From e2dd68b610c3428ca3224b6402bea3fd6517fd70 Mon Sep 17 00:00:00 2001 From: Ana Cabral Date: Fri, 7 Oct 2022 20:18:43 +0200 Subject: [PATCH 1/5] device: allow configuration of VLAN on an unmanaged interface It is not possible to configure a VLAN interface on unmanaged NIC. This forces users who only want to create a VLAN interface to take ownership over possibly shared underlying NIC. In OpenShift, the SR-IOV operator is currently not using NetworkManager to configure VFs. When it starts working with a NIC, it explicitly makes it unmanaged. Then, users cannot create a VLAN interface on PFs managed by the operator. This commit eliminates this issue by allowing configuring VLAN on an interface without requesting it to be managed by NetworkManager. This commit is part of a broader change that eliminates inheriting the unmanaged condition from the parent of a device, for all device types: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1418 https://bugzilla.redhat.com/show_bug.cgi?id=2110307 --- src/core/devices/nm-device-vlan.c | 35 ------------------------------- 1 file changed, 35 deletions(-) diff --git a/src/core/devices/nm-device-vlan.c b/src/core/devices/nm-device-vlan.c index 0c101c70b5..faaa62f53c 100644 --- a/src/core/devices/nm-device-vlan.c +++ b/src/core/devices/nm-device-vlan.c @@ -31,7 +31,6 @@ NM_GOBJECT_PROPERTIES_DEFINE(NMDeviceVlan, PROP_VLAN_ID, ); typedef struct { - gulong parent_state_id; gulong parent_hwaddr_id; gulong parent_mtu_id; guint vlan_id; @@ -53,25 +52,6 @@ G_DEFINE_TYPE(NMDeviceVlan, nm_device_vlan, NM_TYPE_DEVICE) /*****************************************************************************/ -static void -parent_state_changed(NMDevice *parent, - NMDeviceState new_state, - NMDeviceState old_state, - NMDeviceStateReason reason, - gpointer user_data) -{ - NMDeviceVlan *self = NM_DEVICE_VLAN(user_data); - - /* We'll react to our own carrier state notifications. Ignore the parent's. */ - if (nm_device_state_reason_check(reason) == NM_DEVICE_STATE_REASON_CARRIER) - return; - - nm_device_set_unmanaged_by_flags(NM_DEVICE(self), - NM_UNMANAGED_PARENT, - !nm_device_get_managed(parent, FALSE), - reason); -} - static void parent_mtu_maybe_changed(NMDevice *parent, GParamSpec *pspec, gpointer user_data) { @@ -132,19 +112,10 @@ parent_changed_notify(NMDevice *device, NM_DEVICE_CLASS(nm_device_vlan_parent_class) ->parent_changed_notify(device, old_ifindex, old_parent, new_ifindex, new_parent); - /* note that @self doesn't have to clear @parent_state_id on dispose, - * because NMDevice's dispose() will unset the parent, which in turn calls - * parent_changed_notify(). */ - nm_clear_g_signal_handler(old_parent, &priv->parent_state_id); nm_clear_g_signal_handler(old_parent, &priv->parent_hwaddr_id); nm_clear_g_signal_handler(old_parent, &priv->parent_mtu_id); if (new_parent) { - priv->parent_state_id = g_signal_connect(new_parent, - NM_DEVICE_STATE_CHANGED, - G_CALLBACK(parent_state_changed), - device); - priv->parent_hwaddr_id = g_signal_connect(new_parent, "notify::" NM_DEVICE_HW_ADDRESS, G_CALLBACK(parent_hwaddr_maybe_changed), @@ -156,12 +127,6 @@ parent_changed_notify(NMDevice *device, G_CALLBACK(parent_mtu_maybe_changed), device); parent_mtu_maybe_changed(new_parent, NULL, self); - - /* Set parent-dependent unmanaged flag */ - nm_device_set_unmanaged_by_flags(device, - NM_UNMANAGED_PARENT, - !nm_device_get_managed(new_parent, FALSE), - NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED); } /* Recheck availability now that the parent has changed */ From f1a79e97ea07df1a746b873edbc59d8fda38d33e Mon Sep 17 00:00:00 2001 From: Ana Cabral Date: Wed, 12 Oct 2022 18:43:03 +0200 Subject: [PATCH 2/5] device: remove the unmanaged inheritance from the parent for 6lowpan devices This commit is part of a broader change that eliminates inheriting the unmanaged condition from the parent of a device, for all device types: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1418 What motivates this change are the unncessary issues brought by this inheritance. You can see some problems described here: https://bugzilla.redhat.com/show_bug.cgi?id=2110307#c0. --- src/core/devices/nm-device-6lowpan.c | 37 ---------------------------- 1 file changed, 37 deletions(-) diff --git a/src/core/devices/nm-device-6lowpan.c b/src/core/devices/nm-device-6lowpan.c index d654335614..9f9e30452b 100644 --- a/src/core/devices/nm-device-6lowpan.c +++ b/src/core/devices/nm-device-6lowpan.c @@ -21,7 +21,6 @@ /*****************************************************************************/ typedef struct { - gulong parent_state_id; } NMDevice6LowpanPrivate; struct _NMDevice6Lowpan { @@ -40,21 +39,6 @@ G_DEFINE_TYPE(NMDevice6Lowpan, nm_device_6lowpan, NM_TYPE_DEVICE) /*****************************************************************************/ -static void -parent_state_changed(NMDevice *parent, - NMDeviceState new_state, - NMDeviceState old_state, - NMDeviceStateReason reason, - gpointer user_data) -{ - NMDevice6Lowpan *self = NM_DEVICE_6LOWPAN(user_data); - - nm_device_set_unmanaged_by_flags(NM_DEVICE(self), - NM_UNMANAGED_PARENT, - !nm_device_get_managed(parent, FALSE), - reason); -} - static void parent_changed_notify(NMDevice *device, int old_ifindex, @@ -62,30 +46,9 @@ parent_changed_notify(NMDevice *device, int new_ifindex, NMDevice *new_parent) { - NMDevice6Lowpan *self = NM_DEVICE_6LOWPAN(device); - NMDevice6LowpanPrivate *priv = NM_DEVICE_6LOWPAN_GET_PRIVATE(self); - NM_DEVICE_CLASS(nm_device_6lowpan_parent_class) ->parent_changed_notify(device, old_ifindex, old_parent, new_ifindex, new_parent); - /* note that @self doesn't have to clear @parent_state_id on dispose, - * because NMDevice's dispose() will unset the parent, which in turn calls - * parent_changed_notify(). */ - nm_clear_g_signal_handler(old_parent, &priv->parent_state_id); - - if (new_parent) { - priv->parent_state_id = g_signal_connect(new_parent, - NM_DEVICE_STATE_CHANGED, - G_CALLBACK(parent_state_changed), - device); - - /* Set parent-dependent unmanaged flag */ - nm_device_set_unmanaged_by_flags(device, - NM_UNMANAGED_PARENT, - !nm_device_get_managed(new_parent, FALSE), - NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED); - } - if (new_ifindex > 0) { /* Recheck availability now that the parent has changed */ nm_device_queue_recheck_available(device, From 66857bafe8ee7b237b4958c83611a4ebab90d845 Mon Sep 17 00:00:00 2001 From: Ana Cabral Date: Wed, 12 Oct 2022 18:57:52 +0200 Subject: [PATCH 3/5] device: remove the unmanaged inheritance from the parent for macsec devices This commit is part of a broader change that eliminates inheriting the unmanaged condition from the parent of a device, for all device types: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1418 What motivates this change are the unncessary issues brought by this inheritance. You can see some problems described here: https://bugzilla.redhat.com/show_bug.cgi?id=2110307#c0. --- src/core/devices/nm-device-macsec.c | 37 +---------------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/src/core/devices/nm-device-macsec.c b/src/core/devices/nm-device-macsec.c index 5cc0b8da89..cacf248352 100644 --- a/src/core/devices/nm-device-macsec.c +++ b/src/core/devices/nm-device-macsec.c @@ -45,7 +45,6 @@ NM_GOBJECT_PROPERTIES_DEFINE(NMDeviceMacsec, typedef struct { NMPlatformLnkMacsec props; - gulong parent_state_id; gulong parent_mtu_id; struct { @@ -91,25 +90,6 @@ static NM_UTILS_LOOKUP_STR_DEFINE(validation_mode_to_string, NM_UTILS_LOOKUP_STR_ITEM(1, "check"), NM_UTILS_LOOKUP_STR_ITEM(2, "strict"), ); -static void -parent_state_changed(NMDevice *parent, - NMDeviceState new_state, - NMDeviceState old_state, - NMDeviceStateReason reason, - gpointer user_data) -{ - NMDeviceMacsec *self = NM_DEVICE_MACSEC(user_data); - - /* We'll react to our own carrier state notifications. Ignore the parent's. */ - if (nm_device_state_reason_check(reason) == NM_DEVICE_STATE_REASON_CARRIER) - return; - - nm_device_set_unmanaged_by_flags(NM_DEVICE(self), - NM_UNMANAGED_PARENT, - !nm_device_get_managed(parent, FALSE), - reason); -} - static void parent_mtu_maybe_changed(NMDevice *parent, GParamSpec *pspec, gpointer user_data) { @@ -132,27 +112,13 @@ parent_changed_notify(NMDevice *device, NM_DEVICE_CLASS(nm_device_macsec_parent_class) ->parent_changed_notify(device, old_ifindex, old_parent, new_ifindex, new_parent); - /* note that @self doesn't have to clear @parent_state_id on dispose, - * because NMDevice's dispose() will unset the parent, which in turn calls - * parent_changed_notify(). */ - nm_clear_g_signal_handler(old_parent, &priv->parent_state_id); nm_clear_g_signal_handler(old_parent, &priv->parent_mtu_id); if (new_parent) { - priv->parent_state_id = g_signal_connect(new_parent, - NM_DEVICE_STATE_CHANGED, - G_CALLBACK(parent_state_changed), - device); - priv->parent_mtu_id = g_signal_connect(new_parent, + priv->parent_mtu_id = g_signal_connect(new_parent, "notify::" NM_DEVICE_MTU, G_CALLBACK(parent_mtu_maybe_changed), device); - - /* Set parent-dependent unmanaged flag */ - nm_device_set_unmanaged_by_flags(device, - NM_UNMANAGED_PARENT, - !nm_device_get_managed(new_parent, FALSE), - NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED); } /* Recheck availability now that the parent has changed */ @@ -863,7 +829,6 @@ dispose(GObject *object) G_OBJECT_CLASS(nm_device_macsec_parent_class)->dispose(object); - nm_assert(NM_DEVICE_MACSEC_GET_PRIVATE(self)->parent_state_id == 0); nm_assert(NM_DEVICE_MACSEC_GET_PRIVATE(self)->parent_mtu_id == 0); } From 412c7449b3e7922bf354c759632671cc4fccb3e6 Mon Sep 17 00:00:00 2001 From: Ana Cabral Date: Wed, 12 Oct 2022 19:03:27 +0200 Subject: [PATCH 4/5] device: remove the unmanaged inheritance from the parent for macvlan devices This commit is part of a broader change that eliminates inheriting the unmanaged condition from the parent of a device, for all device types: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1418 What motivates this change are the unncessary issues brought by this inheritance. You can see some problems described here: https://bugzilla.redhat.com/show_bug.cgi?id=2110307#c0. --- src/core/devices/nm-device-macvlan.c | 37 +--------------------------- 1 file changed, 1 insertion(+), 36 deletions(-) diff --git a/src/core/devices/nm-device-macvlan.c b/src/core/devices/nm-device-macvlan.c index 40ecc5fd70..fee4cad3ac 100644 --- a/src/core/devices/nm-device-macvlan.c +++ b/src/core/devices/nm-device-macvlan.c @@ -29,7 +29,6 @@ NM_GOBJECT_PROPERTIES_DEFINE(NMDeviceMacvlan, PROP_MODE, PROP_NO_PROMISC, PROP_TAP, ); typedef struct { - gulong parent_state_id; gulong parent_mtu_id; NMPlatformLnkMacvlan props; } NMDeviceMacvlanPrivate; @@ -102,25 +101,6 @@ platform_mode_to_string(guint mode) /*****************************************************************************/ -static void -parent_state_changed(NMDevice *parent, - NMDeviceState new_state, - NMDeviceState old_state, - NMDeviceStateReason reason, - gpointer user_data) -{ - NMDeviceMacvlan *self = NM_DEVICE_MACVLAN(user_data); - - /* We'll react to our own carrier state notifications. Ignore the parent's. */ - if (nm_device_state_reason_check(reason) == NM_DEVICE_STATE_REASON_CARRIER) - return; - - nm_device_set_unmanaged_by_flags(NM_DEVICE(self), - NM_UNMANAGED_PARENT, - !nm_device_get_managed(parent, FALSE), - reason); -} - static void parent_mtu_maybe_changed(NMDevice *parent, GParamSpec *pspec, gpointer user_data) { @@ -143,27 +123,13 @@ parent_changed_notify(NMDevice *device, NM_DEVICE_CLASS(nm_device_macvlan_parent_class) ->parent_changed_notify(device, old_ifindex, old_parent, new_ifindex, new_parent); - /* note that @self doesn't have to clear @parent_state_id on dispose, - * because NMDevice's dispose() will unset the parent, which in turn calls - * parent_changed_notify(). */ - nm_clear_g_signal_handler(old_parent, &priv->parent_state_id); nm_clear_g_signal_handler(old_parent, &priv->parent_mtu_id); if (new_parent) { - priv->parent_state_id = g_signal_connect(new_parent, - NM_DEVICE_STATE_CHANGED, - G_CALLBACK(parent_state_changed), - device); - priv->parent_mtu_id = g_signal_connect(new_parent, + priv->parent_mtu_id = g_signal_connect(new_parent, "notify::" NM_DEVICE_MTU, G_CALLBACK(parent_mtu_maybe_changed), device); - - /* Set parent-dependent unmanaged flag */ - nm_device_set_unmanaged_by_flags(device, - NM_UNMANAGED_PARENT, - !nm_device_get_managed(new_parent, FALSE), - NM_DEVICE_STATE_REASON_PARENT_MANAGED_CHANGED); } if (new_ifindex > 0) { @@ -490,7 +456,6 @@ dispose(GObject *object) { G_OBJECT_CLASS(nm_device_macvlan_parent_class)->dispose(object); - nm_assert(NM_DEVICE_MACVLAN_GET_PRIVATE(object)->parent_state_id == 0); nm_assert(NM_DEVICE_MACVLAN_GET_PRIVATE(object)->parent_mtu_id == 0); } #endif From 0a280c5a9316af8d04c1c4f0a6be6c4cfae8f928 Mon Sep 17 00:00:00 2001 From: Ana Cabral Date: Wed, 12 Oct 2022 19:26:27 +0200 Subject: [PATCH 5/5] device: remove the possibility of inheriting the unmanaged condition from the parent of a device This commit is part of a broader change that eliminates inheriting the unmanaged condition from the parent of a device, for all device types: https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1418 What motivates this change are the unncessary issues brought by this inheritance. You can see some problems described here: https://bugzilla.redhat.com/show_bug.cgi?id=2110307#c0. --- src/core/devices/nm-device.c | 7 +++---- src/core/devices/nm-device.h | 22 ++++++++++------------ src/core/nm-manager.c | 10 ++++------ 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index b5c368ebad..2e23b622c6 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -7667,9 +7667,9 @@ nm_device_unrealize(NMDevice *self, gboolean remove_resources, GError **error) nm_device_set_unmanaged_flags(self, NM_UNMANAGED_PLATFORM_INIT, TRUE); nm_device_set_unmanaged_flags(self, - NM_UNMANAGED_PARENT | NM_UNMANAGED_BY_TYPE - | NM_UNMANAGED_USER_UDEV | NM_UNMANAGED_USER_EXPLICIT - | NM_UNMANAGED_EXTERNAL_DOWN | NM_UNMANAGED_IS_SLAVE, + NM_UNMANAGED_BY_TYPE | NM_UNMANAGED_USER_UDEV + | NM_UNMANAGED_USER_EXPLICIT | NM_UNMANAGED_EXTERNAL_DOWN + | NM_UNMANAGED_IS_SLAVE, NM_UNMAN_FLAG_OP_FORGET); nm_device_state_changed(self, @@ -14142,7 +14142,6 @@ NM_UTILS_FLAGS2STR_DEFINE(nm_unmanaged_flags2str, NMUnmanagedFlags, NM_UTILS_FLAGS2STR(NM_UNMANAGED_SLEEPING, "sleeping"), NM_UTILS_FLAGS2STR(NM_UNMANAGED_QUITTING, "quitting"), - NM_UTILS_FLAGS2STR(NM_UNMANAGED_PARENT, "parent"), NM_UTILS_FLAGS2STR(NM_UNMANAGED_BY_TYPE, "by-type"), NM_UTILS_FLAGS2STR(NM_UNMANAGED_PLATFORM_INIT, "platform-init"), NM_UTILS_FLAGS2STR(NM_UNMANAGED_USER_EXPLICIT, "user-explicit"), diff --git a/src/core/devices/nm-device.h b/src/core/devices/nm-device.h index fea46bb78c..3dfa572064 100644 --- a/src/core/devices/nm-device.h +++ b/src/core/devices/nm-device.h @@ -566,7 +566,6 @@ void nm_device_copy_ip6_dns_config(NMDevice *self, NMDevice *from_device); * @NM_UNMANAGED_NONE: placeholder value * @NM_UNMANAGED_SLEEPING: %TRUE when unmanaged because NM is sleeping. * @NM_UNMANAGED_QUITTING: %TRUE when unmanaged because NM is shutting down. - * @NM_UNMANAGED_PARENT: %TRUE when unmanaged due to parent device being unmanaged * @NM_UNMANAGED_BY_TYPE: %TRUE for unmanaging device by type, like loopback. * @NM_UNMANAGED_PLATFORM_INIT: %TRUE when unmanaged because platform link not * yet initialized. Unrealized device are also unmanaged for this reason. @@ -597,21 +596,20 @@ typedef enum { * the device cannot be managed. */ NM_UNMANAGED_SLEEPING = (1LL << 0), NM_UNMANAGED_QUITTING = (1LL << 1), - NM_UNMANAGED_PARENT = (1LL << 2), - NM_UNMANAGED_BY_TYPE = (1LL << 3), - NM_UNMANAGED_PLATFORM_INIT = (1LL << 4), - NM_UNMANAGED_USER_EXPLICIT = (1LL << 5), - NM_UNMANAGED_USER_SETTINGS = (1LL << 6), + NM_UNMANAGED_BY_TYPE = (1LL << 2), + NM_UNMANAGED_PLATFORM_INIT = (1LL << 3), + NM_UNMANAGED_USER_EXPLICIT = (1LL << 4), + NM_UNMANAGED_USER_SETTINGS = (1LL << 5), /* These flags can be non-effective and be overwritten * by other flags. */ - NM_UNMANAGED_BY_DEFAULT = (1LL << 7), - NM_UNMANAGED_USER_CONF = (1LL << 8), - NM_UNMANAGED_USER_UDEV = (1LL << 9), - NM_UNMANAGED_EXTERNAL_DOWN = (1LL << 10), - NM_UNMANAGED_IS_SLAVE = (1LL << 11), + NM_UNMANAGED_BY_DEFAULT = (1LL << 6), + NM_UNMANAGED_USER_CONF = (1LL << 7), + NM_UNMANAGED_USER_UDEV = (1LL << 8), + NM_UNMANAGED_EXTERNAL_DOWN = (1LL << 9), + NM_UNMANAGED_IS_SLAVE = (1LL << 10), - NM_UNMANAGED_ALL = ((1LL << 12) - 1), + NM_UNMANAGED_ALL = ((1LL << 11) - 1), } NMUnmanagedFlags; typedef enum { diff --git a/src/core/nm-manager.c b/src/core/nm-manager.c index bf1e57a8c6..7c04e8e4e5 100644 --- a/src/core/nm-manager.c +++ b/src/core/nm-manager.c @@ -3083,10 +3083,9 @@ recheck_assume_connection(NMManager *self, NMDevice *device) g_return_val_if_fail(NM_IS_DEVICE(device), FALSE); if (!nm_device_get_managed(device, FALSE)) { - /* If the device is unmanaged by NM_UNMANAGED_PLATFORM_INIT or NM_UNMANAGED_PARENT, + /* If the device is unmanaged by NM_UNMANAGED_PLATFORM_INIT, * don't reset the state now but wait until it becomes managed. */ - if (nm_device_get_unmanaged_flags(device, NM_UNMANAGED_ALL) - & ~(NM_UNMANAGED_PLATFORM_INIT | NM_UNMANAGED_PARENT)) + if (nm_device_get_unmanaged_flags(device, NM_UNMANAGED_ALL) & ~NM_UNMANAGED_PLATFORM_INIT) nm_device_assume_state_reset(device); _LOG2D(LOGD_DEVICE, device, "assume: don't assume because device is not managed"); return FALSE; @@ -3410,10 +3409,9 @@ _device_realize_finish(NMManager *self, NMDevice *device, const NMPlatformLink * nm_device_realize_finish(device, plink); if (!nm_device_get_managed(device, FALSE)) { - /* If the device is unmanaged by NM_UNMANAGED_PLATFORM_INIT or NM_UNMANAGED_PARENT, + /* If the device is unmanaged by NM_UNMANAGED_PLATFORM_INIT, * don't reset the state now but wait until it becomes managed. */ - if (nm_device_get_unmanaged_flags(device, NM_UNMANAGED_ALL) - & ~(NM_UNMANAGED_PLATFORM_INIT | NM_UNMANAGED_PARENT)) + if (nm_device_get_unmanaged_flags(device, NM_UNMANAGED_ALL) & ~NM_UNMANAGED_PLATFORM_INIT) nm_device_assume_state_reset(device); return; }