Re: [PATCH net-next 01/15] net/mlx5: Distinguish VFs/SFs in the SW system image GUID for LAG grouping

[Shay Drori <shayd@nvidia.com>]

On 03/11/2026 19:09, Tariq Toukan wrote:
> From: Shay Drory <shayd@nvidia.com>
> 
> mlx5_query_nic_sw_system_image_guid() builds the key used to group
> devices that may form a LAG. A PF and its VFs/SFs share the same FW
> system image GUID, so without a discriminator a VF/SF that can join a
> virt LAG would be grouped with its PF.
> 
> Appending bare discriminator bytes at conditional offsets is not enough:
> distinct capability sets can collapse to the same key (a PF ending in a
> load_balance byte of 0x01 vs a VF whose marker is 0x01). Give each
> discriminator a fixed position instead, and add the VF and SF as such
> fields. Trailing zero fields are trimmed, so keys of devices that set
> none of the new fields are unchanged.
> 
> Set the VF/SF markers only when mlx5_virt_lag_is_supported(). The same
> key also backs mlx5_same_hw_devs(), so a VF/SF that cannot join a virt
> LAG keeps its current key and is still identified as the same HW as its
> PF.
> 
> 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>
> ---
>   .../net/ethernet/mellanox/mlx5/core/lag/lag.h |  9 ++++
>   .../net/ethernet/mellanox/mlx5/core/vport.c   | 41 +++++++++++++++++--
>   include/linux/mlx5/driver.h                   |  2 +-
>   3 files changed, 48 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.h b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.h
> index 8552792b9035..0ff337983a6d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag/lag.h
> @@ -266,6 +266,15 @@ static inline void mlx5_lag_set_vports_agg_speed(struct mlx5_lag *ldev) {}
>   static inline void mlx5_lag_reset_vports_speed(struct mlx5_lag *ldev) {}
>   #endif
>   
> +static inline bool mlx5_virt_lag_is_supported(const struct mlx5_core_dev *dev)
> +{
> +	if (!MLX5_CAP_GEN(dev, multi_uplink_eswitch) ||
> +	    !MLX5_CAP_GEN(dev, lag_master) ||
> +	    MLX5_CAP_GEN(dev, num_lag_ports) >= 2)
> +		return false;
> +	return true;
> +}
> +
>   static inline bool mlx5_lag_is_supported(struct mlx5_core_dev *dev)
>   {
>   	if (!MLX5_CAP_GEN(dev, vport_group_manager) ||
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/vport.c b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
> index 78f3d15b599d..50d2609374c4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/vport.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/vport.c
> @@ -38,6 +38,7 @@
>   #include "mlx5_core.h"
>   #include "eswitch.h"
>   #include "sf/sf.h"
> +#include "lag/lag.h"
>   
>   /* Mutex to hold while enabling or disabling RoCE */
>   static DEFINE_MUTEX(mlx5_roce_en_lock);
> @@ -1308,10 +1309,37 @@ u64 mlx5_query_nic_system_image_guid(struct mlx5_core_dev *mdev)
>   }
>   EXPORT_SYMBOL_GPL(mlx5_query_nic_system_image_guid);
>   
> +static u8 mlx5_sw_guid_load_balance(const struct mlx5_core_dev *mdev)
> +{
> +	if (MLX5_CAP_GEN_2(mdev, load_balance_id) &&
> +	    MLX5_CAP_GEN_2(mdev, lag_per_mp_group))
> +		return MLX5_CAP_GEN_2(mdev, load_balance_id);
> +	return 0;
> +}
> +
> +static u8 mlx5_sw_guid_sf(const struct mlx5_core_dev *mdev)
> +{
> +	return mlx5_core_is_sf(mdev) && mlx5_virt_lag_is_supported(mdev);
> +}
> +
> +static u8 mlx5_sw_guid_vf(const struct mlx5_core_dev *mdev)
> +{
> +	return mlx5_core_is_vf(mdev) && mlx5_virt_lag_is_supported(mdev);
> +}

answers to net-dev sashiko

Sashiko (Medium):
Does this break PF to VF/SF same-HW detection for the VFs/SFs that do
pass mlx5_virt_lag_is_supported()?
The commit message says a VF/SF that cannot join a virt LAG keeps its
current key and is still identified as the same HW as its PF. A VF/SF
that reports multi_uplink_eswitch, lag_master and num_lag_ports < 2 now
gets a key that differs from its PF's key:
   PF: GUID [+ lb]                 (8 or 9 bytes)
   VF: GUID + lb + 0x01            (10 bytes, lb may be 0)
   SF: GUID + lb + 0x00 + 0x01     (11 bytes, lb may be 0)
mlx5_same_hw_devs() requires the lengths and bytes to match exactly:
drivers/net/ethernet/mellanox/mlx5/core/dev.c:mlx5_same_hw_devs() {
     ...
     return plen && flen && flen == plen &&
         !memcmp(fsystem_guid, psystem_guid, flen);
}
mlx5e_tc_is_vf_tunnel() accepts only a PF out_dev with a VF or SF
route_dev, and then depends on that same check:
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:mlx5e_tc_is_vf_tunnel() {
     ...
     if (out_mdev->coredev_type != MLX5_COREDEV_PF)
         return false;
     if (route_mdev->coredev_type != MLX5_COREDEV_VF &&
         route_mdev->coredev_type != MLX5_COREDEV_SF)
         return false;
     return mlx5e_same_hw_devs(out_priv, route_priv);
}
For these VFs/SFs, it looks like mlx5e_tc_is_vf_tunnel() can never
return true. Would that cause the following?
   mlx5e_tc_tun_route_lookup() skips the VF tunnel branch and never sets
   rx_tun_attr->decap_vport. mlx5e_attach_decap_route() then bails out,
   and the indirect table source vport rewrite is never set up.
   mlx5e_set_vf_tunnel() takes the goto out path and does not rewrite
   the source port to the VF vport.
   mlx5e_update_vf_tunnel() returns -ENODEV.
In NIC mode, tc_act_can_offload_mirred_nic() would also reject a
redirect between a PF and such a VF/SF, or between a VF and an SF, with
"devices are not on same switch HW, can't offload forwarding".
No later patch in the series changes mlx5_same_hw_devs(),
mlx5e_tc_is_vf_tunnel(), the tunnel code or mirred_nic.c. The final
patch, "net/mlx5: LAG, enable LAG over functions that do not represent a
physical port", relies on VF/SF functions that report exactly these
capabilities.
Should mlx5_same_hw_devs() use a key without the VF/SF markers, or
compare only the bytes that identify the hardware?

[SD] No, this is the point of the patch. A VF/SF that reports
multi_uplink_eswitch and lag_master (and num_lag_ports < 2) is a nested
eswitch manager. That makes it a different switch domain from its
PF, not one more vport of the PF's FDB, so it must not be identified as
the same HW as the PF. Without the marker it would also be grouped into
the PF's LAG, which is what the patch prevents.

A VF/SF that does not report these caps, i.e. every VF/SF that exists
today, keeps its key unchanged and is still the same HW as its PF, as
the commit message says. Dropping the markers inside mlx5_same_hw_devs()
would make a nested eswitch manager the same HW as its PF again, i.e.
undo the patch.


> +
> +static u8 (* const mlx5_sw_guid_fields[])(const struct mlx5_core_dev *mdev) = {
> +	mlx5_sw_guid_load_balance,
> +	mlx5_sw_guid_vf,
> +	mlx5_sw_guid_sf,
> +	/* New fields should be added at the end */
> +};
> +
>   void mlx5_query_nic_sw_system_image_guid(struct mlx5_core_dev *mdev, u8 *buf,
>   					 u8 *len)
>   {
>   	u64 fw_system_image_guid;
> +	u8 trim_len;
> +	int i;
>   
>   	*len = 0;
>   
> @@ -1321,10 +1349,17 @@ void mlx5_query_nic_sw_system_image_guid(struct mlx5_core_dev *mdev, u8 *buf,
>   
>   	memcpy(buf, &fw_system_image_guid, sizeof(fw_system_image_guid));
>   	*len += sizeof(fw_system_image_guid);
> +	trim_len = *len;
>   
> -	if (MLX5_CAP_GEN_2(mdev, load_balance_id) &&
> -	    MLX5_CAP_GEN_2(mdev, lag_per_mp_group))
> -		buf[(*len)++] = MLX5_CAP_GEN_2(mdev, load_balance_id);
> +	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;
>   }
>   
>   bool mlx5_vport_use_vhca_id_as_func_id(struct mlx5_core_dev *dev,
> diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
> index abb99371ff13..d9d91faa0a1d 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -1395,7 +1395,7 @@ static inline struct net *mlx5_core_net(struct mlx5_core_dev *dev)
>   	return devlink_net(priv_to_devlink(dev));
>   }
>   
> -#define MLX5_SW_IMAGE_GUID_MAX_BYTES 9
> +#define MLX5_SW_IMAGE_GUID_MAX_BYTES 11
>   
>   static inline bool mlx5_data_direct_supported(struct mlx5_core_dev *mdev)
>   {