Move warning about unsupported BPF firewall right before the firewall would be created

There's no need to warn about the firewall when parsing, because the unit might
not be started at all. Let's warn only when we're actually preparing to start
the firewall.

This changes behaviour:
- the warning is printed just once for all unit types, and not once
  for normal units and once for transient units.
- on repeat warnings, the message is not printed at all. There's already
  detailed debug info from bpf_firewall_compile(), so we don't need to repeat
  ourselves.
- when we are not root, let's say precisely that, not "lack of necessary privileges"
  and "the local system does not support BPF/cgroup firewalling".

Fixes #12673.

diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c
index 723c7b4..32eb870 100644 (file)
--- a/src/core/bpf-firewall.c
+++ b/src/core/bpf-firewall.c
@@ -484,19 +484,17 @@ int bpf_firewall_compile(Unit *u) {
         supported = bpf_firewall_supported();
         if (supported < 0)
                 return supported;
-        if (supported == BPF_FIREWALL_UNSUPPORTED) {
-                log_unit_debug(u, "BPF firewalling not supported on this manager, proceeding without.");
-                return -EOPNOTSUPP;
-        }
-        if (supported != BPF_FIREWALL_SUPPORTED_WITH_MULTI && u->type == UNIT_SLICE) {
+        if (supported == BPF_FIREWALL_UNSUPPORTED)
+                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                            "BPF firewalling not supported on this manager, proceeding without.");
+        if (supported != BPF_FIREWALL_SUPPORTED_WITH_MULTI && u->type == UNIT_SLICE)
                 /* If BPF_F_ALLOW_MULTI is not supported we don't support any BPF magic on inner nodes (i.e. on slice
                  * units), since that would mean leaf nodes couldn't do any BPF anymore at all. Under the assumption
                  * that BPF is more interesting on leaf nodes we hence avoid it on inner nodes in that case. This is
                  * consistent with old systemd behaviour from before v238, where BPF wasn't supported in inner nodes at
                  * all, either. */
-                log_unit_debug(u, "BPF_F_ALLOW_MULTI is not supported on this manager, not doing BPF firewall on slice units.");
-                return -EOPNOTSUPP;
-        }
+                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EOPNOTSUPP),
+                                            "BPF_F_ALLOW_MULTI is not supported on this manager, not doing BPF firewall on slice units.");
 
         /* Note that when we compile a new firewall we first flush out the access maps and the BPF programs themselves,
          * but we reuse the the accounting maps. That way the firewall in effect always maps to the actual
@@ -766,3 +764,15 @@ int bpf_firewall_supported(void) {
                 return supported = BPF_FIREWALL_UNSUPPORTED;
         }
 }
+
+void emit_bpf_firewall_warning(Unit *u) {
+        static bool warned = false;
+
+        if (!warned) {
+                log_unit_warning(u, "unit configures an IP firewall, but %s.\n"
+                                    "(This warning is only shown for the first unit using IP firewalling.)",
+                                 getuid() != 0 ? "not running as root" :
+                                                 "the local system does not support BPF/cgroup firewalling");
+                warned = true;
+        }
+}

diff --git a/src/core/bpf-firewall.h b/src/core/bpf-firewall.h
index 7d38483..10cafcc 100644 (file)
--- a/src/core/bpf-firewall.h
+++ b/src/core/bpf-firewall.h
@@ -18,3 +18,5 @@ int bpf_firewall_install(Unit *u);
 
 int bpf_firewall_read_accounting(int map_fd, uint64_t *ret_bytes, uint64_t *ret_packets);
 int bpf_firewall_reset_accounting(int map_fd);
+
+void emit_bpf_firewall_warning(Unit *u);

diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 042a742..a726385 100644 (file)
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -1540,6 +1540,10 @@ CGroupMask unit_get_target_mask(Unit *u) {
          * hierarchy that shall be enabled for it. */
 
         mask = unit_get_own_mask(u) | unit_get_members_mask(u) | unit_get_siblings_mask(u);
+
+        if (mask & CGROUP_MASK_BPF_FIREWALL & ~u->manager->cgroup_supported)
+                emit_bpf_firewall_warning(u);
+
         mask &= u->manager->cgroup_supported;
         mask &= ~unit_get_ancestor_disable_mask(u);
 

diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c
index d75628c..9f4fd06 100644 (file)
--- a/src/core/dbus-cgroup.c
+++ b/src/core/dbus-cgroup.c
@@ -1426,21 +1426,6 @@ int bus_cgroup_set_property(
                                 return r;
 
                         unit_write_setting(u, flags, name, buf);
-
-                        if (*list) {
-                                r = bpf_firewall_supported();
-                                if (r < 0)
-                                        return r;
-                                if (r == BPF_FIREWALL_UNSUPPORTED) {
-                                        static bool warned = false;
-
-                                        log_full(warned ? LOG_DEBUG : LOG_WARNING,
-                                                 "Transient unit %s configures an IP firewall, but the local system does not support BPF/cgroup firewalling.\n"
-                                                 "Proceeding WITHOUT firewalling in effect! (This warning is only shown for the first started transient unit using IP firewalling.)", u->id);
-
-                                        warned = true;
-                                }
-                        }
                 }
 
                 return 1;

diff --git a/src/core/ip-address-access.c b/src/core/ip-address-access.c
index 1d431bb..36cec70 100644 (file)
--- a/src/core/ip-address-access.c
+++ b/src/core/ip-address-access.c
@@ -134,21 +134,6 @@ int config_parse_ip_address_access(
 
         *list = ip_address_access_reduce(*list);
 
-        if (*list) {
-                r = bpf_firewall_supported();
-                if (r < 0)
-                        return r;
-                if (r == BPF_FIREWALL_UNSUPPORTED) {
-                        static bool warned = false;
-
-                        log_full(warned ? LOG_DEBUG : LOG_WARNING,
-                                 "File %s:%u configures an IP firewall (%s=%s), but the local system does not support BPF/cgroup based firewalling.\n"
-                                 "Proceeding WITHOUT firewalling in effect! (This warning is only shown for the first loaded unit using IP firewalling.)", filename, line, lvalue, rvalue);
-
-                        warned = true;
-                }
-        }
-
         return 0;
 }