Re: [PATCH 1/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default

[Si-Wei Liu <si-wei.liu@oracle.com>]

On 3/13/2023 8:47 PM, Parav Pandit wrote:
>> From: Michael S. Tsirkin <mst@redhat.com>
>> Sent: Monday, March 13, 2023 11:38 PM
>>
>> On Tue, Mar 14, 2023 at 02:05:50AM +0000, Parav Pandit wrote:
>>>> From: Si-Wei Liu <si-wei.liu@oracle.com>
>>>> Sent: Monday, March 13, 2023 6:19 PM
>>>> Actually there's no such burden or requirement to maintain backward
>>>> compatibility for the default 'vdpa dev add' behavior if dedicated
>>>> device_features is not specified. Historically the default vdpa
>>>> creation on mlx5 ever got changed from single queue to 8 queue pairs
>>>> when VIRTIO_NET_F_MQ feature was first introduced to mlx5_vdpa, then
>>>> the default switched back to 1 data queue pair again when max_vqp
>> attribute was added to the vdpa tool.
>>>> Essentially, every addition of new feature to mlx5_vdpa, e.g.
>>>> CTRL_VQ, CTRL_VLAN, and et al, effectively changed the default "vdpa
>>>> dev add" behavior not just only once: the backward compatibility
>>>> guarantee is simply just not there and ever.
>>> This requires that every change in the device attributes will change the
>> behavior for vdpa dev add.
>>> The OR operation of the user supplied feature bits with device defaults
>> feature bit doesn’t look good to me.
>>> It brings uncertain behavior.
>>>
>>> The right behavior should be, if user supplied the feature bits, it should
>> supply all desired bits.
>>
>> I think u mean all that device also supports.
>>
> If user supplied feature bits and device doesn’t support some of the feature bits -> add command fails.
> If user supplied feature bits, than use only the feature bits. Do not OR user supplied feature bits with device supported feature bits.
I think that is the not the current behavior for mlx5_vdpa? As far as I 
can see it would AND user supplied device_features bits with the device 
supported feature bits (find the common part of both feature bits) then 
proceed. I am not sure I follow what is the problem?

> For example,
> User asked to set,
> F_CONFIG_MTU
> F_CONFIG_MAC
> F_PACKED
> In that device should only use above 3 features. (because user is the master).
>
> It should not OR user supplied feature bits with device defaults feature bits.
>   
>>> If user doesn’t supply feature bits, use the device defaults.
> If user doesn’t supply any feature fits, that means user wants to run with some device defaults.
> In that case its fine to run with device defaults.
Noted, we don't have this device defaults exposed to users yet other 
than what's shown in the parent mgmtdev (dev_features). And the current 
and the only safe behavior for mlx5_vdpa is to inherit all device 
supported features from parent mgmtdev, otherwise user can't know in 
advance what default features the device might come up with implicitly 
underneath. Basically it would need to add new CLI interface to achieve 
what you want.

Thanks,
-Siwei

>
>>> But not to mix both feature bits.
>> Can't say I understand what this means. What does "both" mean here?
> Mix Both means, to perform OR operation on user supplied feature bits with device defaults.

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization