RE: [PATCH requirements v4 4/7] net-features: Add notification coalescing requirements

[Parav Pandit <parav@nvidia.com>]

> From: Heng Qi <hengqi@linux.alibaba.com>
> Sent: Wednesday, August 16, 2023 2:01 PM
> 
> å 2023/8/15 äå3:45, Parav Pandit åé:
> > Add virtio net device notification coalescing improvements requirements.
> >
> > Signed-off-by: Parav Pandit <parav@nvidia.com>
> > Acked-by: David Edmondson <david.edmondson@oracle.com>
> >
> > ---
> > changelog:
> > v3->v4:
> > - no change
> >
> > v1->v2:
> > - addressed comments from Stefan
> > - redrafted the requirements to use rearm term and avoid queue enable
> >    confusion
> > v0->v1:
> > - updated the description
> > ---
> >   net-workstream/features-1.4.md | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/net-workstream/features-1.4.md
> > b/net-workstream/features-1.4.md index 72d04bd..cb72442 100644
> > --- a/net-workstream/features-1.4.md
> > +++ b/net-workstream/features-1.4.md
> > @@ -8,6 +8,7 @@ together is desired while updating the virtio net interface.
> >   # 2. Summary
> >   1. Device counters visible to the driver
> >   2. Low latency tx and rx virtqueues for PCI transport
> > +3. Virtqueue notification coalescing re-arming support
> >
> >   # 3. Requirements
> >   ## 3.1 Device counters
> > @@ -172,3 +173,13 @@ struct vnet_rx_completion {
> >      which can be recycled by the driver when the packets from the completed
> >      page is fully consumed.
> >   8. The device should be able to consume multiple pages for a receive GSO
> stream.
> > +
> > +## 3.3 Virtqueue notification coalescing re-arming support 0. Design
> > +goal:
> > +   a. Avoid constant notifications from the device even in conditions when
> > +      the driver may not have acted on the previous pending notification.
> > +1. When Tx and Rx virtqueue notification coalescing is enabled, and when
> such
> > +   a notification is reported by the device, the device stops sending further
> > +   notifications until the driver rearms the notifications of the virtqueue.
> > +2. When the driver rearms the notification of the virtqueue, the device
> > +   to notify again if notification coalescing conditions are met.
> 
> I'm wondering how this relates to the existing notification coalesing[1] and
> notification suppression[2]:
> 
> [1]
> The device sends a used buffer notification once the notification conditions are
> met and if the notifications are not suppressed as explained in \ref{sec:Basic
> Facilities of a Virtio Device / Virtqueues / Used Buffer Notification
> Supppression}.
> 
> [2]
> If the VIRTIO_F_EVENT_IDX feature bit is not negotiated:
> \begin{itemize}
> \item The driver MUST ignore the \field{avail_event} value.
> \item After the driver writes a descriptor index into the available ring:
>  ÂÂ \begin{itemize}
>  ÂÂÂÂÂÂÂÂ \item If \field{flags} is 1, the driver SHOULD NOT send a notification.
>  ÂÂÂÂÂÂÂÂ \item If \field{flags} is 0, the driver MUST send a notification.
>  ÂÂ \end{itemize}
> \end{itemize}
> 
> Otherwise, if the VIRTIO_F_EVENT_IDX feature bit is negotiated:
> \begin{itemize}
> \item The driver MUST ignore the lower bit of \field{flags}.
> \item After the driver writes a descriptor index into the available ring:
>  ÂÂ \begin{itemize}
>  ÂÂÂÂÂÂÂÂ \item If the \field{idx} field in the available ring (which determined
>  ÂÂÂÂÂÂÂÂÂÂ where that descriptor index was placed) was equal to
>  ÂÂÂÂÂÂÂÂÂÂ \field{avail_event}, the driver MUST send a notification.
>  ÂÂÂÂÂÂÂÂ \item Otherwise the driver SHOULD NOT send a notification.
>  ÂÂ \end{itemize}
> \end{itemize}
> 
> Regarding notification suppression:
> 1.When there is VIRTIO_NET_F_EVENT_IDX, even if the notification coalesing
> condition is met, we need to wait for the used_event notification condition to
> be met(the driver does not rearms the notification of the virtqueue now and
> the avail ring is set VRING_AVAIL_F_NO_INTERRUPT in flag).
> 2.When there is no VIRTIO_NET_F_EVENT_IDX, if the driver turns off the
> notification, even if the notidication condition is met, the device cannot send
> the notification.
> 
> Therefore, if I'm not wrong, a device can issue a notification only if the device is
> not suppressed from notifying the driver.
> [1][2] seems to have met this condition.

Notification suppression using _EVENT_IDX for non-memory transport is just sub-optimal for two reasons.

1. It requires device to poll on the used event to learn about when to un-suppress. (arm)
2. this bit also controls driver notifications yet again demand device to arbitrarily poll on new descriptors posting

Hence, an efficient scheme is needed and device notifications to be detached from driver notification.
And now that VQ level notification coalescing is in place, which suppresses the device notifications, it is logical to combine it with VQ device notifications.