Revert "platform: cancel delayed action REFRESH_LINK when receiving an update"

On some kernels (at least RHEL-7.2) we receive a spurious RTM_NEWLINK
message after the RTM_DELLINK message for deleting a bond master.

On RHEL-7, the following commands give:

    # ip link add dummy0 type dummy
    # ip link add bond0 type bond
    # ip link set bond0 up
    # ip link set dummy0 master bond0
    # ip monitor link &
    # ip link del bond0
    21: bond0: <BROADCAST,MULTICAST,MASTER> mtu 1500 qdisc noqueue state DOWN
        link/ether 1e:a6:6c:81:c1:8d brd ff:ff:ff:ff:ff:ff
    Deleted 21: bond0: <BROADCAST,MULTICAST,MASTER> mtu 1500 qdisc noop state DOWN
        link/ether 1e:a6:6c:81:c1:8d brd ff:ff:ff:ff:ff:ff
    20: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN
        link/ether 1e:a6:6c:81:c1:8d brd ff:ff:ff:ff:ff:ff
    21: bond0: <BROADCAST,MULTICAST,MASTER> mtu 1500 qdisc noop state DOWN
        link/ether da:ee:58:70:6f:e5 brd ff:ff:ff:ff:ff:ff

    ^^^^^^^^^^^^^^^ RTM_NEWLINK after RTM_DELLINK (and there follows no
    RTM_DELLINK afterwards)

    21: bond0: <BROADCAST,MULTICAST,MASTER> mtu 1500 qdisc noop state DOWN
        link/ether da:ee:58:70:6f:e5 brd ff:ff:ff:ff:ff:ff
    20: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN
        link/ether 1e:a6:6c:81:c1:8d brd ff:ff:ff:ff:ff:ff
    20: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN
        link/ether 1e:a6:6c:81:c1:8d brd ff:ff:ff:ff:ff:ff

Fix that by reverting clear_REFRESH_LINK(). This fix has two downsides:

- on kernels where this hack is not necessary, we unnecessarily refetch
  a link
- the platform cache first removes the link, adds it again and removes
  it. This is ugly, but should have no real consequences because all
  listeners to the platform signals delay processing the signals to an
  idle handler.

https://bugzilla.redhat.com/show_bug.cgi?id=1285719

This reverts commit f4f4e1cf09.
This commit is contained in:
Thomas Haller 2015-11-26 22:35:25 +01:00
parent 29c293728d
commit 83240f24ae
2 changed files with 51 additions and 28 deletions

View file

@ -2490,33 +2490,6 @@ delayed_action_handle_idle (gpointer user_data)
return G_SOURCE_REMOVE;
}
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)
{
@ -3118,7 +3091,6 @@ event_notification (struct nl_msg *msg, gpointer user_data)
_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:

View file

@ -1443,6 +1443,56 @@ out:
/*****************************************************************************/
static void
test_nl_bugs_spuroius_newlink (void)
{
const char *IFACE_BOND0 = "nm-test-bond0";
const char *IFACE_DUMMY0 = "nm-test-dummy0";
int ifindex_bond0, ifindex_dummy0;
const NMPlatformLink *pllink;
gboolean wait_for_settle;
/* see https://bugzilla.redhat.com/show_bug.cgi?id=1285719 */
nmtstp_run_command_check ("ip link add %s type dummy", IFACE_DUMMY0);
ifindex_dummy0 = nmtstp_assert_wait_for_link (IFACE_DUMMY0, NM_LINK_TYPE_DUMMY, 100)->ifindex;
nmtstp_run_command_check ("ip link add %s type bond", IFACE_BOND0);
ifindex_bond0 = nmtstp_assert_wait_for_link (IFACE_BOND0, NM_LINK_TYPE_BOND, 100)->ifindex;
nmtstp_link_set_updown (-1, ifindex_bond0, TRUE);
nmtstp_run_command_check ("ip link set %s master %s", IFACE_DUMMY0, IFACE_BOND0);
NMTST_WAIT_ASSERT (100, {
nmtstp_wait_for_signal (50);
pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex_dummy0);
g_assert (pllink);
if (pllink->master == ifindex_bond0)
break;
});
nmtstp_run_command_check ("ip link del %s", IFACE_BOND0);
wait_for_settle = TRUE;
nmtstp_wait_for_signal (50);
again:
nm_platform_process_events (NM_PLATFORM_GET);
pllink = nm_platform_link_get (NM_PLATFORM_GET, ifindex_bond0);
g_assert (!pllink);
if (wait_for_settle) {
wait_for_settle = FALSE;
NMTST_WAIT (300, { nmtstp_wait_for_signal (50); });
goto again;
}
nm_platform_link_delete (NM_PLATFORM_GET, ifindex_bond0);
nm_platform_link_delete (NM_PLATFORM_GET, ifindex_dummy0);
}
/*****************************************************************************/
void
init_tests (int *argc, char ***argv)
{
@ -1480,5 +1530,6 @@ setup_tests (void)
g_test_add_func ("/link/software/vlan/set-xgress", test_vlan_set_xgress);
g_test_add_func ("/link/nl-bugs/veth", test_nl_bugs_veth);
g_test_add_func ("/link/nl-bugs/spurious-newlink", test_nl_bugs_spuroius_newlink);
}
}