On 28.08.24 07:54, Si-Wei Liu wrote:
>
>
> On 8/27/2024 9:02 AM, Dragos Tatulea wrote:
>> When the vdpa device is configured without a specific MAC
>> address, the vport MAC address is used. However, this
>> address can be 0 which prevents the driver from properly
>> configuring the MPFS and breaks steering.
>>
>> The solution is to simply generate a random MAC address
>> when no MAC is set on the nic vport.
>>
>> Now it's possible to create a vdpa device without a
>> MAC address and run qemu with this device without needing
>> to configure an explicit MAC address.
>>
>> Signed-off-by: Dragos Tatulea <dtatu...@nvidia.com>
>> Reviewed-by: Jiri Pirko <j...@nvidia.com>
>> ---
>> drivers/vdpa/mlx5/net/mlx5_vnet.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> index fa78e8288ebb..1c26139d02fe 100644
>> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>> @@ -3824,6 +3824,9 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev
>> *v_mdev, const char *name,
>> err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
>> if (err)
>> goto err_alloc;
>> +
>> + if (is_zero_ether_addr(config->mac))
>> + eth_random_addr(config->mac);
> I wonder with this change we no longer honor the historical behaviour to
> retain the zero mac address and clear the _F_MAC bit, should we head to
> remove the below logic? It looks to me below would become dead code
> effectively.
>
It is still possible to create a vdpa device with a zero mac address
explicitly, right?
> } else if ((add_config->mask & BIT_ULL(VDPA_ATTR_DEV_FEATURES)) == 0)
> {
> /*
> * We used to clear _F_MAC feature bit if seeing
> * zero mac address when device features are not
> * specifically provisioned. Keep the behaviour
> * so old scripts do not break.
> */
> device_features &= ~BIT_ULL(VIRTIO_NET_F_MAC);
>
> If we are not going to honor old behaviour any more, looks to me we should
> also block users from creating vdpa device with zero mac address, if the mac
> attribute is specified. There's more sorrow than help the zero mac address
> could buy for users.
That makes sense. There is a small risk of breaking user's scripts that
do this by accident...
Thanks,
Dragos
