core: set number of SR-IOV VFs asynchronously

When changing the number of VFs the kernel can block for very long
time in the write() to sysfs, especially if autoprobe-drivers is
enabled. Turn the nm_platform_link_set_sriov_params() into an
asynchronous function.

src/devices/nm-device.c

@@ -128,6 +128,15 @@ typedef struct {
 	int ifindex;
 } DeleteOnDeactivateData;
 
+typedef struct {
+	NMDevice *device;
+	GCancellable *cancellable;
+	NMPlatformAsyncCallback callback;
+	gpointer callback_data;
+	guint num_vfs;
+	NMTernary autoprobe;
+} SriovOp;
+
 typedef void (*AcdCallback) (NMDevice *, NMIP4Config **, gboolean);
 
 typedef struct {
@@ -575,13 +584,17 @@ typedef struct _NMDevicePrivate {
 
 	guint check_delete_unrealized_id;
 
+	struct {
+		SriovOp *pending;    /* SR-IOV operation currently running */
+		SriovOp *next;       /* next SR-IOV operation scheduled */
+	} sriov;
+
 	struct {
 		guint timeout_id;
 		guint refresh_rate_ms;
 		guint64 tx_bytes;
 		guint64 rx_bytes;
 	} stats;
-
 } NMDevicePrivate;
 
 G_DEFINE_ABSTRACT_TYPE (NMDevice, nm_device, NM_TYPE_DBUS_OBJECT)
@@ -4192,6 +4205,86 @@ nm_device_update_from_platform_link (NMDevice *self, const NMPlatformLink *plink
 	}
 }
 
+static void sriov_op_cb (GError *error, gpointer user_data);
+
+static void
+sriov_op_start (NMDevice *self, SriovOp *op)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+
+	nm_assert (!priv->sriov.pending);
+
+	op->cancellable = g_cancellable_new ();
+	op->device = g_object_ref (self);
+	priv->sriov.pending = op;
+
+	nm_platform_link_set_sriov_params_async (nm_device_get_platform (self),
+	                                         priv->ifindex,
+	                                         op->num_vfs,
+	                                         op->autoprobe,
+	                                         sriov_op_cb,
+	                                         op,
+	                                         op->cancellable);
+}
+
+static void
+sriov_op_cb (GError *error, gpointer user_data)
+{
+	SriovOp *op = user_data;
+	gs_unref_object NMDevice *self = op->device;
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+
+	nm_assert (op == priv->sriov.pending);
+
+	priv->sriov.pending = NULL;
+
+	if (op->callback)
+		op->callback (error, op->callback_data);
+
+	g_clear_object (&op->cancellable);
+	g_slice_free (SriovOp, op);
+
+	if (priv->sriov.next) {
+		sriov_op_start (self,
+		                g_steal_pointer (&priv->sriov.next));
+	}
+}
+
+static void
+sriov_op_queue (NMDevice *self,
+                guint num_vfs,
+                NMTernary autoprobe,
+                NMPlatformAsyncCallback callback,
+                gpointer callback_data)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+	GError *error = NULL;
+	SriovOp *op;
+
+	op = g_slice_new0 (SriovOp);
+	op->num_vfs = num_vfs;
+	op->autoprobe = autoprobe;
+	op->callback = callback;
+	op->callback_data = callback_data;
+
+	if (priv->sriov.next) {
+		/* Cancel the next operation immediately */
+		if (priv->sriov.next->callback) {
+			nm_utils_error_set_cancelled (&error, FALSE, NULL);
+			priv->sriov.next->callback (error, priv->sriov.next->callback_data);
+			g_clear_error (&error);
+		}
+		g_slice_free (SriovOp, priv->sriov.next);
+		priv->sriov.next = NULL;
+	}
+
+	if (priv->sriov.pending) {
+		priv->sriov.next = op;
+		g_cancellable_cancel (priv->sriov.pending->cancellable);
+	} else
+		sriov_op_start (self, op);
+}
+
 static void
 device_init_static_sriov_num_vfs (NMDevice *self)
 {
@@ -4206,10 +4299,8 @@ device_init_static_sriov_num_vfs (NMDevice *self)
 		                                          self,
 		                                          NULL);
 		num_vfs = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT32, -1);
