udev: manage only socket address of device monitor

Previously, the main process of systemd-udevd manages worker process
with their sd_device_monitor object to save the destination address.
Let's save only destination address, and drop worker's sd_device_monitor
object.

diff --git a/src/libsystemd/sd-device/device-monitor-private.h b/src/libsystemd/sd-device/device-monitor-private.h
index 43cc1914e01fa31ed92bb2c4dbfffdbf29b1d385..e37b36a8d33abf17c48ff34a66c6a86b6289391d 100644 (file)
--- a/src/libsystemd/sd-device/device-monitor-private.h
+++ b/src/libsystemd/sd-device/device-monitor-private.h
@@ -20,5 +20,5 @@ int device_monitor_disconnect(sd_device_monitor *m);
 int device_monitor_get_address(sd_device_monitor *m, union sockaddr_union *ret);
 int device_monitor_allow_unicast_sender(sd_device_monitor *m, sd_device_monitor *sender);
 int device_monitor_get_fd(sd_device_monitor *m);
-int device_monitor_send_device(sd_device_monitor *m, sd_device_monitor *destination, sd_device *device);
+int device_monitor_send_device(sd_device_monitor *m, const union sockaddr_union *destination, sd_device *device);
 int device_monitor_receive_device(sd_device_monitor *m, sd_device **ret);

