Re: [PATCH net] net/mlx5: E-switch, Query port function mac only when vport is enabled

[Tariq Toukan <tariqt@nvidia.com>]

On 11/11/2026 23:29, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
> 
> - [Medium] The new `if (vport->enabled)` guard in
>    mlx5_devlink_port_fn_hw_addr_get()…
> 
> --
> 
> net/mlx5: E-switch, Query port function mac only when vport is enabled
> 
> This commit wraps the mlx5_query_nic_vport_mac_address() call in
> mlx5_devlink_port_fn_hw_addr_get() in a vport->enabled check. The goal is
> to stop QUERY_NIC_VPORT_CONTEXT from failing in firmware when a port
> function is dumped while its vport is not enabled.
> 
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> index c712848b202db..b83c6c02670f8 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> @@ -5121,8 +5121,9 @@ int mlx5_devlink_port_fn_hw_addr_get(struct devlink_port *port,
>>   
>>   	mutex_lock(&esw->state_lock);
>>   
>> -	mlx5_query_nic_vport_mac_address(esw->dev, vport->vport, true,
>> -					 vport->info.mac);
>> +	if (vport->enabled)
>> +		mlx5_query_nic_vport_mac_address(esw->dev, vport->vport, true,
>> +						 vport->info.mac);
> 
> [Severity: Medium]
> Can this check ever be false here? The commit message says "A port
> function can be dumped while its vport is not enabled". The ordering of
> devlink port registration against vport enable/disable seems to rule
> that out.
> 
> mlx5_devlink_port_fn_hw_addr_get() is reachable only through the
> .port_fn_hw_addr_get op in mlx5_esw_pf_vf_dl_port_ops and
> mlx5_esw_dl_sf_port_ops. Devlink calls that op only on registered ports.
> 
> Those ports are registered in mlx5_esw_offloads_devlink_port_register(),
> via mlx5_esw_offloads_load_rep(). That happens only after the vport has
> been enabled:
> 
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c:mlx5_eswitch_load_vport() {
>      ...
> 	err = mlx5_esw_vport_enable(esw, vport, enabled_events);
> 	if (err)
> 		return err;
> 
> 	err = mlx5_esw_offloads_load_rep(esw, vport);
>      ...
> }
> 
> On teardown, the devlink port is unregistered before the vport is
> disabled:
> 
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c:mlx5_eswitch_unload_vport() {
> 	mlx5_esw_offloads_unload_rep(esw, vport);
> 	mlx5_esw_vport_disable(esw, vport);
> }
> 
> mlx5_esw_vport_enable() and mlx5_esw_vport_disable() are the only places
> that write vport->enabled. Rep netdev notifications that reach this op
> also run inside the same window. So every dump, doit or notify that gets
> here should already see vport->enabled == true.
> 
> If so, would the quoted QUERY_NIC_VPORT_CONTEXT "bad parameter(0x3),
> syndrome (0x29d722)" error still occur after this patch?
> 
> One possibility is a vport the driver has enabled but firmware still
> rejects for the query, such as an inactive SF. In that case this query
> still runs and still fails.
> 
> Another candidate is the unconditional query that the same Fixes commit
> eea31f21dce1 added to esw_vport_setup(). It runs on every vport enable,
> including bulk SF creation:
> 
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.c:esw_vport_setup() {
>      ...
> 	mlx5_query_nic_vport_mac_address(esw->dev, vport_num, true,
> 					 vport->info.mac);
>      ...
> }
> 
> This patch leaves that query unchanged. Was the failing path confirmed
> to be mlx5_devlink_port_fn_hw_addr_get() and not esw_vport_setup()?
> 

Ack.

pw-bot: cr

>>   	ether_addr_copy(hw_addr, vport->info.mac);
>>   	*hw_addr_len = ETH_ALEN;
>>   	mutex_unlock(&esw->state_lock);
>