-		if (num_vfs >= 0) {
+		if (num_vfs >= 0)
-			nm_platform_link_set_sriov_params (nm_device_get_platform (self),
+			sriov_op_queue (self, num_vfs, NM_TERNARY_DEFAULT, NULL, NULL);
-			                                   priv->ifindex, num_vfs, NM_TERNARY_DEFAULT);
-		}
 	}
 }
 
@@ -4223,11 +4314,11 @@ config_changed (NMConfig *config,
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
 	if (   priv->state <= NM_DEVICE_STATE_DISCONNECTED
-	    || priv->state > NM_DEVICE_STATE_ACTIVATED)
+	    || priv->state > NM_DEVICE_STATE_ACTIVATED) {
 		priv->ignore_carrier = nm_config_data_get_ignore_carrier (config_data, self);
-
+		if (NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_VALUES))
-	if (NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_VALUES))
+			device_init_static_sriov_num_vfs (self);
-		device_init_static_sriov_num_vfs (self);
+	}
 }
 
 static void
@@ -6265,6 +6356,41 @@ sriov_vf_config_to_platform (NMDevice *self,
 	return g_steal_pointer (&plat_vf);
 }
 
+static void
+sriov_params_cb (GError *error, gpointer data)
+{
+	NMDevice *self;
+	NMDevicePrivate *priv;
+	nm_auto_freev NMPlatformVF **plat_vfs = NULL;
+
+	nm_utils_user_data_unpack (data, &self, &plat_vfs);
+
+	if (nm_utils_error_is_cancelled (error, TRUE))
+		return;
+
+	priv = NM_DEVICE_GET_PRIVATE (self);
+
+	if (error) {
+		_LOGE (LOGD_DEVICE, "failed to set SR-IOV parameters: %s", error->message);
+		nm_device_state_changed (self,
+		                         NM_DEVICE_STATE_FAILED,
+		                         NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED);
+		return;
+	}
+
+	if (!nm_platform_link_set_sriov_vfs (nm_device_get_platform (self),
+	                                     priv->ifindex,
+	                                     (const NMPlatformVF *const *) plat_vfs)) {
+		_LOGE (LOGD_DEVICE, "failed to apply SR-IOV VFs");
+		nm_device_state_changed (self,
+		                         NM_DEVICE_STATE_FAILED,
+		                         NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED);
+		return;
+	}
+
+	nm_device_activate_schedule_stage2_device_config (self);
+}
+
 static NMActStageReturn
 act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason)
 {
@@ -6279,6 +6405,7 @@ act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason)
 		gs_free_error GError *error = NULL;
 		NMSriovVF *vf;
 		NMTernary autoprobe;
+		gpointer *data;
 
 		autoprobe = nm_setting_sriov_get_autoprobe_drivers (s_sriov);
 		if (autoprobe == NM_TERNARY_DEFAULT) {
@@ -6305,24 +6432,19 @@ act_stage1_prepare (NMDevice *self, NMDeviceStateReason *out_failure_reason)
 			}
 		}
 
