From e426876c708c54032fc141ec46c688544f9591d7 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Thu, 11 Dec 2025 18:14:14 +0100 Subject: [PATCH] device: use the internal ping implementation Currently NetworkManager depends on the external ping binary to perform the reachability check on IP addresses. This means that the NM daemon package must depend on another package. On Fedora the iputils package is 800KiB. Implement the same functionality natively so that we can drop such dependency. --- NEWS | 3 + src/core/devices/nm-device.c | 427 ++++++++++++++--------------------- 2 files changed, 168 insertions(+), 262 deletions(-) diff --git a/NEWS b/NEWS index f720a1bda6..eb9210cf09 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,9 @@ USE AT YOUR OWN RISK. NOT RECOMMENDED FOR PRODUCTION USE! the 802.1X certificates and keys set in the connection. * Introduce a libnm function that can be used by VPN plugins to check user permissions on certificate and keys. +* Use an internal implementation of the ping functionality when the + "connection.gateway-ping-timeout" or "connection.ip-ping-addresses" + properties are set, instead of relying on the "ping" tool. ============================================= NetworkManager-1.56 diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index f51aea53ae..de8edc8064 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -805,7 +805,7 @@ typedef struct _NMDevicePrivate { GVariant *ports_variant; /* Array of port devices D-Bus path */ char *prop_ip_iface; /* IP interface D-Bus property */ - GList *ping_operations; + CList ping_ops_lst_head; GSource *ping_timeout; } NMDevicePrivate; @@ -850,7 +850,6 @@ static const char *_activation_func_to_string(ActivationHandleFunc func); static void _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, gboolean quitting); static void queued_state_clear(NMDevice *device); -static void ip_check_ping_watch_cb(GPid pid, int status, gpointer user_data); static void nm_device_start_ip_check(NMDevice *self); static void realize_start_setup(NMDevice *self, const NMPlatformLink *plink, @@ -15381,36 +15380,16 @@ _dispatcher_complete_proceed_state(NMDispatcherCallId *call_id, gpointer user_da /*****************************************************************************/ typedef struct { - NMLogDomain log_domain; - NMDevice *device; - gboolean ping_addresses_require_all; - GSource *watch; - GPid pid; - char *binary; - char *address; - guint deadline; + char *addr_str; + NMIPAddrTyped addr_bin; + + NMLogDomain log_domain; + NMDevice *device; + GCancellable *cancellable; + gboolean require_all; + CList ping_ops_lst; } PingOperation; -static PingOperation * -ping_operation_new(NMDevice *self, - NMLogDomain log_domain, - const char *address, - const char *ping_binary, - guint ping_timeout, - gboolean ip_ping_addresses_require_all) -{ - PingOperation *ping_op = g_new0(PingOperation, 1); - - ping_op->device = self; - ping_op->log_domain = log_domain; - ping_op->address = g_strdup(address); - ping_op->binary = g_strdup(ping_binary); - ping_op->deadline = ping_timeout + 10; - ping_op->ping_addresses_require_all = ip_ping_addresses_require_all; - - return ping_op; -} - static void ip_check_pre_up(NMDevice *self) { @@ -15433,188 +15412,154 @@ ip_check_pre_up(NMDevice *self) } static void -cleanup_ping_operation(PingOperation *ping_op) +ping_op_cleanup(PingOperation *ping_op) { - if (ping_op->watch) { - nm_clear_g_source_inst(&ping_op->watch); - } - - if (ping_op->pid) { - nm_utils_kill_child_async(ping_op->pid, - SIGTERM, - ping_op->log_domain, - "ping", - 1000, - NULL, - NULL); - ping_op->pid = 0; - } - - nm_clear_g_free(&ping_op->binary); - nm_clear_g_free(&ping_op->address); + nm_clear_g_cancellable(&ping_op->cancellable); + nm_clear_g_free(&ping_op->addr_str); + c_list_unlink_stale(&ping_op->ping_ops_lst); g_free(ping_op); } -static gboolean -spawn_ping_for_operation(NMDevice *self, PingOperation *ping_op) +static void +ping_cleanup(NMDevice *self) { - gs_free char *str_timeout = NULL; - gs_free char *tmp_str = NULL; - const char *args[] = {ping_op->binary, - "-I", - nm_device_get_ip_iface(self), - "-c", - "1", - "-w", - NULL, - ping_op->address, - NULL}; - gs_free_error GError *error = NULL; - gboolean ret; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + PingOperation *ping_op; - args[6] = str_timeout = g_strdup_printf("%u", ping_op->deadline); - - tmp_str = g_strjoinv(" ", (char **) args); - _LOGD(ping_op->log_domain, "ping: running '%s'", tmp_str); - - ret = g_spawn_async("/", - (char **) args, - NULL, - G_SPAWN_DO_NOT_REAP_CHILD, - NULL, - NULL, - &ping_op->pid, - &error); - - if (ret) { - ping_op->watch = nm_g_child_watch_add_source(ping_op->pid, ip_check_ping_watch_cb, ping_op); - } else { - _LOGD(ping_op->log_domain, "ping: could not spawn %s: %s", ping_op->binary, error->message); + while ((ping_op = c_list_first_entry(&priv->ping_ops_lst_head, PingOperation, ping_ops_lst))) { + ping_op_cleanup(ping_op); } - return ret; -} - -static gboolean -respawn_ping_cb(gpointer user_data) -{ - PingOperation *ping_op = (PingOperation *) user_data; - NMDevice *self = ping_op->device; - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - - nm_clear_g_source_inst(&ping_op->watch); - - if (!spawn_ping_for_operation(self, ping_op)) { - priv->ping_operations = g_list_remove(priv->ping_operations, ping_op); - cleanup_ping_operation(ping_op); - - if (g_list_length(priv->ping_operations) == 0) { - ip_check_pre_up(self); - } - } - - return FALSE; + nm_clear_g_source_inst(&priv->ping_timeout); } static void -ip_check_ping_watch_cb(GPid pid, int status, gpointer user_data) +ping_host_cb(GObject *source, GAsyncResult *result, gpointer user_data) { - PingOperation *ping_op = (PingOperation *) user_data; - NMDevice *self = ping_op->device; - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - gboolean success = FALSE; + NMDevice *self; + NMDevicePrivate *priv; + PingOperation *ping_op = user_data; + gs_free_error GError *error = NULL; + gboolean success; + NMLogDomain log_domain; + gs_free char *addr_str = NULL; - if (!ping_op->watch) + success = nm_utils_ping_host_finish(result, &error); + if (nm_utils_error_is_cancelled(error)) return; - nm_clear_g_source_inst(&ping_op->watch); - ping_op->pid = 0; + self = NM_DEVICE(ping_op->device); + priv = NM_DEVICE_GET_PRIVATE(self); + log_domain = ping_op->log_domain; + addr_str = g_steal_pointer(&ping_op->addr_str); - if (WIFEXITED(status)) { - if (WEXITSTATUS(status) == 0) { - _LOGD(ping_op->log_domain, "ping: ping succeeded on %s", ping_op->address); - success = TRUE; - } else { - _LOGD(ping_op->log_domain, - "ping: ping failed with error code %d on %s", - WEXITSTATUS(status), - ping_op->address); - } - } else { - _LOGD(ping_op->log_domain, - "ping: stopped unexpectedly with status %d on %s", - status, - ping_op->address); + if (!success) { + /* it should never fail because we set an infinite timeout */ + nm_assert_not_reached(); + return; } - if (success) { - if (ping_op->ping_addresses_require_all) { - priv->ping_operations = g_list_remove(priv->ping_operations, ping_op); - if (g_list_length(priv->ping_operations) == 0) { - _LOGD(ping_op->log_domain, - "ping: ip-ping-addresses requires all, all ping checks on ip-ping-addresses " - "succeeded"); - if (priv->ping_timeout) - nm_clear_g_source_inst(&priv->ping_timeout); - ip_check_pre_up(self); - } - cleanup_ping_operation(ping_op); - } else { - nm_assert(priv->ping_operations); - - g_list_free_full(priv->ping_operations, (GDestroyNotify) cleanup_ping_operation); - priv->ping_operations = NULL; - - if (priv->ping_timeout) - nm_clear_g_source_inst(&priv->ping_timeout); - - _LOGD(ping_op->log_domain, - "ping: ip-ping-addresses requires any, one ping check on ip-ping-addresses " - "succeeded"); - ip_check_pre_up(self); + if (ping_op->require_all) { + ping_op_cleanup(ping_op); + if (!c_list_is_empty(&priv->ping_ops_lst_head)) { + _LOGD(log_domain, + "ping: check on address %s succeeded, waiting for other addresses", + addr_str); + return; } - } else { - /* If ping exited with an error it may have returned early, - * wait 1 second and restart it */ - ping_op->watch = nm_g_timeout_add_seconds_source(1, respawn_ping_cb, ping_op); } + + _LOGD(log_domain, "ping: check on address %s succeeded, continuing the activation", addr_str); + + ping_cleanup(self); + ip_check_pre_up(self); +} + +static void +ping_operation_start(NMDevice *self, + const char *addr_str, + NMIPAddrTyped *addr_bin, + gboolean require_all) +{ + PingOperation *ping_op; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + NMIPAddrTyped addr_bin_local; + char buf[NM_INET_ADDRSTRLEN]; + int addr_family; + int ret; + + /* Exactly one of the two must be set */ + nm_assert(!!addr_str ^ !!addr_bin); + + /* Derive the string address from the binary and vice versa */ + if (addr_str) { + ret = nm_inet_parse_bin_full(AF_UNSPEC, + FALSE, + addr_str, + &addr_family, + &addr_bin_local.addr.addr_ptr); + nm_assert(ret); + addr_bin_local.addr_family = addr_family; + addr_bin = &addr_bin_local; + } else { + nm_inet_ntop(addr_bin->addr_family, addr_bin->addr.addr_ptr, buf); + addr_str = buf; + } + + /* When pinging the gateway, the caller must ensure that the IP configuration is ready. + * For the ip-ping-addresses property, a valid connection always has may-fail=no for + * the families of all the target addresses. Thus, at this point the IP configuration + * must also be ready. */ + nm_assert(priv->ip_data_x[NM_IS_IPv4(addr_bin->addr_family)].state == NM_DEVICE_IP_STATE_READY); + + ping_op = g_new(PingOperation, 1); + *ping_op = (PingOperation) { + .device = self, + .cancellable = g_cancellable_new(), + .require_all = require_all, + .addr_bin = *addr_bin, + .addr_str = g_strdup(addr_str), + .log_domain = (addr_bin->addr_family == AF_INET) ? LOGD_IP4 : LOGD_IP6, + }; + + /* Start the asynchronous ping operation */ + nm_utils_ping_host(ping_op->addr_bin, + nm_device_get_ip_ifindex(self), + 0, /* try forever */ + ping_op->cancellable, + ping_host_cb, + ping_op); + + c_list_link_tail(&priv->ping_ops_lst_head, &ping_op->ping_ops_lst); } static gboolean ip_check_ping_timeout_cb(gpointer user_data) { - NMDevice *self = NM_DEVICE(user_data); - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + NMDevice *self = NM_DEVICE(user_data); + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); + PingOperation *ping_op; + nm_auto_free_gstring GString *str = NULL; - _LOGW(LOGD_DEVICE, "ping timeout: unreachable gateway or ip-ping-addresses"); - - if (priv->ping_operations) { - g_list_free_full(priv->ping_operations, (GDestroyNotify) cleanup_ping_operation); - priv->ping_operations = NULL; + if (_LOGW_ENABLED(LOGD_DEVICE)) { + str = g_string_new(""); + c_list_for_each_entry (ping_op, &priv->ping_ops_lst_head, ping_ops_lst) { + if (str->len != 0) + g_string_append(str, ", "); + g_string_append(str, ping_op->addr_str); + } + _LOGW(LOGD_DEVICE, + "ping: the following addresses were not reachable within the timeout: %s", + str->str); } - if (priv->ping_timeout) - nm_clear_g_source_inst(&priv->ping_timeout); + ping_cleanup(self); ip_check_pre_up(self); return FALSE; } -static gboolean -start_ping(NMDevice *self, PingOperation *ping_op) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE(self); - - if (spawn_ping_for_operation(self, ping_op)) { - priv->ping_operations = g_list_append(priv->ping_operations, ping_op); - return TRUE; - } - - cleanup_ping_operation(ping_op); - return FALSE; -} - static void nm_device_start_ip_check(NMDevice *self) { @@ -15623,17 +15568,13 @@ nm_device_start_ip_check(NMDevice *self) NMSettingConnection *s_con; guint gw_ping_timeout = 0; guint ip_ping_timeout = 0; - const char *ping_binary = NULL; - char buf[NM_INET_ADDRSTRLEN]; - NMLogDomain log_domain = LOGD_IP4; - gboolean ip_ping_addresses_require_all; - gboolean ping_started = FALSE; + gboolean require_all; + NMIPAddrTyped addr_bin = {}; /* Shouldn't be any active ping here, since IP_CHECK happens after the * first IP method completes. Any subsequently completing IP method doesn't - * get checked. - */ - g_return_if_fail(priv->ping_operations == NULL); + * get checked. */ + g_return_if_fail(c_list_is_empty(&priv->ping_ops_lst_head)); g_return_if_fail(priv->ip_data_4.state == NM_DEVICE_IP_STATE_READY || priv->ip_data_6.state == NM_DEVICE_IP_STATE_READY); @@ -15642,100 +15583,71 @@ nm_device_start_ip_check(NMDevice *self) s_con = nm_connection_get_setting_connection(connection); g_assert(s_con); - gw_ping_timeout = nm_setting_connection_get_gateway_ping_timeout(s_con); - ip_ping_addresses_require_all = _prop_get_connection_ip_ping_addresses_require_all(self, s_con); - ip_ping_timeout = nm_setting_connection_get_ip_ping_timeout(s_con); + gw_ping_timeout = nm_setting_connection_get_gateway_ping_timeout(s_con); + ip_ping_timeout = nm_setting_connection_get_ip_ping_timeout(s_con); + require_all = _prop_get_connection_ip_ping_addresses_require_all(self, s_con); - buf[0] = '\0'; - if (gw_ping_timeout != 0 && ip_ping_timeout == 0) { + /* the timeouts are mutually exclusive */ + nm_assert(gw_ping_timeout == 0 || ip_ping_timeout == 0); + + if (gw_ping_timeout > 0) { const NMPObject *gw; const NML3ConfigData *l3cd; - _LOGD(LOGD_DEVICE, "starting ping gateway..."); - l3cd = priv->l3cfg ? nm_l3cfg_get_combined_l3cd(priv->l3cfg, TRUE) : NULL; if (!l3cd) { /* pass */ } else if (priv->ip_data_4.state == NM_DEVICE_IP_STATE_READY) { gw = nm_l3_config_data_get_best_default_route(l3cd, AF_INET); if (gw) { - nm_inet4_ntop(NMP_OBJECT_CAST_IP4_ROUTE(gw)->gateway, buf); - ping_binary = nm_utils_find_helper("ping", "/usr/bin/ping", NULL); - log_domain = LOGD_IP4; + addr_bin.addr_family = AF_INET; + addr_bin.addr.addr4 = NMP_OBJECT_CAST_IP4_ROUTE(gw)->gateway; } } else if (priv->ip_data_6.state == NM_DEVICE_IP_STATE_READY) { gw = nm_l3_config_data_get_best_default_route(l3cd, AF_INET6); if (gw) { - nm_inet6_ntop(&NMP_OBJECT_CAST_IP6_ROUTE(gw)->gateway, buf); - ping_binary = nm_utils_find_helper("ping6", "/usr/bin/ping6", NULL); - log_domain = LOGD_IP6; + addr_bin.addr_family = AF_INET6; + addr_bin.addr.addr6 = NMP_OBJECT_CAST_IP6_ROUTE(gw)->gateway; } } - } - if (buf[0]) { - PingOperation *ping_op = ping_operation_new(self, - log_domain, - buf, - ping_binary, - gw_ping_timeout, - ip_ping_addresses_require_all); - - if (start_ping(self, ping_op)) - ping_started = TRUE; - } - - if (gw_ping_timeout == 0 && ip_ping_timeout != 0) { + if (addr_bin.addr_family != AF_UNSPEC) { + _LOGD(LOGD_DEVICE, + "starting ping on the IPv%c gateway with a %u seconds timeout", + nm_utils_addr_family_to_char(addr_bin.addr_family), + gw_ping_timeout); + ping_operation_start(self, NULL, &addr_bin, require_all); + } + } else if (ip_ping_timeout > 0) { const NML3ConfigData *l3cd; + GArray *ip_ping_addresses; + const char *const *strv; guint i; - GArray *ip_ping_addresses = _nm_setting_connection_get_ip_ping_addresses(s_con); - const char *const *strv = nm_strvarray_get_strv_notempty(ip_ping_addresses, NULL); - _LOGD(LOGD_DEVICE, "starting ping ip addresses..."); + ip_ping_addresses = _nm_setting_connection_get_ip_ping_addresses(s_con); + strv = nm_strvarray_get_strv_notnull(ip_ping_addresses, NULL); + + _LOGD(LOGD_DEVICE, + "starting ping on the ip-ping-addresses with a %u seconds timeout", + ip_ping_timeout); l3cd = priv->l3cfg ? nm_l3cfg_get_combined_l3cd(priv->l3cfg, TRUE) : NULL; - if (l3cd) { for (i = 0; strv[i]; i++) { - const char *s = strv[i]; - struct in_addr ipv4_addr; - struct in6_addr ipv6_addr; - - if (priv->ip_data_4.state == NM_DEVICE_IP_STATE_READY - && inet_pton(AF_INET, (const char *) s, &ipv4_addr)) { - ping_binary = nm_utils_find_helper("ping", "/usr/bin/ping", NULL); - log_domain = LOGD_IP4; - } else if (priv->ip_data_6.state == NM_DEVICE_IP_STATE_READY - && inet_pton(AF_INET6, (const char *) s, &ipv6_addr)) { - ping_binary = nm_utils_find_helper("ping6", "/usr/bin/ping6", NULL); - log_domain = LOGD_IP6; - } else - continue; - - if (s[0]) { - PingOperation *ping_op = ping_operation_new(self, - log_domain, - s, - ping_binary, - ip_ping_timeout, - ip_ping_addresses_require_all); - - if (start_ping(self, ping_op)) - ping_started = TRUE; - } + ping_operation_start(self, strv[i], NULL, require_all); } } } - if (ping_started) { + if (c_list_is_empty(&priv->ping_ops_lst_head)) { + /* No ping operation in progress, advance to pre-up */ + ip_check_pre_up(self); + } else { priv->ping_timeout = - nm_g_timeout_add_seconds_source(gw_ping_timeout ? gw_ping_timeout : ip_ping_timeout, + nm_g_timeout_add_seconds_source(ip_ping_timeout > 0 ? ip_ping_timeout : gw_ping_timeout, ip_check_ping_timeout_cb, self); } - /* If no ping was started, just advance to pre_up. */ - else - ip_check_pre_up(self); } /*****************************************************************************/ @@ -17194,17 +17106,11 @@ _cancel_activation(NMDevice *self) _dispatcher_cleanup(self); - if (priv->ping_operations) { - g_list_free_full(priv->ping_operations, (GDestroyNotify) cleanup_ping_operation); - priv->ping_operations = NULL; - } - - if (priv->ping_timeout) - nm_clear_g_source_inst(&priv->ping_timeout); - _dev_ip_state_cleanup(self, AF_INET, FALSE); _dev_ip_state_cleanup(self, AF_INET6, FALSE); + ping_cleanup(self); + /* Break the activation chain */ activation_source_clear(self); } @@ -17960,13 +17866,8 @@ _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, break; } case NM_DEVICE_STATE_SECONDARIES: - if (priv->ping_operations) { - g_list_free_full(priv->ping_operations, (GDestroyNotify) cleanup_ping_operation); - priv->ping_operations = NULL; - } - if (priv->ping_timeout) - nm_clear_g_source_inst(&priv->ping_timeout); _LOGD(LOGD_DEVICE, "device entered SECONDARIES state"); + ping_cleanup(self); break; default: break; @@ -19631,6 +19532,8 @@ nm_device_init(NMDevice *self) priv->available_connections = g_hash_table_new_full(nm_direct_hash, NULL, g_object_unref, NULL); priv->ip6_saved_properties = g_hash_table_new_full(nm_str_hash, g_str_equal, NULL, g_free); + c_list_init(&priv->ping_ops_lst_head); + priv->managed_type_ = NM_DEVICE_MANAGED_TYPE_EXTERNAL; /* If networking is already disabled at boot, we want to manage all devices * after re-enabling networking; hence, the initial state is MANAGED. */