network/sysctl: several cleanups for sysctl_add_monitor()

- rename rootcg -> root_cgroup_fd, to emphasize it is a fd,
- drop nested function call, and check error code.

src/network/bpf/sysctl_monitor/sysctl-monitor.bpf.c

@@ -36,23 +36,22 @@ struct str {
 static long cut_last(u32 i, struct str *str) {
         char *s;
 
+        /* Sanity check for the preverifier */
+        if (i >= str->l)
+                return 1; /* exit from the loop */
+
         i = str->l - i - 1;
         s = str->s + i;
 
-        /* Sanity check for the preverifier */
-        if (i >= str->l)
-                return 1;
-
         if (*s == 0)
-                return 0;
+                return 0; /* continue */
 
         if (*s == '\n' || *s == '\r' || *s == ' ' || *s == '\t') {
                 *s = 0;
-
+                return 0; /* continue */
-                return 0;
         }
 
-        return 1;
+        return 1; /* exit from the loop */
 }
 
 /* Cut off trailing whitespace and newlines */

src/network/networkd-sysctl.c

@@ -34,13 +34,7 @@ static struct sysctl_monitor_bpf* sysctl_monitor_bpf_free(struct sysctl_monitor_
         return NULL;
 }
 
-static struct ring_buffer* rb_free(struct ring_buffer *rb) {
-        sym_ring_buffer__free(rb);
-        return NULL;
-}
-
 DEFINE_TRIVIAL_CLEANUP_FUNC(struct sysctl_monitor_bpf *, sysctl_monitor_bpf_free);
-DEFINE_TRIVIAL_CLEANUP_FUNC(struct ring_buffer *, rb_free);
 
 static int sysctl_event_handler(void *ctx, void *data, size_t data_sz) {
         struct sysctl_write_event *we = ASSERT_PTR(data);
@@ -99,10 +93,10 @@ static int on_ringbuf_io(sd_event_source *s, int fd, uint32_t revents, void *use
 int sysctl_add_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_(rb_freep) struct ring_buffer *sysctl_buffer = NULL;
+        _cleanup_(bpf_ring_buffer_freep) struct ring_buffer *sysctl_buffer = NULL;
-        _cleanup_close_ int cgroup_fd = -EBADF, rootcg = -EBADF;
+        _cleanup_close_ int cgroup_fd = -EBADF, root_cgroup_fd = -EBADF;
         _cleanup_free_ char *cgroup = NULL;
-        int idx = 0, r;
+        int idx = 0, r, fd;
 
         assert(manager);
 
@@ -116,9 +110,9 @@ int sysctl_add_monitor(Manager *manager) {
         if (r < 0)
                 return log_warning_errno(r, "Failed to get cgroup path, ignoring: %m.");
 
-        rootcg = cg_path_open(SYSTEMD_CGROUP_CONTROLLER, "/");
+        root_cgroup_fd = cg_path_open(SYSTEMD_CGROUP_CONTROLLER, "/");
-        if (rootcg < 0)
+        if (root_cgroup_fd < 0)
-                return log_warning_errno(rootcg, "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) {
@@ -133,21 +127,27 @@ int sysctl_add_monitor(Manager *manager) {
         if (sym_bpf_map_update_elem(sym_bpf_map__fd(obj->maps.cgroup_map), &idx, &cgroup_fd, BPF_ANY))
                 return log_warning_errno(errno, "Failed to update cgroup map: %m");
 
-        sysctl_link = sym_bpf_program__attach_cgroup(obj->progs.sysctl_monitor, rootcg);
+        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;
         }
 
-        sysctl_buffer = sym_ring_buffer__new(
+        fd = sym_bpf_map__fd(obj->maps.written_sysctls);
-                        sym_bpf_map__fd(obj->maps.written_sysctls),
+        if (fd < 0)
-                        sysctl_event_handler, &manager->sysctl_shadow, NULL);
+                return log_warning_errno(fd, "Failed to get fd of sysctl maps: %m");
+
+        sysctl_buffer = sym_ring_buffer__new(fd, sysctl_event_handler, &manager->sysctl_shadow, NULL);
         if (!sysctl_buffer)
                 return log_warning_errno(errno, "Failed to create ring buffer: %m");
 
+        fd = sym_ring_buffer__epoll_fd(sysctl_buffer);
+        if (fd < 0)
+                return log_warning_errno(fd, "Failed to get poll fd of ring buffer: %m");
+
         r = sd_event_add_io(manager->event, &manager->sysctl_event_source,
-                        sym_ring_buffer__epoll_fd(sysctl_buffer), EPOLLIN, on_ringbuf_io, sysctl_buffer);
+                            fd, EPOLLIN, on_ringbuf_io, sysctl_buffer);
         if (r < 0)
                 return log_warning_errno(r, "Failed to watch sysctl event ringbuffer: %m");
 
@@ -163,22 +163,9 @@ void sysctl_remove_monitor(Manager *manager) {
         assert(manager);
 
         manager->sysctl_event_source = sd_event_source_disable_unref(manager->sysctl_event_source);
-
+        manager->sysctl_buffer = bpf_ring_buffer_free(manager->sysctl_buffer);
-        if (manager->sysctl_buffer) {
+        manager->sysctl_link = bpf_link_free(manager->sysctl_link);
-                sym_ring_buffer__free(manager->sysctl_buffer);
+        manager->sysctl_skel = sysctl_monitor_bpf_free(manager->sysctl_skel);
-                manager->sysctl_buffer = NULL;
-        }
-
-        if (manager->sysctl_link) {
-                sym_bpf_link__destroy(manager->sysctl_link);
-                manager->sysctl_link = NULL;
-        }
-
-        if (manager->sysctl_skel) {
-                sysctl_monitor_bpf__destroy(manager->sysctl_skel);
-                manager->sysctl_skel = NULL;
-        }
-
         manager->cgroup_fd = safe_close(manager->cgroup_fd);
 }
 

src/nsresourced/nsresourced-manager.c

@@ -6,6 +6,7 @@
 #include "sd-daemon.h"
 
 #include "bpf-dlopen.h"
+#include "bpf-link.h"
 #include "build-path.h"
 #include "common-signal.h"
 #include "env-util.h"
@@ -141,8 +142,7 @@ Manager* manager_free(Manager *m) {
 
 #if HAVE_VMLINUX_H
         sd_event_source_disable_unref(m->userns_restrict_bpf_ring_buffer_event_source);
-        if (m->userns_restrict_bpf_ring_buffer)
+        bpf_ring_buffer_free(m->userns_restrict_bpf_ring_buffer);
-                sym_ring_buffer__free(m->userns_restrict_bpf_ring_buffer);
         userns_restrict_bpf_free(m->userns_restrict_bpf);
 #endif
 

src/shared/bpf-link.c

@@ -31,7 +31,7 @@ int bpf_serialize_link(FILE *f, FDSet *fds, const char *key, struct bpf_link *li
         return serialize_fd(f, fds, key, sym_bpf_link__fd(link));
 }
 
-struct bpf_link *bpf_link_free(struct bpf_link *link) {
+struct bpf_link* bpf_link_free(struct bpf_link *link) {
         /* If libbpf wasn't dlopen()ed, sym_bpf_link__destroy might be unresolved (NULL), so let's not try to
          * call it if link is NULL. link might also be a non-null "error pointer", but such a value can only
          * originate from a call to libbpf, but that means that libbpf is available, and we can let
@@ -41,3 +41,10 @@ struct bpf_link *bpf_link_free(struct bpf_link *link) {
 
         return NULL;
 }
+
+struct ring_buffer* bpf_ring_buffer_free(struct ring_buffer *rb) {
+        if (rb) /* See the comment in bpf_link_free(). */
+                sym_ring_buffer__free(rb);
+
+        return NULL;
+}

src/shared/bpf-link.h

@@ -12,5 +12,8 @@ bool bpf_can_link_program(struct bpf_program *prog);
 
 int bpf_serialize_link(FILE *f, FDSet *fds, const char *key, struct bpf_link *link);
 
-struct bpf_link *bpf_link_free(struct bpf_link *p);
+struct bpf_link* bpf_link_free(struct bpf_link *p);
 DEFINE_TRIVIAL_CLEANUP_FUNC(struct bpf_link *, bpf_link_free);
+
+struct ring_buffer* bpf_ring_buffer_free(struct ring_buffer *rb);
+DEFINE_TRIVIAL_CLEANUP_FUNC(struct ring_buffer *, bpf_ring_buffer_free);