RE: [virtio-comment] Re: [PATCH v18] virtio-net: support inner header hash

[Parav Pandit <parav@nvidia.com>]

> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Michael S. Tsirkin
> Sent: Thursday, June 22, 2023 1:04 PM
> To: Parav Pandit <parav@nvidia.com>
> 
> On Thu, Jun 22, 2023 at 04:54:48PM +0000, Parav Pandit wrote:
> >
> >
> > > From: Michael S. Tsirkin <mst@redhat.com>
> > > Sent: Thursday, June 22, 2023 12:47 PM
> >
> > >
> > > The hardware footprint of keeping this in memory is also fairly
> > > small :) I care about a messy interface because this mess builds up over time.
> > >
> > It is really a simple GET command.
> > It is actually messy for the device to implement functionality in two places in
> cfg space and cvq.
> >
> > > And I am worried about capabilities really. My bad that I missed
> > > this change in v13. I only can say in my defence that I already had
> > > to rewrite huge chunks of this proposal to make it readable so one
> > > can't say I'm only delaying things, I also made an effort to help
> > > this progress faster :)
> > >
> > > I feel we need a single place where device capabilities can live. So
> > > far they were in config space.  It's consistent, yes I get this has
> > > hardware costs *if* there's a huge number of VFs and *if* there's a
> > > way to provision each VF with a different configuration.
> > All the ifs are valid today.
> >
> > > And yes querying VFs over MMIO is kind of ugly. But it does at least
> > > work, and works fine while VF is assigned.  So we can build
> > > migration around that *today*.
> > >
> > Other way to say migration can be skipped for this feature bit, and it still
> works for rest.
> 
> If VF is assigned then we can't really control what does guest enable.
>
All parameters set over the CVQ needs to be accessible to the migration entity.
Including RSS and including this bit.
Either by trapping the CVQ or by having AQ command to query member dev capabilities.

> > > But querying over cvq while VF is assigned clearly *doesn't* work.
> > >
> > That is not the idea at all.
> > Querying VF capabilities is the role of the admin command for which we built
> it.
> 
> This GET is exactly that though.
> 
Not exactly.
This GET command is needed for the member driver to know what is supported for the device it got.

> > > So what is the solution proposed for this?
> > >
> > 1. Query member device capabilities via admin command
> 
> But that's not 1.3 material.
> 
> > > Yes the current migration is broken in many ways but that's what we
> > > have. Let's build something better sure but that is not 1.3 material.
> >
> > True, it is not 1.3 material, hence the proposal was to have the GET command.
> > Once/if we reach agreement that no new fields to be added to config space
> starting 1.4 and should be queried using non intercepted cfgvq, it makes sense
> to let this go in cfg space.
> > Else GET command seems the elegant and right approach.
> 
> I expect no such agreement at all. Instead, I expect that we'll have an alternative
> way to access config space. guest virtio core then needs to learn both ways, and
> devices can support one or both.
> 
Yeah, we disagree.
Because alternative way that you propose is not predictable way to build the device efficiently.
It always needs to account for old driver to support.
This is clearly sub-optimal as the capabilities grow.

> A good implementation of virtio_cread can abstract that easily so we don't need
> to change drivers.

There is no backward compat issue for the GET command being new.