[PATCH 1/1] device: delay handling of link-changed platform event

[Thomas Haller <thaller redhat com>]

When inside a state-change, we set for example the device up.
This triggers a link-changed event, which then causes further
state-changes of the devices.

The handling of the link-changed event must be delayed and invoked
idly.

This avoid a serious assertions that can hit since platform-refactoring
(commit 35dcd8ac33d631c54532aa3bd0b3d2e026ec6407).
---
 src/devices/nm-device.c | 88 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 60 insertions(+), 28 deletions(-)

diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
index 9531ff2..b06635d 100644
--- a/src/devices/nm-device.c
+++ b/src/devices/nm-device.c
@@ -179,6 +179,9 @@ typedef struct {
        gboolean initialized;
        gboolean platform_link_initialized;
 
+       guint link_changed_cb_delayed_ifindex;
+       guint link_changed_cb_delayed_ip_ifindex;
+
        NMDeviceState state;
        NMDeviceStateReason state_reason;
        QueuedState   queued_state;
@@ -1333,8 +1336,8 @@ device_set_master (NMDevice *self, int ifindex)
        }
 }
 
-static void
-device_link_changed (NMDevice *self, NMPlatformLink *info)
+static gboolean
+device_link_changed (NMDevice *self)
 {
        NMDeviceClass *klass = NM_DEVICE_GET_CLASS (self);
        NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
@@ -1342,8 +1345,16 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
        gboolean ip_ifname_changed = FALSE;
        gboolean platform_unmanaged = FALSE;
        const char *udi;
+       NMPlatformLink info;
+       int ifindex;
+
+       priv->link_changed_cb_delayed_ifindex = 0;
+
+       ifindex = nm_device_get_ifindex (self);
+       if (!nm_platform_link_get (NM_PLATFORM_GET, ifindex, &info))
+               return G_SOURCE_REMOVE;
 
-       udi = nm_platform_link_get_udi (NM_PLATFORM_GET, info->ifindex);
+       udi = nm_platform_link_get_udi (NM_PLATFORM_GET, info.ifindex);
        if (udi && g_strcmp0 (udi, priv->udi)) {
                /* Update UDI to what udev gives us */
                g_free (priv->udi);
@@ -1351,24 +1362,24 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
                g_object_notify (G_OBJECT (self), NM_DEVICE_UDI);
        }
 
-       if (g_strcmp0 (info->driver, priv->driver)) {
+       if (g_strcmp0 (info.driver, priv->driver)) {
                /* Update driver to what udev gives us */
                g_free (priv->driver);
-               priv->driver = g_strdup (info->driver);
+               priv->driver = g_strdup (info.driver);
                g_object_notify (G_OBJECT (self), NM_DEVICE_DRIVER);
        }
 
        /* Update MTU if it has changed. */
-       if (priv->mtu != info->mtu) {
-               priv->mtu = info->mtu;
+       if (priv->mtu != info.mtu) {
+               priv->mtu = info.mtu;
                g_object_notify (G_OBJECT (self), NM_DEVICE_MTU);
        }
 
-       if (info->name[0] && strcmp (priv->iface, info->name) != 0) {
+       if (info.name[0] && strcmp (priv->iface, info.name) != 0) {
                _LOGI (LOGD_DEVICE, "interface index %d renamed iface from '%s' to '%s'",
-                      priv->ifindex, priv->iface, info->name);
+                      priv->ifindex, priv->iface, info.name);
                g_free (priv->iface);
-               priv->iface = g_strdup (info->name);
+               priv->iface = g_strdup (info.name);
 
                /* If the device has no explicit ip_iface, then changing iface changes ip_iface too. */
                ip_ifname_changed = !priv->ip_iface;
@@ -1387,10 +1398,10 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
        }
 
        /* Update slave status for external changes */
-       if (priv->enslaved && info->master != nm_device_get_ifindex (priv->master))
+       if (priv->enslaved && info.master != nm_device_get_ifindex (priv->master))
                nm_device_release_one_slave (priv->master, self, FALSE, NM_DEVICE_STATE_REASON_NONE);
-       if (info->master && !priv->enslaved) {
-               device_set_master (self, info->master);
+       if (info.master && !priv->enslaved) {
+               device_set_master (self, info.master);
                if (priv->master)
                        nm_device_enslave_slave (priv->master, self, NULL);
        }
@@ -1402,21 +1413,21 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
        }
 
        if (klass->link_changed)
-               klass->link_changed (self, info);
+               klass->link_changed (self, &info);
 
        /* Update DHCP, etc, if needed */
        if (ip_ifname_changed)
                update_for_ip_ifname_change (self);
 
-       if (priv->up != NM_FLAGS_HAS (info->flags, IFF_UP)) {
-               priv->up = NM_FLAGS_HAS (info->flags, IFF_UP);
+       if (priv->up != NM_FLAGS_HAS (info.flags, IFF_UP)) {
+               priv->up = NM_FLAGS_HAS (info.flags, IFF_UP);
 
                /* Manage externally-created software interfaces only when they are IFF_UP */
                g_assert (priv->ifindex > 0);
                if (NM_DEVICE_GET_CLASS (self)->can_unmanaged_external_down (self)) {
                        gboolean external_down = nm_device_get_unmanaged_flag (self, 
NM_UNMANAGED_EXTERNAL_DOWN);
 
-                       if (external_down && NM_FLAGS_HAS (info->flags, IFF_UP)) {
+                       if (external_down && NM_FLAGS_HAS (info.flags, IFF_UP)) {
                                if (nm_device_get_state (self) < NM_DEVICE_STATE_DISCONNECTED) {
                                        /* Ensure the assume check is queued before any queued state changes
                                         * from the transition to UNAVAILABLE.
@@ -1439,7 +1450,7 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
                                         */
                                        priv->unmanaged_flags &= ~NM_UNMANAGED_EXTERNAL_DOWN;
                                }
-                       } else if (!external_down && !NM_FLAGS_HAS (info->flags, IFF_UP) && 
nm_device_get_state (self) <= NM_DEVICE_STATE_DISCONNECTED) {
+                       } else if (!external_down && !NM_FLAGS_HAS (info.flags, IFF_UP) && 
nm_device_get_state (self) <= NM_DEVICE_STATE_DISCONNECTED) {
                                /* If the device is already disconnected and is set !IFF_UP,
                                 * unmanage it.
                                 */
@@ -1451,7 +1462,7 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
                }
        }
 
-       if (priv->ifindex > 0 && !priv->platform_link_initialized && info->initialized) {
+       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)) {
@@ -1466,23 +1477,34 @@ device_link_changed (NMDevice *self, NMPlatformLink *info)
                                         FALSE,
                                         NM_DEVICE_STATE_REASON_NOW_MANAGED);
        }
+
+       return G_SOURCE_REMOVE;
 }
 
-static void
-device_ip_link_changed (NMDevice *self, NMPlatformLink *info)
+static gboolean
+device_ip_link_changed (NMDevice *self)
 {
        NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+       NMPlatformLink info;
+       int ip_ifindex;
 
-       if (info->name[0] && g_strcmp0 (priv->ip_iface, info->name)) {
+       priv->link_changed_cb_delayed_ip_ifindex = 0;
+
+       ip_ifindex = nm_device_get_ip_ifindex (self);
+       if (!nm_platform_link_get (NM_PLATFORM_GET, ip_ifindex, &info))
+               return G_SOURCE_REMOVE;
+
+       if (info.name[0] && g_strcmp0 (priv->ip_iface, info.name)) {
                _LOGI (LOGD_DEVICE, "interface index %d renamed ip_iface (%d) from '%s' to '%s'",
                       priv->ifindex, nm_device_get_ip_ifindex (self),
-                      priv->ip_iface, info->name);
+                      priv->ip_iface, info.name);
                g_free (priv->ip_iface);
-               priv->ip_iface = g_strdup (info->name);
+               priv->ip_iface = g_strdup (info.name);
 
                g_object_notify (G_OBJECT (self), NM_DEVICE_IP_IFACE);
                update_for_ip_ifname_change (self);
        }
+       return G_SOURCE_REMOVE;
 }
 
 static void
@@ -1493,19 +1515,26 @@ link_changed_cb (NMPlatform *platform,
                  NMPlatformReason reason,
                  NMDevice *self)
 {
+       NMDevicePrivate *priv;
+
        if (change_type != NM_PLATFORM_SIGNAL_CHANGED)
                return;
 
+       priv = NM_DEVICE_GET_PRIVATE (self);
+
        /* We don't filter by 'reason' because we are interested in *all* link
         * changes. For example a call to nm_platform_link_set_up() may result
         * in an internal carrier change (i.e. we ask the kernel to set IFF_UP
         * and it results in also setting IFF_LOWER_UP.
         */
 
-       if (ifindex == nm_device_get_ifindex (self))
-               device_link_changed (self, info);
-       else if (ifindex == nm_device_get_ip_ifindex (self))
-               device_ip_link_changed (self, info);
+       if (ifindex == nm_device_get_ifindex (self)) {
+               if (!priv->link_changed_cb_delayed_ifindex)
+                       priv->link_changed_cb_delayed_ifindex = g_idle_add ((GSourceFunc) 
device_link_changed, self);
+       } else if (ifindex == nm_device_get_ip_ifindex (self)) {
+               if (!priv->link_changed_cb_delayed_ip_ifindex)
+                       priv->link_changed_cb_delayed_ip_ifindex = g_idle_add ((GSourceFunc) 
device_ip_link_changed, self);
+       }
 }
 
 static void
@@ -8888,6 +8917,9 @@ dispose (GObject *object)
        g_signal_handlers_disconnect_by_func (platform, G_CALLBACK (device_ip_changed), self);
        g_signal_handlers_disconnect_by_func (platform, G_CALLBACK (link_changed_cb), self);
 
+       nm_clear_g_source (&priv->link_changed_cb_delayed_ifindex);
+       nm_clear_g_source (&priv->link_changed_cb_delayed_ip_ifindex);
+
        G_OBJECT_CLASS (nm_device_parent_class)->dispose (object);
 }
 
-- 
2.4.3