Re: [PATCH net-next 11/15] net/mlx5: E-switch, add match-metadata allocator for nested VF/SF eswitches

[Shay Drory <shayd@nvidia.com>]

On 03/11/2026 19:10, Tariq Toukan wrote:
> From: Shay Drory <shayd@nvidia.com>
>
> The eswitch source-port match metadata is laid out as 4 bits of PFNUM
> and 12 bits of a unique id (ESW_SOURCE_PORT_METADATA_BITS). PFNUM is the
> native_port_num reported in HCA_CAP, so all VFs/SFs of the same PF share
> a single PFNUM. Each VF/SF eswitch allocates its id from its own
> vport_metadata_ida, restarting from the lowest value, so two VFs/SFs of
> the same PF can end up with identical metadata. Once their eswitches
> share an FDB (virt LAG), that metadata no longer uniquely identifies the
> source port.
>
> Add a dedicated allocator for nested (virt LAG) eswitches that replaces
> the PFNUM field with the VF/SF's slot in the LAG (its xarray index in
> ldev->fns). That index is unique per function within the LAG, so
> combined with a per-VF/SF id it yields unique metadata regardless of
> which PF the VFs/SFs belong to. The split is 8 bits for the virt LAG
> index - which fits the U8_MAX cap on the LAG xarray - and the remaining
> 8 bits for the id; the two still sum to ESW_SOURCE_PORT_METADATA_BITS,
> so nothing downstream of the match changes.
>
> The PFNUM value 0xf is reserved for internal port offload, which stamps
> it into reg_c_0 and matches it with the same mask used for vport
> metadata. That offload is gated on switchdev mode only, so it can be
> used on a nested eswitch as well. Cap the LAG index accordingly.
>
> Select the nested layout for any non-PF eswitch, so every PF keeps the
> PFNUM layout regardless of its virt LAG capabilities.
>
> Signed-off-by: Shay Drory <shayd@nvidia.com>
> Reviewed-by: Mark Bloch <mbloch@nvidia.com>
> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>
> Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> ---
>  .../mellanox/mlx5/core/eswitch_offloads.c     | 48 +++++++++++++++++++
>  .../net/ethernet/mellanox/mlx5/core/lag/lag.c | 17 +++++++
>  .../ethernet/mellanox/mlx5/core/mlx5_core.h   |  1 +
>  3 files changed, 66 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index 70e6dd9561a6..ef1043517e9e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@ -3651,6 +3651,48 @@ static u32 mlx5_esw_match_metadata_reserved(struct mlx5_eswitch *esw)
>  	return MLX5_ESW_METADATA_RSVD_UPLINK;
>  }
>  
> +/* Nested (virt LAG) source-port metadata layout: the upper ESW_NESTED_IDX_BITS
> + * hold the VF/SF's slot in the LAG and the lower ESW_NESTED_VPORT_BITS hold a
> + * vport id unique within its eswitch. The two fields still sum to
> + * ESW_SOURCE_PORT_METADATA_BITS. The LAG slot (0-254, per the xa_alloc() limit
> + * in mlx5_ldev_add_mdev()) fits in ESW_NESTED_IDX_BITS; slots 0xf0 and up would
> + * drive the PFNUM bits to the 0xf reserved by internal port offload, so only
> + * 0-239 are used.
> + */
> +#define ESW_NESTED_VPORT_BITS 8
> +#define ESW_NESTED_IDX_BITS (ESW_SOURCE_PORT_METADATA_BITS - ESW_NESTED_VPORT_BITS)
> +
> +static u32 mlx5_nested_esw_match_metadata_alloc(struct mlx5_eswitch *esw)
> +{
> +	u32 vport_end_ida = (1 << ESW_NESTED_VPORT_BITS) - 1;
> +	u32 max_idx = (1 << ESW_NESTED_IDX_BITS) -
> +		      (1 << (ESW_NESTED_IDX_BITS - ESW_PFNUM_BITS)) - 1;
> +	int idx;
> +	int id;
> +
> +	idx = mlx5_lag_get_member_idx(esw->dev);
> +	if (idx < 0 || idx > max_idx)
> +		return 0;
> +
> +	/* Use only non-zero id's (2-255). */
> +	id = ida_alloc_range(&esw->offloads.vport_metadata_ida,
> +			     MLX5_ESW_METADATA_RSVD_UPLINK + 1,
> +			     vport_end_ida, GFP_KERNEL);
> +	if (id < 0)
> +		return 0;
> +	id = (idx << ESW_NESTED_VPORT_BITS) | id;
> +	return id;
> +}
> +
> +static void mlx5_nested_esw_match_metadata_free(struct mlx5_eswitch *esw,
> +						u32 metadata)
> +{
> +	u32 vport_bit_mask = (1 << ESW_NESTED_VPORT_BITS) - 1;
> +
> +	/* Metadata holds only ESW_NESTED_VPORT_BITS of actual ida id. */
> +	ida_free(&esw->offloads.vport_metadata_ida, metadata & vport_bit_mask);
> +}
> +
>  u32 mlx5_esw_match_metadata_alloc(struct mlx5_eswitch *esw)
>  {
>  	u32 vport_end_ida = (1 << ESW_VPORT_BITS) - 1;
> @@ -3659,6 +3701,9 @@ u32 mlx5_esw_match_metadata_alloc(struct mlx5_eswitch *esw)
>  	int pf_num;
>  	int id;
>  
> +	if (!mlx5_core_is_pf(esw->dev) && !mlx5_virt_lag_nonmember(esw->dev))
> +		return mlx5_nested_esw_match_metadata_alloc(esw);
> +

answers to net-dev sashiko

Sashiko (High):
Does this predicate pick the intended devices? mlx5_virt_lag_nonmember()
is defined as:
static inline bool mlx5_virt_lag_nonmember(struct mlx5_core_dev *dev)
{
    return mlx5_virt_lag_is_supported(dev) && !mlx5_lag_dev(dev);
}
So !mlx5_virt_lag_nonmember() is also true for any VF/SF whose firmware
does not report the virt LAG caps (multi_uplink_eswitch, lag_master,
num_lag_ports < 2). That holds whether or not the device is in a LAG.
Such a VF/SF can still own an eswitch,

[SD] FW doesn't expose such a VF/SF.

because mlx5_eswitch_init() only
requires MLX5_ESWITCH_MANAGER. It never gets an ldev, though, because
mlx5_lag_add_mdev() returns early:
    if (!mlx5_lag_is_supported(dev))
        return;
For that device the enable path looks like this:
esw_offloads_enable()
  esw_offloads_metadata_init()
    esw_offloads_vport_metadata_setup()
      mlx5_esw_match_metadata_alloc()
        mlx5_nested_esw_match_metadata_alloc()
          mlx5_lag_get_member_idx()   <- ldev is NULL, returns -ENOENT
          return 0;
esw_offloads_vport_metadata_setup() then turns the 0 into -ENOSPC:
    vport->metadata = vport->default_metadata;
    return vport->metadata ? 0 : -ENOSPC;
Would that make switchdev mode fail on these non-PF eswitch managers?
Before this patch they got a PFNUM-layout id from mlx5_sd_pf_num_get().
mlx5_enable_lag_rep_bond() in en/rep/bond.c also calls
mlx5_esw_match_metadata_alloc(), so it would fail with -ENOSPC on the same
devices.
The behavior also seems to differ from the commit message:
    Select the nested layout for any non-PF eswitch, so every PF keeps the
    PFNUM layout regardless of its virt LAG capabilities.
With this predicate, a non-PF eswitch that is virt LAG capable but not in a
LAG keeps the PFNUM layout. A non-PF eswitch without virt LAG caps gets the
nested layout, which can never succeed for it.
At this commit, the virt LAG capable VFs/SFs cannot get an ldev either,
because mlx5_lag_add_mdev() still requires mlx5_lag_is_supported(). A later
commit in the series, "net/mlx5: LAG, enable LAG over functions that do not
represent a physical port", fixes that part. The predicate is unchanged at
the end of the series, so the failure for VF/SF eswitch managers without
virt LAG caps remains.


[SD] True, the sentence is looser than the code: a virt-capable non-PF
that is not in a LAG, e.g. one refused as a late joiner and left
standalone, keeps the PFNUM layout. That is fine: "net/mlx5: LAG, do not
add a device to an already-active LAG" also keeps such a device out of
the eswitch devcom component, so it never pairs with the eswitches of
the LAG members, and its metadata lives by itself and is never compared
against theirs.
In case of v2, The commit message wording will be fixed.


Would it work better to test for actual LAG membership? For example:
    if (!mlx5_core_is_pf(esw->dev) && mlx5_lag_dev(esw->dev))
This could optionally also check mlx5_virt_lag_is_supported(). The same
check would need to be used in mlx5_esw_match_metadata_free() so the alloc
and free sides stay consistent.

>  	/* Only 4 bits of pf_num */
>  	pf_num = mlx5_sd_pf_num_get(esw->dev);
>  	if (pf_num < 0 || pf_num > max_pf_num)
> @@ -3679,6 +3724,9 @@ void mlx5_esw_match_metadata_free(struct mlx5_eswitch *esw, u32 metadata)
>  {
>  	u32 vport_bit_mask = (1 << ESW_VPORT_BITS) - 1;
>  
> +	if (!mlx5_core_is_pf(esw->dev) && !mlx5_virt_lag_nonmember(esw->dev))
> +		return mlx5_nested_esw_match_metadata_free(esw, metadata);
> +
>  	/* Metadata contains only 12 bits of actual ida id */
>  	ida_free(&esw->offloads.vport_metadata_ida, metadata & vport_bit_mask);
>  }
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> index d73bac8441f7..4e15e16ae6e8 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.c
> @@ -516,6 +516,23 @@ int mlx5_lag_get_dev_seq(struct mlx5_core_dev *dev)
>  }
>  EXPORT_SYMBOL(mlx5_lag_get_dev_seq);
>  
> +int mlx5_lag_get_member_idx(struct mlx5_core_dev *dev)
> +{
> +	struct mlx5_lag *ldev = mlx5_lag_dev(dev);
> +	struct lag_fn *fn;
> +	int idx;
> +
> +	if (!ldev)
> +		return -ENOENT;
> +
> +	xa_lock(&ldev->fns);
> +	fn = mlx5_lag_fn_by_dev(ldev, dev);
> +	idx = fn ? fn->idx : -ENOENT;
> +	xa_unlock(&ldev->fns);
> +
> +	return idx;
> +}
> +
>  /* seq 0 = master, then all remaining devices */
>  static int mlx5_lag_get_dev_index_by_seq_all(struct mlx5_lag *ldev, int seq)
>  {
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
> index d84d0424d04e..f5f105762d88 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
> @@ -275,6 +275,7 @@ void mlx5_lag_add_mdev(struct mlx5_core_dev *dev);
>  void mlx5_lag_remove_mdev(struct mlx5_core_dev *dev);
>  void mlx5_lag_disable_change(struct mlx5_core_dev *dev);
>  void mlx5_lag_enable_change(struct mlx5_core_dev *dev);
> +int mlx5_lag_get_member_idx(struct mlx5_core_dev *dev);
>  
>  int mlx5_events_init(struct mlx5_core_dev *dev);
>  void mlx5_events_cleanup(struct mlx5_core_dev *dev);