diff options
author | Beniamino Galvani <bgalvani@redhat.com> | 2019-03-05 14:39:29 +0100 |
---|---|---|
committer | Beniamino Galvani <bgalvani@redhat.com> | 2019-05-28 10:35:04 +0200 |
commit | 121c58f0c48de9fb64a87ef02e3e090d90d2e96e (patch) | |
tree | 2802d1f3bef0cfbffd5e1050a5bedf2e97109fb7 | |
parent | abec66762abbe28461959f26db1cd95187c66252 (diff) |
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.
-rw-r--r-- | src/devices/nm-device.c | 224 | ||||
-rw-r--r-- | src/platform/nm-linux-platform.c | 129 | ||||
-rw-r--r-- | src/platform/nm-platform.c | 32 | ||||
-rw-r--r-- | src/platform/nm-platform.h | 18 |
4 files changed, 314 insertions, 89 deletions
diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 493288c197..ca0114f522 100644 --- a/src/devices/nm-device.c +++ b/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 { @@ -576,12 +585,16 @@ 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) { - nm_platform_link_set_sriov_params (nm_device_get_platform (self), - priv->ifindex, num_vfs, NM_TERNARY_DEFAULT); - } + if (num_vfs >= 0) + sriov_op_queue (self, num_vfs, NM_TERNARY_DEFAULT, NULL, NULL); } } @@ -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)) - device_init_static_sriov_num_vfs (self); + if (NM_FLAGS_HAS (changes, NM_CONFIG_CHANGE_VALUES)) + 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), - priv->ifindex, - nm_setting_sriov_get_total_vfs (s_sriov), - autoprobe)) { - _LOGE (LOGD_DEVICE, "failed to apply SR-IOV parameters"); - NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; - } - - 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_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_SRIOV_CONFIGURATION_FAILED); - return NM_ACT_STAGE_RETURN_FAILURE; - } + /* 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. Do it asynchronously + * to avoid blocking the entire NM process. + */ + data = nm_utils_user_data_pack (self, g_steal_pointer (&plat_vfs)); + sriov_op_queue (self, + nm_setting_sriov_get_total_vfs (s_sriov), + autoprobe, + sriov_params_cb, + data); + return NM_ACT_STAGE_RETURN_POSTPONE; } - return NM_ACT_STAGE_RETURN_SUCCESS; } @@ -14801,6 +14923,34 @@ ip6_managed_setup (NMDevice *self) } 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, gpointer user_data) @@ -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); diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 8eb30d86f2..ac8e869d8e 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -6814,35 +6814,74 @@ nla_put_failure: g_return_val_if_reached (FALSE); } -static gboolean -link_set_sriov_params (NMPlatform *platform, - int ifindex, - guint num_vfs, - NMTernary autoprobe) +static void +sriov_idle_cb (gpointer user_data, + GCancellable *cancellable) +{ + 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)) - return FALSE; + g_return_if_fail (callback || !data); + 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) - return FALSE; + if (!dirfd) { + 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) - return FALSE; + if (errno) { + 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; - - 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; + goto out_idle; 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; - _LOGW ("link: couldn't set SR-IOV drivers-autoprobe to %d: %s", (int) autoprobe, nm_strerror_native (errsv)); - return FALSE; + g_set_error (&error, + NM_UTILS_ERROR, + 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, - NMP_SYSCTL_PATHID_NETDIR (dirfd, - 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; - } + if (current_num == 0 && num_vfs == 0) + goto out_idle; - 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; diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index de5ccdd8bd..4a98df84e8 100644 --- a/src/platform/nm-platform.c +++ b/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 -nm_platform_link_set_sriov_params (NMPlatform *self, - int ifindex, - guint num_vfs, - NMTernary autoprobe) +void +nm_platform_link_set_sriov_params_async (NMPlatform *self, + int ifindex, + guint num_vfs, + 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 diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 49d6da5686..a6b5f2c4c3 100644 --- a/src/platform/nm-platform.h +++ b/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); |