fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2025-12-20 04:40:04 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

ovs: set the tun interface up before stage3

Browse source 
When using the netdev datapath, we wait that the tun link appears, we
call nm_device_set_ip_ifindex() (which also brings the link up) and
then we check that the link is ready, i.e. that udev has announced the
link and the MAC address is correct. After that, we schedule stage3
(ip-config).

In this, there is a race condition that occurs sometimes in NMCI test
ovs_datapath_type_netdev_with_cloned_mac. In rare conditions,
nm_device_set_ip_ifindex() bring the interface up but then ovs-vswitch
changes again the flags of the interface without IFF_UP. The result is
that the interface stays down, breaking communications.

To fix this, we need to always call nm_device_bring_up() after the tun
device is ready. The problem is that we can't do it in
_netdev_tun_link_cb() because that function is already invoked
synchronously from platform code.

Instead, simplify the handling of the netdev datapath. Every
"link-changed" event from platform is handled by
_netdev_tun_link_cb(), which always schedule a delayed function
_netdev_tun_link_cb_in_idle(). This function just assigns the
ip-ifindex to the device if missing, and starts stage3 if the link is
ready. While doing so, it also bring the interface up.

Fixes: 99a6c6eda6 ('ovs, dpdk: fix creating ovs-interface when the ovs-bridge is netdev')

https://issues.redhat.com/browse/RHEL-17358

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/2218
This commit is contained in:
Beniamino Galvani 2025-06-04 10:57:51 +02:00
parent b678ceab9f
commit 46e0d2b4e4
1 changed files with 41 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

98
src/core/devices/ovs/nm-device-ovs-interface.c
View file

