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

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jun 06, 2023 at 11:08:12PM +0000, Parav Pandit wrote:
> 
> 
> > 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.

just don't keep listing this in each feature. my plan is to work
on cfgvq so we can keep using config space without these
limitations.

> > > >
> > > > 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.

maybe. but notice how you have a specific design in mind so you are jumping ahead.
you asked how we could make these synch, i told you how.
the actual point is you want to say "we don't need this synchronous",
design idea: use cvq


> > > > 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.

yes but this one I don't know what it means, spec wise.
some kind of block? what for?

> > 
> > > > 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.

i guess device might only support part of counters then?

> > > 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.

for debugging it's easiest if you just stop the vq, instead of drops.

> > 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.

I would have a separate debug section.

-- 
MST