From 4669f01eb0fb35d32e397c372b14689088cfb59e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=8D=C3=B1igo=20Huguet?= Date: Wed, 14 Feb 2024 11:30:03 +0100 Subject: [PATCH] sriov: set the devlink's eswitch inline-mode and encap-mode Set these parameters according to the values set in the new properties sriov.eswitch-inline-mode and sriov.eswitch-encap-mode. The number of parameters related to SR-IOV was becoming too big. Refactor to group them in a NMPlatformSriovParams struct and pass it around. --- src/core/devices/nm-device.c | 41 +++++--- src/libnm-platform/devlink/nm-devlink.c | 12 +-- src/libnm-platform/nm-linux-platform.c | 119 +++++++++++++++--------- src/libnm-platform/nm-platform.c | 17 ++-- src/libnm-platform/nm-platform.h | 16 ++-- 5 files changed, 125 insertions(+), 80 deletions(-) diff --git a/src/core/devices/nm-device.c b/src/core/devices/nm-device.c index 07a763429e..34022efbde 100644 --- a/src/core/devices/nm-device.c +++ b/src/core/devices/nm-device.c @@ -137,9 +137,7 @@ typedef struct { GCancellable *cancellable; NMPlatformAsyncCallback callback; gpointer callback_data; - guint num_vfs; - NMOptionBool autoprobe; - NMSriovEswitchMode eswitch_mode; + NMPlatformSriovParams sriov_params; } SriovOp; typedef enum { @@ -7707,9 +7705,7 @@ sriov_op_start(NMDevice *self, SriovOp *op) nm_platform_link_set_sriov_params_async(nm_device_get_platform(self), priv->ifindex, - op->num_vfs, - op->autoprobe, - (_NMSriovEswitchMode) op->eswitch_mode, + op->sriov_params, sriov_op_cb, op, op->cancellable); @@ -7770,12 +7766,14 @@ sriov_op_queue_op(NMDevice *self, SriovOp *op) } static void -sriov_op_queue(NMDevice *self, - guint num_vfs, - NMOptionBool autoprobe, - NMSriovEswitchMode eswitch_mode, - NMPlatformAsyncCallback callback, - gpointer callback_data) +sriov_op_queue(NMDevice *self, + guint num_vfs, + NMOptionBool autoprobe, + NMSriovEswitchMode eswitch_mode, + NMSriovEswitchInlineMode eswitch_inline_mode, + NMSriovEswitchEncapMode eswitch_encap_mode, + NMPlatformAsyncCallback callback, + gpointer callback_data) { SriovOp *op; @@ -7800,9 +7798,14 @@ sriov_op_queue(NMDevice *self, op = g_slice_new(SriovOp); *op = (SriovOp){ - .num_vfs = num_vfs, - .autoprobe = autoprobe, - .eswitch_mode = eswitch_mode, + .sriov_params = + (NMPlatformSriovParams){ + .num_vfs = num_vfs, + .autoprobe = autoprobe, + .eswitch_mode = (_NMSriovEswitchMode) eswitch_mode, + .eswitch_inline_mode = (_NMSriovEswitchInlineMode) eswitch_inline_mode, + .eswitch_encap_mode = (_NMSriovEswitchEncapMode) eswitch_encap_mode, + }, .callback = callback, .callback_data = callback_data, }; @@ -7831,6 +7834,8 @@ device_init_static_sriov_num_vfs(NMDevice *self) num_vfs, NM_OPTION_BOOL_DEFAULT, NM_SRIOV_ESWITCH_MODE_PRESERVE, + NM_SRIOV_ESWITCH_INLINE_MODE_PRESERVE, + NM_SRIOV_ESWITCH_ENCAP_MODE_PRESERVE, NULL, NULL); } @@ -10014,6 +10019,8 @@ activate_stage1_device_prepare(NMDevice *self) nm_setting_sriov_get_total_vfs(s_sriov), NM_TERNARY_TO_OPTION_BOOL(autoprobe), nm_setting_sriov_get_eswitch_mode(s_sriov), + nm_setting_sriov_get_eswitch_inline_mode(s_sriov), + nm_setting_sriov_get_eswitch_encap_mode(s_sriov), sriov_params_cb, nm_utils_user_data_pack(self, g_steal_pointer(&plat_vfs))); priv->stage1_sriov_state = NM_DEVICE_STAGE_STATE_PENDING; @@ -16731,6 +16738,8 @@ _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, 0, NM_OPTION_BOOL_TRUE, NM_SRIOV_ESWITCH_MODE_PRESERVE, + NM_SRIOV_ESWITCH_INLINE_MODE_PRESERVE, + NM_SRIOV_ESWITCH_ENCAP_MODE_PRESERVE, sriov_reset_on_deactivate_cb, nm_utils_user_data_pack(self, GINT_TO_POINTER(reason))); } @@ -16784,6 +16793,8 @@ _set_state_full(NMDevice *self, NMDeviceState state, NMDeviceStateReason reason, 0, NM_OPTION_BOOL_TRUE, NM_SRIOV_ESWITCH_MODE_PRESERVE, + NM_SRIOV_ESWITCH_INLINE_MODE_PRESERVE, + NM_SRIOV_ESWITCH_ENCAP_MODE_PRESERVE, sriov_reset_on_failure_cb, self); break; diff --git a/src/libnm-platform/devlink/nm-devlink.c b/src/libnm-platform/devlink/nm-devlink.c index a2fee3bcf3..dbd8563850 100644 --- a/src/libnm-platform/devlink/nm-devlink.c +++ b/src/libnm-platform/devlink/nm-devlink.c @@ -171,7 +171,7 @@ err_cb_handler(const struct sockaddr_nl *nla, const struct nlmsgerr *err, void * *result = err->error; nlmsg_parse_error(nlmsg_undata(err), &extack_msg); - _LOGT("error response (%d) %s", err->error, extack_msg ?: nm_strerror(err->error)); + _LOGT("error response (%d - %s)", err->error, extack_msg ?: nm_strerror(err->error)); if (err_msg) *err_msg = g_strdup(extack_msg ?: nm_strerror(err->error)); @@ -212,7 +212,7 @@ devlink_send_and_recv(NMDevlink *self, while (cb_result == CB_RESULT_PENDING) { nle = nl_recvmsgs(self->genl_sock_sync, &cb); if (nle < 0 && nle != -EAGAIN) { - _LOGW("nl_recvmsgs() error: (%d) %s", nle, nm_strerror(nle)); + _LOGW("nl_recvmsgs() error (%d - %s)", nle, nm_strerror(nle)); break; } } @@ -286,13 +286,13 @@ nm_devlink_get_eswitch_params(NMDevlink *self, NMDevlinkEswitchParams *out_param g_set_error(error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "devlink: eswitch get: failed (%d) %s", + "devlink: eswitch get failed (%d - %s)", rc, err_msg); return FALSE; } - _LOGD("eswitch get: success"); + _LOGD("eswitch get success"); return TRUE; @@ -346,13 +346,13 @@ nm_devlink_set_eswitch_params(NMDevlink *self, NMDevlinkEswitchParams params, GE g_set_error(error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, - "devlink: eswitch set: failed (%d) %s", + "devlink: eswitch set failed (%d - %s)", rc, err_msg); return FALSE; } - _LOGD("eswitch set: success"); + _LOGD("eswitch set success"); return TRUE; diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index 765b02e788..7d34fa939d 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -8955,10 +8955,9 @@ sriov_set_autoprobe(NMPlatform *platform, #define _SRIOV_ASYNC_MAX_STEPS 4 typedef struct _SriovAsyncState { - NMPlatform *platform; - int ifindex; - guint num_vfs; - _NMSriovEswitchMode eswitch_mode; + NMPlatform *platform; + int ifindex; + NMPlatformSriovParams sriov_params; void (*steps[_SRIOV_ASYNC_MAX_STEPS])(struct _SriovAsyncState *); int current_step; NMPlatformAsyncCallback callback; @@ -9074,17 +9073,20 @@ sriov_async_step1_destroy_vfs(SriovAsyncState *async_state) static void sriov_async_step2_set_eswitch_mode(SriovAsyncState *async_state) { - NMPlatform *platform = async_state->platform; - NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE(platform); - gs_free NMDevlink *devlink = NULL; - gs_free_error GError *error = NULL; - NMDevlinkEswitchParams eswitch_params; + NMPlatform *platform = async_state->platform; + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE(platform); + gs_free NMDevlink *devlink = NULL; + gs_free_error GError *error = NULL; + NMDevlinkEswitchParams eswitch_params = { + .mode = async_state->sriov_params.eswitch_mode, + .inline_mode = async_state->sriov_params.eswitch_inline_mode, + .encap_mode = async_state->sriov_params.eswitch_encap_mode, + }; - _LOGD("setting eswitch-mode to '%d'", (int) async_state->eswitch_mode); - - eswitch_params.mode = (_NMSriovEswitchMode) async_state->eswitch_mode; - eswitch_params.inline_mode = _NM_SRIOV_ESWITCH_INLINE_MODE_PRESERVE; - eswitch_params.encap_mode = _NM_SRIOV_ESWITCH_ENCAP_MODE_PRESERVE; + _LOGD("setting eswitch params (mode=%d, inline-mode=%d, encap-mode=%d)", + (int) eswitch_params.mode, + (int) eswitch_params.inline_mode, + (int) eswitch_params.encap_mode); /* We set eswitch mode as a sriov_async step because it's in the middle of * other steps that are async. However, this step itself is synchronous. */ @@ -9101,9 +9103,9 @@ static void sriov_async_step3_create_vfs(SriovAsyncState *async_state) { NMPlatform *platform = async_state->platform; - const char *val = nm_sprintf_bufa(32, "%u", async_state->num_vfs); + const char *val = nm_sprintf_bufa(32, "%u", async_state->sriov_params.num_vfs); - _LOGD("setting sriov_numvfs to %u", async_state->num_vfs); + _LOGD("setting sriov_numvfs to %u", async_state->sriov_params.num_vfs); sriov_async_set_num_vfs(async_state, val); } @@ -9114,6 +9116,41 @@ sriov_async_step_finish_ok(SriovAsyncState *async_state) sriov_async_finish_err(async_state, NULL); } +static int +sriov_eswitch_get_needs_change(SriovAsyncState *async_state, + gboolean *out_needs_change, + GError **error) +{ + NMPlatform *platform = async_state->platform; + NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE(platform); + _NMSriovEswitchMode mode = async_state->sriov_params.eswitch_mode; + _NMSriovEswitchInlineMode inline_mode = async_state->sriov_params.eswitch_inline_mode; + _NMSriovEswitchEncapMode encap_mode = async_state->sriov_params.eswitch_encap_mode; + NMDevlinkEswitchParams current_params; + gs_free NMDevlink *devlink = NULL; + + nm_assert(out_needs_change); + + if (mode == _NM_SRIOV_ESWITCH_MODE_PRESERVE + && inline_mode == _NM_SRIOV_ESWITCH_INLINE_MODE_PRESERVE + && encap_mode == _NM_SRIOV_ESWITCH_ENCAP_MODE_PRESERVE) { + *out_needs_change = FALSE; + return 0; + } + + devlink = nm_devlink_new(platform, priv->sk_genl_sync, async_state->ifindex); + + if (!nm_devlink_get_eswitch_params(devlink, ¤t_params, error)) + return -1; + + *out_needs_change = (mode != _NM_SRIOV_ESWITCH_MODE_PRESERVE && mode != current_params.mode) + || (inline_mode != _NM_SRIOV_ESWITCH_INLINE_MODE_PRESERVE + && inline_mode != current_params.inline_mode) + || (encap_mode != _NM_SRIOV_ESWITCH_ENCAP_MODE_PRESERVE + && encap_mode != current_params.encap_mode); + return 0; +} + /* * Take special care when setting new values: * - don't touch anything if the right values are already set @@ -9123,24 +9160,19 @@ sriov_async_step_finish_ok(SriovAsyncState *async_state) static void link_set_sriov_params_async(NMPlatform *platform, int ifindex, - guint num_vfs, - NMOptionBool autoprobe, - _NMSriovEswitchMode eswitch_mode, + NMPlatformSriovParams sriov_params, NMPlatformAsyncCallback callback, gpointer data, GCancellable *cancellable) { SriovAsyncState *async_state; - NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE(platform); nm_auto_pop_netns NMPNetns *netns = NULL; gs_free_error GError *error = NULL; nm_auto_close int dirfd = -1; char ifname[IFNAMSIZ]; int max_vfs; int current_num_vfs; - int current_eswitch_mode = _NM_SRIOV_ESWITCH_MODE_PRESERVE; - NMDevlinkEswitchParams eswitch_params; - gboolean need_change_eswitch_mode; + gboolean need_change_eswitch_params; gboolean need_change_vfs; gboolean need_destroy_vfs; gboolean need_create_vfs; @@ -9152,7 +9184,7 @@ link_set_sriov_params_async(NMPlatform *platform, async_state = g_new0(SriovAsyncState, 1); async_state->platform = g_object_ref(platform); async_state->ifindex = ifindex; - async_state->eswitch_mode = eswitch_mode; + async_state->sriov_params = sriov_params; async_state->current_step = -1; async_state->callback = callback; async_state->data = data; @@ -9190,30 +9222,23 @@ link_set_sriov_params_async(NMPlatform *platform, return; } - if (num_vfs > max_vfs) { - _LOGW("link: device %d only supports %u VFs (requested %u)", ifindex, max_vfs, num_vfs); + if (sriov_params.num_vfs > max_vfs) { + _LOGW("link: device %d only supports %u VFs (requested %u)", + ifindex, + max_vfs, + sriov_params.num_vfs); _LOGW("link: reducing num_vfs to %u for device %d", max_vfs, ifindex); - num_vfs = max_vfs; + sriov_params.num_vfs = max_vfs; + async_state->sriov_params.num_vfs = max_vfs; } - async_state->num_vfs = num_vfs; - /* Setting autoprobe goes first, we can do it synchronously */ - if (num_vfs > 0 && !sriov_set_autoprobe(platform, dirfd, ifname, autoprobe, &error)) { + if (sriov_params.num_vfs > 0 + && !sriov_set_autoprobe(platform, dirfd, ifname, sriov_params.autoprobe, &error)) { sriov_async_finish_err(async_state, g_steal_pointer(&error)); return; } - if (eswitch_mode != _NM_SRIOV_ESWITCH_MODE_PRESERVE) { - gs_free NMDevlink *devlink = nm_devlink_new(platform, priv->sk_genl_sync, ifindex); - - if (!nm_devlink_get_eswitch_params(devlink, &eswitch_params, &error)) { - sriov_async_finish_err(async_state, g_steal_pointer(&error)); - return; - } - current_eswitch_mode = eswitch_params.mode; - } - /* Decide what actions we must do. Note that we might need to destroy the VFs even * if num_vfs == current_num_vfs, for example to change the eswitch mode. Because of * that, we might need to create VFs even if num_vfs == current_num_vfs. @@ -9223,16 +9248,18 @@ link_set_sriov_params_async(NMPlatform *platform, * 3. Create VFs * 4. Invoke caller's callback */ - need_change_eswitch_mode = - eswitch_mode != _NM_SRIOV_ESWITCH_MODE_PRESERVE && current_eswitch_mode != eswitch_mode; - need_change_vfs = num_vfs != current_num_vfs; - need_destroy_vfs = current_num_vfs > 0 && (need_change_eswitch_mode || need_change_vfs); - need_create_vfs = (current_num_vfs == 0 || need_destroy_vfs) && num_vfs > 0; + if (sriov_eswitch_get_needs_change(async_state, &need_change_eswitch_params, &error) < 0) { + sriov_async_finish_err(async_state, g_steal_pointer(&error)); + return; + } + need_change_vfs = sriov_params.num_vfs != current_num_vfs; + need_destroy_vfs = current_num_vfs > 0 && (need_change_eswitch_params || need_change_vfs); + need_create_vfs = (current_num_vfs == 0 || need_destroy_vfs) && sriov_params.num_vfs > 0; i = 0; if (need_destroy_vfs) async_state->steps[i++] = sriov_async_step1_destroy_vfs; - if (need_change_eswitch_mode) + if (need_change_eswitch_params) async_state->steps[i++] = sriov_async_step2_set_eswitch_mode; if (need_create_vfs) async_state->steps[i++] = sriov_async_step3_create_vfs; diff --git a/src/libnm-platform/nm-platform.c b/src/libnm-platform/nm-platform.c index 7fb2a2e2e7..b89b035925 100644 --- a/src/libnm-platform/nm-platform.c +++ b/src/libnm-platform/nm-platform.c @@ -2022,9 +2022,7 @@ nm_platform_link_supports_sriov(NMPlatform *self, int ifindex) void nm_platform_link_set_sriov_params_async(NMPlatform *self, int ifindex, - guint num_vfs, - NMOptionBool autoprobe, - _NMSriovEswitchMode eswitch_mode, + NMPlatformSriovParams sriov_params, NMPlatformAsyncCallback callback, gpointer callback_data, GCancellable *cancellable) @@ -2033,12 +2031,17 @@ nm_platform_link_set_sriov_params_async(NMPlatform *self, g_return_if_fail(ifindex > 0); - _LOG3D("link: setting %u total VFs and autoprobe %d", num_vfs, (int) autoprobe); + _LOG3D("link: setting SR-IOV params (numvfs=%u, autoprobe=%d, eswitch mode=%d inline-mode=%d " + "encap-mode=%d)", + sriov_params.num_vfs, + (int) sriov_params.autoprobe, + (int) sriov_params.eswitch_mode, + (int) sriov_params.eswitch_inline_mode, + (int) sriov_params.eswitch_encap_mode); + klass->link_set_sriov_params_async(self, ifindex, - num_vfs, - autoprobe, - eswitch_mode, + sriov_params, callback, callback_data, cancellable); diff --git a/src/libnm-platform/nm-platform.h b/src/libnm-platform/nm-platform.h index 8aba0e5c11..f6a6ba082b 100644 --- a/src/libnm-platform/nm-platform.h +++ b/src/libnm-platform/nm-platform.h @@ -993,6 +993,14 @@ typedef struct { guint8 public_key[NMP_WIREGUARD_PUBLIC_KEY_LEN]; } _nm_alignas(NMPlatformObject) NMPlatformLnkWireGuard; +typedef struct { + guint num_vfs; + NMOptionBool autoprobe; + _NMSriovEswitchMode eswitch_mode; + _NMSriovEswitchInlineMode eswitch_inline_mode; + _NMSriovEswitchEncapMode eswitch_encap_mode; +} NMPlatformSriovParams; + typedef enum { NM_PLATFORM_WIREGUARD_CHANGE_FLAG_NONE = 0, NM_PLATFORM_WIREGUARD_CHANGE_FLAG_REPLACE_PEERS = (1LL << 0), @@ -1172,9 +1180,7 @@ typedef struct { gboolean (*link_set_name)(NMPlatform *self, int ifindex, const char *name); void (*link_set_sriov_params_async)(NMPlatform *self, int ifindex, - guint num_vfs, - NMOptionBool autoprobe, - _NMSriovEswitchMode eswitch_mode, + NMPlatformSriovParams sriov_params, NMPlatformAsyncCallback callback, gpointer callback_data, GCancellable *cancellable); @@ -2036,9 +2042,7 @@ gboolean nm_platform_link_set_name(NMPlatform *self, int ifindex, const char *na void nm_platform_link_set_sriov_params_async(NMPlatform *self, int ifindex, - guint num_vfs, - NMOptionBool autoprobe, - _NMSriovEswitchMode eswitch_mode, + NMPlatformSriovParams sriov_params, NMPlatformAsyncCallback callback, gpointer callback_data, GCancellable *cancellable);