Re: [PATCH net-next V2 3/7] net/mlx5: E-Switch, Add support for adjacent functions vports discovery

[Saeed Mahameed <saeed@kernel.org>]

On 28 Aug 12:34, Simon Horman wrote:
>On Tue, Aug 26, 2025 at 09:45:12PM -0700, Saeed Mahameed wrote:
>> From: Adithya Jayachandran <ajayachandra@nvidia.com>
>>
>> Adding driver support to query adjacent functions vports, AKA
>> delegated vports.
>>
>> Adjacent functions can delegate their sriov vfs to other sibling PF in
>> the system, to be managed by the eswitch capable sibling PF.
>> E.g, ECPF to Host PF, multi host PF between each other, etc.
>>
>> Only supported in switchdev mode.
>>
>> Signed-off-by: Adithya Jayachandran <ajayachandra@nvidia.com>
>> Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
>
>...
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
>
>...
>
>> +static int
>> +mlx5_eswitch_load_adj_vf_vports(struct mlx5_eswitch *esw,
>> +				enum mlx5_eswitch_vport_event enabled_events)
>> +{
>> +	struct mlx5_vport *vport;
>> +	unsigned long i;
>> +	int err;
>> +
>> +	mlx5_esw_for_each_vf_vport(esw, i, vport, U16_MAX) {
>> +		if (!vport->adjacent)
>> +			continue;
>> +		err = mlx5_eswitch_load_pf_vf_vport(esw, vport->vport,
>> +						    enabled_events);
>> +		if (err)
>> +			goto unload_adj_vf_vport;
>> +	}
>> +
>> +	return 0;
>> +
>> +unload_adj_vf_vport:
>> +	mlx5_eswitch_unload_adj_vf_vports(esw);
>> +	return err;
>> +}
>> +
>>  int mlx5_eswitch_load_vf_vports(struct mlx5_eswitch *esw, u16 num_vfs,
>>  				enum mlx5_eswitch_vport_event enabled_events)
>>  {
>> @@ -1345,7 +1382,15 @@ mlx5_eswitch_enable_pf_vf_vports(struct mlx5_eswitch *esw,
>>  					  enabled_events);
>>  	if (ret)
>>  		goto vf_err;
>> +
>> +	/* Enable adjacent VF vports */
>> +	ret = mlx5_eswitch_load_adj_vf_vports(esw, enabled_events);
>> +	if (ret)
>> +		goto unload_adj_vf_vports;
>> +
>>  	return 0;
>> +unload_adj_vf_vports:
>> +	mlx5_eswitch_unload_adj_vf_vports(esw);
>>
>
>Hi Adithya and Saeed,
>
>mlx5_eswitch_load_adj_vf_vports() already unwinds on error,
>calling mlx5_eswitch_unload_adj_vf_vports().
>
>While resources allocated by the preceding call to
>mlx5_eswitch_load_vf_vports() (just before this hunk) seem to be leaked.
>
>So I wonder if the above two lines should be:
>
>unload_vf_vports:
>	mlx5_eswitch_unload_vf_vports(esw, num_vfs);
>

Yep, looks incorrect, let me double check and submit v3.

>>  vf_err:
>>  	if (mlx5_core_ec_sriov_enabled(esw->dev))
>
>...
>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> index cfd6b1b8c6f4..9f8bb397eae5 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> @@ -216,6 +216,7 @@ struct mlx5_vport {
>>  	u32                     metadata;
>>  	int                     vhca_id;
>>
>> +	bool adjacent; /* delegated vhca from adjacent function */
>>  	struct mlx5_vport_info  info;
>>
>>  	/* Protected with the E-Switch qos domain lock. The Vport QoS can
>> @@ -384,6 +385,7 @@ struct mlx5_eswitch {
>>
>>  	struct mlx5_esw_bridge_offloads *br_offloads;
>>  	struct mlx5_esw_offload offloads;
>> +	u32 last_vport_idx; /* ++ every time a vport is created */
>
>The comment above documents one operation that can occur
>on this field. But the code also performs others: decrement and set.
>
>So perhaps this is more appropriate?
>
>	u32 last_vport_idx; /* Number of vports created */
>
>Or dropping the comment entirely: it seems obvious enough.
>

Agreed, will drop.