RE: [virtio-comment] Re: [PATCH v6 2/5] virtio-net: Add flow filter capabilities read commands

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Friday, November 24, 2023 11:37 AM
> 
> On Fri, Nov 24, 2023 at 05:53:02AM +0000, Parav Pandit wrote:
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Friday, November 24, 2023 11:03 AM
> > >
> > > On Fri, Nov 24, 2023 at 12:02:23PM +0800, Jason Wang wrote:
> > > > > > I won't be able to absorb this comment of DMA interface.
> > > > > > If I discuss further, I will repeat the whole document [1] and
> > > > > > I will avoid
> > > that now.
> > > > > >
> > > > > > [1]
> > > > > > https://docs.google.com/document/d/1Iyn-
> > > l3Nm0yls3pZaul4lZiVj8x1s73
> > > > > > Ed6rOsmn6LfXc/edit#heading=h.qexbtyc2jpwr
> > > > >
> > > > >
> > > > > I really worry about how provisioning will work. And I do not at
> > > > > all cherish replicating all of these query capability commands for
> provisioning.
> > > >
> > > > +1
> > > >
> > > > There's nothing that prevents the config space from being
> > > > implemented in a way other than registers.
> > >
> > > Care doing it finally? Let's see if what Parav is worrying about is
> > > then addressed.
> >
> > The whole concept that everything must be in one giant config space is just
> simply bad.
> > It does not exist either in virtio spec today.
> 
> But it does, this is what transports are doing.
I don't understand what is "it".
Transport like pci transport bits.

CVQ is functional object that helps to arrange the bits in logical, functional manner instead of trying to place them in bit array.

> 
> > once can see that what is presented in the commands cannot be placed in
> config space at dynamic location.
> > Same was the case with statistics too.
> > Same was the case with VQ coaleasing knobs.
> > Same with hash knobs.
> > With flow filters,
> > With rss contexts
> > With rtc
> > With new queues creation apis.
> >
> > The endless list continues...
> >
> > And reserving bits for future (for other than pad bytes) for future addition in
> config space is equally not elegant design.
> > Bits will get spread out at random location making things even harder to
> maintain.
> >
> > The device is no longer a simple mac_addr + N queues device with some
> static rss config anymore.
> >
> > With all modern work, every capability query and run time configuration is
> done over cvq interface today.
> > Single get/set channel from driver to device all using existing resources.
> >
> > The real hw device also does not need to refer to two places of config and
> cvq when serving cvq commands.
> > Oh, the list of advantages just continues with what 1.3 spec has done.
> 
> I don't see the problem sorry. We've been doing this for many years with many
> ways to access config space.  It scaled well.
> 
I don't see it. sorry.
The configuration of the device is done using cvq.
> 
> Then hardware offload guys come and say that in PCI spec current transport is
> forcing use of on-device memory, and they want to build cheap offload PCI
> based devices. Fine, let's build a transport variant that does not force this.

All new capabilities and control is over the cvq. What is baked until 1.2 is sort of legacy.

>  And
> we want optional compatibility, so let's also find a way to do that.  This makes
> much more sense than forcing transport specific issues on everyone.
> 
Trying to attribute as some transport specific issue is just not aligned to the spec written today.

> To add to that, what did not historicall scale well is transport-specific registers.
Then you should have put the VQ notification coalescing functionality in a horrible virtio_net_config register like how a queue reset in the common config space.
Thank God that mistake was not done and many similar mistakes were avoided.

> That was a bad design, with all transports doing exactly the same thing is
> slightly different ways.  And what you are advocating for with CVQ is exactly
> replicating the bad design not the good one.

CVQ design is what extends the current spec in good way. Followed by many other non nvidia nics listed in the doc for reference.