From 22583a002eba2e95c66e76f0023cbc1eb953445d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 18 Dec 2024 14:26:39 +0100 Subject: [PATCH 1/2] networkd-sysctl: rename functions to match our typical prefixes --- src/network/networkd-link.c | 2 +- src/network/networkd-manager.c | 4 ++-- src/network/networkd-sysctl.c | 12 ++++++------ src/network/networkd-sysctl.h | 12 ++++++------ 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/network/networkd-link.c b/src/network/networkd-link.c index 3c042e6c18..22756fc8fb 100644 --- a/src/network/networkd-link.c +++ b/src/network/networkd-link.c @@ -252,7 +252,7 @@ static void link_free_engines(Link *link) { static Link *link_free(Link *link) { assert(link); - (void) sysctl_clear_link_shadows(link); + (void) link_clear_sysctl_shadows(link); link_ntp_settings_clear(link); link_dns_settings_clear(link); diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c index 96cedbec38..829937958b 100644 --- a/src/network/networkd-manager.c +++ b/src/network/networkd-manager.c @@ -666,7 +666,7 @@ Manager* manager_free(Manager *m) { if (!m) return NULL; - sysctl_remove_monitor(m); + manager_remove_sysctl_monitor(m); free(m->state_file); @@ -742,7 +742,7 @@ int manager_start(Manager *m) { log_debug("Starting..."); - (void) sysctl_add_monitor(m); + (void) manager_install_sysctl_monitor(m); /* Loading BPF programs requires CAP_SYS_ADMIN and CAP_BPF. * Drop the capabilities here, regardless if the load succeeds or not. */ diff --git a/src/network/networkd-sysctl.c b/src/network/networkd-sysctl.c index fb2864eb4f..c59a5d341d 100644 --- a/src/network/networkd-sysctl.c +++ b/src/network/networkd-sysctl.c @@ -47,7 +47,7 @@ static int sysctl_event_handler(void *ctx, void *data, size_t data_sz) { * so do it only in case of a fatal error like a version mismatch. */ if (we->version != 1) return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), - "Unexpected sysctl event, disabling sysctl monitoring: %d", we->version); + "Unexpected sysctl event, disabling sysctl monitoring: %d", we->version); if (we->errorcode != 0) { log_warning_errno(we->errorcode, "Sysctl monitor BPF returned error: %m"); @@ -56,7 +56,7 @@ static int sysctl_event_handler(void *ctx, void *data, size_t data_sz) { path = path_join("/proc/sys", we->path); if (!path) { - log_oom(); + log_oom_warning(); return 0; } @@ -91,7 +91,7 @@ static int on_ringbuf_io(sd_event_source *s, int fd, uint32_t revents, void *use return 0; } -int sysctl_add_monitor(Manager *manager) { +int manager_install_sysctl_monitor(Manager *manager) { _cleanup_(sysctl_monitor_bpf_freep) struct sysctl_monitor_bpf *obj = NULL; _cleanup_(bpf_link_freep) struct bpf_link *sysctl_link = NULL; _cleanup_(bpf_ring_buffer_freep) struct ring_buffer *sysctl_buffer = NULL; @@ -160,7 +160,7 @@ int sysctl_add_monitor(Manager *manager) { return 0; } -void sysctl_remove_monitor(Manager *manager) { +void manager_remove_sysctl_monitor(Manager *manager) { assert(manager); manager->sysctl_event_source = sd_event_source_disable_unref(manager->sysctl_event_source); @@ -171,7 +171,7 @@ void sysctl_remove_monitor(Manager *manager) { manager->sysctl_shadow = hashmap_free(manager->sysctl_shadow); } -int sysctl_clear_link_shadows(Link *link) { +int link_clear_sysctl_shadows(Link *link) { _cleanup_free_ char *ipv4 = NULL, *ipv6 = NULL; char *key = NULL, *value = NULL; @@ -188,7 +188,7 @@ int sysctl_clear_link_shadows(Link *link) { HASHMAP_FOREACH_KEY(value, key, link->manager->sysctl_shadow) if (path_startswith(key, ipv4) || path_startswith(key, ipv6)) { - assert_se(hashmap_remove_value(link->manager->sysctl_shadow, key, value) == value); + assert_se(hashmap_remove_value(link->manager->sysctl_shadow, key, value)); free(key); free(value); } diff --git a/src/network/networkd-sysctl.h b/src/network/networkd-sysctl.h index ea1d2bc897..2a1f1ae248 100644 --- a/src/network/networkd-sysctl.h +++ b/src/network/networkd-sysctl.h @@ -28,13 +28,13 @@ typedef enum IPReversePathFilter { } IPReversePathFilter; #if HAVE_VMLINUX_H -int sysctl_add_monitor(Manager *manager); -void sysctl_remove_monitor(Manager *manager); -int sysctl_clear_link_shadows(Link *link); +int manager_install_sysctl_monitor(Manager *manager); +void manager_remove_sysctl_monitor(Manager *manager); +int link_clear_sysctl_shadows(Link *link); #else -static inline int sysctl_add_monitor(Manager *manager) { return 0; } -static inline void sysctl_remove_monitor(Manager *manager) { } -static inline int sysctl_clear_link_shadows(Link *link) { return 0; } +static inline int manager_install_sysctl_monitor(Manager *manager) { return 0; } +static inline void manager_remove_sysctl_monitor(Manager *manager) { } +static inline int link_clear_sysctl_shadows(Link *link) { return 0; } #endif void manager_set_sysctl(Manager *manager); From cffae6e113a8c078adaf610c05e734346233ee3d Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 18 Dec 2024 14:34:59 +0100 Subject: [PATCH 2/2] networkd-sysctl: tweak error handling and log level a bit Follow-up for 6d9ef22acdeac4b429efb75164341233955484af - Downgrade log level for bpf not installed or kernel version being too old to LOG_DEBUG. Otherwise, on kernels older than 6.12 the log becomes quite annoying. - Always propagate the error and ignore only on caller's side. The current style is a messy mix. --- src/network/networkd-sysctl.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/network/networkd-sysctl.c b/src/network/networkd-sysctl.c index c59a5d341d..d6d9fe81eb 100644 --- a/src/network/networkd-sysctl.c +++ b/src/network/networkd-sysctl.c @@ -102,10 +102,10 @@ int manager_install_sysctl_monitor(Manager *manager) { assert(manager); r = dlopen_bpf(); - if (r < 0) { - log_info_errno(r, "sysctl monitor disabled, as BPF support is not available."); - return 0; - } + if (ERRNO_IS_NEG_NOT_SUPPORTED(r)) + return log_debug_errno(r, "sysctl monitor disabled, as BPF support is not available."); + if (r < 0) + return log_warning_errno(r, "Failed to load libbpf, not installing sysctl monitor: %m"); r = cg_pid_get_path(SYSTEMD_CGROUP_CONTROLLER, 0, &cgroup); if (r < 0) @@ -113,13 +113,12 @@ int manager_install_sysctl_monitor(Manager *manager) { root_cgroup_fd = cg_path_open(SYSTEMD_CGROUP_CONTROLLER, "/"); if (root_cgroup_fd < 0) - return log_warning_errno(root_cgroup_fd, "Failed to open cgroup, ignoring: %m."); + return log_warning_errno(root_cgroup_fd, "Failed to open cgroup, ignoring: %m"); obj = sysctl_monitor_bpf__open_and_load(); - if (!obj) { - log_info_errno(errno, "Unable to load sysctl monitor BPF program, ignoring: %m."); - return 0; - } + if (!obj) + return log_full_errno(errno == EINVAL ? LOG_DEBUG : LOG_INFO, errno, + "Unable to load sysctl monitor BPF program, ignoring: %m"); cgroup_fd = cg_path_open(SYSTEMD_CGROUP_CONTROLLER, cgroup); if (cgroup_fd < 0) @@ -130,10 +129,8 @@ int manager_install_sysctl_monitor(Manager *manager) { sysctl_link = sym_bpf_program__attach_cgroup(obj->progs.sysctl_monitor, root_cgroup_fd); r = bpf_get_error_translated(sysctl_link); - if (r < 0) { - log_info_errno(r, "Unable to attach sysctl monitor BPF program to cgroup, ignoring: %m."); - return 0; - } + if (r < 0) + return log_warning_errno(r, "Unable to attach sysctl monitor BPF program to cgroup, ignoring: %m"); fd = sym_bpf_map__fd(obj->maps.written_sysctls); if (fd < 0)