-		if (!nm_platform_link_set_sriov_params (nm_device_get_platform (self),
+		/* When changing the number of VFs the kernel can block
-		                                        priv->ifindex,
+		 * for very long time in the write to sysfs, especially
-		                                        nm_setting_sriov_get_total_vfs (s_sriov),
+		 * if autoprobe-drivers is enabled. Do it asynchronously
-		                                        autoprobe)) {
+		 * to avoid blocking the entire NM process.
-			_LOGE (LOGD_DEVICE, "failed to apply SR-IOV parameters");
+		 */
-			NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED);
+		data = nm_utils_user_data_pack (self, g_steal_pointer (&plat_vfs));
-			return NM_ACT_STAGE_RETURN_FAILURE;
+		sriov_op_queue (self,
-		}
+		                nm_setting_sriov_get_total_vfs (s_sriov),
-
+		                autoprobe,
-		if (!nm_platform_link_set_sriov_vfs (nm_device_get_platform (self),
+		                sriov_params_cb,
-		                                     priv->ifindex,
+		                data);
-		                                     (const NMPlatformVF *const *) plat_vfs)) {
+		return NM_ACT_STAGE_RETURN_POSTPONE;
-			_LOGE (LOGD_DEVICE, "failed to apply SR-IOV VFs");
-			NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED);
-			return NM_ACT_STAGE_RETURN_FAILURE;
-		}
 	}
-
 	return NM_ACT_STAGE_RETURN_SUCCESS;
 }
 
@@ -14800,6 +14922,34 @@ ip6_managed_setup (NMDevice *self)
 	nm_device_sysctl_ip_conf_set (self, AF_INET6, "forwarding", "0");
 }
 
+static void
+deactivate_ready (NMDevice *self, NMDeviceStateReason reason)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+
+	if (priv->dispatcher.call_id)
+		return;
+
+	if (priv->sriov.pending)
+		return;
+	nm_assert (!priv->sriov.next);
+
+	nm_device_queue_state (self, NM_DEVICE_STATE_DISCONNECTED, reason);
+}
+
+static void
+sriov_deactivate_cb (GError *error, gpointer user_data)
+{
+	NMDevice *self;
+	gpointer reason;
+
+	if (nm_utils_error_is_cancelled (error, TRUE))
+		return;
+
+	nm_utils_user_data_unpack (user_data, &self, &reason);
+	deactivate_ready (self, (NMDeviceStateReason) reason);
+}
+
 static void
 deactivate_async_ready (NMDevice *self,
                         GError *error,
@@ -14820,7 +14970,8 @@ deactivate_async_ready (NMDevice *self,
 		_LOGW (LOGD_DEVICE, "Deactivation failed: %s",
 		       error->message);
 	}
-	nm_device_queue_state (self, NM_DEVICE_STATE_DISCONNECTED, reason);
+
+	deactivate_ready (self, reason);
 }
 
 static void
@@ -14833,7 +14984,7 @@ deactivate_dispatcher_complete (NMDispatcherCallId *call_id, gpointer user_data)
 	g_return_if_fail (call_id == priv->dispatcher.call_id);
 	g_return_if_fail (priv->dispatcher.post_state == NM_DEVICE_STATE_DISCONNECTED);
 
-	reason = priv->dispatcher.post_state_reason;
+	reason = priv->state_reason;
 
 	priv->dispatcher.call_id = NULL;
 	priv->dispatcher.post_state = NM_DEVICE_STATE_UNKNOWN;
@@ -14849,7 +15000,7 @@ deactivate_dispatcher_complete (NMDispatcherCallId *call_id, gpointer user_data)
 		                                              deactivate_async_ready,
 		                                              GUINT_TO_POINTER (reason));
 	} else
-		nm_device_queue_state (self, NM_DEVICE_STATE_DISCONNECTED, reason);
+		deactivate_ready (self, reason);
 }
 
 static void
@@ -15062,12 +15213,6 @@ _set_state_full (NMDevice *self,
 		}
 		break;
 	case NM_DEVICE_STATE_DEACTIVATING:
-		if (   (s_sriov = nm_device_get_applied_setting (self, NM_TYPE_SETTING_SRIOV))
-		    && priv->ifindex > 0) {
-			nm_platform_link_set_sriov_params (nm_device_get_platform (self),
-			                                   priv->ifindex, 0, NM_TERNARY_TRUE);
-		}
-
 		_cancel_activation (self);
 
 		/* We cache the ignore_carrier state to not react on config-reloads while the connection
@@ -15089,6 +15234,15 @@ _set_state_full (NMDevice *self,
 				/* Just proceed on errors */
 				deactivate_dispatcher_complete (0, self);
 			}