@ -28,16 +28,18 @@ typedef struct {
NMOvsdb *ovsdb;
struct {
/* The source for the idle handler to set the TUN ifindex */
GSource *tun_set_ifindex_idle_source;
/* The signal id for the TUN link-changed event */
gulong tun_link_signal_id;
/* The idle handler source for the TUN link-changed event */
GSource *tun_link_idle_source;
/* The ifindex for the TUN link-changed event */
int tun_ifindex;
/* The cloned MAC to set */
char *cloned_mac;
/* The id for the signal watching the TUN link to appear/change */
gulong tun_link_signal_id;
/* The TUN ifindex to set in the idle handler */
int tun_ifindex;
/* Whether we have determined the cloned MAC */
bool cloned_mac_evaluated : 1;
/* Whether we are waiting for the kernel link */
bool waiting : 1;
} wait_link;
@ -263,39 +265,33 @@ ready_for_ip_config(NMDevice *device, gboolean is_manual)
}
static gboolean
_set_ip_ifindex_tun(gpointer user_data)
_netdev_tun_link_cb_in_idle(gpointer user_data)
{
NMDevice *device = user_data;
NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE(device);
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE(self);
_LOGT(LOGD_CORE,
"ovs-wait-link: setting ip-ifindex %d from tun interface",
priv->wait_link.tun_ifindex);
if (nm_device_get_ip_ifindex(device) <= 0) {
_LOGT(LOGD_CORE,
"ovs-wait-link: setting ip-ifindex %d from tun link",
priv->wait_link.tun_ifindex);
nm_device_set_ip_ifindex(device, priv->wait_link.tun_ifindex);
}
nm_clear_g_source_inst(&priv->wait_link.tun_set_ifindex_idle_source);
nm_device_set_ip_ifindex(device, priv->wait_link.tun_ifindex);
if (check_waiting_for_link(device, "set-ip-ifindex-tun")) {
/* If the link is not ready, it means the MAC is not set yet. We don't have
* a convenient way to monitor for ip-ifindex changes other than listening
* for platform events again.*/
nm_assert(!priv->wait_link.tun_link_signal_id);
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),
self);
if (check_waiting_for_link(device, "tun-link-changed")) {
nm_clear_g_source_inst(&priv->wait_link.tun_link_idle_source);
return G_SOURCE_CONTINUE;
}
_LOGT(LOGD_CORE, "tun link is ready");
_LOGT(LOGD_CORE, "ovs-wait-link: tun link is ready");
nm_device_link_properties_set(device, FALSE);
nm_device_bring_up(device);
nm_device_devip_set_state(device, AF_INET, NM_DEVICE_IP_STATE_PENDING, NULL);
nm_device_devip_set_state(device, AF_INET6, NM_DEVICE_IP_STATE_PENDING, NULL);
nm_device_activate_schedule_stage3_ip_config(device, FALSE);
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_link_idle_source);
return G_SOURCE_CONTINUE;
}
@ -311,40 +307,28 @@ _netdev_tun_link_cb(NMPlatform *platform,
const NMPlatformSignalChangeType change_type = change_type_i;
NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE(device);
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE(self);
int ip_ifindex;
/* This is the handler for the link-changed platform events. It is triggered for all
* link changes. Keep only the ones matching our device. */
if (!NM_IN_SET(change_type, NM_PLATFORM_SIGNAL_ADDED, NM_PLATFORM_SIGNAL_CHANGED))
return;
if (pllink->type != NM_LINK_TYPE_TUN || !nm_streq0(pllink->name, nm_device_get_iface(device)))
return;
ip_ifindex = nm_device_get_ip_ifindex(device);
if (ip_ifindex > 0) {
/* When we have an ifindex, we are only waiting for the MAC to settle */
if (change_type != NM_PLATFORM_SIGNAL_CHANGED)
return;
if (!check_waiting_for_link(device, "tun-link-changed")) {
_LOGT(LOGD_CORE, "ovs-wait-link: tun link is ready, cloned MAC is set");
nm_clear_g_signal_handler(platform, &priv->wait_link.tun_link_signal_id);
nm_device_link_properties_set(device, FALSE);
nm_device_devip_set_state(device, AF_INET, NM_DEVICE_IP_STATE_PENDING, NULL);
nm_device_devip_set_state(device, AF_INET6, NM_DEVICE_IP_STATE_PENDING, NULL);
nm_device_activate_schedule_stage3_ip_config(device, FALSE);
}
return;
}
/* No ip-ifindex on the device, set it when the link appears */
if (change_type != NM_PLATFORM_SIGNAL_ADDED)
return;
_LOGT(LOGD_CORE,
"ovs-wait-link: found matching tun interface, schedule set-ip-ifindex(%d)",
"ovs-wait-link: got platform event \'%s\' for ifindex %d, scheduling idle handler",
change_type == NM_PLATFORM_SIGNAL_ADDED ? "added" : "changed",
ifindex);
nm_clear_g_signal_handler(platform, &priv->wait_link.tun_link_signal_id);
priv->wait_link.tun_ifindex = ifindex;
priv->wait_link.tun_set_ifindex_idle_source = nm_g_idle_add_source(_set_ip_ifindex_tun, device);
/* The handler is invoked by the platform synchronously in the netlink receive loop.
* We can't perform other platform operations (like bringing the interface up) since
* the code there is not re-entrant. Schedule an idle handler. */
nm_clear_g_source_inst(&priv->wait_link.tun_link_idle_source);
priv->wait_link.tun_link_idle_source =
nm_g_idle_add_source(_netdev_tun_link_cb_in_idle, device);
priv->wait_link.tun_ifindex = ifindex;
return;
}
static gboolean
@ -466,7 +450,7 @@ act_stage3_ip_config(NMDevice *device, int addr_family)
nm_device_activate_schedule_stage3_ip_config(device, TRUE);
return;
}
nm_clear_g_source_inst(&priv->wait_link.tun_set_ifindex_idle_source);
nm_clear_g_source_inst(&priv->wait_link.tun_link_idle_source);
nm_clear_g_signal_handler(nm_device_get_platform(device), &priv->wait_link.tun_link_signal_id);
nm_device_link_properties_set(device, FALSE);
@ -490,7 +474,7 @@ deactivate(NMDevice *device)
priv->wait_link.cloned_mac_evaluated = FALSE;
nm_clear_g_free(&priv->wait_link.cloned_mac);
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);
nm_clear_g_source_inst(&priv->wait_link.tun_link_idle_source);
}
typedef struct {
@ -583,7 +567,7 @@ deactivate_async(NMDevice *device,
_LOGT(LOGD_CORE, "deactivate: start async");
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);
nm_clear_g_source_inst(&priv->wait_link.tun_link_idle_source);
priv->wait_link.tun_ifindex = -1;
priv->wait_link.cloned_mac_evaluated = FALSE;
nm_clear_g_free(&priv->wait_link.cloned_mac);
@ -706,7 +690,7 @@ dispose(GObject *object)
nm_assert(!priv->wait_link.waiting);
nm_assert(priv->wait_link.tun_link_signal_id == 0);
nm_assert(!priv->wait_link.tun_set_ifindex_idle_source);
nm_assert(!priv->wait_link.tun_link_idle_source);
if (priv->ovsdb) {
g_signal_handlers_disconnect_by_func(priv->ovsdb, G_CALLBACK(ovsdb_ready), self);