netdev - Re: [PATCH V3 5/6] vDPA: answer num of queue pairs = 1 to userspace when VIRTIO_NET_F

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Jul 2022 10:11:02 +0800
From:   "Zhu, Lingshan" <lingshan.zhu@...el.com>
To:     Parav Pandit <parav@...dia.com>,
        "Michael S. Tsirkin" <mst@...hat.com>
Cc:     "jasowang@...hat.com" <jasowang@...hat.com>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "xieyongji@...edance.com" <xieyongji@...edance.com>,
        "gautam.dawar@....com" <gautam.dawar@....com>
Subject: Re: [PATCH V3 5/6] vDPA: answer num of queue pairs = 1 to userspace
 when VIRTIO_NET_F_MQ == 0



On 7/27/2022 4:53 AM, Parav Pandit wrote:
>> From: Michael S. Tsirkin<mst@...hat.com>
>> Sent: Tuesday, July 26, 2022 3:49 PM
>>
>> On Tue, Jul 26, 2022 at 03:54:06PM +0000, Parav Pandit wrote:
>>>> From: Michael S. Tsirkin<mst@...hat.com>
>>>> Sent: Wednesday, July 13, 2022 1:27 AM
>>>>
>>>> On Fri, Jul 01, 2022 at 10:07:59PM +0000, Parav Pandit wrote:
>>>>>> From: Zhu Lingshan<lingshan.zhu@...el.com>
>>>>>> Sent: Friday, July 1, 2022 9:28 AM If VIRTIO_NET_F_MQ == 0, the
>>>>>> virtio device should have one queue pair, so when userspace
>>>>>> querying queue pair numbers, it should return mq=1 than zero.
>>>>>>
>>>>>> Function vdpa_dev_net_config_fill() fills the attributions of
>>>>>> the vDPA devices, so that it should call
>>>>>> vdpa_dev_net_mq_config_fill() so the parameter in
>>>>>> vdpa_dev_net_mq_config_fill() should be feature_device than
>>>>>> feature_driver for the vDPA devices themselves
>>>>>>
>>>>>> Before this change, when MQ = 0, iproute2 output:
>>>>>> $vdpa dev config show vdpa0
>>>>>> vdpa0: mac 00:e8:ca:11:be:05 link up link_announce false
>>>>>> max_vq_pairs 0 mtu 1500
>>>>>>
>>>>> The fix belongs to user space.
>>>>> When a feature bit _MQ is not negotiated, vdpa kernel space will
>>>>> not add
>>>> attribute VDPA_ATTR_DEV_NET_CFG_MAX_VQP.
>>>>> When such attribute is not returned by kernel, max_vq_pairs should
>>>>> not be
>>>> shown by the iproute2.
>>>>> We have many config space fields that depend on the feature bits
>>>>> and
>>>> some of them do not have any defaults.
>>>>> To keep consistency of existence of config space fields among all,
>>>>> we don't
>>>> want to show default like below.
>>>>> Please fix the iproute2 to not print max_vq_pairs when it is not
>>>>> returned by
>>>> the kernel.
>>>>
>>>> Parav I read the discussion and don't get your argument. From
>>>> driver's POV _MQ with 1 VQ pair and !_MQ are exactly functionally
>> equivalent.
>>> But we are talking from user POV here.
>>  From spec POV there's just driver and device, user would be part of driver here.
> User space application still need to inspect the _MQ bit to

>>>> It's true that iproute probably needs to be fixed too, to handle old kernels.
>>>> But iproute is not the only userspace, why not make it's life easier
>>>> by fixing the kernel?
>>> Because it cannot be fixed for other config space fields which are control by
>> feature bits those do not have any defaults.
>>> So better to treat all in same way from user POV.
>> Consistency is good for sure. What are these other fields though?
>> Can you give examples so I understand please?
> speed only exists if VIRTIO_NET_F_SPEED_DUPLEX.
> rss_max_key_size exists only if VIRTIO_NET_F_RSS exists.
That's different cases from the MQ case.

There are no default values for speed and rss_max_key_size. And talking 
on speed without VIRTIO_NET_F_SEPPD_DUPLEX or rss_max_key_size without 
VIRTIO_NET_F_RSS are meaningless.
But for MQ, if without MQ, we know it has to be 1 queue pair to be a 
functional virtio-net, and this is meaningful.