+
+			if (   priv->ifindex > 0
+			    && (s_sriov = nm_device_get_applied_setting (self, NM_TYPE_SETTING_SRIOV))) {
+				sriov_op_queue (self,
+				                0,
+				                NM_TERNARY_TRUE,
+				                sriov_deactivate_cb,
+				                nm_utils_user_data_pack (self, (gpointer) reason));
+			}
 		}
 
 		nm_pacrunner_manager_remove_clear (&priv->pacrunner_conf_id);

src/platform/nm-linux-platform.c

@@ -6814,35 +6814,74 @@ nla_put_failure:
 	g_return_val_if_reached (FALSE);
 }
 
-static gboolean
+static void
-link_set_sriov_params (NMPlatform *platform,
+sriov_idle_cb (gpointer user_data,
-                       int ifindex,
+               GCancellable *cancellable)
-                       guint num_vfs,
+{
-                       NMTernary autoprobe)
+	gs_unref_object NMPlatform *platform = NULL;
+	gs_free_error GError *cancelled_error = NULL;
+	gs_free_error GError *error = NULL;
+	NMPlatformAsyncCallback callback;
+	gpointer callback_data;
+
+	g_cancellable_set_error_if_cancelled (cancellable, &cancelled_error);
+	nm_utils_user_data_unpack (user_data, &platform, &error, &callback, &callback_data);
+	callback (cancelled_error ?: error, callback_data);
+}
+
+static void
+link_set_sriov_params_async (NMPlatform *platform,
+                             int ifindex,
+                             guint num_vfs,
+                             NMTernary autoprobe,
+                             NMPlatformAsyncCallback callback,
+                             gpointer data,
+                             GCancellable *cancellable)
 {
 	nm_auto_pop_netns NMPNetns *netns = NULL;
+	gs_free_error GError *error = NULL;
 	nm_auto_close int dirfd = -1;
 	int current_autoprobe;
-	guint total;
+	guint i, total;
 	gint64 current_num;
 	char ifname[IFNAMSIZ];
+	gpointer packed;
+	const char *values[3];
 	char buf[64];
-	int errsv;
 
-	if (!nm_platform_netns_push (platform, &netns))
+	g_return_if_fail (callback || !data);
-		return FALSE;
+	g_return_if_fail (cancellable);
+
+	if (!nm_platform_netns_push (platform, &netns)) {
+		g_set_error_literal (&error,
+		                     NM_UTILS_ERROR,
+		                     NM_UTILS_ERROR_UNKNOWN,
+		                     "couldn't change namespace");
+		goto out_idle;
+	}
 
 	dirfd = nm_platform_sysctl_open_netdir (platform, ifindex, ifname);
-	if (!dirfd)
+	if (!dirfd) {
-		return FALSE;
+		g_set_error_literal (&error,
+		                     NM_UTILS_ERROR,
+		                     NM_UTILS_ERROR_UNKNOWN,
+		                     "couldn't open netdir");
+		goto out_idle;
+	}
 
 	total = nm_platform_sysctl_get_int_checked (platform,
 	                                            NMP_SYSCTL_PATHID_NETDIR (dirfd,
 	                                                                      ifname,
 	                                                                      "device/sriov_totalvfs"),
 	                                            10, 0, G_MAXUINT, 0);
-	if (errno)
+	if (errno) {
-		return FALSE;
+		g_set_error (&error,
+		             NM_UTILS_ERROR,
+		             NM_UTILS_ERROR_UNKNOWN,
+		             "failed reading sriov_totalvfs value: %s",
+		             nm_strerror_native (errno));
+		goto out_idle;
+	}
 	if (num_vfs > total) {
 		_LOGW ("link: %d only supports %u VFs (requested %u)", ifindex, total, num_vfs);
 		num_vfs = total;
@@ -6874,23 +6913,7 @@ link_set_sriov_params (NMPlatform *platform,
 
 	if (   current_num == num_vfs
 	    && (autoprobe == NM_TERNARY_DEFAULT || current_autoprobe == autoprobe))
-		return TRUE;
+		goto out_idle;
-
-	if (current_num != 0) {
-		/* We need to destroy all other VFs before changing any value */
-		if (!nm_platform_sysctl_set (NM_PLATFORM_GET,
-		                             NMP_SYSCTL_PATHID_NETDIR (dirfd,
-		                                                       ifname,
-		                                                      "device/sriov_numvfs"),
-		                             "0")) {
-			errsv = errno;
-			_LOGW ("link: couldn't reset SR-IOV num_vfs: %s", nm_strerror_native (errsv));
-			return FALSE;
-		}
-	}
-
-	if (num_vfs == 0)
-		return TRUE;
 
 	if (   NM_IN_SET (autoprobe, NM_TERNARY_TRUE, NM_TERNARY_FALSE)
 	    && current_autoprobe != autoprobe
@@ -6899,22 +6922,40 @@ link_set_sriov_params (NMPlatform *platform,
 	                                                          ifname,
 	                                                          "device/sriov_drivers_autoprobe"),
 	                                nm_sprintf_buf (buf, "%d", (int) autoprobe))) {
-		errsv = errno;
+		g_set_error (&error,
-		_LOGW ("link: couldn't set SR-IOV drivers-autoprobe to %d: %s", (int) autoprobe, nm_strerror_native (errsv));
+		             NM_UTILS_ERROR,
-		return FALSE;
+		             NM_UTILS_ERROR_UNKNOWN,
+		             "couldn't set SR-IOV drivers-autoprobe to %d: %s",
+		            (int) autoprobe, nm_strerror_native (errno));
+		goto out_idle;
 	}
 
-	if (!nm_platform_sysctl_set (NM_PLATFORM_GET,
+	if (current_num == 0 && num_vfs == 0)
-	                             NMP_SYSCTL_PATHID_NETDIR (dirfd,
+		goto out_idle;
-	                                                       ifname,
-	                                                       "device/sriov_numvfs"),
-	                             nm_sprintf_buf (buf, "%u", num_vfs))) {
-		errsv = errno;
-		_LOGW ("link: couldn't set SR-IOV num_vfs to %d: %s", num_vfs, nm_strerror_native (errsv));
-		return FALSE;
-	}
 
-	return TRUE;
+	i = 0;
+	if (current_num != 0)
+		values[i++] = "0";
+	if (num_vfs != 0)
+		values[i++] = nm_sprintf_bufa (32, "%u", num_vfs);
+	values[i++] = NULL;
+
+	sysctl_set_async (platform,
+	                  NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "device/sriov_numvfs"),
+	                  values,
+	                  callback,
+	                  data,
+	                  cancellable);
+	return;
+
+out_idle:
+	if (callback) {
+		packed = nm_utils_user_data_pack (g_object_ref (platform),
+		                                  g_steal_pointer (&error),
+		                                  callback,
+		                                  data);
+		nm_utils_invoke_on_idle (sriov_idle_cb, packed, cancellable);
+	}
 }
 
 static gboolean
