From bdd826a0441965e4f8c6f2936f0e69c3c0621828 Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 16 Dec 2022 09:43:20 +0100 Subject: [PATCH 1/2] veth: improve comment about skipping creation of interfaces --- src/core/devices/nm-device-veth.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/devices/nm-device-veth.c b/src/core/devices/nm-device-veth.c index 82762ba9b2..44d4052ea1 100644 --- a/src/core/devices/nm-device-veth.c +++ b/src/core/devices/nm-device-veth.c @@ -98,11 +98,14 @@ create_and_realize(NMDevice *device, return FALSE; } + /* For veths, users can define two connection profiles referencing each + * other as 'veth.peer'. Only the first to be activated will actually + * create the veth pair; the other must detect that interfaces already + * exist and proceed. */ peer = nm_setting_veth_get_peer(s_veth); peer_device = nm_manager_get_device(NM_MANAGER_GET, peer, NM_DEVICE_TYPE_VETH); if (peer_device) { if (nm_device_parent_get_device(peer_device)) - /* The veth device and its peer already exist. No need to create it again. */ return TRUE; } From 50f738bde5b441b5ca52024c1a0998399b87337b Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Fri, 16 Dec 2022 10:13:18 +0100 Subject: [PATCH 2/2] veth: fix detection of existing interfaces in create_and_realize() The current implementation only checks that a device with name equal to veth.peer exists and it has a parent device; it doesn't check that its parent is actually the device we want to create. So for example, if the profile specifies interface-name A and peer B, while in platform we have a veth pair {B,C}, we'll skip the interface creation and the device will remain without a ifindex, leading to a crash later. Fix this by adding the missing check. While at it, don't implement the check by inspecting NMDevices but look directly at the platform cache; that seems more robust because devices are often updated from platform events via idle handlers and so the information there could be outdated. Fixes: 07e0ab48d194 ('veth: drop iface peer check during create_and_realize()') https://bugzilla.redhat.com/show_bug.cgi?id=2129829 --- src/core/devices/nm-device-veth.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/core/devices/nm-device-veth.c b/src/core/devices/nm-device-veth.c index 44d4052ea1..c4b9e23456 100644 --- a/src/core/devices/nm-device-veth.c +++ b/src/core/devices/nm-device-veth.c @@ -81,11 +81,13 @@ create_and_realize(NMDevice *device, const NMPlatformLink **out_plink, GError **error) { - const char *iface = nm_device_get_iface(device); - const char *peer; - NMDevice *peer_device; - NMSettingVeth *s_veth; - int r; + NMPlatform *platform = nm_device_get_platform(device); + const char *iface = nm_device_get_iface(device); + NMSettingVeth *s_veth; + const NMPlatformLink *plink; + const NMPlatformLink *peer_plink; + int peer_ifindex; + int r; s_veth = _nm_connection_get_setting(connection, NM_TYPE_SETTING_VETH); if (!s_veth) { @@ -102,14 +104,19 @@ create_and_realize(NMDevice *device, * other as 'veth.peer'. Only the first to be activated will actually * create the veth pair; the other must detect that interfaces already * exist and proceed. */ - peer = nm_setting_veth_get_peer(s_veth); - peer_device = nm_manager_get_device(NM_MANAGER_GET, peer, NM_DEVICE_TYPE_VETH); - if (peer_device) { - if (nm_device_parent_get_device(peer_device)) + plink = nm_platform_link_get_by_ifname(platform, iface); + if (plink && nm_platform_link_veth_get_properties(platform, plink->ifindex, &peer_ifindex)) { + peer_plink = nm_platform_link_get(platform, peer_ifindex); + if (peer_plink && peer_plink->type == NM_LINK_TYPE_VETH + && nm_streq0(peer_plink->name, nm_setting_veth_get_peer(s_veth))) { return TRUE; + } } - r = nm_platform_link_veth_add(nm_device_get_platform(device), iface, peer, out_plink); + r = nm_platform_link_veth_add(nm_device_get_platform(device), + iface, + nm_setting_veth_get_peer(s_veth), + out_plink); if (r < 0) { g_set_error(error, NM_DEVICE_ERROR,