Re: [PATCH net-next] net/mlx5: Support devlink port state for host PF
From: Paolo Abeni <pabeni@redhat.com>
Date: 2026-02-05 15:57:17
Also in:
linux-rdma, lkml
On 2/3/26 11:24 AM, Tariq Toukan wrote:
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 4b7a1ce7f406..5fbfabe28bdb 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c@@ -1304,24 +1304,52 @@ static int mlx5_eswitch_load_ec_vf_vports(struct mlx5_eswitch *esw, u16 num_ec_v return err; } -static int host_pf_enable_hca(struct mlx5_core_dev *dev) +int mlx5_esw_host_pf_enable_hca(struct mlx5_core_dev *dev) { - if (!mlx5_core_is_ecpf(dev)) + struct mlx5_eswitch *esw = dev->priv.eswitch; + struct mlx5_vport *vport; + int err; + + if (!mlx5_core_is_ecpf(dev) || !mlx5_esw_allowed(esw)) return 0;
I was able to miss the AI feedback here: --- The old host_pf_enable_hca() only checked mlx5_core_is_ecpf(dev) before calling mlx5_cmd_host_pf_enable_hca(). The new function adds a check for mlx5_esw_allowed(esw), which returns false when esw is NULL or when the device is not an eswitch manager. When called from mlx5_host_pf_init() in ecpf.c on an ECPF device that is not an eswitch manager (the path when mlx5_ecpf_esw_admins_host_pf() returns false), this new condition will cause the function to return 0 without enabling the HCA. Is this behavior change intentional? The old code would enable the HCA in this configuration, but the new code skips it. The same concern applies to mlx5_esw_host_pf_disable_hca() below. --- and indeed it looks relevant. I think you have to follow-up or send a revert, whatever it's easier/faster.
quoted hunk ↗ jump to hunk
@@ -1347,7 +1375,7 @@ mlx5_eswitch_enable_pf_vf_vports(struct mlx5_eswitch *esw, if (mlx5_esw_host_functions_enabled(esw->dev)) { /* Enable external host PF HCA */ - ret = host_pf_enable_hca(esw->dev); + ret = mlx5_esw_host_pf_enable_hca(esw->dev);
Just FTR, more AI feedback here: --- The old host_pf_disable_hca() was a void function. The new mlx5_esw_host_pf_disable_hca() returns int and can fail, but the return value is not checked here in the error path. If mlx5_esw_host_pf_disable_hca() fails, it returns without setting vport->pf_activated = false. This leaves pf_activated set to true even though the HCA state may be inconsistent. Later, mlx5_devlink_pf_port_fn_state_get() reads vport->pf_activated to report state to userspace, which could then report incorrect state. Should the return value be checked, or should the pf_activated flag be updated unconditionally to reflect the intended state? The same pattern appears in mlx5_eswitch_disable_pf_vf_vports(). ---