Re: [PATCH net-next 14/15] net/mlx5e: Register eswitch-manager VF/SF self-port with switch-id

[Shay Drory]

On 03/11/2026 19:10, Tariq Toukan wrote:
> From: Shay Drory <shayd@xxxxxxxxxx>
>
> A non-PF function's self-port (its own uplink netdev) was always
> registered with DEVLINK_PORT_FLAVOUR_VIRTUAL, which doesn't carries
> switch_id. Consequently a function that acts as an eswitch manager - a
> VF (or SF) running its own nested eswitch - exposed no phys_switch_id on
> its uplink netdev, so it could not be grouped with the representors it
> manages.
>
> When the function is an eswitch manager, register its self-port with the
> switch_id taken from the system image guid. The uplink netdev is already
> bound to this devlink port, so phys_switch_id is derived by the core,
> and the managed representors - which stamp the same switch_id - group
> with it.
>
> Signed-off-by: Shay Drory <shayd@xxxxxxxxxx>
> Reviewed-by: Moshe Shemesh <moshe@xxxxxxxxxx>
> Reviewed-by: Mark Bloch <mbloch@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
> index 8818f65d1fbc..79adb1ecc450 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
> @@ -63,6 +63,11 @@ int mlx5e_devlink_port_register(struct mlx5e_dev *mlx5e_dev,
>  		dl_port_index = mlx5_esw_vport_to_devlink_port_index(mdev,
>  								     MLX5_VPORT_UPLINK);
>  	} else {
> +		if (MLX5_ESWITCH_MANAGER(mdev)) {
> +			mlx5e_devlink_get_port_parent_id(mdev, &ppid);
> +			memcpy(attrs.switch_id.id, ppid.id, ppid.id_len);
> +			attrs.switch_id.id_len = ppid.id_len;
> +		}

answers to net-dev sashiko

Sashiko (Medium):
Can this give the VF/SF uplink netdev the same phys_switch_id as its
parent PF's switch?
The only condition here is MLX5_ESWITCH_MANAGER(mdev). However,
mlx5_query_nic_sw_system_image_guid() makes a VF/SF id differ from the
PF's only when virt-LAG is supported:
drivers/net/ethernet/mellanox/mlx5/core/vport.c:
static u8 mlx5_sw_guid_vf(const struct mlx5_core_dev *mdev)
{
    return mlx5_core_is_vf(mdev) && mlx5_virt_lag_is_supported(mdev);
}
It also trims zero discriminator bytes from the result:
    for (i = 0; i < ARRAY_SIZE(mlx5_sw_guid_fields); i++) {
        u8 val = mlx5_sw_guid_fields[i](mdev);
        buf[(*len)++] = val;
        if (val)
            trim_len = *len;
    }
    *len = trim_len;
mlx5_virt_lag_is_supported() needs multi_uplink_eswitch, lag_master and
num_lag_ports < 2. mlx5_eswitch_init() creates an eswitch for any
function with MLX5_VPORT_MANAGER or MLX5_ESWITCH_MANAGER and does not
check those caps.
Take a VF/SF eswitch manager without the virt-LAG caps, for example a VF
that reports num_lag_ports >= 2. Would its uplink netdev now report the
plain FW system image GUID? That is the same switch_id that the parent
PF's uplink and representors use.
This also seems to affect TC offload on the PF side. tc_act_parse_mirred()
in en/tc/act/mirred.c calls parse_mirred() only when:
    else if (netdev_port_same_parent_id(priv->netdev, out_dev))
        err = parse_mirred(parse_state, act, priv, attr);
Before this patch, that check always failed for the VF/SF self netdev,
because the port had no switch_id.
Suppose the VF's nested eswitch is in switchdev mode in the same host as
the PF reps. A mirred redirect from a PF rep to the VF uplink would now
reach mlx5e_is_valid_eswitch_fwd_dev(). There,
same_hw_reps()->mlx5_same_hw_devs() compares the same identical sw GUIDs.
If the PF is in LAG (is_lag_dev()) or MPESW (is_multiport_eligible()),
the forward is accepted.
parse_mirred() would then program a PF FDB destination with:
    esw_attr->dests[esw_attr->out_count].vport = rpriv->rep->vport;
    esw_attr->dests[esw_attr->out_count].mdev = out_priv->mdev;
That destination is MLX5_VPORT_UPLINK of the VF's mdev, which belongs to
a different, nested eswitch. Is that intended?
The nested eswitch representors already used this colliding id before
this patch, through mlx5_esw_get_port_parent_id() in esw/devlink_port.c.
This patch extends the collision to the self/uplink netdev.
The earlier change "net/mlx5: Distinguish VFs/SFs in the SW system image
GUID for LAG grouping" kept the PF key for non virt-LAG VFs/SFs on
purpose, so they still count as the same HW in mlx5_same_hw_devs(). That
reasoning does not seem to cover publishing the key as a switch identity.
The later patches in this series do not appear to change en/devlink.c,
vport.c, lag.h or esw/devlink_port.c.
The driver code alone can't tell us whether firmware ever grants
eswitch_manager to a VF/SF without the virt-LAG caps. Nothing in the
driver seems to rule it out, though.
Should this branch set the switch_id only when the resulting GUID differs
from the PF's?

[SD] A VF/SF eswitch manager without the virt LAG caps doesn't exist. New
FW will let a VF/SF be an eswitch manager only as a nested eswitch
manager, and such a function reports multi_uplink_eswitch, lag_master and
num_lag_ports < 2, so mlx5_query_nic_sw_system_image_guid() gives it the
VF/SF marker and its switch_id differs from its PF's.

>  		attrs.flavour = DEVLINK_PORT_FLAVOUR_VIRTUAL;
>  		dl_port_index = mlx5_esw_vport_to_devlink_port_index(mdev, 0);
>  	}