From: 'Simon Horman' <horms@kernel.org>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
net/mlx5: Use v1 response layout for query_esw_functions
This patch teaches mlx5_esw_query_functions() to use the v1 response
layout (MLX5_QUERY_ESW_FUNC_OP_MOD_LAYOUT_V1) when the firmware
advertises query_host_net_function_v1, allocates the output buffer
based on query_host_net_function_num_max, and adds a parser for the
new network_function_params layout in mlx5_esw_get_host_pf_info().
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 861e79ddb489..8b62dde7eb70 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1100,12 +1133,55 @@ mlx5_esw_host_pf_from_host_params(const void *entry)
};
}
-struct mlx5_esw_pf_info mlx5_esw_get_host_pf_info(const u32 *out)
+static struct mlx5_esw_pf_info
+mlx5_esw_host_pf_from_net_func_params(const u8 *entry, int num_entries)
+{
+ int i;
+
+ for (i = 0; i < num_entries; i++) {
+ int pf_type, state;
+
+ pf_type = MLX5_GET(network_function_params, entry, pci_pf_type);
+ if (pf_type != MLX5_PCI_PF_TYPE_EXTERNAL_HOST_PF) {
+ entry += MLX5_UN_SZ_BYTES(net_function_params);
+ continue;
+ }
+
+ state = MLX5_GET(network_function_params, entry, vhca_state);
+
+ return (struct mlx5_esw_pf_info) {
+ .pf_disabled = state != MLX5_VHCA_STATE_IN_USE,
[Medium]
The commit message says "this change does not introduce new
functionality," but the legacy parser reads an explicit firmware
host_pf_disabled bit while the v1 parser derives pf_disabled from a
multi-valued vhca_state enum:
.pf_disabled = state != MLX5_VHCA_STATE_IN_USE,
Is this mapping intended to be exactly equivalent to the legacy
host_pf_disabled bit across all five vhca_state values
(INVALID/ALLOCATED/ACTIVE/IN_USE/TEARDOWN_REQUEST)?
The same vhca_state value is interpreted differently elsewhere in
mlx5 for SFs. mlx5_sf_to_devlink_opstate() in
drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c reports:
case MLX5_VHCA_STATE_IN_USE:
case MLX5_VHCA_STATE_TEARDOWN_REQUEST:
return DEVLINK_PORT_FN_OPSTATE_ATTACHED;
case MLX5_VHCA_STATE_INVALID:
case MLX5_VHCA_STATE_ALLOCATED:
case MLX5_VHCA_STATE_ACTIVE:
default:
return DEVLINK_PORT_FN_OPSTATE_DETACHED;
so SFs in TEARDOWN_REQUEST are reported as ATTACHED, while a host
PF in TEARDOWN_REQUEST would now end up as pf_disabled = true and
therefore DETACHED via mlx5_devlink_pf_port_fn_state_get():
quoted hunk ↗ jump to hunk
+ .num_of_vfs = MLX5_GET(network_function_params,
+ entry, pci_num_vfs),
+ .total_vfs = MLX5_GET(network_function_params,
+ entry, pci_total_vfs),
+ .host_number = MLX5_GET(network_function_params,
+ entry, host_number),
+ };
+ }
[Medium]
pf_disabled also feeds esw_vfs_changed_event_handler(), which skips
VF reload when it is true:
quoted hunk ↗ jump to hunk
@@ -3716,7 +3716,7 @@ static void esw_vfs_changed_event_handler(struct mlx5_eswitch *esw)
[ ... ]
quoted hunk ↗ jump to hunk
- host_pf_info = mlx5_esw_get_host_pf_info(out);
+ host_pf_info = mlx5_esw_get_host_pf_info(esw->dev, out);
new_num_vfs = host_pf_info.num_of_vfs;
if (new_num_vfs == esw->esw_funcs.num_vfs || host_pf_info.pf_disabled)
Could the commit message spell out the firmware/PRM mapping between
the legacy host_pf_disabled bit and the vhca_state enum so it is
clear that ACTIVE, ALLOCATED and TEARDOWN_REQUEST really should be
treated as disabled here?
Would it also be worth aligning with mlx5_sf_to_devlink_opstate()
(at least for TEARDOWN_REQUEST) so the devlink opstate reported via
mlx5_devlink_pf_port_fn_state_get() stays consistent between SFs and
host PFs on v1-capable firmware?