Merge pull request #171 from teg/rtnl-broadcast-2

sd-rtnl: make joining broadcast groups implicit

13 files changed, 87 insertions, 77 deletions

diff --git a/src/core/loopback-setup.c b/src/core/loopback-setup.c
index 63b15c1200..938f3ab068 100644
--- a/src/core/loopback-setup.c
+++ b/src/core/loopback-setup.c
@@ -70,7 +70,7 @@ int loopback_setup(void) {
         _cleanup_rtnl_unref_ sd_rtnl *rtnl = NULL;
         int r;
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return r;
 
diff --git a/src/libsystemd-network/test-pppoe.c b/src/libsystemd-network/test-pppoe.c
index 9c8d6f7779..cc51197295 100644
--- a/src/libsystemd-network/test-pppoe.c
+++ b/src/libsystemd-network/test-pppoe.c
@@ -94,7 +94,7 @@ static int test_pppoe_server(sd_event *e) {
 
         assert_se(r >= 0);
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
         assert_se(sd_rtnl_attach_event(rtnl, e, 0) >= 0);
 
         assert_se(sd_rtnl_message_new_link(rtnl, &m, RTM_NEWLINK, 0) >= 0);
diff --git a/src/libsystemd/sd-rtnl/local-addresses.c b/src/libsystemd/sd-rtnl/local-addresses.c
index 31bfa06066..f05cbf1f47 100644
--- a/src/libsystemd/sd-rtnl/local-addresses.c
+++ b/src/libsystemd/sd-rtnl/local-addresses.c
@@ -66,7 +66,7 @@ int local_addresses(sd_rtnl *context, int ifindex, int af, struct local_address
         if (context)
                 rtnl = sd_rtnl_ref(context);
         else {
-                r = sd_rtnl_open(&rtnl, 0);
+                r = sd_rtnl_open(&rtnl);
                 if (r < 0)
                         return r;
         }
@@ -177,7 +177,7 @@ int local_gateways(sd_rtnl *context, int ifindex, int af, struct local_address *
         if (context)
                 rtnl = sd_rtnl_ref(context);
         else {
-                r = sd_rtnl_open(&rtnl, 0);
+                r = sd_rtnl_open(&rtnl);
                 if (r < 0)
                         return r;
         }
diff --git a/src/libsystemd/sd-rtnl/rtnl-util.c b/src/libsystemd/sd-rtnl/rtnl-util.c
index 9ddf074c24..c2b1a5c65d 100644
--- a/src/libsystemd/sd-rtnl/rtnl-util.c
+++ b/src/libsystemd/sd-rtnl/rtnl-util.c
@@ -34,7 +34,7 @@ int rtnl_set_link_name(sd_rtnl **rtnl, int ifindex, const char *name) {
         assert(name);
 
         if (!*rtnl) {
-                r = sd_rtnl_open(rtnl, 0);
+                r = sd_rtnl_open(rtnl);
                 if (r < 0)
                         return r;
         }
@@ -66,7 +66,7 @@ int rtnl_set_link_properties(sd_rtnl **rtnl, int ifindex, const char *alias,
                 return 0;
 
         if (!*rtnl) {
-                r = sd_rtnl_open(rtnl, 0);
+                r = sd_rtnl_open(rtnl);
                 if (r < 0)
                         return r;
         }
diff --git a/src/libsystemd/sd-rtnl/sd-rtnl.c b/src/libsystemd/sd-rtnl/sd-rtnl.c
index ae9a40c0fe..5bafc282c0 100644
--- a/src/libsystemd/sd-rtnl/sd-rtnl.c
+++ b/src/libsystemd/sd-rtnl/sd-rtnl.c
@@ -106,25 +106,7 @@ static bool rtnl_pid_changed(sd_rtnl *rtnl) {
         return rtnl->original_pid != getpid();
 }
 
-static int rtnl_compute_groups_ap(uint32_t *_groups, unsigned n_groups, va_list ap) {
-        uint32_t groups = 0;
-        unsigned i;
-
-        for (i = 0; i < n_groups; i++) {
-                unsigned group;
-
-                group = va_arg(ap, unsigned);
-                assert_return(group < 32, -EINVAL);
-
-                groups |= group ? (1 << (group - 1)) : 0;
-        }
-
-        *_groups = groups;
-
-        return 0;
-}
-
-static int rtnl_open_fd_ap(sd_rtnl **ret, int fd, unsigned n_groups, va_list ap) {
+int sd_rtnl_open_fd(sd_rtnl **ret, int fd) {
         _cleanup_rtnl_unref_ sd_rtnl *rtnl = NULL;
         socklen_t addrlen;
         int r, one = 1;
@@ -140,10 +122,6 @@ static int rtnl_open_fd_ap(sd_rtnl **ret, int fd, unsigned n_groups, va_list ap)
         if (r < 0)
                 return -errno;
 
-        r = rtnl_compute_groups_ap(&rtnl->sockaddr.nl.nl_groups, n_groups, ap);
-        if (r < 0)
-                return r;
-
         addrlen = sizeof(rtnl->sockaddr);
 
         r = bind(fd, &rtnl->sockaddr.sa, addrlen);
@@ -163,33 +141,33 @@ static int rtnl_open_fd_ap(sd_rtnl **ret, int fd, unsigned n_groups, va_list ap)
         return 0;
 }
 
-int sd_rtnl_open_fd(sd_rtnl **ret, int fd, unsigned n_groups, ...) {
-        va_list ap;
+int sd_rtnl_open(sd_rtnl **ret) {
+        _cleanup_close_ int fd = -1;
         int r;
 
-        va_start(ap, n_groups);
-        r = rtnl_open_fd_ap(ret, fd, n_groups, ap);
-        va_end(ap);
-
-        return r;
-}
-
-int sd_rtnl_open(sd_rtnl **ret, unsigned n_groups, ...) {
-        va_list ap;
-        int fd, r;
-
         fd = socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC|SOCK_NONBLOCK, NETLINK_ROUTE);
         if (fd < 0)
                 return -errno;
 
-        va_start(ap, n_groups);
-        r = rtnl_open_fd_ap(ret, fd, n_groups, ap);
-        va_end(ap);
-
-        if (r < 0) {
-                safe_close(fd);
+        r = sd_rtnl_open_fd(ret, fd);
+        if (r < 0)
                 return r;
-        }
+
+        fd = -1;
+
+        return 0;
+}
+
+static int rtnl_join_broadcast_group(sd_rtnl *rtnl, unsigned group) {
+        int r;
+
+        assert(rtnl);
+        assert(rtnl->fd >= 0);
+        assert(group > 0);
+
+        r = setsockopt(rtnl->fd, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, &group, sizeof(group));
+        if (r < 0)
+                return -errno;
 
         return 0;
 }
@@ -1001,14 +979,12 @@ int sd_rtnl_add_match(sd_rtnl *rtnl,
                       uint16_t type,
                       sd_rtnl_message_handler_t callback,
                       void *userdata) {
-        struct match_callback *c;
+        _cleanup_free_ struct match_callback *c = NULL;
+        int r;
 
         assert_return(rtnl, -EINVAL);
         assert_return(callback, -EINVAL);
         assert_return(!rtnl_pid_changed(rtnl), -ECHILD);
-        assert_return(rtnl_message_type_is_link(type) ||
-                      rtnl_message_type_is_addr(type) ||
-                      rtnl_message_type_is_route(type), -EOPNOTSUPP);
 
         c = new0(struct match_callback, 1);
         if (!c)
@@ -1018,8 +994,36 @@ int sd_rtnl_add_match(sd_rtnl *rtnl,
         c->type = type;
         c->userdata = userdata;
 
+        switch (type) {
+                case RTM_NEWLINK:
+                case RTM_SETLINK:
+                case RTM_GETLINK:
+                case RTM_DELLINK:
+                        r = rtnl_join_broadcast_group(rtnl, RTNLGRP_LINK);
+                        if (r < 0)
+                                return r;
+
+                        break;
+                case RTM_NEWADDR:
+                case RTM_GETADDR:
+                case RTM_DELADDR:
+                        r = rtnl_join_broadcast_group(rtnl, RTNLGRP_IPV4_IFADDR);
+                        if (r < 0)
+                                return r;
+
+                        r = rtnl_join_broadcast_group(rtnl, RTNLGRP_IPV6_IFADDR);
+                        if (r < 0)
+                                return r;
+
+                        break;
+                default:
+                        return -EOPNOTSUPP;
+        }
+
         LIST_PREPEND(match_callbacks, rtnl->match_callbacks, c);
 
+        c = NULL;
+
         return 0;
 }
 
@@ -1033,6 +1037,13 @@ int sd_rtnl_remove_match(sd_rtnl *rtnl,
         assert_return(callback, -EINVAL);
         assert_return(!rtnl_pid_changed(rtnl), -ECHILD);
 
+        /* we should unsubscribe from the broadcast groups at this point, but it is not so
+           trivial for a few reasons: the refcounting is a bit of a mess and not obvious
+           how it will look like after we add genetlink support, and it is also not possible
+           to query what broadcast groups were subscribed to when we inherit the socket to get
+           the initial refcount. The latter could indeed be done for the first 32 broadcast
+           groups (which incidentally is all we currently support in .socket units anyway),
+           but we better not rely on only ever using 32 groups. */
         LIST_FOREACH(match_callbacks, c, rtnl->match_callbacks)
                 if (c->callback == callback && c->type == type && c->userdata == userdata) {
                         LIST_REMOVE(match_callbacks, rtnl->match_callbacks, c);
diff --git a/src/libsystemd/sd-rtnl/test-rtnl.c b/src/libsystemd/sd-rtnl/test-rtnl.c
index 47cce64816..94b1cb7c72 100644
--- a/src/libsystemd/sd-rtnl/test-rtnl.c
+++ b/src/libsystemd/sd-rtnl/test-rtnl.c
@@ -184,8 +184,8 @@ static void test_route(void) {
 static void test_multiple(void) {
         sd_rtnl *rtnl1, *rtnl2;
 
-        assert_se(sd_rtnl_open(&rtnl1, 0) >= 0);
-        assert_se(sd_rtnl_open(&rtnl2, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl1) >= 0);
+        assert_se(sd_rtnl_open(&rtnl2) >= 0);
 
         rtnl1 = sd_rtnl_unref(rtnl1);
         rtnl2 = sd_rtnl_unref(rtnl2);
@@ -216,7 +216,7 @@ static void test_event_loop(int ifindex) {
         ifname = strdup("lo2");
         assert_se(ifname);
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
         assert_se(sd_rtnl_message_new_link(rtnl, &m, RTM_GETLINK, ifindex) >= 0);
 
         assert_se(sd_rtnl_call_async(rtnl, m, &link_handler, ifname, 0, NULL) >= 0);
@@ -256,7 +256,7 @@ static void test_async(int ifindex) {
         ifname = strdup("lo");
         assert_se(ifname);
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
 
         assert_se(sd_rtnl_message_new_link(rtnl, &m, RTM_GETLINK, ifindex) >= 0);
 
@@ -273,7 +273,7 @@ static void test_pipe(int ifindex) {
         _cleanup_rtnl_message_unref_ sd_rtnl_message *m1 = NULL, *m2 = NULL;
         int counter = 0;
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
 
         assert_se(sd_rtnl_message_new_link(rtnl, &m1, RTM_GETLINK, ifindex) >= 0);
         assert_se(sd_rtnl_message_new_link(rtnl, &m2, RTM_GETLINK, ifindex) >= 0);
@@ -330,7 +330,7 @@ static void test_container(void) {
 static void test_match(void) {
         _cleanup_rtnl_unref_ sd_rtnl *rtnl = NULL;
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
 
         assert_se(sd_rtnl_add_match(rtnl, RTM_NEWLINK, &link_handler, NULL) >= 0);
         assert_se(sd_rtnl_add_match(rtnl, RTM_NEWLINK, &link_handler, NULL) >= 0);
@@ -395,7 +395,7 @@ int main(void) {
 
         test_container();
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
         assert_se(rtnl);
 
         if_loopback = (int) if_nametoindex("lo");
diff --git a/src/network/networkctl.c b/src/network/networkctl.c
index 68925debe6..8e20f70aae 100644
--- a/src/network/networkctl.c
+++ b/src/network/networkctl.c
@@ -199,7 +199,7 @@ static int list_links(int argc, char *argv[], void *userdata) {
 
         pager_open_if_enabled();
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
@@ -670,7 +670,7 @@ static int link_status(int argc, char *argv[], void *userdata) {
         char **name;
         int r;
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
@@ -910,7 +910,7 @@ static int link_lldp_status(int argc, char *argv[], void *userdata) {
 
         pager_open_if_enabled();
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
diff --git a/src/network/networkd-manager.c b/src/network/networkd-manager.c
index db737ad484..9936a8e7cd 100644
--- a/src/network/networkd-manager.c
+++ b/src/network/networkd-manager.c
@@ -385,9 +385,9 @@ static int manager_connect_rtnl(Manager *m) {
 
         fd = systemd_netlink_fd();
         if (fd < 0)
-                r = sd_rtnl_open(&m->rtnl, 3, RTNLGRP_LINK, RTNLGRP_IPV4_IFADDR, RTNLGRP_IPV6_IFADDR);
+                r = sd_rtnl_open(&m->rtnl);
         else
-                r = sd_rtnl_open_fd(&m->rtnl, fd, 0);
+                r = sd_rtnl_open_fd(&m->rtnl, fd);
         if (r < 0)
                 return r;
 
diff --git a/src/network/networkd-wait-online-manager.c b/src/network/networkd-wait-online-manager.c
index 1c997a50a4..39f3845633 100644
--- a/src/network/networkd-wait-online-manager.c
+++ b/src/network/networkd-wait-online-manager.c
@@ -177,7 +177,7 @@ static int manager_rtnl_listen(Manager *m) {
         assert(m);
 
         /* First, subscribe to interfaces coming and going */
-        r = sd_rtnl_open(&m->rtnl, 3, RTNLGRP_LINK, RTNLGRP_IPV4_IFADDR, RTNLGRP_IPV6_IFADDR);
+        r = sd_rtnl_open(&m->rtnl);
         if (r < 0)
                 return r;
 
diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c
index 251af4a67c..6a21ed5471 100644
--- a/src/nspawn/nspawn.c
+++ b/src/nspawn/nspawn.c
@@ -2062,7 +2062,7 @@ static int watch_rtnl(sd_event *event, int recv_fd, union in_addr_union *exposed
         assert(cmsg->cmsg_len == CMSG_LEN(sizeof(int)));
         memcpy(&fd, CMSG_DATA(cmsg), sizeof(int));
 
-        r = sd_rtnl_open_fd(&rtnl, fd, 1, RTNLGRP_IPV4_IFADDR);
+        r = sd_rtnl_open_fd(&rtnl, fd);
         if (r < 0) {
                 safe_close(fd);
                 return log_error_errno(r, "Failed to create rtnl object: %m");
@@ -2574,7 +2574,7 @@ static int setup_veth(pid_t pid, char iface_name[IFNAMSIZ], int *ifi) {
         if (r < 0)
                 return log_error_errno(r, "Failed to generate predictable MAC address for host side: %m");
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
@@ -2659,7 +2659,7 @@ static int setup_bridge(const char veth_name[], int *ifi) {
 
         *ifi = bridge;
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
@@ -2720,7 +2720,7 @@ static int move_network_interfaces(pid_t pid) {
         if (strv_isempty(arg_network_interfaces))
                 return 0;
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
@@ -2767,7 +2767,7 @@ static int setup_macvlan(pid_t pid) {
         if (strv_isempty(arg_network_macvlan))
                 return 0;
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
@@ -2857,7 +2857,7 @@ static int setup_ipvlan(pid_t pid) {
         if (strv_isempty(arg_network_ipvlan))
                 return 0;
 
-        r = sd_rtnl_open(&rtnl, 0);
+        r = sd_rtnl_open(&rtnl);
         if (r < 0)
                 return log_error_errno(r, "Failed to connect to netlink: %m");
 
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index f8d4db7aad..aa78885ac3 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -195,7 +195,7 @@ static int manager_rtnl_listen(Manager *m) {
         assert(m);
 
         /* First, subscribe to interfaces coming and going */
-        r = sd_rtnl_open(&m->rtnl, 3, RTNLGRP_LINK, RTNLGRP_IPV4_IFADDR, RTNLGRP_IPV6_IFADDR);
+        r = sd_rtnl_open(&m->rtnl);
         if (r < 0)
                 return r;
 
diff --git a/src/systemd/sd-rtnl.h b/src/systemd/sd-rtnl.h
index be318e58a8..b05690cb1c 100644
--- a/src/systemd/sd-rtnl.h
+++ b/src/systemd/sd-rtnl.h
@@ -42,8 +42,8 @@ typedef int (*sd_rtnl_message_handler_t)(sd_rtnl *rtnl, sd_rtnl_message *m, void
 
 /* bus */
 int sd_rtnl_new_from_netlink(sd_rtnl **nl, int fd);
-int sd_rtnl_open(sd_rtnl **nl, unsigned n_groups, ...);
-int sd_rtnl_open_fd(sd_rtnl **nl, int fd, unsigned n_groups, ...);
+int sd_rtnl_open(sd_rtnl **nl);
+int sd_rtnl_open_fd(sd_rtnl **nl, int fd);
 int sd_rtnl_inc_rcvbuf(const sd_rtnl *const rtnl, const int size);
 
 sd_rtnl *sd_rtnl_ref(sd_rtnl *nl);
@@ -57,7 +57,6 @@ int sd_rtnl_call_async_cancel(sd_rtnl *nl, uint32_t serial);
 int sd_rtnl_call(sd_rtnl *nl, sd_rtnl_message *message, uint64_t timeout,
                  sd_rtnl_message **reply);
 
-
 int sd_rtnl_get_events(sd_rtnl *nl);
 int sd_rtnl_get_timeout(sd_rtnl *nl, uint64_t *timeout);
 int sd_rtnl_process(sd_rtnl *nl, sd_rtnl_message **ret);
diff --git a/src/test/test-rtnl-manual.c b/src/test/test-rtnl-manual.c
index c406454f77..9fc860d6a5 100644
--- a/src/test/test-rtnl-manual.c
+++ b/src/test/test-rtnl-manual.c
@@ -136,7 +136,7 @@ int main(int argc, char *argv[]) {
         sd_rtnl *rtnl;
         int r;
 
-        assert_se(sd_rtnl_open(&rtnl, 0) >= 0);
+        assert_se(sd_rtnl_open(&rtnl) >= 0);
         assert_se(rtnl);
 
         r = test_tunnel_configure(rtnl);