diff --git a/src/libsystemd/sd-device/device-monitor.c b/src/libsystemd/sd-device/device-monitor.c
index 56f8836ef317452698333d554fac95c81129f1c4..c4113ae1b5fedb98d7b6a07cced3714b7ad71517 100644 (file)
--- a/src/libsystemd/sd-device/device-monitor.c
+++ b/src/libsystemd/sd-device/device-monitor.c
@@ -694,7 +694,7 @@ static uint64_t string_bloom64(const char *str) {
 
 int device_monitor_send_device(
                 sd_device_monitor *m,
-                sd_device_monitor *destination,
+                const union sockaddr_union *destination,
                 sd_device *device) {
 
         monitor_netlink_header nlh = {
@@ -710,7 +710,7 @@ int device_monitor_send_device(
                 .msg_iovlen = 2,
         };
         /* default destination for sending */
-        union sockaddr_union default_destination = {
+        static const union sockaddr_union default_destination = {
                 .nl.nl_family = AF_NETLINK,
                 .nl.nl_groups = MONITOR_GROUP_UDEV,
         };
@@ -760,7 +760,7 @@ int device_monitor_send_device(
          * If we send to a multicast group, we will get
          * ECONNREFUSED, which is expected.
          */
-        smsg.msg_name = destination ? &destination->snl : &default_destination;
+        smsg.msg_name = (struct sockaddr_nl*) &(destination ?: &default_destination)->nl;
         smsg.msg_namelen = sizeof(struct sockaddr_nl);
         count = sendmsg(m->sock, &smsg, 0);
         if (count < 0) {

diff --git a/src/libsystemd/sd-device/test-sd-device-monitor.c b/src/libsystemd/sd-device/test-sd-device-monitor.c
index 50505a5b121fcaaa26e6161757047cc1250ce38a..651490ad1cb31dc772c1c78735ecd0e544b6109b 100644 (file)
--- a/src/libsystemd/sd-device/test-sd-device-monitor.c
+++ b/src/libsystemd/sd-device/test-sd-device-monitor.c
@@ -52,7 +52,7 @@ static int monitor_handler(sd_device_monitor *m, sd_device *d, void *userdata) {
         return sd_event_exit(sd_device_monitor_get_event(m), 100);
 }
 
-static void prepare_monitor(sd_device_monitor **ret_server, sd_device_monitor **ret_client) {
+static void prepare_monitor(sd_device_monitor **ret_server, sd_device_monitor **ret_client, union sockaddr_union *ret_address) {
         _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL;
 
         ASSERT_OK(device_monitor_new_full(&monitor_server, MONITOR_GROUP_NONE, -1));
@@ -62,6 +62,7 @@ static void prepare_monitor(sd_device_monitor **ret_server, sd_device_monitor **
         ASSERT_OK(device_monitor_new_full(&monitor_client, MONITOR_GROUP_NONE, -1));
         ASSERT_OK(sd_device_monitor_set_description(monitor_client, "client"));
         ASSERT_OK(device_monitor_allow_unicast_sender(monitor_client, monitor_server));
+        ASSERT_OK(device_monitor_get_address(monitor_client, ret_address));
 
         *ret_server = TAKE_PTR(monitor_server);
         *ret_client = TAKE_PTR(monitor_client);
@@ -69,7 +70,7 @@ static void prepare_monitor(sd_device_monitor **ret_server, sd_device_monitor **
 
 static void send_by_enumerator(
                 sd_device_monitor *monitor_server,
-                sd_device_monitor *monitor_client,
+                const union sockaddr_union *address,
                 sd_device_enumerator *e,
                 size_t n,
                 const char *syspath_filter) {
@@ -89,7 +90,7 @@ static void send_by_enumerator(
                 ASSERT_OK(device_add_property(d, "SEQNUM", "10"));
 
                 log_device_debug(d, "Sending device subsystem:%s syspath:%s", s, p);
-                ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, d));
+                ASSERT_OK(device_monitor_send_device(monitor_server, address, d));
 
                 /* The sysattr and parent filters are not implemented in BPF yet. So, sending multiple
                  * devices may fills up buffer and device_monitor_send_device() may return EAGAIN. Let's
@@ -115,6 +116,7 @@ TEST(sd_device_monitor_is_running) {
 TEST(refuse_invalid_device) {
         _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL;
         _cleanup_(sd_device_unrefp) sd_device *loopback = NULL;
+        union sockaddr_union sa;
         const char *syspath;
 
         /* Try to send device with invalid action and without seqnum. */
@@ -123,23 +125,24 @@ TEST(refuse_invalid_device) {
 
         ASSERT_OK(sd_device_get_syspath(loopback, &syspath));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, loopback));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, loopback));
         ASSERT_OK(sd_event_run(sd_device_monitor_get_event(monitor_client), 0));
 }
 
 static void test_send_receive_one(sd_device *device, bool subsystem_filter, bool tag_filter, bool use_bpf) {
         _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL;
         const char *syspath, *subsystem, *devtype = NULL;
+        union sockaddr_union sa;
 
         log_device_info(device, "/* %s(subsystem_filter=%s, tag_filter=%s, use_bpf=%s) */", __func__,
                         true_false(subsystem_filter), true_false(tag_filter), true_false(use_bpf));
 
         ASSERT_OK(sd_device_get_syspath(device, &syspath));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         if (subsystem_filter) {
                 ASSERT_OK(sd_device_get_subsystem(device, &subsystem));
@@ -155,7 +158,7 @@ static void test_send_receive_one(sd_device *device, bool subsystem_filter, bool
                 ASSERT_OK(sd_device_monitor_filter_update(monitor_client));
 
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100);
 }
 
@@ -190,23 +193,24 @@ TEST(sd_device_monitor_filter_add_match_subsystem_devtype) {
         _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL;
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
         const char *syspath, *subsystem;
+        union sockaddr_union sa;
 
         prepare_loopback(&device);
 
         ASSERT_OK(sd_device_get_syspath(device, &syspath));
         ASSERT_OK(sd_device_get_subsystem(device, &subsystem));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         ASSERT_OK(sd_device_monitor_filter_add_match_subsystem_devtype(monitor_client, subsystem, NULL));
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
 
         ASSERT_OK(sd_device_enumerator_new(&e));
         ASSERT_OK(sd_device_enumerator_add_match_subsystem(e, subsystem, false));
-        send_by_enumerator(monitor_server, monitor_client, e, SIZE_MAX, NULL);
+        send_by_enumerator(monitor_server, &sa, e, SIZE_MAX, NULL);
 
         log_device_info(device, "Sending device subsystem:%s syspath:%s", subsystem, syspath);
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100);
 }
 
@@ -214,22 +218,23 @@ TEST(sd_device_monitor_filter_add_match_tag) {
         _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL;
         _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL;
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
+        union sockaddr_union sa;
         const char *syspath;
 
         prepare_loopback(&device);
 
         ASSERT_OK(sd_device_get_syspath(device, &syspath));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         ASSERT_OK(sd_device_monitor_filter_add_match_tag(monitor_client, "TEST_SD_DEVICE_MONITOR"));
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
 
         ASSERT_OK(sd_device_enumerator_new(&e));
-        send_by_enumerator(monitor_server, monitor_client, e, SIZE_MAX, NULL);
+        send_by_enumerator(monitor_server, &sa, e, SIZE_MAX, NULL);
 
         log_device_info(device, "Sending device syspath:%s", syspath);
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100);
 }
 
@@ -239,23 +244,24 @@ TEST(sd_device_monitor_filter_add_match_sysattr) {
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
         static const char *sysattr = "ifindex";
         const char *syspath, *sysattr_value;
+        union sockaddr_union sa;
 
         prepare_loopback(&device);
 
         ASSERT_OK(sd_device_get_syspath(device, &syspath));
         ASSERT_OK(sd_device_get_sysattr_value(device, sysattr, &sysattr_value));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         ASSERT_OK(sd_device_monitor_filter_add_match_sysattr(monitor_client, sysattr, sysattr_value, true));
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
 
         ASSERT_OK(sd_device_enumerator_new(&e));
         ASSERT_OK(sd_device_enumerator_add_match_sysattr(e, sysattr, sysattr_value, false));
-        send_by_enumerator(monitor_server, monitor_client, e, 5, NULL);
+        send_by_enumerator(monitor_server, &sa, e, 5, NULL);
 
         log_device_info(device, "Sending device syspath:%s", syspath);
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100);
 }
 
@@ -264,6 +270,7 @@ TEST(sd_device_monitor_add_match_parent) {
         _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL;
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
         const char *syspath, *parent_syspath;
+        union sockaddr_union sa;
         sd_device *parent;
         int r;
 
@@ -279,39 +286,40 @@ TEST(sd_device_monitor_add_match_parent) {
 
         ASSERT_OK(sd_device_get_syspath(parent, &parent_syspath));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         ASSERT_OK(sd_device_monitor_filter_add_match_parent(monitor_client, parent, true));
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
 
         ASSERT_OK(sd_device_enumerator_new(&e));
-        send_by_enumerator(monitor_server, monitor_client, e, 5, parent_syspath);
+        send_by_enumerator(monitor_server, &sa, e, 5, parent_syspath);
 
         log_device_info(device, "Sending device syspath:%s", syspath);
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100);
 }
 
 TEST(sd_device_monitor_filter_remove) {
         _cleanup_(sd_device_monitor_unrefp) sd_device_monitor *monitor_server = NULL, *monitor_client = NULL;
         _cleanup_(sd_device_unrefp) sd_device *device = NULL;
+        union sockaddr_union sa;
         const char *syspath;
 
         prepare_loopback(&device);
 
         ASSERT_OK(sd_device_get_syspath(device, &syspath));
 
-        prepare_monitor(&monitor_server, &monitor_client);
+        prepare_monitor(&monitor_server, &monitor_client, &sa);
 
         ASSERT_OK(sd_device_monitor_filter_add_match_subsystem_devtype(monitor_client, "hoge", NULL));
         ASSERT_OK(sd_device_monitor_start(monitor_client, monitor_handler, (void *) syspath));
 
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_OK(sd_event_run(sd_device_monitor_get_event(monitor_client), 0));
 
         ASSERT_OK(sd_device_monitor_filter_remove(monitor_client));
 
-        ASSERT_OK(device_monitor_send_device(monitor_server, monitor_client, device));
+        ASSERT_OK(device_monitor_send_device(monitor_server, &sa, device));
         ASSERT_EQ(sd_event_loop(sd_device_monitor_get_event(monitor_client)), 100);
 }
 

diff --git a/src/udev/udev-manager.c b/src/udev/udev-manager.c
index e44b7eb0ceb3e1dcdc438fa6c6466d29fc07edf8..5d0d6543c7949da355ceda341d15c55adba9fe26 100644 (file)
--- a/src/udev/udev-manager.c
+++ b/src/udev/udev-manager.c
@@ -84,7 +84,7 @@ typedef struct Worker {
         Manager *manager;
         pid_t pid;
         sd_event_source *child_event_source;
-        sd_device_monitor *monitor;
+        union sockaddr_union address;
         WorkerState state;
         Event *event;
 } Worker;
@@ -125,7 +125,6 @@ static Worker *worker_free(Worker *worker) {
                 hashmap_remove(worker->manager->workers, PID_TO_PTR(worker->pid));
 
         sd_event_source_unref(worker->child_event_source);
-        sd_device_monitor_unref(worker->monitor);
         event_free(worker->event);
 
         return mfree(worker);
@@ -173,18 +172,18 @@ static int worker_new(Worker **ret, Manager *manager, sd_device_monitor *worker_
         assert(worker_monitor);
         assert(pid > 1);
 
-        /* close monitor, but keep address around */
-        device_monitor_disconnect(worker_monitor);
-
         worker = new(Worker, 1);
         if (!worker)
                 return -ENOMEM;
 
         *worker = (Worker) {
-                .monitor = sd_device_monitor_ref(worker_monitor),
                 .pid = pid,
         };
 
+        r = device_monitor_get_address(worker_monitor, &worker->address);
+        if (r < 0)
+                return r;
+
         r = sd_event_add_child(manager->event, &worker->child_event_source, pid, WEXITED, on_sigchld, worker);
         if (r < 0)
                 return r;
@@ -451,7 +450,7 @@ static int event_run(Event *event) {
                 if (worker->state != WORKER_IDLE)
                         continue;
 
-                r = device_monitor_send_device(manager->monitor, worker->monitor, event->dev);
+                r = device_monitor_send_device(manager->monitor, &worker->address, event->dev);
                 if (r < 0) {
                         log_device_error_errno(event->dev, r, "Worker ["PID_FMT"] did not accept message, killing the worker: %m",
                                                worker->pid);