From e29ab54335c6a5ef1ce6bac525f1f18a8e81b96e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Oct 2015 11:11:42 +0200 Subject: 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 --- src/devices/nm-device.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 1f8eaada8d..fe48260a7d 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1376,6 +1376,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; @@ -1451,8 +1453,31 @@ 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) { + gboolean platform_unmanaged = FALSE; + + 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); @@ -1494,25 +1519,8 @@ device_link_changed (NMDevice *self) } } - if (priv->ifindex > 0 && !priv->platform_link_initialized && info.initialized) { - gboolean platform_unmanaged = FALSE; - - 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; } -- cgit v1.2.3