@@ -9223,7 +9264,7 @@ nm_linux_platform_class_init (NMLinuxPlatformClass *klass)
 	platform_class->link_get_permanent_address = link_get_permanent_address;
 	platform_class->link_set_mtu = link_set_mtu;
 	platform_class->link_set_name = link_set_name;
-	platform_class->link_set_sriov_params = link_set_sriov_params;
+	platform_class->link_set_sriov_params_async = link_set_sriov_params_async;
 	platform_class->link_set_sriov_vfs = link_set_sriov_vfs;
 	platform_class->link_set_bridge_vlans = link_set_bridge_vlans;
 

src/platform/nm-platform.c

@@ -1587,19 +1587,35 @@ nm_platform_link_supports_sriov (NMPlatform *self, int ifindex)
  * @num_vfs: the number of VFs to create
  * @autoprobe: the new autoprobe-drivers value (pass
  *     %NM_TERNARY_DEFAULT to keep current value)
+ * @callback: called when the operation finishes
+ * @callback_data: data passed to @callback
+ * @cancellable: cancellable to abort the operation
+ *
+ * Sets SR-IOV parameters asynchronously without
+ * blocking the main thread. The callback function is
+ * always invoked, and asynchronously.
  */
-gboolean
+void
-nm_platform_link_set_sriov_params (NMPlatform *self,
+nm_platform_link_set_sriov_params_async (NMPlatform *self,
-                                   int ifindex,
+                                         int ifindex,
-                                   guint num_vfs,
+                                         guint num_vfs,
-                                   NMTernary autoprobe)
+                                         NMTernary autoprobe,
+                                         NMPlatformAsyncCallback callback,
+                                         gpointer callback_data,
+                                         GCancellable *cancellable)
 {
-	_CHECK_SELF (self, klass, FALSE);
+	_CHECK_SELF_VOID (self, klass);
 
-	g_return_val_if_fail (ifindex > 0, FALSE);
+	g_return_if_fail (ifindex > 0);
 
 	_LOG3D ("link: setting %u total VFs and autoprobe %d", num_vfs, (int) autoprobe);
-	return klass->link_set_sriov_params (self, ifindex, num_vfs, autoprobe);
+	klass->link_set_sriov_params_async (self,
+	                                    ifindex,
+	                                    num_vfs,
+	                                    autoprobe,
+	                                    callback,
+	                                    callback_data,
+	                                    cancellable);
 }
 
 gboolean

