ovs: fix assertion failure in netdev datapath mode

When using the netdev datapath, we wait for the link to appear in
different steps:

 1. initially, in act_stage3_ip_config() connects to platform's
   "link-changed" signal to detect when the TUN interface appears;

 2. when the interface appears, _netdev_tun_link_cb() schedules
   _set_ip_ifindex_tun() in a idle handler;

 3. _set_ip_ifindex_tun() checks if the link is ready (e.g. if the MAC
   address is correct) and in that case it reschedules stage3, which
   will move forward with the activation;

 4. if the link is not ready in _set_ip_ifindex_tun(), the function
   connects again to platform's "link-changed" signal to react to link
   changes;

 5. after the link changes and it is ready, _netdev_tun_link_cb()
   reschedules stage3, which moves forward with the activation;

With the current implementation it is possible that after step 2, if
act_stage3_ip_config() runs because it was already scheduled, it
registers again to the "link-changed" event; then when
_set_ip_ifindex_tun() is invoked it will hit assertion:

  nm_assert(!priv->wait_link.tun_link_signal_id);

Fix this by preventing that the signal gets registered again after
step 2.

Fixes-test: @ovs_datapath_type_netdev_with_cloned_mac

Fixes: acf485196c ('ovs-interface: wait that the cloned MAC changes instead of setting it')

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2024
This commit is contained in:
Beniamino Galvani
2024-08-28 20:58:05 +02:00
committed by Íñigo Huguet
parent 67416d52f4
commit b6e69f3467

View File

@@ -438,7 +438,7 @@ act_stage3_ip_config(NMDevice *device, int addr_family)
if (check_waiting_for_link(device, addr_family == AF_INET ? "stage3-ipv4" : "stage3-ipv6")) {
nm_device_devip_set_state(device, addr_family, NM_DEVICE_IP_STATE_PENDING, NULL);
if (nm_device_get_ip_ifindex(device) <= 0 && priv->wait_link.tun_link_signal_id == 0
&& ovs_interface_is_netdev_datapath(self)) {
&& priv->wait_link.tun_ifindex <= 0 && ovs_interface_is_netdev_datapath(self)) {
priv->wait_link.tun_link_signal_id = g_signal_connect(nm_device_get_platform(device),
NM_PLATFORM_SIGNAL_LINK_CHANGED,
G_CALLBACK(_netdev_tun_link_cb),
@@ -483,6 +483,7 @@ deactivate(NMDevice *device)
NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE(device);
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE(self);
priv->wait_link.tun_ifindex = -1;
priv->wait_link.waiting = FALSE;
priv->wait_link.cloned_mac_evaluated = FALSE;
nm_clear_g_free(&priv->wait_link.cloned_mac);
@@ -581,6 +582,7 @@ deactivate_async(NMDevice *device,
nm_clear_g_signal_handler(nm_device_get_platform(device), &priv->wait_link.tun_link_signal_id);
nm_clear_g_source_inst(&priv->wait_link.tun_set_ifindex_idle_source);
priv->wait_link.tun_ifindex = -1;
priv->wait_link.cloned_mac_evaluated = FALSE;
nm_clear_g_free(&priv->wait_link.cloned_mac);
@@ -668,6 +670,8 @@ nm_device_ovs_interface_init(NMDeviceOvsInterface *self)
if (!nm_ovsdb_is_ready(priv->ovsdb))
g_signal_connect(priv->ovsdb, NM_OVSDB_READY, G_CALLBACK(ovsdb_ready), self);
priv->wait_link.tun_ifindex = -1;
}
static void