From 891e3b8f1bd14d4f7eea689ec3f0d7c1845d0ccd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Sep 2015 16:40:51 +0200 Subject: [PATCH 1/3] platform: fix handling refresh-link delayed actions Due to a bug, we would only handle one REFRESH_LINK delayed action and ignore the ones queued afterwards. Fixes: 051cf8bbde9b73cdb3718be94e78237758b451ff (cherry picked from commit eee240ffe82d1cf4e3e84688a37fbdc215c119d0) (cherry picked from commit bdbc3bbb7b919307b0a353b5de6bb1369127df5d) --- src/platform/nm-linux-platform.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index cd0a4e0..cc24707 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1617,7 +1617,7 @@ delayed_action_handle_one (NMPlatform *platform) user_data = priv->delayed_action.list_refresh_link->pdata[0]; g_ptr_array_remove_index_fast (priv->delayed_action.list_refresh_link, 0); - if (priv->delayed_action.list_master_connected->len == 0) + if (priv->delayed_action.list_refresh_link->len == 0) priv->delayed_action.flags &= ~DELAYED_ACTION_TYPE_REFRESH_LINK; nm_assert (_nm_utils_ptrarray_find_first (priv->delayed_action.list_refresh_link->pdata, priv->delayed_action.list_refresh_link->len, user_data) < 0); -- 2.4.3 From 211a064f15cbcc63374f14d39d939f0ab6c1070f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Sep 2015 15:05:00 +0200 Subject: [PATCH 2/3] platform: refresh links when parent gets removed When moving a link to another netns, it gets removed from NMPlatform's view. Currently kernel does not sent a notification to inform about that change (see related bug rh#1262908). Ensure that we reload all linked interfaces which now might have an invisible parent. (cherry picked from commit 2cd6aaa918333813879c97f7a1cb228e706a0586) (cherry picked from commit e79caf9b4ad888e9d78c4256f402b9eb8a524641) --- src/platform/nm-linux-platform.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index cc24707..6b65135 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1869,6 +1869,37 @@ cache_pre_hook (NMPCache *cache, const NMPObject *old, const NMPObject *new, NMP } } { + int ifindex = -1; + + /* removal of a link could be caused by moving the link to another netns. + * In this case, we potentially have to update other links that have this link as parent. + * Currently, kernel misses to sent us a notification in this case (rh #1262908). */ + + if ( ops_type == NMP_CACHE_OPS_REMOVED + && old /* <-- nonsensical, make coverity happy */ + && old->_link.netlink.is_in_netlink) + ifindex = old->link.ifindex; + else if ( ops_type == NMP_CACHE_OPS_UPDATED + && old && new /* <-- nonsensical, make coverity happy */ + && old->_link.netlink.is_in_netlink + && !new->_link.netlink.is_in_netlink) + ifindex = new->link.ifindex; + + if (ifindex > 0) { + const NMPlatformLink *const *links; + + links = cache_lookup_all_objects (NMPlatformLink, platform, NMP_OBJECT_TYPE_LINK, FALSE); + if (links) { + for (; *links; links++) { + const NMPlatformLink *l = (*links); + + if (l->parent == ifindex) + delayed_action_schedule (platform, DELAYED_ACTION_TYPE_REFRESH_LINK, GINT_TO_POINTER (l->ifindex)); + } + } + } + } + { /* if a link goes down, we must refresh routes */ if ( ops_type == NMP_CACHE_OPS_UPDATED && old && new /* <-- nonsensical, make coverity happy */ -- 2.4.3 From 729222d9e2997f262e070c1820b742e6aea07fa7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 14 Sep 2015 15:27:36 +0200 Subject: [PATCH 3/3] platform: cancel delayed action REFRESH_LINK when receiving an update When we receive an update for a link, cancel a scheduled REFRESH_LINK delayed-action for that ifindex. At the point when we scheduled refrehing the link, we only cared about receiving a notification that was newer then the current state. We scheduled requesting this new notification to resync the cache. It is not necessary to actually request a new update, any update we receive *after* requesting a new update will suffice. This potentially saves extra round-trips re-requesting the link. (cherry picked from commit f4f4e1cf092ce5236aa9394ad5c485ad1c5885c2) (cherry picked from commit 91c00072f2fd23e90a840a7c466c3ed95217eb68) --- src/platform/nm-linux-platform.c | 43 ++++++++++++++++++++++++++++++++++------ 1 file changed, 37 insertions(+), 6 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 6b65135..ac6bb05 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1653,6 +1653,33 @@ delayed_action_handle_idle (gpointer user_data) } static void +delayed_action_clear_REFRESH_LINK (NMPlatform *platform, int ifindex) +{ + NMLinuxPlatformPrivate *priv; + gssize idx; + gpointer user_data; + + if (ifindex <= 0) + return; + + priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); + if (!NM_FLAGS_HAS (priv->delayed_action.flags, DELAYED_ACTION_TYPE_REFRESH_LINK)) + return; + + user_data = GINT_TO_POINTER (ifindex); + + idx = _nm_utils_ptrarray_find_first (priv->delayed_action.list_refresh_link->pdata, priv->delayed_action.list_refresh_link->len, user_data); + if (idx < 0) + return; + + _LOGT_delayed_action (DELAYED_ACTION_TYPE_REFRESH_LINK, user_data, "clear"); + + g_ptr_array_remove_index_fast (priv->delayed_action.list_refresh_link, idx); + if (priv->delayed_action.list_refresh_link->len == 0) + priv->delayed_action.flags &= ~DELAYED_ACTION_TYPE_REFRESH_LINK; +} + +static void delayed_action_schedule (NMPlatform *platform, DelayedActionType action_type, gpointer user_data) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); @@ -2069,9 +2096,11 @@ do_request_all (NMPlatform *platform, DelayedActionType action_type, gboolean ha /* clear any delayed action that request a refresh of this object type. */ priv->delayed_action.flags &= ~iflags; + _LOGT_delayed_action (iflags, NULL, "handle (do-request-all)"); if (obj_type == NMP_OBJECT_TYPE_LINK) { priv->delayed_action.flags &= ~DELAYED_ACTION_TYPE_REFRESH_LINK; g_ptr_array_set_size (priv->delayed_action.list_refresh_link, 0); + _LOGT_delayed_action (DELAYED_ACTION_TYPE_REFRESH_LINK, NULL, "clear (do-request-all)"); } event_handler_read_netlink_all (platform, FALSE); @@ -2382,12 +2411,14 @@ event_notification (struct nl_msg *msg, gpointer user_data) switch (msghdr->nlmsg_type) { case RTM_NEWLINK: - if ( NMP_OBJECT_GET_TYPE (obj) == NMP_OBJECT_TYPE_LINK - && g_hash_table_lookup (priv->delayed_deletion, obj) != NULL) { - /* the object is scheduled for delayed deletion. Replace that object - * by clearing the value from priv->delayed_deletion. */ - _LOGT ("delayed-deletion: clear delayed deletion of protected object %s", nmp_object_to_string (obj, NMP_OBJECT_TO_STRING_ID, NULL, 0)); - g_hash_table_insert (priv->delayed_deletion, nmp_object_ref (obj), NULL); + if (NMP_OBJECT_GET_TYPE (obj) == NMP_OBJECT_TYPE_LINK) { + if (g_hash_table_lookup (priv->delayed_deletion, obj) != NULL) { + /* the object is scheduled for delayed deletion. Replace that object + * by clearing the value from priv->delayed_deletion. */ + _LOGT ("delayed-deletion: clear delayed deletion of protected object %s", nmp_object_to_string (obj, NMP_OBJECT_TO_STRING_ID, NULL, 0)); + g_hash_table_insert (priv->delayed_deletion, nmp_object_ref (obj), NULL); + } + delayed_action_clear_REFRESH_LINK (platform, obj->link.ifindex); } /* fall-through */ case RTM_NEWADDR: -- 2.4.3