Discussion of the VIRTIO specification
 help / color / mirror / Atom feed
From: Heng Qi <hengqi@linux.alibaba.com>
To: Parav Pandit <parav@nvidia.com>
Cc: Shahaf Shuler <shahafs@nvidia.com>,
	"virtio@lists.oasis-open.org" <virtio@lists.oasis-open.org>,
	"virtio-comment@lists.oasis-open.org"
	<virtio-comment@lists.oasis-open.org>,
	"david.edmondson@oracle.com" <david.edmondson@oracle.com>,
	"xuanzhuo@linux.alibaba.com" <xuanzhuo@linux.alibaba.com>,
	"sburla@marvell.com" <sburla@marvell.com>
Subject: [virtio-comment] Re: [PATCH requirements v4 4/7] net-features: Add notification coalescing requirements
Date: Wed, 16 Aug 2023 20:36:45 +0800	[thread overview]
Message-ID: <5ce972f4-1efd-400d-bc45-c03e9956f2ce@linux.alibaba.com> (raw)
In-Reply-To: <PH0PR12MB54815DBA73F066BFAE3824CBDC15A@PH0PR12MB5481.namprd12.prod.outlook.com>



在 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.
2. When used idx notification is not satisfied, even if coalescing is 
satisfied, the device still cannot notify the driver.
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}."

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.

Thanks!


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


  reply	other threads:[~2023-08-16 12:36 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-15  7:45 [virtio-comment] [PATCH requirements v4 0/7] virtio net requirements for 1.4 Parav Pandit
2023-08-15  7:45 ` [virtio-comment] [PATCH requirements v4 1/7] net-features: Add requirements document for release 1.4 Parav Pandit
2023-08-15  8:56   ` David Edmondson
2023-08-15  9:11     ` Parav Pandit
2023-08-15  9:14       ` David Edmondson
2023-08-15 12:20         ` Parav Pandit
2023-08-15  7:45 ` [virtio-comment] [PATCH requirements v4 2/7] net-features: Add low latency transmit queue requirements Parav Pandit
2023-08-15  8:45   ` [virtio-comment] " David Edmondson
2023-08-15  8:50     ` [virtio-comment] " Parav Pandit
2023-08-15  7:45 ` [virtio-comment] [PATCH requirements v4 3/7] net-features: Add low latency receive " Parav Pandit
2023-08-15  8:50   ` [virtio-comment] " David Edmondson
2023-08-15  7:45 ` [virtio-comment] [PATCH requirements v4 4/7] net-features: Add notification coalescing requirements Parav Pandit
2023-08-16  8:30   ` [virtio-comment] " Heng Qi
2023-08-16 10:46     ` [virtio-comment] " Parav Pandit
2023-08-16 12:36       ` Heng Qi [this message]
2023-08-17  4:57         ` Parav Pandit
2023-08-17  5:13           ` [virtio-comment] " Heng Qi
2023-08-17  5:20             ` [virtio-comment] " Parav Pandit
2023-08-15  7:45 ` [virtio-comment] [PATCH requirements v4 5/7] net-features: Add n-tuple receive flow filters requirements Parav Pandit
2023-08-16  6:27   ` [virtio-comment] " Parav Pandit
2023-08-16  7:38     ` [virtio-comment] " Heng Qi
2023-08-16 10:31       ` [virtio-comment] " Parav Pandit
2023-08-16 11:10         ` [virtio-comment] " Heng Qi
2023-08-16 11:18           ` [virtio-comment] " Parav Pandit
2023-08-16 11:42   ` [virtio-comment] " Heng Qi
2023-08-17  4:52     ` [virtio-comment] " Parav Pandit
2023-08-17  5:14       ` [virtio-comment] " Heng Qi
2023-08-15  7:45 ` [virtio-comment] [PATCH requirements v4 6/7] net-features: Add packet timestamp requirements Parav Pandit
2023-08-15  7:46 ` [virtio-comment] [PATCH requirements v4 7/7] net-features: Add header data split requirements Parav Pandit
2023-08-15  8:52   ` [virtio-comment] " David Edmondson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5ce972f4-1efd-400d-bc45-c03e9956f2ce@linux.alibaba.com \
    --to=hengqi@linux.alibaba.com \
    --cc=david.edmondson@oracle.com \
    --cc=parav@nvidia.com \
    --cc=sburla@marvell.com \
    --cc=shahafs@nvidia.com \
    --cc=virtio-comment@lists.oasis-open.org \
    --cc=virtio@lists.oasis-open.org \
    --cc=xuanzhuo@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox