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 6:07 PM
> 
> 
> å 2023/8/16 äå6:46, Parav Pandit åé:
> >
> >> 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.
> >
> 
> Let me summarize:
> 1. When used idx notification is satisfied, but coalescing is not satisfied, the
> driver continues to suppress device notifications.
Ack.

> 2. When used idx notification is not satisfied, even if coalescing is satisfied, the
> device still cannot notify the driver.
Ack.

> I think that's what coalescing does, and the description below has satisfied this
> behavior:
> "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}."
>
Ack.
the proposal here is to not use EVENT_IDX scheme, instead driver to enable/disable notification coalescing in different way, even when notification coalescing parameters are configured.
And this to be done in fairly fast way (not like a cvq) command. For example like driver notifications.
 
> Or we want to say that it has nothing to do with the used idx notification. 

When
> the coalescing is satisfied and the driver rearms the notification of the
> virtqueue, the device now send a notification.
> 
Right.
F_NOTIFION_ARM is mutually exclusive with F_EVENT_IDX.
(Like packed vq is mutually exclusive with split q.)