From 84d2744bc56a1876af90fba9c16df953f9c2cb84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 4 Jun 2019 15:01:27 +0200 Subject: [PATCH 1/2] 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. --- src/core/bpf-firewall.c | 26 ++++++++++++++++++-------- src/core/bpf-firewall.h | 2 ++ src/core/cgroup.c | 4 ++++ src/core/dbus-cgroup.c | 15 --------------- src/core/ip-address-access.c | 15 --------------- 5 files changed, 24 insertions(+), 38 deletions(-) diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c index 723c7b4b4e..32eb8700e3 100644 --- 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 7d38483dbd..10cafcc02e 100644 --- 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 042a742fa9..a7263855dc 100644 --- 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 d75628c663..9f4fd06dc4 100644 --- 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 1d431bb674..36cec70c2c 100644 --- 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; } From f140ed02f764c731a948dc2b4c3349ba6e2eff24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 4 Jun 2019 17:22:12 +0200 Subject: [PATCH 2/2] Silence warning about BPF firewall in containers We'd get a warning on every nspawn invocation: dev-hugepages.mount: unit configures an IP firewall, but the local system does not support BPF/cgroup firewalling. (This warning is only shown for the first unit using IP firewalling.) Before the previous commit, I'd generally get a warning about systemd-udev.service, even though that service is not started in containers. But are still many other units which that declare a firewall, which is currently unsupported in containers. Let's stop warning about this. The warning is still emitted e.g. if legacy cgroups are used. This is something that can be configured, so it makes more sense to emit the warning. --- src/core/bpf-firewall.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c index 32eb8700e3..7b9aeb6fbf 100644 --- a/src/core/bpf-firewall.c +++ b/src/core/bpf-firewall.c @@ -23,6 +23,7 @@ #include "memory-util.h" #include "missing_syscall.h" #include "unit.h" +#include "virt.h" enum { MAP_KEY_PACKETS, @@ -643,6 +644,8 @@ int bpf_firewall_reset_accounting(int map_fd) { return bpf_map_update_element(map_fd, &key, &value); } +static int bpf_firewall_unsupported_reason = 0; + int bpf_firewall_supported(void) { struct bpf_insn trivial[] = { BPF_MOV64_IMM(BPF_REG_0, 1), @@ -667,7 +670,9 @@ int bpf_firewall_supported(void) { return supported; if (geteuid() != 0) { - log_debug("Not enough privileges, BPF firewalling is not supported."); + bpf_firewall_unsupported_reason = + log_debug_errno(SYNTHETIC_ERRNO(EACCES), + "Not enough privileges, BPF firewalling is not supported."); return supported = BPF_FIREWALL_UNSUPPORTED; } @@ -675,7 +680,9 @@ int bpf_firewall_supported(void) { if (r < 0) return log_error_errno(r, "Can't determine whether the unified hierarchy is used: %m"); if (r == 0) { - log_debug("Not running with unified cgroups, BPF firewalling is not supported."); + bpf_firewall_unsupported_reason = + log_debug_errno(SYNTHETIC_ERRNO(EUCLEAN), + "Not running with unified cgroups, BPF firewalling is not supported."); return supported = BPF_FIREWALL_UNSUPPORTED; } @@ -685,7 +692,8 @@ int bpf_firewall_supported(void) { 1, BPF_F_NO_PREALLOC); if (fd < 0) { - log_debug_errno(fd, "Can't allocate BPF LPM TRIE map, BPF firewalling is not supported: %m"); + bpf_firewall_unsupported_reason = + log_debug_errno(fd, "Can't allocate BPF LPM TRIE map, BPF firewalling is not supported: %m"); return supported = BPF_FIREWALL_UNSUPPORTED; } @@ -693,19 +701,22 @@ int bpf_firewall_supported(void) { r = bpf_program_new(BPF_PROG_TYPE_CGROUP_SKB, &program); if (r < 0) { - log_debug_errno(r, "Can't allocate CGROUP SKB BPF program, BPF firewalling is not supported: %m"); + bpf_firewall_unsupported_reason = + log_debug_errno(r, "Can't allocate CGROUP SKB BPF program, BPF firewalling is not supported: %m"); return supported = BPF_FIREWALL_UNSUPPORTED; } r = bpf_program_add_instructions(program, trivial, ELEMENTSOF(trivial)); if (r < 0) { - log_debug_errno(r, "Can't add trivial instructions to CGROUP SKB BPF program, BPF firewalling is not supported: %m"); + bpf_firewall_unsupported_reason = + log_debug_errno(r, "Can't add trivial instructions to CGROUP SKB BPF program, BPF firewalling is not supported: %m"); return supported = BPF_FIREWALL_UNSUPPORTED; } r = bpf_program_load_kernel(program, NULL, 0); if (r < 0) { - log_debug_errno(r, "Can't load kernel CGROUP SKB BPF program, BPF firewalling is not supported: %m"); + bpf_firewall_unsupported_reason = + log_debug_errno(r, "Can't load kernel CGROUP SKB BPF program, BPF firewalling is not supported: %m"); return supported = BPF_FIREWALL_UNSUPPORTED; } @@ -725,7 +736,8 @@ int bpf_firewall_supported(void) { if (bpf(BPF_PROG_DETACH, &attr, sizeof(attr)) < 0) { if (errno != EBADF) { - log_debug_errno(errno, "Didn't get EBADF from BPF_PROG_DETACH, BPF firewalling is not supported: %m"); + bpf_firewall_unsupported_reason = + log_debug_errno(errno, "Didn't get EBADF from BPF_PROG_DETACH, BPF firewalling is not supported: %m"); return supported = BPF_FIREWALL_UNSUPPORTED; } @@ -769,10 +781,13 @@ 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"); + bool quiet = bpf_firewall_unsupported_reason == -EPERM && detect_container(); + + log_unit_full(u, quiet ? LOG_DEBUG : LOG_WARNING, bpf_firewall_unsupported_reason, + "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; } }