ovs: rework asynchronous deactivation of ovs interfaces

Tracking the deletion of link by ifindex is difficult because the
ifindex of the device is updated through delayed (idle) calls in
NMDevice and so there is the possibility that at a certain time the
device ifindex is not in sync with platform state. It seems simpler to
watch instead the interface name. The ugly thing is that the interface
name can be changed externally, but if users do that on an activating
device they are looking for trouble.

Also change the deactivate code to deal with the scenario where we
already created the interface in the ovsdb but the link didn't show up
yet. To ensure a proper cleanup we must wait that the link appears and
then goes away; however the link may never appear if vswitchd sees
only the last state in ovsdb, and so we must use a ugly timeout to
avoid waiting forever.

https://bugzilla.redhat.com/show_bug.cgi?id=1787989
(cherry picked from commit 9c49f8a879)
This commit is contained in:
Beniamino Galvani 2020-02-13 14:52:11 +01:00
parent 644151b07a
commit 2e5e409bf2

View file

@ -21,7 +21,6 @@ _LOG_DECLARE_SELF(NMDeviceOvsInterface);
typedef struct {
bool waiting_for_interface:1;
int link_ifindex;
} NMDeviceOvsInterfacePrivate;
struct _NMDeviceOvsInterface {
@ -99,13 +98,12 @@ link_changed (NMDevice *device,
{
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);
if (pllink)
priv->link_ifindex = pllink->ifindex;
if (!pllink || !priv->waiting_for_interface)
return;
if ( pllink
&& priv->waiting_for_interface
&& nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) {
priv->waiting_for_interface = FALSE;
priv->waiting_for_interface = FALSE;
if (nm_device_get_state (device) == NM_DEVICE_STATE_IP_CONFIG) {
nm_device_bring_up (device, TRUE, NULL);
nm_device_activate_schedule_stage3_ip_config_start (device);
}
@ -129,12 +127,14 @@ act_stage3_ip_config_start (NMDevice *device,
gpointer *out_config,
NMDeviceStateReason *out_failure_reason)
{
NMDeviceOvsInterface *self = NM_DEVICE_OVS_INTERFACE (device);
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (device);
if (!_is_internal_interface (device))
return NM_ACT_STAGE_RETURN_IP_FAIL;
if (nm_device_get_ip_ifindex (device) <= 0) {
_LOGT (LOGD_DEVICE, "waiting for link to appear");
priv->waiting_for_interface = TRUE;
return NM_ACT_STAGE_RETURN_POSTPONE;
}
@ -164,11 +164,17 @@ typedef struct {
gpointer callback_user_data;
gulong link_changed_id;
gulong cancelled_id;
guint link_timeout_id;
} DeactivateData;
static void
deactivate_invoke_cb (DeactivateData *data, GError *error)
{
NMDeviceOvsInterface *self = data->self;
_LOGT (LOGD_CORE,
"deactivate: async callback (%s)",
error ? error->message : "success");
data->callback (NM_DEVICE (data->self),
error,
data->callback_user_data);
@ -177,34 +183,43 @@ deactivate_invoke_cb (DeactivateData *data, GError *error)
&data->link_changed_id);
nm_clear_g_signal_handler (data->cancellable,
&data->cancelled_id);
nm_clear_g_source (&data->link_timeout_id);
g_object_unref (data->self);
g_object_unref (data->cancellable);
nm_g_slice_free (data);
}
static void
link_changed_cb (NMPlatform *platform,
int obj_type_i,
int ifindex,
NMPlatformLink *info,
int change_type_i,
DeactivateData *data)
deactivate_link_changed_cb (NMPlatform *platform,
int obj_type_i,
int ifindex,
NMPlatformLink *info,
int change_type_i,
DeactivateData *data)
{
NMDeviceOvsInterface *self = data->self;
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (self);
const NMPlatformSignalChangeType change_type = change_type_i;
if ( change_type == NM_PLATFORM_SIGNAL_REMOVED
&& ifindex == priv->link_ifindex) {
_LOGT (LOGD_DEVICE,
"link %d gone, proceeding with deactivation",
priv->link_ifindex);
priv->link_ifindex = 0;
&& nm_streq0 (info->name, nm_device_get_iface (NM_DEVICE (self)))) {
_LOGT (LOGD_DEVICE, "deactivate: link removed, proceeding");
nm_device_update_from_platform_link (NM_DEVICE (self), NULL);
deactivate_invoke_cb (data, NULL);
return;
}
}
static gboolean
deactivate_link_timeout (gpointer user_data)
{
DeactivateData *data = user_data;
NMDeviceOvsInterface *self = data->self;
_LOGT (LOGD_DEVICE, "deactivate: timeout waiting link removal");
deactivate_invoke_cb (data, NULL);
return G_SOURCE_REMOVE;
}
static void
deactivate_cancelled_cb (GCancellable *cancellable,
gpointer user_data)
@ -236,7 +251,14 @@ deactivate_async (NMDevice *device,
NMDeviceOvsInterfacePrivate *priv = NM_DEVICE_OVS_INTERFACE_GET_PRIVATE (self);
DeactivateData *data;
priv->waiting_for_interface = FALSE;
_LOGT (LOGD_CORE, "deactivate: start async");
/* We want to ensure that the kernel link for this device is
* removed upon disconnection so that it will not interfere with
* later activations of the same device. Unfortunately there is
* no synchronization mechanism with vswitchd, we only update
* ovsdb and wait that changes are picked up.
*/
data = g_slice_new (DeactivateData);
*data = (DeactivateData) {
@ -246,16 +268,26 @@ deactivate_async (NMDevice *device,
.callback_user_data = callback_user_data,
};
if ( !priv->link_ifindex
|| !nm_platform_link_get (nm_device_get_platform (device), priv->link_ifindex)) {
priv->link_ifindex = 0;
if ( !priv->waiting_for_interface
&& !nm_platform_link_get_by_ifname (nm_device_get_platform (device),
nm_device_get_iface (device))) {
_LOGT (LOGD_CORE, "deactivate: link not present, proceeding");
nm_device_update_from_platform_link (NM_DEVICE (self), NULL);
nm_utils_invoke_on_idle (deactivate_cb_on_idle, data, cancellable);
return;
}
_LOGT (LOGD_DEVICE,
"async deactivation: waiting for link %d to disappear",
priv->link_ifindex);
if (priv->waiting_for_interface) {
/* At this point we have issued an INSERT and a DELETE
* command for the interface to ovsdb. We don't know if
* vswitchd will see the two updates or only one. We
* must add a timeout to avoid waiting forever in case
* the link doesn't appear.
*/
data->link_timeout_id = g_timeout_add (6000, deactivate_link_timeout, data);
_LOGT (LOGD_DEVICE, "deactivate: waiting for link to disappear in 6 seconds");
} else
_LOGT (LOGD_DEVICE, "deactivate: waiting for link to disappear");
data->cancelled_id = g_cancellable_connect (cancellable,
G_CALLBACK (deactivate_cancelled_cb),
@ -263,7 +295,7 @@ deactivate_async (NMDevice *device,
NULL);
data->link_changed_id = g_signal_connect (nm_device_get_platform (device),
NM_PLATFORM_SIGNAL_LINK_CHANGED,
G_CALLBACK (link_changed_cb),
G_CALLBACK (deactivate_link_changed_cb),
data);
}