RE: [virtio-comment] [PATCH requirements 1/7] net-features: Add requirements document for release 1.4

[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: Tuesday, June 6, 2023 6:57 PM

> > It matters for requirements, so we produce design that addresses it.
> > We don't want to add config space every growing bit map which may be
> different between different devices.
> 
> then say that you want to conserve config space, that is the requirement. not
> cvq specifically.
>
Well in one meeting you specifically told that requirements and design to be combined together, so it is drafted this way.
Instead of very abstract like "conserve config space".

We can debate and change from cvq to new cfgvq as part of the journey.
It is fine.
 
> > >
> > > what matters is whether they have to be synchronized with a given
> > > queue - I get it they don't have to.
> > They don't have to be.
> 
> Then say that.
>
I thought this is very obvious as querying counter is hundred-time slower operation than packet processing.
 
> > I don't see how it can be every synchronized.
> 
> you could program them through the vq itself.
> 
Do you mean in the packet transmit and receive completions itself?
It would be too heavy to do so to mix the control fields in the data path.
> > > we don't cover migration currently don't see how this is a spec rquirement.
> > > unless maybe it's justification for 4?
> > True, but design need to keep this in mind if it has some touch points like of
> #4 so when migration arrives, it has the building block in place.
> 
> so again, let's make sure we capture the actual spec requirement.
>
It is an actual spec requirement to be done and drafted in the spec.
We may not do everything in the first phase, this are the broad requirements.
And in design, we will say, requirement #4 is phase 2.

> 
> > > so maybe it means there needs to be a way to set counters?
> > > so there's no need to mention migration - just that it should be
> > > possible to move counters between devices.
> > >
> > > > +4. If a virtio device is group member device, a group owner should be
> able
> > > > +   to query all the counter attributes using the admin queue command
> which
> > > > +   a virtio device will expose via a control vq to the driver.
> > >
> > >
> > > this seems weirdly specific.
> > > what is the actual requirement?
> > >
> > I don't follow the question.
> > When a device migrates from src to dst, group owner needs to know if both
> side underlying member device has same counter attributes or not.
> 
> whether it's through a command or not is not a requirement and I still do not
> know what the requirement is.
> what does "same counter attributes" mean? you never mentioned attributes
> before.
>
I will refine this further and drop the word "attribute".
If device support X counters, group owner should be able to know this bitmap.
 
> > Yes, will change the GUEST surfaced from the current F_GUEST terminology of
> the net device.
> 
> So?  this predates 1.x spec we never bothered changing them.
>
Will remove the guest wording and will change to transmit and receive.

 
> > > > +4. le64 rx_pkts: Total packets received by the device
> > >
> > > including dropped ones or not?
> > >
> > Not including. Will add this clarification further in v1.
> >
> > > > +
> > > > +### 3.1.2 Per transmit queue counters 1. le64 tx_bad_desc_errors:
> > > > +Descriptors dropped by the device due to errors in
> > > > +   descriptors
> > >
> > > since when do we drop packets on error in descriptor?
> > > we just as likely stall ...
> > >
> > It is left to the device to implement on what to do on bad desc.
> 
> then how can you count drops? 
A device may count the drops based on errors.
It is counting drops based on the error it got.

> why does this even matter?
To debug.

> and why on tx specifically?
Missed the rx, will add.

> I feel addressing descriptor errors is a completely separate project.

Not sure if it is that big project.