From 0551c1cb8106bf3483433fd9da6e391b9c2c2712 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Oct 2015 11:11:42 +0200 Subject: [PATCH 1/1] device: fix race wrongly managing external-down device due to late udev signal Executing: # brctl addbr lbr0 # ip addr add 10.1.1.1/24 dev lbr0 # ip link set lbr0 up can result in a race so that NetworkManager would manage the device (and clear the IP addresses). It happens, when NetworkManager first receives platform signals that the device is already up: signal: link changed: 11: lbr0 mtu 1500 arp 1 bridge* not-init addrgenmode eui64 addr D2:A1:B4:17:18:F2 driver bridge Note that the device is still unknown via udev (not-init). The unmanaged-state NM_UNMANAGED_EXTERNAL_DOWN gets cleared, but the device still stays unmanaged. Only afterwards the device is known in udev: signal: link changed: 11: lbr0 mtu 1500 arp 1 bridge* init addrgenmode eui64 addr D2:A1:B4:17:18:F2 driver bridge At this point, we also clear NM_UNMANAGED_PLATFORM_INIT, making the device managed with reason NM_DEVICE_STATE_REASON_NOW_MANAGED. That results in managing the external device. Fix that by only clearing NM_UNMANAGED_EXTERNAL_DOWN after the device is no longer NM_UNMANAGED_PLATFORM_INIT. https://bugzilla.redhat.com/show_bug.cgi?id=1269199 (cherry picked from commit e29ab54335c6a5ef1ce6bac525f1f18a8e81b96e) (cherry picked from commit cde15dda2e513517433e6273ca62e8b8c2c35ccb) --- src/devices/nm-device.c | 44 ++++++++++++++++++++++++++------------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 83cd6e9..7c49f3f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1375,6 +1375,8 @@ device_link_changed (NMDevice *self) NMPlatformLink info; const NMPlatformLink *pllink; int ifindex; + gboolean emit_link_initialized = FALSE; + gboolean was_up; priv->device_link_changed_id = 0; @@ -1450,8 +1452,29 @@ device_link_changed (NMDevice *self) if (ip_ifname_changed) update_dynamic_ip_setup (self); - if (priv->up != NM_FLAGS_HAS (info.flags, IFF_UP)) { - priv->up = NM_FLAGS_HAS (info.flags, IFF_UP); + if (priv->ifindex > 0 && !priv->platform_link_initialized && info.initialized) { + priv->platform_link_initialized = TRUE; + + if (nm_platform_link_get_unmanaged (NM_PLATFORM_GET, priv->ifindex, &platform_unmanaged)) { + nm_device_set_unmanaged (self, + NM_UNMANAGED_DEFAULT, + platform_unmanaged, + NM_DEVICE_STATE_REASON_USER_REQUESTED); + } + + nm_device_set_unmanaged (self, + NM_UNMANAGED_PLATFORM_INIT, + FALSE, + NM_DEVICE_STATE_REASON_NOW_MANAGED); + + emit_link_initialized = TRUE; + } + + was_up = priv->up; + priv->up = NM_FLAGS_HAS (info.flags, IFF_UP); + + if ( priv->platform_link_initialized + && (emit_link_initialized || priv->up != was_up)) { /* Manage externally-created software interfaces only when they are IFF_UP */ g_assert (priv->ifindex > 0); @@ -1493,23 +1516,8 @@ device_link_changed (NMDevice *self) } } - if (priv->ifindex > 0 && !priv->platform_link_initialized && info.initialized) { - priv->platform_link_initialized = TRUE; - - if (nm_platform_link_get_unmanaged (NM_PLATFORM_GET, priv->ifindex, &platform_unmanaged)) { - nm_device_set_unmanaged (self, - NM_UNMANAGED_DEFAULT, - platform_unmanaged, - NM_DEVICE_STATE_REASON_USER_REQUESTED); - } - - nm_device_set_unmanaged (self, - NM_UNMANAGED_PLATFORM_INIT, - FALSE, - NM_DEVICE_STATE_REASON_NOW_MANAGED); - + if (emit_link_initialized) g_signal_emit (self, signals[LINK_INITIALIZED], 0); - } return G_SOURCE_REMOVE; } -- 2.4.3