From 21e7cd5f413f2b69e6c77ee7b825b8409e92cc8b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 3 Dec 2014 14:47:44 +0100 Subject: device: don't disconnect assumed connections Transition them to activated status when they fail. https://bugzilla.redhat.com/show_bug.cgi?id=1141264 --- src/devices/nm-device.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a5c13f6d88..121135fd74 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6915,6 +6915,13 @@ _set_state_full (NMDevice *device, "Activation (%s) failed for connection '%s'", nm_device_get_iface (device), connection ? nm_connection_get_id (connection) : ""); + if (req && nm_active_connection_get_assumed (NM_ACTIVE_CONNECTION (req))) { + /* Avoid tearing down assumed connection, assume it's connected */ + nm_device_queue_state (device, + NM_DEVICE_STATE_ACTIVATED, + NM_DEVICE_STATE_REASON_CONNECTION_ASSUMED); + break; + } /* Notify any slaves of the unexpected failure */ nm_device_master_release_slaves (device); -- cgit v1.2.1 From 0c032107f38d8daf269664015312110f03c0ec09 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Thu, 4 Dec 2014 16:22:39 +0100 Subject: platform: ensure all objects in link cache are of AF_UNSPEC family We assume that in nm_nl_cache_search() and correctly set that in get_kernel_object(), but we rtnl_link_alloc_cache() can initialize the cache with devices of other families. The consequence is that we don't notify when the bridge changes to IFF_UP as we fail to match and remove the old downed object from the cache: nm_device_bring_up(): [0xf506c0] (bridge0): bringing up device. nm_platform_link_set_up(): link: setting up 'bridge0' (12) link_change_flags(): link: change 12: flags set 'up' (1) get_kernel_object(): get_kernel_object for link: bridge0 (12, family 7) log_link(): signal: link added: 12: bridge0 mtu 1500 bridge driver 'bridge' udi '/sys/devices/virtual/net/bridge0' get_kernel_object(): get_kernel_object for link: bridge0 (12, family 7) log_link(): signal: link changed: 12: bridge0 mtu 1500 bridge driver 'bridge' udi '/sys/devices/virtual/net/bridge0' log_link(): signal: link changed: 12: bridge0 mtu 1500 bridge driver 'bridge' udi '/sys/devices/virtual/net/bridge0' (bridge0): device not up after timeout! (bridge0): preparing device --- src/platform/nm-linux-platform.c | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 1f925a65a3..32f0aba916 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3818,6 +3818,43 @@ ip6_route_exists (NMPlatform *platform, int ifindex, struct in6_addr network, in /******************************************************************/ +/* Initialize the link cache while ensuring all links are of AF_UNSPEC, + * family (even though the kernel might set AF_BRIDGE for bridges). + * See also: _nl_link_family_unset() */ +static void +init_link_cache (NMPlatform *platform) +{ + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); + struct nl_object *object = NULL; + + rtnl_link_alloc_cache (priv->nlh, AF_UNSPEC, &priv->link_cache); + + do { + for (object = nl_cache_get_first (priv->link_cache); object; object = nl_cache_get_next (object)) { + if (rtnl_link_get_family ((struct rtnl_link *)object) != AF_UNSPEC) + break; + } + + if (object) { + /* A non-AF_UNSPEC object encoutnered */ + struct nl_object *existing; + + nl_object_get (object); + nl_cache_remove (object); + rtnl_link_set_family ((struct rtnl_link *)object, AF_UNSPEC); + existing = nl_cache_search (priv->link_cache, object); + if (existing) { + nl_object_put (existing); + } else { + nl_cache_add (priv->link_cache, object); + } + nl_object_put (object); + } + } while (object); +} + +/******************************************************************/ + #define EVENT_CONDITIONS ((GIOCondition) (G_IO_IN | G_IO_PRI)) #define ERROR_CONDITIONS ((GIOCondition) (G_IO_ERR | G_IO_NVAL)) #define DISCONNECT_CONDITIONS ((GIOCondition) (G_IO_HUP)) @@ -4089,7 +4126,7 @@ setup (NMPlatform *platform) event_handler, platform); /* Allocate netlink caches */ - rtnl_link_alloc_cache (priv->nlh, AF_UNSPEC, &priv->link_cache); + init_link_cache (platform); rtnl_addr_alloc_cache (priv->nlh, &priv->address_cache); rtnl_route_alloc_cache (priv->nlh, AF_UNSPEC, 0, &priv->route_cache); g_assert (priv->link_cache && priv->address_cache && priv->route_cache); -- cgit v1.2.1 From 54ce1bcfdaf6bf3dae829213a2d8851a449d7bea Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 29 Sep 2014 17:58:44 +0200 Subject: platform: refactor the object comparison logic into a separate function One from libnl is not good enough (see comment). --- src/platform/nm-linux-platform.c | 48 +++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 32f0aba916..ff65d31365 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1818,6 +1818,33 @@ _rtnl_addr_timestamps_equal_fuzzy (guint32 ts1, guint32 ts2) return diff <= 2; } +static gboolean +nm_nl_object_diff (struct nl_object *_a, struct nl_object *_b, ObjectType type) +{ + if (nl_object_diff (_a, _b)) { + /* libnl thinks objects are different*/ + return TRUE; + } + + if (type == OBJECT_TYPE_IP4_ADDRESS || type == OBJECT_TYPE_IP6_ADDRESS) { + struct rtnl_addr *a = (struct rtnl_addr *) _a; + struct rtnl_addr *b = (struct rtnl_addr *) _b; + + /* libnl nl_object_diff() ignores differences in timestamp. Let's care about + * them (if they are large enough). + * + * Note that these valid and preferred timestamps are absolute, after + * _rtnl_addr_hack_lifetimes_rel_to_abs(). */ + if ( !_rtnl_addr_timestamps_equal_fuzzy (rtnl_addr_get_preferred_lifetime (a), + rtnl_addr_get_preferred_lifetime (b)) + || !_rtnl_addr_timestamps_equal_fuzzy (rtnl_addr_get_valid_lifetime (a), + rtnl_addr_get_valid_lifetime (b))) + return TRUE; + } + + return FALSE; +} + /* This function does all the magic to avoid race conditions caused * by concurrent usage of synchronous commands and an asynchronous cache. This * might be a nice future addition to libnl but it requires to do all operations @@ -1928,24 +1955,9 @@ event_notification (struct nl_msg *msg, gpointer user_data) * This also catches notifications for internal addition or change, unless * another action occured very soon after it. */ - if (!nl_object_diff (kernel_object, cached_object)) { - if (type == OBJECT_TYPE_IP4_ADDRESS || type == OBJECT_TYPE_IP6_ADDRESS) { - struct rtnl_addr *c = (struct rtnl_addr *) cached_object; - struct rtnl_addr *k = (struct rtnl_addr *) kernel_object; - - /* libnl nl_object_diff() ignores differences in timestamp. Let's care about - * them (if they are large enough). - * - * Note that these valid and preferred timestamps are absolute, after - * _rtnl_addr_hack_lifetimes_rel_to_abs(). */ - if ( _rtnl_addr_timestamps_equal_fuzzy (rtnl_addr_get_preferred_lifetime (c), - rtnl_addr_get_preferred_lifetime (k)) - && _rtnl_addr_timestamps_equal_fuzzy (rtnl_addr_get_valid_lifetime (c), - rtnl_addr_get_valid_lifetime (k))) - return NL_OK; - } else - return NL_OK; - } + if (!nm_nl_object_diff (kernel_object, cached_object, type)) + return NL_OK; + /* Handle external change */ nl_cache_remove (cached_object); nle = nl_cache_add (cache, kernel_object); -- cgit v1.2.1 From 16f5430ebdd6471291583625ca5324117e96f089 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Mon, 8 Dec 2014 18:08:42 +0100 Subject: platform: resynchronize with kernel when we're out of buffer space Kernel can return ENOBUFS in variety of reasons. If that happens, we know we've lost events and should pick up kernel state. Simple reproducer that triggers an ENOBUFS condition no matter how big our netlink socket buffer is: ip link add bridge0 type bridge for i in seq $(0 1023); do ip link add dummy$i type dummy; \ ip link set dummy$i master bridge0; done ip link del bridge0 --- src/platform/nm-linux-platform.c | 158 ++++++++++++++++++++++++++++----------- 1 file changed, 116 insertions(+), 42 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index ff65d31365..cf2164d748 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -3865,6 +3865,115 @@ init_link_cache (NMPlatform *platform) } while (object); } +/* Calls announce_object with appropriate arguments for all objects + * which are not coherent between old and new caches and deallocates + * the old cache. */ +static gboolean +sync_cache (NMPlatform *platform, struct nl_cache *new, struct nl_cache *old) +{ + struct nl_object *object; + gboolean changed = FALSE; + + if (!old) + return changed; + + for (object = nl_cache_get_first (new); object; object = nl_cache_get_next (object)) { + struct nl_object *cached_object = nm_nl_cache_search (old, object); + + if (cached_object) { + ObjectType type = object_type_from_nl_object (object); + if (nm_nl_object_diff (object, cached_object, type)) { + announce_object (platform, object, NM_PLATFORM_SIGNAL_CHANGED, NM_PLATFORM_REASON_EXTERNAL); + changed = TRUE; + } + nl_object_put (cached_object); + } else { + announce_object (platform, object, NM_PLATFORM_SIGNAL_ADDED, NM_PLATFORM_REASON_EXTERNAL); + changed = TRUE; + } + } + for (object = nl_cache_get_first (old); object; object = nl_cache_get_next (object)) { + struct nl_object *cached_object = nm_nl_cache_search (new, object); + if (cached_object) { + nl_object_put (cached_object); + } else { + announce_object (platform, object, NM_PLATFORM_SIGNAL_REMOVED, NM_PLATFORM_REASON_EXTERNAL); + changed = TRUE; + } + } + + nl_cache_free (old); + return changed; +} + +/* The cache should always avoid containing objects not handled by NM, like + * e.g. addresses of the AF_PHONET family. */ +static void +cache_remove_unknown (struct nl_cache *cache) +{ + GPtrArray *objects_to_remove = NULL; + struct nl_object *object; + + for (object = nl_cache_get_first (cache); object; object = nl_cache_get_next (object)) { + if (object_type_from_nl_object (object) == OBJECT_TYPE_UNKNOWN) { + if (!objects_to_remove) + objects_to_remove = g_ptr_array_new_with_free_func ((GDestroyNotify) nl_object_put); + nl_object_get (object); + g_ptr_array_add (objects_to_remove, object); + } + } + + if (objects_to_remove) { + guint i; + + for (i = 0; i < objects_to_remove->len; i++) + nl_cache_remove (g_ptr_array_index (objects_to_remove, i)); + + g_ptr_array_free (objects_to_remove, TRUE); + } +} + +/* Creates and populates the netlink object caches. Called upon platform init + * and when we run out of sync (out of buffer space, netlink congestion control). + * Returns TRUE if there were changes, + * Does not guarrantee the caches are coherent with kernel -- we could have missed + * events while synchronizing. Should be re-called when there were changes. */ +static gboolean +sync_platform (NMPlatform *platform) +{ + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); + struct nl_cache *old_link_cache = priv->link_cache; + struct nl_cache *old_address_cache = priv->address_cache; + struct nl_cache *old_route_cache = priv->route_cache; + struct nl_object *object; + gboolean changed = FALSE; + + /* Allocate new netlink caches */ + init_link_cache (platform); + rtnl_addr_alloc_cache (priv->nlh, &priv->address_cache); + rtnl_route_alloc_cache (priv->nlh, AF_UNSPEC, 0, &priv->route_cache); + g_assert (priv->link_cache && priv->address_cache && priv->route_cache); + + /* Remove all unknown objects from the caches */ + cache_remove_unknown (priv->link_cache); + cache_remove_unknown (priv->address_cache); + cache_remove_unknown (priv->route_cache); + + for (object = nl_cache_get_first (priv->address_cache); object; object = nl_cache_get_next (object)) { + _rtnl_addr_hack_lifetimes_rel_to_abs ((struct rtnl_addr *) object); + } + + /* Make sure all changes we've missed are announced. */ + if (sync_cache (platform, priv->link_cache, old_link_cache)) + changed = TRUE; + if (sync_cache (platform, priv->address_cache, old_address_cache)) + changed = TRUE; + if (sync_cache (platform, priv->route_cache, old_route_cache)) + changed = TRUE; + + return changed; +} + /******************************************************************/ #define EVENT_CONDITIONS ((GIOCondition) (G_IO_IN | G_IO_PRI)) @@ -3893,7 +4002,8 @@ event_handler (GIOChannel *channel, GIOCondition io_condition, gpointer user_data) { - NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (user_data); + NMPlatform *platform = NM_PLATFORM (user_data); + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); int nle; nle = nl_recvmsgs_default (priv->nlh_event); @@ -3905,6 +4015,10 @@ event_handler (GIOChannel *channel, * and can happen easily. */ debug ("Uncritical failure to retrieve incoming events: %s (%d)", nl_geterror (nle), nle); break; + case -NLE_NOMEM: + warning ("Too many netlink events. Trying to resynchronize: %s (%d)", nl_geterror (nle), nle); + while (sync_platform (platform)); + break; default: error ("Failed to retrieve incoming events: %s (%d)", nl_geterror (nle), nle); break; @@ -4064,33 +4178,6 @@ nm_linux_platform_init (NMLinuxPlatform *platform) { } -/* The cache should always avoid containing objects not handled by NM, like - * e.g. addresses of the AF_PHONET family. */ -static void -cache_remove_unknown (struct nl_cache *cache) -{ - GPtrArray *objects_to_remove = NULL; - struct nl_object *object; - - for (object = nl_cache_get_first (cache); object; object = nl_cache_get_next (object)) { - if (object_type_from_nl_object (object) == OBJECT_TYPE_UNKNOWN) { - if (!objects_to_remove) - objects_to_remove = g_ptr_array_new_with_free_func ((GDestroyNotify) nl_object_put); - nl_object_get (object); - g_ptr_array_add (objects_to_remove, object); - } - } - - if (objects_to_remove) { - guint i; - - for (i = 0; i < objects_to_remove->len; i++) - nl_cache_remove (g_ptr_array_index (objects_to_remove, i)); - - g_ptr_array_free (objects_to_remove, TRUE); - } -} - static gboolean setup (NMPlatform *platform) { @@ -4101,7 +4188,6 @@ setup (NMPlatform *platform) int channel_flags; gboolean status; int nle; - struct nl_object *object; /* Initialize netlink socket for requests */ priv->nlh = setup_socket (FALSE, platform); @@ -4137,19 +4223,7 @@ setup (NMPlatform *platform) (EVENT_CONDITIONS | ERROR_CONDITIONS | DISCONNECT_CONDITIONS), event_handler, platform); - /* Allocate netlink caches */ - init_link_cache (platform); - rtnl_addr_alloc_cache (priv->nlh, &priv->address_cache); - rtnl_route_alloc_cache (priv->nlh, AF_UNSPEC, 0, &priv->route_cache); - g_assert (priv->link_cache && priv->address_cache && priv->route_cache); - - /* Remove all unknown objects from the caches */ - cache_remove_unknown (priv->link_cache); - cache_remove_unknown (priv->address_cache); - cache_remove_unknown (priv->route_cache); - - for (object = nl_cache_get_first (priv->address_cache); object; object = nl_cache_get_next (object)) - _rtnl_addr_hack_lifetimes_rel_to_abs ((struct rtnl_addr *) object); + sync_platform (platform); /* Set up udev monitoring */ priv->udev_client = g_udev_client_new (udev_subsys); -- cgit v1.2.1 From 253b4fb88b92fe9ae1b7a9dabf94273183384aa7 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 5 Dec 2014 13:10:51 +0100 Subject: device: release and enslave an interface if its master changed In case of an atomic master change, we'd not notice that the master changed: ip link set dummy0 master bridge0 ip link set dummy0 master bridge1 --- src/devices/nm-device.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 121135fd74..73384aa369 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1060,6 +1060,8 @@ device_link_changed (NMDevice *device, NMPlatformLink *info) } /* Update slave status for external changes */ + if (priv->enslaved && info->master != nm_device_get_ifindex (priv->master)) + nm_device_release_one_slave (priv->master, device, FALSE, NM_DEVICE_STATE_REASON_NONE); if (info->master && !priv->enslaved) { NMDevice *master; @@ -1079,8 +1081,7 @@ device_link_changed (NMDevice *device, NMPlatformLink *info) info->master, nm_platform_link_get_name (info->master)); } - } else if (priv->enslaved && !info->master) - nm_device_release_one_slave (priv->master, device, FALSE, NM_DEVICE_STATE_REASON_NONE); + } if (klass->link_changed) klass->link_changed (device, info); -- cgit v1.2.1 From 763031ad2bdd637ec841a30a38782d418f6a72df Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 5 Dec 2014 13:12:55 +0100 Subject: device: set the master on device addition Otherwise we won't notice the device is a slave on NM startup until someone changes the link or tries to activate the device. --- src/devices/nm-device.c | 67 ++++++++++++++++++++++++++++++++++--------------- src/devices/nm-device.h | 2 ++ src/nm-manager.c | 1 + 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 73384aa369..55eb1d8281 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -854,6 +854,22 @@ nm_device_release_one_slave (NMDevice *dev, NMDevice *slave, gboolean configure, return success; } +/** + * nm_device_finish_init: + * @self: the master device + * + * Whatever needs to be done post-initialization, when the device has a DBus + * object name. + */ +void +nm_device_finish_init (NMDevice *self) +{ + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + if (priv->master) + nm_device_enslave_slave (priv->master, self, NULL); +} + static void carrier_changed (NMDevice *device, gboolean carrier) { @@ -1017,6 +1033,27 @@ update_for_ip_ifname_change (NMDevice *device) } } +static void +device_set_master (NMDevice *self, int ifindex) +{ + NMDevice *master; + NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); + + master = nm_manager_get_device_by_ifindex (nm_manager_get (), ifindex); + if (master && NM_DEVICE_GET_CLASS (master)->enslave_slave) { + g_clear_object (&priv->master); + priv->master = g_object_ref (master); + nm_device_master_add_slave (master, self, FALSE); + } else if (master) { + nm_log_info (LOGD_DEVICE, "(%s): enslaved to non-master-type device %s; ignoring", + nm_device_get_iface (self), + nm_device_get_iface (master)); + } else { + nm_log_warn (LOGD_DEVICE, "(%s): enslaved to unknown device", + nm_device_get_iface (self)); + } +} + static void device_link_changed (NMDevice *device, NMPlatformLink *info) { @@ -1062,26 +1099,10 @@ device_link_changed (NMDevice *device, NMPlatformLink *info) /* Update slave status for external changes */ if (priv->enslaved && info->master != nm_device_get_ifindex (priv->master)) nm_device_release_one_slave (priv->master, device, FALSE, NM_DEVICE_STATE_REASON_NONE); - if (info->master && !priv->enslaved) { - NMDevice *master; - - master = nm_manager_get_device_by_ifindex (nm_manager_get (), info->master); - if (master && NM_DEVICE_GET_CLASS (master)->enslave_slave) { - g_clear_object (&priv->master); - priv->master = g_object_ref (master); - nm_device_master_add_slave (master, device, FALSE); - nm_device_enslave_slave (master, device, NULL); - } else if (master) { - nm_log_info (LOGD_DEVICE, "(%s): enslaved to non-master-type device %s; ignoring", - nm_device_get_iface (device), - nm_device_get_iface (master)); - } else { - nm_log_warn (LOGD_DEVICE, "(%s): enslaved to unknown device %d %s", - nm_device_get_iface (device), - info->master, - nm_platform_link_get_name (info->master)); - } - } + if (info->master && !priv->enslaved) + device_set_master (device, info->master); + if (priv->master) + nm_device_enslave_slave (priv->master, device, NULL); if (klass->link_changed) klass->link_changed (device, info); @@ -7417,6 +7438,7 @@ constructed (GObject *object) { NMDevice *dev = NM_DEVICE (object); NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (dev); + int master; nm_device_update_hw_address (dev); @@ -7449,6 +7471,11 @@ constructed (GObject *object) if (priv->ifindex > 0) priv->mtu = nm_platform_link_get_mtu (priv->ifindex); + /* Enslave ourselves */ + master = nm_platform_link_get_master (priv->ifindex); + if (master) + device_set_master (dev, master); + priv->con_provider = nm_connection_provider_get (); g_assert (priv->con_provider); g_signal_connect (priv->con_provider, diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index a2f276934e..f6ceff7116 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -220,6 +220,8 @@ GType nm_device_get_type (void); const char * nm_device_get_path (NMDevice *dev); void nm_device_dbus_export (NMDevice *device); +void nm_device_finish_init (NMDevice *device); + const char * nm_device_get_udi (NMDevice *dev); const char * nm_device_get_iface (NMDevice *dev); int nm_device_get_ifindex (NMDevice *dev); diff --git a/src/nm-manager.c b/src/nm-manager.c index 6be83380eb..0214d47f36 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -1790,6 +1790,7 @@ add_device (NMManager *self, NMDevice *device, gboolean generate_con) nm_device_set_initial_unmanaged_flag (device, NM_UNMANAGED_INTERNAL, sleeping); nm_device_dbus_export (device); + nm_device_finish_init (device); /* Don't generate a connection e.g. for devices NM just created, or * for the loopback, or when we're sleeping. */ -- cgit v1.2.1 From fbba98de6b2e38713a13396cc80b069e4f837de9 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Fri, 5 Dec 2014 17:49:11 +0100 Subject: device: assume connections for device with slaves If a bridge/team/bond has slaves, assume it's connected. Recheck as devices appear. https://bugzilla.redhat.com/show_bug.cgi?id=1141266 --- src/devices/nm-device.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 55eb1d8281..a31673bfc3 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1274,6 +1274,7 @@ nm_device_master_add_slave (NMDevice *dev, NMDevice *slave, gboolean configure) G_CALLBACK (slave_state_changed), dev); priv->slaves = g_slist_append (priv->slaves, info); } + nm_device_queue_recheck_assume (dev); return TRUE; } @@ -1782,8 +1783,9 @@ nm_device_generate_connection (NMDevice *device) ip6_method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP6_CONFIG); if ( g_strcmp0 (ip4_method, NM_SETTING_IP4_CONFIG_METHOD_DISABLED) == 0 && g_strcmp0 (ip6_method, NM_SETTING_IP6_CONFIG_METHOD_IGNORE) == 0 - && !nm_setting_connection_get_master (NM_SETTING_CONNECTION (s_con))) { - nm_log_dbg (LOGD_DEVICE, "(%s): ignoring generated connection (no IP and not slave)", ifname); + && !nm_setting_connection_get_master (NM_SETTING_CONNECTION (s_con)) + && !priv->slaves) { + nm_log_dbg (LOGD_DEVICE, "(%s): ignoring generated connection (no IP and not in master-slave relationship)", ifname); g_object_unref (connection); connection = NULL; } @@ -1993,7 +1995,7 @@ nm_device_emit_recheck_assume (gpointer self) NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); priv->recheck_assume_id = 0; - if (!nm_device_get_act_request (self) && (priv->ip4_config || priv->ip6_config)) + if (!nm_device_get_act_request (self)) g_signal_emit (self, signals[RECHECK_ASSUME], 0); return G_SOURCE_REMOVE; } -- cgit v1.2.1 From f1a5d995383f2133ca3b3d7fc40bb5cc5f08269c Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 11 Dec 2014 17:52:09 -0600 Subject: core: fix attaching managed slaves to master devices (rh #1141266) Broken by 25387cd1ffc4b3135a13e9222c0b0e5dea506c80 When an activation request comes in via D-Bus for a slave, the slave device's priv->master is set in stage1 in master_ready_cb(). Then nm_device_bring_up() is called on the slave, which triggers link_changed_cb() and device_link_changed(). That then executes this code: if (priv->master) nm_device_enslave_slave (priv->master, self, NULL); which enslaves the slave, but due to the NULL will not configure the slave. This code was only meant to be run for externally triggered master/slave changes. (cherry picked from commit 9337a13a87dcc408eb0ca1a51997fc7ce9377078) --- src/devices/nm-device.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a31673bfc3..ff6971fef8 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1099,10 +1099,11 @@ device_link_changed (NMDevice *device, NMPlatformLink *info) /* Update slave status for external changes */ if (priv->enslaved && info->master != nm_device_get_ifindex (priv->master)) nm_device_release_one_slave (priv->master, device, FALSE, NM_DEVICE_STATE_REASON_NONE); - if (info->master && !priv->enslaved) + if (info->master && !priv->enslaved) { device_set_master (device, info->master); - if (priv->master) - nm_device_enslave_slave (priv->master, device, NULL); + if (priv->master) + nm_device_enslave_slave (priv->master, device, NULL); + } if (klass->link_changed) klass->link_changed (device, info); -- cgit v1.2.1 From b48a4ec63516ff957d175db1afaaef6d8aa07aeb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 17 Dec 2014 00:40:05 +0100 Subject: device: fix assertion in nm_device_slave_notify_release() logging the master device #0 0x00007f6c3aed34e9 in g_logv (log_domain=0x7f6c3ea7341c "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=, args=args@entry=0x7fff0a33fb60) at gmessages.c:989 #1 0x00007f6c3aed363f in g_log (log_domain=, log_level=, format=) at gmessages.c:1025 #2 0x00007f6c3e8ead4f in nm_device_get_iface (self=0x0) at devices/nm-device.c:502 #3 0x00007f6c3e904f59 in nm_device_slave_notify_release (self=0x7f6c3fb48e60, reason=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:1618 #4 0x00007f6c3e8ed69f in nm_device_release_one_slave (self=0x7f6c3fb22670, slave=0x7f6c3fb48e60, configure=1, reason=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:968 #5 0x00007f6c3e904bf7 in slave_state_changed (slave=0x7f6c3fb48e60, slave_new_state=NM_DEVICE_STATE_UNMANAGED, slave_old_state=NM_DEVICE_STATE_ACTIVATED, reason=NM_DEVICE_STATE_REASON_REMOVED, self=0x7f6c3fb22670) at devices/nm-device.c:1368 #6 0x00007f6c39829d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #7 0x00007f6c398296bc in ffi_call (cif=cif@entry=0x7fff0a340070, fn=0x7f6c3e9049d0 , rvalue=0x7fff0a33ffe0, avalue=avalue@entry=0x7fff0a33ff60) at ../src/x86/ffi64.c:522 #8 0x00007f6c3b1bfad8 in g_cclosure_marshal_generic (closure=0x7f6c3fb5c8c0, return_gvalue=0x0, n_param_values=, param_values=, invocation_hint=, marshal_data=0x0) at gclosure.c:1454 #9 0x00007f6c3b1bf298 in g_closure_invoke (closure=0x7f6c3fb5c8c0, return_value=return_value@entry=0x0, n_param_values=4, param_values=param_values@entry=0x7fff0a340270, invocation_hint=invocation_hint@entry=0x7fff0a340210) at gclosure.c:777 #10 0x00007f6c3b1d135d in signal_emit_unlocked_R (node=node@entry=0x7f6c3faf5d10, detail=detail@entry=0, instance=instance@entry=0x7f6c3fb48e60, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fff0a340270) at gsignal.c:3586 #11 0x00007f6c3b1d90f2 in g_signal_emit_valist (instance=instance@entry=0x7f6c3fb48e60, signal_id=signal_id@entry=64, detail=detail@entry=0, var_args=var_args@entry=0x7fff0a3404a8) at gsignal.c:3330 #12 0x00007f6c3b1d98f8 in g_signal_emit_by_name (instance=0x7f6c3fb48e60, detailed_signal=0x7f6c3ea70f83 "state-changed") at gsignal.c:3426 #13 0x00007f6c3e8f894f in _set_state_full (self=0x7f6c3fb48e60, state=NM_DEVICE_STATE_UNMANAGED, reason=NM_DEVICE_STATE_REASON_REMOVED, quitting=0) at devices/nm-device.c:7486 #14 0x00007f6c3e8f0706 in nm_device_state_changed (self=0x7f6c3fb48e60, state=NM_DEVICE_STATE_UNMANAGED, reason=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:7623 #15 0x00007f6c3e8f808b in nm_device_set_unmanaged (self=0x7f6c3fb48e60, flag=NM_UNMANAGED_INTERNAL, unmanaged=1, reason=NM_DEVICE_STATE_REASON_REMOVED) at devices/nm-device.c:6652 #16 0x00007f6c3e9943d0 in remove_device (manager=0x7f6c3fb20150, device=0x7f6c3fb48e60, quitting=0, allow_unmanage=1) at nm-manager.c:752 #17 0x00007f6c3e995c29 in platform_link_cb (platform=0x7f6c3fa7a870, ifindex=73, plink=0x7fff0a341260, change_type=NM_PLATFORM_SIGNAL_REMOVED, reason=NM_PLATFORM_REASON_EXTERNAL, user_data=0x7f6c3fb20150) at nm-manager.c:2182 #18 0x00007f6c39829d8c in ffi_call_unix64 () at ../src/x86/unix64.S:76 #19 0x00007f6c398296bc in ffi_call (cif=cif@entry=0x7fff0a340bc0, fn=0x7f6c3e995b60 , rvalue=0x7fff0a340b30, avalue=avalue@entry=0x7fff0a340ab0) at ../src/x86/ffi64.c:522 #20 0x00007f6c3b1bfad8 in g_cclosure_marshal_generic (closure=0x7f6c3fb14cf0, return_gvalue=0x0, n_param_values=, param_values=, invocation_hint=, marshal_data=0x0) at gclosure.c:1454 #21 0x00007f6c3b1bf298 in g_closure_invoke (closure=0x7f6c3fb14cf0, return_value=return_value@entry=0x0, n_param_values=5, param_values=param_values@entry=0x7fff0a340dc0, invocation_hint=invocation_hint@entry=0x7fff0a340d60) at gclosure.c:777 #22 0x00007f6c3b1d135d in signal_emit_unlocked_R (node=node@entry=0x7f6c3fa76f00, detail=detail@entry=0, instance=instance@entry=0x7f6c3fa7a870, emission_return=emission_return@entry=0x0, instance_and_params=instance_and_params@entry=0x7fff0a340dc0) at gsignal.c:3586 #23 0x00007f6c3b1d90f2 in g_signal_emit_valist (instance=instance@entry=0x7f6c3fa7a870, signal_id=signal_id@entry=2, detail=detail@entry=0, var_args=var_args@entry=0x7fff0a341018) at gsignal.c:3330 #24 0x00007f6c3b1d98f8 in g_signal_emit_by_name (instance=0x7f6c3fa7a870, detailed_signal=0x7f6c3ea5f1fa "link-changed") at gsignal.c:3426 #25 0x00007f6c3e92412a in announce_object (platform=0x7f6c3fa7a870, object=0x7f6c3fbb6fd0, change_type=NM_PLATFORM_SIGNAL_REMOVED, reason=NM_PLATFORM_REASON_EXTERNAL) at platform/nm-linux-platform.c:1625 #26 0x00007f6c3e92b0f9 in event_notification (msg=0x7f6c3fa946f0, user_data=0x7f6c3fa7a870) at platform/nm-linux-platform.c:1986 #27 0x00007f6c3c35812f in nl_cb_call (msg=, type=, cb=) at ../include/netlink-private/netlink.h:141 #28 recvmsgs (cb=0x7f6c3fa7a620, sk=0x7f6c3fa7a710) at nl.c:952 #29 nl_recvmsgs_report (sk=0x7f6c3fa7a710, cb=0x7f6c3fa7a620) at nl.c:1003 #30 0x00007f6c3c3584f9 in nl_recvmsgs (sk=, cb=) at nl.c:1027 #31 0x00007f6c3e929dca in event_handler (channel=0x7f6c3fa78810, io_condition=G_IO_IN, user_data=0x7f6c3fa7a870) at platform/nm-linux-platform.c:4127 #32 0x00007f6c3aecc2a6 in g_main_dispatch (context=0x7f6c3fa68490) at gmain.c:3066 #33 g_main_context_dispatch (context=context@entry=0x7f6c3fa68490) at gmain.c:3642 #34 0x00007f6c3aecc628 in g_main_context_iterate (context=0x7f6c3fa68490, block=block@entry=1, dispatch=dispatch@entry=1, self=) at gmain.c:3713 #35 0x00007f6c3aecca3a in g_main_loop_run (loop=0x7f6c3fa68550) at gmain.c:3907 #36 0x00007f6c3e8e9fff in main (argc=1, argv=0x7fff0a341c88) at main.c:483 https://bugzilla.gnome.org/show_bug.cgi?id=741651 (cherry picked from commit 3ea9169c81805885fcc0d2527bfd856de3a0ecc0) --- src/devices/nm-device.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index ff6971fef8..5b284e9a23 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1495,11 +1495,16 @@ nm_device_slave_notify_release (NMDevice *dev, NMDeviceStateReason reason) master_status); nm_device_queue_state (dev, new_state, reason); - } else { + } else if (priv->master) { nm_log_info (LOGD_DEVICE, "(%s): released from master %s", nm_device_get_iface (dev), nm_device_get_iface (priv->master)); + } else { + nm_log_info (LOGD_DEVICE, + "(%s): released from master%s", + nm_device_get_iface (dev), + priv->enslaved ? "" : " (was not enslaved)"); } if (priv->enslaved) { -- cgit v1.2.1 From 2022a5adf154c88a28f8ef7e3b3ef957bb132071 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Tue, 3 Mar 2015 09:23:50 -0600 Subject: device: move to UNAVAILABLE state when deactivating if device is not available If the device assumed a connection but some of the resources for that connection (eg, carrier) are no longer available, the device will not be deactivated after "device: don't disconnect assumed connections" to ensure NM does not clean up something it shouldn't touch. But if the user manually deactivates the device, ensure that if it is no longer available, it's not moved to DISCONNECTED state when it really should be moved to UNAVAILABLE. If the device becomes available again during the DEACTIVATING state processing, then when it finally hits UNAVAILABLE it will be moved again to DISCONNECTED automatically. Reproducer: - add ethernet interface to an externally managed bridge - plug cable into ethernet interface - remove cable - remove interface from the bridge; it stays in 'activated' state - nmcli dev disconnect - device moves to 'disconnected' state even though there is no carrier --- src/devices/nm-device.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 5b284e9a23..997284878a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -6909,7 +6909,10 @@ _set_state_full (NMDevice *device, nm_act_request_get_connection (req), device); } else { - priv->dispatcher.post_state = NM_DEVICE_STATE_DISCONNECTED; + if (nm_device_is_available (device)) + priv->dispatcher.post_state = NM_DEVICE_STATE_DISCONNECTED; + else + priv->dispatcher.post_state = NM_DEVICE_STATE_UNAVAILABLE; priv->dispatcher.post_state_reason = reason; if (!nm_dispatcher_call (DISPATCHER_ACTION_PRE_DOWN, nm_act_request_get_connection (req), -- cgit v1.2.1