From: Yu Watanabe Date: Mon, 27 May 2024 03:05:24 +0000 (+0900) Subject: udev: manage only socket address of device monitor X-Git-Tag: v257-rc1~772^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=98ca8eac110dded71872ff671591579a747f41ac;p=thirdparty%2Fsystemd.git 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 43cc1914e01..e37b36a8d33 100644 --- 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 56f8836ef31..c4113ae1b5f 100644 --- 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 50505a5b121..651490ad1cb 100644 --- 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 e44b7eb0ceb..5d0d6543c79 100644 --- 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);