From 36fb335b286be0de403d58d368c6945b4a72c2a1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 30 Oct 2023 17:59:15 +0100 Subject: [PATCH 1/5] device/ethernet: honor ID_NET_AUTO_LINK_LOCAL_ONLY udev property We honored "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY" udev property, for when we generate a "Wired connection 1" (aka the "auto-default connection"). Systemd now also honors and may set ID_NET_AUTO_LINK_LOCAL_ONLY for a similar purpose. Honore that too. The NM specific variable still is preferred, also because "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY" is about something very NetworkManager specific (controlling "Wired connection 1"). Maybe one day, we should drop "data/90-nm-thunderbolt.rules" and only rely on what systemd provides. But not yet. See-also: https://github.com/systemd/systemd/pull/29767 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1413 --- NEWS | 4 +++- man/NetworkManager.xml | 18 ++++++++++++++++++ src/core/devices/nm-device-ethernet.c | 11 +++++++---- 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/NEWS b/NEWS index 6d479faa75..0079fd8487 100644 --- a/NEWS +++ b/NEWS @@ -1,11 +1,13 @@ ============================================= NetworkManager-1.46 -Overview of changes since NetworkManager-1.46 +Overview of changes since NetworkManager-1.44 ============================================= * Change internal ABI of NMSetting types and NMSimpleConnection. The layout of these structs was already hidden from public headers since 1.34 and this change should not be noticeable. +* Honor udev property ID_NET_AUTO_LINK_LOCAL_ONLY=1 for enabling + link local addresses on default wired connection. ============================================= NetworkManager-1.44 diff --git a/man/NetworkManager.xml b/man/NetworkManager.xml index bd5429952f..455925ab74 100644 --- a/man/NetworkManager.xml +++ b/man/NetworkManager.xml @@ -193,6 +193,24 @@ + + NM_AUTO_DEFAULT_LINK_LOCAL_ONLY + + If set to "1" or "true", the + automatically generated connections "Wired connection N" will only + enable link local addressing for IPv4 and IPv6. This can be useful + on thunderbolt devices or host-to-host USB devices. + + + + + ID_NET_AUTO_LINK_LOCAL_ONLY + + Honored and treated the same as if NM_AUTO_DEFAULT_LINK_LOCAL_ONLY + were set. + + + diff --git a/src/core/devices/nm-device-ethernet.c b/src/core/devices/nm-device-ethernet.c index aedacc248b..1b6101d107 100644 --- a/src/core/devices/nm-device-ethernet.c +++ b/src/core/devices/nm-device-ethernet.c @@ -1707,6 +1707,7 @@ complete_connection(NMDevice *device, static NMConnection * new_default_connection(NMDevice *self) { + const char *link_local_only = NULL; NMConnection *connection; NMSettingsConnection *const *connections; NMSetting *setting; @@ -1714,7 +1715,6 @@ new_default_connection(NMDevice *self) struct udev_device *dev; const char *perm_hw_addr; const char *iface; - const char *uprop = "0"; gs_free char *defname = NULL; gs_free char *uuid = NULL; guint i, n_connections; @@ -1763,10 +1763,13 @@ new_default_connection(NMDevice *self) /* Check if we should create a Link-Local only connection */ dev = nm_platform_link_get_udev_device(nm_device_get_platform(NM_DEVICE(self)), nm_device_get_ip_ifindex(self)); - if (dev) - uprop = udev_device_get_property_value(dev, "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY"); + if (dev) { + link_local_only = udev_device_get_property_value(dev, "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY"); + if (!link_local_only) + link_local_only = udev_device_get_property_value(dev, "ID_NET_AUTO_LINK_LOCAL_ONLY"); + } - if (_nm_utils_ascii_str_to_bool(uprop, FALSE)) { + if (_nm_utils_ascii_str_to_bool(link_local_only, FALSE)) { setting = nm_setting_ip4_config_new(); g_object_set(setting, NM_SETTING_IP_CONFIG_METHOD, From 74cb2400406a78d6b9b554f2890b1c823b011481 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 31 Oct 2023 12:22:28 +0100 Subject: [PATCH 2/5] man: document "ID_NET_DHCP_BROADCAST" udev property https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/issues/1414 --- man/NetworkManager.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/man/NetworkManager.xml b/man/NetworkManager.xml index 455925ab74..5c17bd52fc 100644 --- a/man/NetworkManager.xml +++ b/man/NetworkManager.xml @@ -211,6 +211,15 @@ + + ID_NET_DHCP_BROADCAST + + If set to "1" or "true", use + broadcast requests for DHCPv4 offers. This can make sense of devices + that can't handle unicast messages until being configured. + + + From 175865c8d310098c5de87c7c439bf36041256f87 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 31 Oct 2023 12:07:20 +0100 Subject: [PATCH 3/5] core: refactor nm_platform_link_get_unmanaged() to return ternary value It seems easier to understand this way and to implement. Next, another udev property will be honored. In light of that, the change makes more sense. --- src/core/devices/nm-device.c | 17 ++++++++++------- src/libnm-platform/nm-platform.c | 27 +++++++++++++++------------ src/libnm-platform/nm-platform.h | 6 +++--- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index b8c2f88cc7..74a967e123 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -15097,20 +15097,23 @@ nm_device_set_unmanaged_by_user_settings(NMDevice *self, gboolean now) void nm_device_set_unmanaged_by_user_udev(NMDevice *self) { - int ifindex; - gboolean platform_unmanaged = FALSE; + NMOptionBool platform_unmanaged; + int ifindex; ifindex = self->_priv->ifindex; - if (ifindex <= 0 - || !nm_platform_link_get_unmanaged(nm_device_get_platform(self), - ifindex, - &platform_unmanaged)) + if (ifindex <= 0) + return; + + platform_unmanaged = nm_platform_link_get_unmanaged(nm_device_get_platform(self), ifindex); + if (platform_unmanaged == NM_OPTION_BOOL_DEFAULT) return; nm_device_set_unmanaged_by_flags(self, NM_UNMANAGED_USER_UDEV, - platform_unmanaged, + platform_unmanaged == NM_OPTION_BOOL_TRUE + ? NM_UNMAN_FLAG_OP_SET_UNMANAGED + : NM_UNMAN_FLAG_OP_SET_MANAGED, NM_DEVICE_STATE_REASON_USER_REQUESTED); } diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index 1b513d34be..e55c749931 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -1636,7 +1636,7 @@ nm_platform_link_get_udev_property(NMPlatform *self, const char *name, const char **out_value) { - struct udev_device *udevice = NULL; + struct udev_device *udevice; const char *uproperty; udevice = nm_platform_link_get_udev_device(self, ifindex); @@ -1655,22 +1655,25 @@ nm_platform_link_get_udev_property(NMPlatform *self, * nm_platform_link_get_unmanaged: * @self: platform instance * @ifindex: interface index - * @unmanaged: management status (in case %TRUE is returned) * - * Returns: %TRUE if platform overrides NM default-unmanaged status, - * %FALSE otherwise (with @unmanaged unmodified). + * Returns: %NM_OPTION_BOOL_DEFAULT if the udev property NM_UNMANAGED + * is not set. Otherwise, return NM_UNMANAGED as boolean. */ -gboolean -nm_platform_link_get_unmanaged(NMPlatform *self, int ifindex, gboolean *unmanaged) +NMOptionBool +nm_platform_link_get_unmanaged(NMPlatform *self, int ifindex) { - const char *value; + struct udev_device *udevice; + const char *val; - if (nm_platform_link_get_udev_property(self, ifindex, "NM_UNMANAGED", &value)) { - NM_SET_OUT(unmanaged, _nm_utils_ascii_str_to_bool(value, FALSE)); - return TRUE; - } + udevice = nm_platform_link_get_udev_device(self, ifindex); + if (!udevice) + return NM_OPTION_BOOL_DEFAULT; - return FALSE; + val = udev_device_get_property_value(udevice, "NM_UNMANAGED"); + if (val) + return _nm_utils_ascii_str_to_bool(val, FALSE); + + return NM_OPTION_BOOL_DEFAULT; } /** diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index bc5bc5e8b4..81d2b9d6fe 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -1941,9 +1941,9 @@ int nm_platform_link_get_master(NMPlatform *self, int slave); gboolean nm_platform_link_can_assume(NMPlatform *self, int ifindex); -gboolean nm_platform_link_get_unmanaged(NMPlatform *self, int ifindex, gboolean *unmanaged); -gboolean nm_platform_link_supports_slaves(NMPlatform *self, int ifindex); -const char *nm_platform_link_get_type_name(NMPlatform *self, int ifindex); +NMOptionBool nm_platform_link_get_unmanaged(NMPlatform *self, int ifindex); +gboolean nm_platform_link_supports_slaves(NMPlatform *self, int ifindex); +const char *nm_platform_link_get_type_name(NMPlatform *self, int ifindex); gboolean nm_platform_link_refresh(NMPlatform *self, int ifindex); void nm_platform_process_events(NMPlatform *self); From 86db3df6ac331558b8cf48ab3747cabc4f3c249c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 31 Oct 2023 12:11:55 +0100 Subject: [PATCH 4/5] core: honor ID_NET_MANAGED_BY="org.freedesktop.NetworkManager" to manage device If ID_NET_MANAGED_BY= attribute is set, we have an indication who is responsible for the device. If this is set to anything but "org.freedesktop.NetworkManager", then the device is unmanaged. The effect is the same as setting NM_UNMANAGED= attribute. NM_UNMANAGED= takes precedence over this setting. See-also: https://github.com/systemd/systemd/issues/29768 See-also: https://github.com/systemd/systemd/pull/29782 --- NEWS | 2 ++ man/NetworkManager.xml | 9 +++++++++ src/libnm-platform/nm-platform.c | 9 +++++++++ 3 files changed, 20 insertions(+) diff --git a/NEWS b/NEWS index 0079fd8487..e7838bd35f 100644 --- a/NEWS +++ b/NEWS @@ -8,6 +8,8 @@ Overview of changes since NetworkManager-1.44 should not be noticeable. * Honor udev property ID_NET_AUTO_LINK_LOCAL_ONLY=1 for enabling link local addresses on default wired connection. +* Honor udev property ID_NET_MANAGED_BY to only manage an interface + when set to "org.freedesktop.NetworkManager". ============================================= NetworkManager-1.44 diff --git a/man/NetworkManager.xml b/man/NetworkManager.xml index 5c17bd52fc..ff1635cd29 100644 --- a/man/NetworkManager.xml +++ b/man/NetworkManager.xml @@ -193,6 +193,15 @@ + + ID_NET_MANAGED_BY + + If NM_UNMANAGED is set, this has no effect. Otherwise, + if the attribute is set to anything but + "org.freedesktop.NetworkManager", the device is unmanaged. + + + NM_AUTO_DEFAULT_LINK_LOCAL_ONLY diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index e55c749931..d5acec98f5 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -1673,6 +1673,15 @@ nm_platform_link_get_unmanaged(NMPlatform *self, int ifindex) if (val) return _nm_utils_ascii_str_to_bool(val, FALSE); + val = udev_device_get_property_value(udevice, "ID_NET_MANAGED_BY"); + if (val) { + if (!nm_streq(val, "org.freedesktop.NetworkManager")) { + /* There is another manager. UNMANAGED. */ + return TRUE; + } + return FALSE; + } + return NM_OPTION_BOOL_DEFAULT; } From a7259047b15a13ecaae65b20a4b19b82cb5a918a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Tue, 31 Oct 2023 13:23:22 +0100 Subject: [PATCH 5/5] auto-default: check NM_AUTO_DEFAULT_LINK_LOCAL_ONLY from nm-device When creating default connections automatically, we check if udev has set the NM_AUTO_DEFAULT_LINK_LOCAL_ONLY variable, and if so, we create the connection with method=link-local. It was checked only for ethernet connection type, which works fine because it's the only device type that we create connections automatically for. However, link-local connections are not specific to Ethernet, and if we add auto-default connections for more devices in the future we should honor this configuration too. Do it from nm-device, but only if the device class has defined a "new_default_connection" method so the behaviour is identical as the current one, but will be used by future implementors of this method too. See-also: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1780 https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1779 --- src/core/devices/nm-device-ethernet.c | 29 --------------------- src/core/devices/nm-device.c | 37 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 29 deletions(-) diff --git a/src/core/devices/nm-device-ethernet.c b/src/core/devices/nm-device-ethernet.c index 1b6101d107..5d58b69d67 100644 --- a/src/core/devices/nm-device-ethernet.c +++ b/src/core/devices/nm-device-ethernet.c @@ -1707,12 +1707,10 @@ complete_connection(NMDevice *device, static NMConnection * new_default_connection(NMDevice *self) { - const char *link_local_only = NULL; NMConnection *connection; NMSettingsConnection *const *connections; NMSetting *setting; gs_unref_hashtable GHashTable *existing_ids = NULL; - struct udev_device *dev; const char *perm_hw_addr; const char *iface; gs_free char *defname = NULL; @@ -1760,33 +1758,6 @@ new_default_connection(NMDevice *self) iface, NULL); - /* Check if we should create a Link-Local only connection */ - dev = nm_platform_link_get_udev_device(nm_device_get_platform(NM_DEVICE(self)), - nm_device_get_ip_ifindex(self)); - if (dev) { - link_local_only = udev_device_get_property_value(dev, "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY"); - if (!link_local_only) - link_local_only = udev_device_get_property_value(dev, "ID_NET_AUTO_LINK_LOCAL_ONLY"); - } - - if (_nm_utils_ascii_str_to_bool(link_local_only, FALSE)) { - setting = nm_setting_ip4_config_new(); - g_object_set(setting, - NM_SETTING_IP_CONFIG_METHOD, - NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL, - NULL); - nm_connection_add_setting(connection, setting); - - setting = nm_setting_ip6_config_new(); - g_object_set(setting, - NM_SETTING_IP_CONFIG_METHOD, - NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL, - NM_SETTING_IP_CONFIG_MAY_FAIL, - TRUE, - NULL); - nm_connection_add_setting(connection, setting); - } - return connection; } diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 74a967e123..8031bd5dd4 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -22,6 +22,7 @@ #include #include #include +#include #include "libnm-std-aux/unaligned.h" #include "libnm-glib-aux/nm-uuid.h" @@ -8098,6 +8099,40 @@ nm_device_owns_iface(NMDevice *self, const char *iface) return FALSE; } +static void +apply_udev_auto_default_configs(NMDevice *self, NMConnection *connection) +{ + struct udev_device *dev; + const char *uprop; + NMSetting *setting; + + dev = nm_platform_link_get_udev_device(nm_device_get_platform(NM_DEVICE(self)), + nm_device_get_ip_ifindex(self)); + if (!dev) + return; + + uprop = udev_device_get_property_value(dev, "NM_AUTO_DEFAULT_LINK_LOCAL_ONLY"); + uprop = uprop ?: udev_device_get_property_value(dev, "ID_NET_AUTO_LINK_LOCAL_ONLY"); + + if (_nm_utils_ascii_str_to_bool(uprop, FALSE)) { + setting = nm_setting_ip4_config_new(); + g_object_set(setting, + NM_SETTING_IP_CONFIG_METHOD, + NM_SETTING_IP4_CONFIG_METHOD_LINK_LOCAL, + NULL); + nm_connection_add_setting(connection, setting); + + setting = nm_setting_ip6_config_new(); + g_object_set(setting, + NM_SETTING_IP_CONFIG_METHOD, + NM_SETTING_IP6_CONFIG_METHOD_LINK_LOCAL, + NM_SETTING_IP_CONFIG_MAY_FAIL, + TRUE, + NULL); + nm_connection_add_setting(connection, setting); + } +} + NMConnection * nm_device_new_default_connection(NMDevice *self) { @@ -8111,6 +8146,8 @@ nm_device_new_default_connection(NMDevice *self) if (!connection) return NULL; + apply_udev_auto_default_configs(self, connection); + if (!nm_connection_normalize(connection, NULL, NULL, &error)) { _LOGD(LOGD_DEVICE, "device generated an invalid default connection: %s", error->message); g_error_free(error);