src/platform/nm-platform.h

@@ -997,7 +997,13 @@ typedef struct {
 	int (*link_set_address) (NMPlatform *self, int ifindex, gconstpointer address, size_t length);
 	int (*link_set_mtu) (NMPlatform *self, int ifindex, guint32 mtu);
 	gboolean (*link_set_name) (NMPlatform *self, int ifindex, const char *name);
-	gboolean (*link_set_sriov_params) (NMPlatform *self, int ifindex, guint num_vfs, int autoprobe);
+	void (*link_set_sriov_params_async) (NMPlatform *self,
+	                                     int ifindex,
+	                                     guint num_vfs,
+	                                     int autoprobe,
+	                                     NMPlatformAsyncCallback callback,
+	                                     gpointer callback_data,
+	                                     GCancellable *cancellable);
 	gboolean (*link_set_sriov_vfs) (NMPlatform *self, int ifindex, const NMPlatformVF *const *vfs);
 	gboolean (*link_set_bridge_vlans) (NMPlatform *self, int ifindex, gboolean on_master, const NMPlatformBridgeVlan *const *vlans);
 
@@ -1434,7 +1440,15 @@ gboolean nm_platform_link_get_permanent_address (NMPlatform *self, int ifindex,
 int nm_platform_link_set_address (NMPlatform *self, int ifindex, const void *address, size_t length);
 int nm_platform_link_set_mtu (NMPlatform *self, int ifindex, guint32 mtu);
 gboolean nm_platform_link_set_name (NMPlatform *self, int ifindex, const char *name);
-gboolean nm_platform_link_set_sriov_params (NMPlatform *self, int ifindex, guint num_vfs, int autoprobe);
+
+void nm_platform_link_set_sriov_params_async (NMPlatform *self,
+                                              int ifindex,
+                                              guint num_vfs,
+                                              int autoprobe,
+                                              NMPlatformAsyncCallback callback,
+                                              gpointer callback_data,
+                                              GCancellable *cancellable);
+
 gboolean nm_platform_link_set_sriov_vfs (NMPlatform *self, int ifindex, const NMPlatformVF *const *vfs);
 gboolean nm_platform_link_set_bridge_vlans (NMPlatform *self, int ifindex, gboolean on_master, const NMPlatformBridgeVlan *const *vlans);