Re: [PATCH v7] virtio-net: Add support for the flexible driver notification structure.

[Cornelia Huck <cohuck@redhat.com>]

On Thu, 26 Mar 2020 07:26:16 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Mar 26, 2020 at 10:43:20AM +0100, Cornelia Huck wrote:
> > On Wed, 18 Mar 2020 18:29:19 +0000
> > Vitaly Mireyno <vmireyno@marvell.com> wrote:

> > > +
> > > +\begin{note}
> > > +If \field{notify_off_multiplier} > 0, the virtqueue number can potentially be
> > > +derived by the device from the Queue Notify address, so \field{vqn} may be
> > > +redundant.  
> 
> I feel this is confusing the reader more than it explains.
> the field is always there. It's either the virtqueue number or
> whatever we get from the device.

"the virtqueue number can potentially be derived by the device from the
Queue Notify address, so obtaining the virtqueue number from
\field{vqn} is not needed"

?

> 
> > > +\end{note}
> > > +
(...)
> > > @@ -4180,6 +4205,33 @@ \subsubsection{Legacy Interface: Framing Requirements}\label{sec:Device
> > >  See \ref{sec:Basic
> > >  Facilities of a Virtio Device / Virtqueues / Message Framing}.
> > >  
> > > +\subsubsection{Driver Notifications}\label{sec:Device Types / Network Device / Device Operation / Driver Notifications}
> > > +As mentioned in section \ref{sec:Virtqueues / Driver notifications}, when the
> > > +driver is required to send an available buffer notification to the device, it
> > > +always sends the virtqueue number to be notified. The method of delivering
> > > +notifications is transport specific. With the PCI transport, the device can
> > > +optionally provide a distinct per-virtqueue address for driver notifications
> > > +(Queue Notify address). In this case, the virtqueue number can potentially be
> > > +derived by the device from the Queue Notify address. For devices, that provide
> > > +per-virtqueue Queue Notify addresses and are able to derive the virtqueue number
> > > +from that address, the virtqueue number in the notification data is redundant.  
> > 
> > I would keep the remark that the queue number might be redundant with
> > the PCI transport.  
> 
> I feel redundant is too strong a word. If anything sending data we just
> read from device back to it is definitely redundant.
> Spec says send it so we send it.
> Maybe say it can be ignored by the device?

Maybe "not needed"?

(see above)

> 
> > > +On the other hand, some devices can benefit from receiving additional data with
> > > +driver notifications. An example could be a hardware device implementing multiple
> > > +protocols (with virtio being one of them), so the extra notification data could
> > > +serve as a notification type indication or a protocol indication.  
> > 
> > Hm... what about replacing the whole text above with
> > 
> > "Driver notifications (\ref{sec:Virtqueues / Driver notifications})
> > always contain at least the virtqueue number for which buffers are
> > available. Some devices can benefit from receiving additional data with
> > driver notifications. For example, a hardware device implementing
> > multiple protocols (with virtio being one of them) could benefit from a
> > notification type indication or a protocol indication."  
> 
> 
> My problem with both versions is that it talks about
> "additional information" while in fact it's information that
> comes from the device itself.

"per-device information", maybe?