All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression
@ 2019-01-31 13:16 Savir, Gil
  2019-02-11  6:58 ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Savir, Gil @ 2019-01-31 13:16 UTC (permalink / raw)
  To: virtio-comment; +Cc: Elmaleh, Liron

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

Hi,

If VIRTIO_F_EVENT_IDX feature bit is negotiated, then Available Buffer Notification Suppression mechanism used is avail event (not flags).
The spec (both v1.0 / v1.1-draft) states that the device MAY use this mechanism (Paragraph 2.4.9.2 / 2.6.10.2 respectively).
This statement implies that the device may choose not to use this suppression mechanism (even if VIRTIO_F_EVENT_IDX was negotiated).

However - there's no way for the device to inform the driver that he is not using avail_event.
As consequence, since there will be a default value in avail_event (probably 0x0), then the driver will always assume that it has to send notify "once-per ring".
This will render performance futile, or force the device to actively update avail_event.

Is there a way for the device to inform the driver that he is not using avail_event (and I missed it)?

If yes, than my apologies for wasting your time.
If no, then I suggest one of the following:

*         Either, to change the "MAY" (referred above) to "MUST",

*        Or, to add way for the device to inform the driver that he is not using avail_event (flag /certain reserved value in avail_event /other mechanism).

Thanks,
Gil Savir
Intel Corporation

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[-- Attachment #2: Type: text/html, Size: 7893 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression
  2019-01-31 13:16 [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression Savir, Gil
@ 2019-02-11  6:58 ` Michael S. Tsirkin
  2019-02-11  7:02   ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2019-02-11  6:58 UTC (permalink / raw)
  To: Savir, Gil; +Cc: virtio-comment, Elmaleh, Liron, cunming.liang

On Thu, Jan 31, 2019 at 01:16:29PM +0000, Savir, Gil wrote:
> Hi,
> 
>  
> 
> If VIRTIO_F_EVENT_IDX feature bit is negotiated, then Available Buffer
> Notification Suppression mechanism used is avail event (not flags).
> 
> The spec (both v1.0 / v1.1-draft) states that the device MAY use this mechanism
> (Paragraph 2.4.9.2 / 2.6.10.2 respectively).
> 
> This statement implies that the device may choose not to use this suppression
> mechanism (even if VIRTIO_F_EVENT_IDX was negotiated).
> 
>  
> 
> However – there’s no way for the device to inform the driver that he is not
> using avail_event.
> 
> As consequence, since there will be a default value in avail_event (probably
> 0x0), then the driver will always assume that it has to send notify “once-per
> ring”.


No I think this part is wrong, pls see below.


> This will render performance futile, or force the device to actively update
> avail_event.
> 
>  
> 
> Is there a way for the device to inform the driver that he is not using
> avail_event (and I missed it)?
> 
>  
> 
> If yes, than my apologies for wasting your time.
> 
> If no, then I suggest one of the following:
> 
> ·         Either, to change the “MAY” (referred above) to “MUST”,
> 

Thanks for the feedback!

So we are talking about split queues.

If avail_event is never set and remains 0, driver will send
notifications every time index wraps around to 0, which
would be every 2^16.

This is I think expected since the spec says:

The device MUST handle spurious notifications from the driver.

2^16 does not seem excessive so I don't think it will render performance
futile.

> ·        Or, to add way for the device to inform the driver that he is not
> using avail_event (flag /certain reserved value in avail_event /other
> mechanism).
> 
>  
> 
> Thanks,
> 
> Gil Savir
> 
> Intel Corporation

It might be a good addition to spec, probably along the lines
of an option to send notifications on
even suppression changes (which was proposed in the past, but
was not included due to lack of time).

-- 
MST

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/


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression
  2019-02-11  6:58 ` Michael S. Tsirkin
@ 2019-02-11  7:02   ` Michael S. Tsirkin
  2019-02-21  8:57     ` Savir, Gil
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2019-02-11  7:02 UTC (permalink / raw)
  To: Savir, Gil; +Cc: virtio-comment, Elmaleh, Liron, cunming.liang

On Mon, Feb 11, 2019 at 01:58:09AM -0500, Michael S. Tsirkin wrote:
> On Thu, Jan 31, 2019 at 01:16:29PM +0000, Savir, Gil wrote:
> > Hi,
> > 
> >  
> > 
> > If VIRTIO_F_EVENT_IDX feature bit is negotiated, then Available Buffer
> > Notification Suppression mechanism used is avail event (not flags).
> > 
> > The spec (both v1.0 / v1.1-draft) states that the device MAY use this mechanism
> > (Paragraph 2.4.9.2 / 2.6.10.2 respectively).
> > 
> > This statement implies that the device may choose not to use this suppression
> > mechanism (even if VIRTIO_F_EVENT_IDX was negotiated).
> > 
> >  
> > 
> > However – there’s no way for the device to inform the driver that he is not
> > using avail_event.
> > 
> > As consequence, since there will be a default value in avail_event (probably
> > 0x0), then the driver will always assume that it has to send notify “once-per
> > ring”.
> 
> 
> No I think this part is wrong, pls see below.
> 
> 
> > This will render performance futile, or force the device to actively update
> > avail_event.
> > 
> >  
> > 
> > Is there a way for the device to inform the driver that he is not using
> > avail_event (and I missed it)?
> > 
> >  
> > 
> > If yes, than my apologies for wasting your time.
> > 
> > If no, then I suggest one of the following:
> > 
> > ·         Either, to change the “MAY” (referred above) to “MUST”,
> > 
> 
> Thanks for the feedback!
> 
> So we are talking about split queues.
> 
> If avail_event is never set and remains 0, driver will send
> notifications every time index wraps around to 0, which
> would be every 2^16.
> 
> This is I think expected since the spec says:
> 
> The device MUST handle spurious notifications from the driver.
> 
> 2^16 does not seem excessive so I don't think it will render performance
> futile.


To add to that, above is based on

2.6.10.1
Driver Requirements: Available Buffer Notification Suppression

which states:

After the driver writes a descriptor index into the available ring:
– If the idx field in the available ring (which determined where that descriptor index was placed)
was equal to avail_event, the driver MUST send a notification.
– Otherwise the driver SHOULD NOT send a notification.



Pls let the TC know whether taking the above into account you think
further clarification in the spec is necessary.

Thanks!


> > ·        Or, to add way for the device to inform the driver that he is not
> > using avail_event (flag /certain reserved value in avail_event /other
> > mechanism).
> > 
> >  
> > 
> > Thanks,
> > 
> > Gil Savir
> > 
> > Intel Corporation
> 
> It might be a good addition to spec, probably along the lines
> of an option to send notifications on
> even suppression changes (which was proposed in the past, but
> was not included due to lack of time).
> 
> -- 
> MST

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/


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression
  2019-02-11  7:02   ` Michael S. Tsirkin
@ 2019-02-21  8:57     ` Savir, Gil
  2019-02-22  1:28       ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Savir, Gil @ 2019-02-21  8:57 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-comment, Elmaleh, Liron, Liang, Cunming



> -----Original Message-----
> From: virtio-comment@lists.oasis-open.org [mailto:virtio-comment@lists.oasis-
> open.org] On Behalf Of Michael S. Tsirkin
> Sent: Monday, 11 February, 2019 09:03
> To: Savir, Gil <gil.savir@intel.com>
> Cc: virtio-comment@lists.oasis-open.org; Elmaleh, Liron
> <liron.elmaleh@intel.com>; Liang, Cunming <cunming.liang@intel.com>
> Subject: Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification
> Suppression
> 
> On Mon, Feb 11, 2019 at 01:58:09AM -0500, Michael S. Tsirkin wrote:
> > On Thu, Jan 31, 2019 at 01:16:29PM +0000, Savir, Gil wrote:
> > > Hi,
> > >
> > >
> > >
> > > If VIRTIO_F_EVENT_IDX feature bit is negotiated, then Available
> > > Buffer Notification Suppression mechanism used is avail event (not flags).
> > >
> > > The spec (both v1.0 / v1.1-draft) states that the device MAY use
> > > this mechanism (Paragraph 2.4.9.2 / 2.6.10.2 respectively).
> > >
> > > This statement implies that the device may choose not to use this
> > > suppression mechanism (even if VIRTIO_F_EVENT_IDX was negotiated).
> > >
> > >
> > >
> > > However – there’s no way for the device to inform the driver that he
> > > is not using avail_event.
> > >
> > > As consequence, since there will be a default value in avail_event
> > > (probably 0x0), then the driver will always assume that it has to
> > > send notify “once-per ring”.
> >
> >
> > No I think this part is wrong, pls see below.
> >
> >
> > > This will render performance futile, or force the device to actively
> > > update avail_event.
> > >
> > >
> > >
> > > Is there a way for the device to inform the driver that he is not
> > > using avail_event (and I missed it)?
> > >
> > >
> > >
> > > If yes, than my apologies for wasting your time.
> > >
> > > If no, then I suggest one of the following:
> > >
> > > ·         Either, to change the “MAY” (referred above) to “MUST”,
> > >
> >
> > Thanks for the feedback!
> >
> > So we are talking about split queues.
> >
> > If avail_event is never set and remains 0, driver will send
> > notifications every time index wraps around to 0, which would be every
> > 2^16.
> >
> > This is I think expected since the spec says:
> >
> > The device MUST handle spurious notifications from the driver.
> >
> > 2^16 does not seem excessive so I don't think it will render
> > performance futile.

Performance degradation is not caused due to the amount of packets served each notification.

Let's take, for example, net-device rx virtqueue.
This scenario leads to situation where only once per ring (no matter its negotiated size), the device will hand-over ALL ring buffers to the driver, and will have no buffers available for incoming rx-packets. 
From this moment (in HW implementation, earlier, due to pipelining nature), until the driver finish to process some packets, and reallocate empty buffers back to the device, the device stands still, and cannot serve incoming rx-packets.
In lossy-networks, the device will have to drop incoming packets during this time.
In lossless-networks, the device will have to backpressure (send pause).

In both cases, the result is underperformance.

Similar issue occurs with the counterpart - used_event suppression mechanism, on the tx-virtqueue.


I suggest to split VIRTIO_F_EVENT_IDX into (say) VIRTIO_F_GUEST_EVENT_IDX, VIRTIO_F_HOST_EVENT_IDX, such that used_event and avail_event usage will be independent.
Such separation makes sense, since the driver is responsible for used_event mechanism, and the device for avail_event mechanism.	

> 
> 
> To add to that, above is based on
> 
> 2.6.10.1
> Driver Requirements: Available Buffer Notification Suppression
> 
> which states:
> 
> After the driver writes a descriptor index into the available ring:
> – If the idx field in the available ring (which determined where that descriptor
> index was placed) was equal to avail_event, the driver MUST send a notification.
> – Otherwise the driver SHOULD NOT send a notification.
> 
> 
> 
> Pls let the TC know whether taking the above into account you think further
> clarification in the spec is necessary.
> 
> Thanks!
> 
> 
> > > ·        Or, to add way for the device to inform the driver that he is not
> > > using avail_event (flag /certain reserved value in avail_event
> > > /other mechanism).
> > >
> > >
> > >
> > > Thanks,
> > >
> > > Gil Savir
> > >
> > > Intel Corporation
> >
> > It might be a good addition to spec, probably along the lines of an
> > option to send notifications on even suppression changes (which was
> > proposed in the past, but was not included due to lack of time).
> >
> > --
> > MST
> 

Gil Savir
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression
  2019-02-21  8:57     ` Savir, Gil
@ 2019-02-22  1:28       ` Michael S. Tsirkin
  2019-02-26 20:30         ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2019-02-22  1:28 UTC (permalink / raw)
  To: Savir, Gil; +Cc: virtio-comment, Elmaleh, Liron, Liang, Cunming

On Thu, Feb 21, 2019 at 08:57:15AM +0000, Savir, Gil wrote:
> 
> 
> > -----Original Message-----
> > From: virtio-comment@lists.oasis-open.org [mailto:virtio-comment@lists.oasis-
> > open.org] On Behalf Of Michael S. Tsirkin
> > Sent: Monday, 11 February, 2019 09:03
> > To: Savir, Gil <gil.savir@intel.com>
> > Cc: virtio-comment@lists.oasis-open.org; Elmaleh, Liron
> > <liron.elmaleh@intel.com>; Liang, Cunming <cunming.liang@intel.com>
> > Subject: Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification
> > Suppression
> > 
> > On Mon, Feb 11, 2019 at 01:58:09AM -0500, Michael S. Tsirkin wrote:
> > > On Thu, Jan 31, 2019 at 01:16:29PM +0000, Savir, Gil wrote:
> > > > Hi,
> > > >
> > > >
> > > >
> > > > If VIRTIO_F_EVENT_IDX feature bit is negotiated, then Available
> > > > Buffer Notification Suppression mechanism used is avail event (not flags).
> > > >
> > > > The spec (both v1.0 / v1.1-draft) states that the device MAY use
> > > > this mechanism (Paragraph 2.4.9.2 / 2.6.10.2 respectively).
> > > >
> > > > This statement implies that the device may choose not to use this
> > > > suppression mechanism (even if VIRTIO_F_EVENT_IDX was negotiated).
> > > >
> > > >
> > > >
> > > > However – there’s no way for the device to inform the driver that he
> > > > is not using avail_event.
> > > >
> > > > As consequence, since there will be a default value in avail_event
> > > > (probably 0x0), then the driver will always assume that it has to
> > > > send notify “once-per ring”.
> > >
> > >
> > > No I think this part is wrong, pls see below.
> > >
> > >
> > > > This will render performance futile, or force the device to actively
> > > > update avail_event.
> > > >
> > > >
> > > >
> > > > Is there a way for the device to inform the driver that he is not
> > > > using avail_event (and I missed it)?
> > > >
> > > >
> > > >
> > > > If yes, than my apologies for wasting your time.
> > > >
> > > > If no, then I suggest one of the following:
> > > >
> > > > ·         Either, to change the “MAY” (referred above) to “MUST”,
> > > >
> > >
> > > Thanks for the feedback!
> > >
> > > So we are talking about split queues.
> > >
> > > If avail_event is never set and remains 0, driver will send
> > > notifications every time index wraps around to 0, which would be every
> > > 2^16.
> > >
> > > This is I think expected since the spec says:
> > >
> > > The device MUST handle spurious notifications from the driver.
> > >
> > > 2^16 does not seem excessive so I don't think it will render
> > > performance futile.
> 
> Performance degradation is not caused due to the amount of packets served each notification.
> 
> Let's take, for example, net-device rx virtqueue.
> This scenario leads to situation where only once per ring (no matter its negotiated size), the device will hand-over ALL ring buffers to the driver, and will have no buffers available for incoming rx-packets. 
> From this moment (in HW implementation, earlier, due to pipelining nature), until the driver finish to process some packets, and reallocate empty buffers back to the device, the device stands still, and cannot serve incoming rx-packets.
> In lossy-networks, the device will have to drop incoming packets during this time.
> In lossless-networks, the device will have to backpressure (send pause).
> 
> In both cases, the result is underperformance.
> 
> Similar issue occurs with the counterpart - used_event suppression mechanism, on the tx-virtqueue.
> 
> 
> I suggest to split VIRTIO_F_EVENT_IDX into (say) VIRTIO_F_GUEST_EVENT_IDX, VIRTIO_F_HOST_EVENT_IDX, such that used_event and avail_event usage will be independent.
> Such separation makes sense, since the driver is responsible for used_event mechanism, and the device for avail_event mechanism.	

This sounds like a reasonable feature to add.

However I wonder: when packed ring is used, it is possible for the
device to enable notifications unconditionally even when EVENT_IDX has
been negotiated.

Would that be enough?




> > 
> > 
> > To add to that, above is based on
> > 
> > 2.6.10.1
> > Driver Requirements: Available Buffer Notification Suppression
> > 
> > which states:
> > 
> > After the driver writes a descriptor index into the available ring:
> > – If the idx field in the available ring (which determined where that descriptor
> > index was placed) was equal to avail_event, the driver MUST send a notification.
> > – Otherwise the driver SHOULD NOT send a notification.
> > 
> > 
> > 
> > Pls let the TC know whether taking the above into account you think further
> > clarification in the spec is necessary.
> > 
> > Thanks!
> > 
> > 
> > > > ·        Or, to add way for the device to inform the driver that he is not
> > > > using avail_event (flag /certain reserved value in avail_event
> > > > /other mechanism).
> > > >
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > Gil Savir
> > > >
> > > > Intel Corporation
> > >
> > > It might be a good addition to spec, probably along the lines of an
> > > option to send notifications on even suppression changes (which was
> > > proposed in the past, but was not included due to lack of time).
> > >
> > > --
> > > MST
> > 
> 
> Gil Savir
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
> 
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.

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/


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression
  2019-02-22  1:28       ` Michael S. Tsirkin
@ 2019-02-26 20:30         ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2019-02-26 20:30 UTC (permalink / raw)
  To: Savir, Gil; +Cc: virtio-comment, Elmaleh, Liron, Liang, Cunming

On Thu, Feb 21, 2019 at 08:28:33PM -0500, Michael S. Tsirkin wrote:
> On Thu, Feb 21, 2019 at 08:57:15AM +0000, Savir, Gil wrote:
> > 
> > 
> > > -----Original Message-----
> > > From: virtio-comment@lists.oasis-open.org [mailto:virtio-comment@lists.oasis-
> > > open.org] On Behalf Of Michael S. Tsirkin
> > > Sent: Monday, 11 February, 2019 09:03
> > > To: Savir, Gil <gil.savir@intel.com>
> > > Cc: virtio-comment@lists.oasis-open.org; Elmaleh, Liron
> > > <liron.elmaleh@intel.com>; Liang, Cunming <cunming.liang@intel.com>
> > > Subject: Re: [virtio-comment] VirtIO spec issue - Available Buffer Notification
> > > Suppression
> > > 
> > > On Mon, Feb 11, 2019 at 01:58:09AM -0500, Michael S. Tsirkin wrote:
> > > > On Thu, Jan 31, 2019 at 01:16:29PM +0000, Savir, Gil wrote:
> > > > > Hi,
> > > > >
> > > > >
> > > > >
> > > > > If VIRTIO_F_EVENT_IDX feature bit is negotiated, then Available
> > > > > Buffer Notification Suppression mechanism used is avail event (not flags).
> > > > >
> > > > > The spec (both v1.0 / v1.1-draft) states that the device MAY use
> > > > > this mechanism (Paragraph 2.4.9.2 / 2.6.10.2 respectively).
> > > > >
> > > > > This statement implies that the device may choose not to use this
> > > > > suppression mechanism (even if VIRTIO_F_EVENT_IDX was negotiated).
> > > > >
> > > > >
> > > > >
> > > > > However – there’s no way for the device to inform the driver that he
> > > > > is not using avail_event.
> > > > >
> > > > > As consequence, since there will be a default value in avail_event
> > > > > (probably 0x0), then the driver will always assume that it has to
> > > > > send notify “once-per ring”.
> > > >
> > > >
> > > > No I think this part is wrong, pls see below.
> > > >
> > > >
> > > > > This will render performance futile, or force the device to actively
> > > > > update avail_event.
> > > > >
> > > > >
> > > > >
> > > > > Is there a way for the device to inform the driver that he is not
> > > > > using avail_event (and I missed it)?
> > > > >
> > > > >
> > > > >
> > > > > If yes, than my apologies for wasting your time.
> > > > >
> > > > > If no, then I suggest one of the following:
> > > > >
> > > > > ·         Either, to change the “MAY” (referred above) to “MUST”,
> > > > >
> > > >
> > > > Thanks for the feedback!
> > > >
> > > > So we are talking about split queues.
> > > >
> > > > If avail_event is never set and remains 0, driver will send
> > > > notifications every time index wraps around to 0, which would be every
> > > > 2^16.
> > > >
> > > > This is I think expected since the spec says:
> > > >
> > > > The device MUST handle spurious notifications from the driver.
> > > >
> > > > 2^16 does not seem excessive so I don't think it will render
> > > > performance futile.
> > 
> > Performance degradation is not caused due to the amount of packets served each notification.
> > 
> > Let's take, for example, net-device rx virtqueue.
> > This scenario leads to situation where only once per ring (no matter its negotiated size), the device will hand-over ALL ring buffers to the driver, and will have no buffers available for incoming rx-packets. 
> > From this moment (in HW implementation, earlier, due to pipelining nature), until the driver finish to process some packets, and reallocate empty buffers back to the device, the device stands still, and cannot serve incoming rx-packets.
> > In lossy-networks, the device will have to drop incoming packets during this time.
> > In lossless-networks, the device will have to backpressure (send pause).
> > 
> > In both cases, the result is underperformance.
> > 
> > Similar issue occurs with the counterpart - used_event suppression mechanism, on the tx-virtqueue.
> > 
> > 
> > I suggest to split VIRTIO_F_EVENT_IDX into (say) VIRTIO_F_GUEST_EVENT_IDX, VIRTIO_F_HOST_EVENT_IDX, such that used_event and avail_event usage will be independent.
> > Such separation makes sense, since the driver is responsible for used_event mechanism, and the device for avail_event mechanism.	
> 
> This sounds like a reasonable feature to add.
> 
> However I wonder: when packed ring is used, it is possible for the
> device to enable notifications unconditionally even when EVENT_IDX has
> been negotiated.
> 
> Would that be enough?
> 

I filed this as https://github.com/oasis-tcs/virtio-spec/issues/35
I think this is something to consider for after 1.2 if
there is interest.


> 
> 
> > > 
> > > 
> > > To add to that, above is based on
> > > 
> > > 2.6.10.1
> > > Driver Requirements: Available Buffer Notification Suppression
> > > 
> > > which states:
> > > 
> > > After the driver writes a descriptor index into the available ring:
> > > – If the idx field in the available ring (which determined where that descriptor
> > > index was placed) was equal to avail_event, the driver MUST send a notification.
> > > – Otherwise the driver SHOULD NOT send a notification.
> > > 
> > > 
> > > 
> > > Pls let the TC know whether taking the above into account you think further
> > > clarification in the spec is necessary.
> > > 
> > > Thanks!
> > > 
> > > 
> > > > > ·        Or, to add way for the device to inform the driver that he is not
> > > > > using avail_event (flag /certain reserved value in avail_event
> > > > > /other mechanism).
> > > > >
> > > > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Gil Savir
> > > > >
> > > > > Intel Corporation
> > > >
> > > > It might be a good addition to spec, probably along the lines of an
> > > > option to send notifications on even suppression changes (which was
> > > > proposed in the past, but was not included due to lack of time).
> > > >
> > > > --
> > > > MST
> > > 
> > 
> > Gil Savir
> > ---------------------------------------------------------------------
> > Intel Israel (74) Limited
> > 
> > This e-mail and any attachments may contain confidential material for
> > the sole use of the intended recipient(s). Any review or distribution
> > by others is strictly prohibited. If you are not the intended
> > recipient, please contact the sender and delete all copies.
> 
> 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/

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/


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-02-26 20:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-31 13:16 [virtio-comment] VirtIO spec issue - Available Buffer Notification Suppression Savir, Gil
2019-02-11  6:58 ` Michael S. Tsirkin
2019-02-11  7:02   ` Michael S. Tsirkin
2019-02-21  8:57     ` Savir, Gil
2019-02-22  1:28       ` Michael S. Tsirkin
2019-02-26 20:30         ` Michael S. Tsirkin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.