netdev - Re: [PATCH net-next v3 01/11] virtio-pci: Expose generic device capability operations

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]

Message-ID: <bc70cd99-31a5-44d4-9648-20dcd80e9f8f@nvidia.com>
Date: Thu, 25 Sep 2025 11:53:50 -0500
From: Dan Jurgens <danielj@...dia.com>
To: "Michael S. Tsirkin" <mst@...hat.com>, Parav Pandit <parav@...dia.com>
Cc: Jason Wang <jasowang@...hat.com>, netdev@...r.kernel.org,
 alex.williamson@...hat.com, pabeni@...hat.com,
 virtualization@...ts.linux.dev, shshitrit@...dia.com, yohadt@...dia.com,
 xuanzhuo@...ux.alibaba.com, eperezma@...hat.com,
 shameerali.kolothum.thodi@...wei.com, jgg@...pe.ca, kevin.tian@...el.com,
 kuba@...nel.org, andrew+netdev@...n.ch, edumazet@...gle.com,
 Yishai Hadas <yishaih@...dia.com>
Subject: Re: [PATCH net-next v3 01/11] virtio-pci: Expose generic device
 capability operations

On 9/25/25 8:08 AM, Michael S. Tsirkin wrote:
> On Thu, Sep 25, 2025 at 05:39:54PM +0530, Parav Pandit wrote:
>>
>> On 25-09-2025 05:19 pm, Michael S. Tsirkin wrote:
>>> On Thu, Sep 25, 2025 at 04:15:19PM +0530, Parav Pandit wrote:
>>>> On 25-09-2025 04:05 pm, Michael S. Tsirkin wrote:
>>>>> On Thu, Sep 25, 2025 at 03:21:38PM +0530, Parav Pandit wrote:
>>>>>> Function pointers are there for multiple transports to implement their own
>>>>>> implementation.
>>>>> My understanding is that you want to use flow control admin commands
>>>>> in virtio net, without making it depend on virtio pci.
>>>> No flow control in vnet.
>>>>> This why the callbacks are here. Is that right?
>>>> No. callbacks are there so that transport agnostic layer can invoke it,
>>>> which is drivers/virtio/virtio.c.
>>>>
>>>> And transport specific code stays in transport layer, which is presently
>>>> following config_ops design.
>>>>
>>>>> That is fair enough, but it looks like every new command then
>>>>> needs a lot of boilerplate code with a callback a wrapper and
>>>>> a transport implementation.
>>>> Not really. I dont see any callbacks or wrapper in current proposed patches.
>>>>
>>>> All it has is transport specific implementation of admin commands.
>>>>
>>>>>
>>>>> Why not just put all this code in virtio core? It looks like the
>>>>> transport just needs to expose an API to find the admin vq.
>>>> Can you please be specific of which line in the current code can be moved to
>>>> virtio core?
>>>>
>>>> When the spec was drafted, _one_ was thinking of admin command transport
>>>> over non admin vq also.
>>>>
>>>> So current implementation of letting transport decide on how to transport a
>>>> command seems right to me.
>>>>
>>>> But sure, if you can pin point the lines of code that can be shifted to
>>>> generic layer, that would be good.
>>> I imagine a get_admin_vq operation in config_ops. The rest of the
>>> code seems to be transport independent and could be part of
>>> the core. WDYT?
>>>
>> IMHV, the code before vp_modern_admin_cmd_exec() can be part of
>> drivers/virtio/virtio_admin_cmds.c and admin_cmd_exec() can be part of the
>> config ops.
>>
>> However such refactor can be differed when it actually becomes boiler plate
>> code where there is more than one transport and/or more than one way to send
>> admin cmds.
> 
> Well administration virtqueue section is currently not a part of a
> transport section in the spec.  But if you think it will change and so
> find it cleaner for transports to expose, instead of a VQ, a generic
> interfaces to send an admin command, that's fine too. That is still a
> far cry from adding all the object management in the transport. 
> 
> 
> Well we have all the new code you are writing, and hacking around
> the fact it's in the wrong module with a level of indirection
> seems wrong.
> If you need help moving this code let me know, it's not hard.
> 
>> Even if its done, it probably will require vfio-virtio-pci to interact with
>> generic virtio layer. Not sure added value of that complication to be part
>> of this series.
>>
>>
>> Dan,
>>
>> WDYT?
> 
> 
> virtio pci pulls in the core already, and VFIO only uses the SRIOV
> group, so it can keep using the existing pci device based interfaces,
> if you prefer.
> 

I can make changes here. I'd appreciate if you review the rest of the
series while I do so. Patches 3+ are isolated from this, so it won't be
a waste of your time.