All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
@ 2023-02-15  9:54 Alvaro Karsz
  2023-02-15 11:42 ` [virtio-dev] " Michael S. Tsirkin
  2023-02-15 13:43 ` [virtio-comment] " David Edmondson
  0 siblings, 2 replies; 8+ messages in thread
From: Alvaro Karsz @ 2023-02-15  9:54 UTC (permalink / raw)
  To: virtio-comment, virtio-dev; +Cc: jasowang, mst, hengqi, parav, Alvaro Karsz

This patch makes several improvements to the notification coalescing
feature, including:

- Consolidating virtio_net_ctrl_coal_tx and virtio_net_ctrl_coal_rx
  into a single struct, virtio_net_ctrl_coal, as they are identical.
- Emphasizing that the coalescing commands are best-effort.
- Defining the behavior of coalescing with regards to delivering
  notifications when a change occur.
- Stating that the commands should apply to all the receive/transmit
  virtqueues.
- Stating that every receive/transmit virtqueue should count it's own
  packets.
- A new intro explaining the entire coalescing operation.

Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
---
v2:
	- Add the last 2 points to the patch.
	- Rephrase the "commands are best-effort" note.
	- Replace "notification" with "used buffer notification" to be
	  more consistent.
v3:
	- Add an intro explaining the entire coalescing operation.

 device-types/net/description.tex | 63 ++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index 1741c79..2f4835e 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -1514,15 +1514,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
 send control commands for dynamically changing the coalescing parameters.
 
-\begin{lstlisting}
-struct virtio_net_ctrl_coal_rx {
-    le32 rx_max_packets;
-    le32 rx_usecs;
-};
+\begin{note}
+In general, these commands are best-effort: spurious notifications could still arrive.
+\end{note}
 
-struct virtio_net_ctrl_coal_tx {
-    le32 tx_max_packets;
-    le32 tx_usecs;
+\begin{lstlisting}
+struct virtio_net_ctrl_coal {
+    le32 max_packets;
+    le32 usecs;
 };
 
 #define VIRTIO_NET_CTRL_NOTF_COAL 6
@@ -1532,49 +1531,65 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 Coalescing parameters:
 \begin{itemize}
-\item \field{rx_usecs}: Maximum number of usecs to delay a RX notification.
-\item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
-\item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
-\item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
+\item \field{usecs} for RX: Maximum number of usecs to delay a RX notification.
+\item \field{usecs} for TX: Maximum number of usecs to delay a TX notification.
+\item \field{max_packets} for RX: Maximum number of packets to receive before a RX notification.
+\item \field{max_packets} for TX: Maximum number of packets to send before a TX notification.
 \end{itemize}
 
-
 The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
 \begin{enumerate}
-\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
-\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
+\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{usecs} and \field{max_packets} parameters for all the transmit virtqueues.
+\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{usecs} and \field{max_packets} parameters for all the receive virtqueues.
 \end{enumerate}
 
-\subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
+\subparagraph{Operation}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Operation}
+
+The device sends a used buffer notification once the notification conditions are met, if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
+
+When the device has non-zero \field{usecs} and non-zero \field{max_packets}, it starts counting usecs and packets upon receiving/sending a packet.
+The device counts packets and usecs for each receive virtqueue and transmit virtqueue separately.
+In this case, the notification conditions are met when \field{usecs} usecs elapses, or upon sending/receiving \field{max_packets} packets, whichever happens first.
+
+When the device has \field{usecs} = 0 or \field{max_packets} = 0, the notification conditions are met after every packet received/sent.
+
+\subparagraph{RX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Example}
 
 If, for example:
 \begin{itemize}
-\item \field{rx_usecs} = 10.
-\item \field{rx_max_packets} = 15.
+\item \field{usecs} = 10.
+\item \field{max_packets} = 15.
 \end{itemize}
 
-The device will operate as follows:
+A device with a single receive virtqueue will operate as follows:
 
 \begin{itemize}
 \item The device will count received packets until it accumulates 15, or until 10 usecs elapsed since the first one was received.
 \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
 \end{itemize}
 
-\subparagraph{TX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / TX Notifications}
+\subparagraph{TX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / TX Example}
 
 If, for example:
 \begin{itemize}
-\item \field{tx_usecs} = 10.
-\item \field{tx_max_packets} = 15.
+\item \field{usecs} = 10.
+\item \field{max_packets} = 15.
 \end{itemize}
 
-The device will operate as follows:
+A device with a single transmit virtqueue will operate as follows:
 
 \begin{itemize}
 \item The device will count sent packets until it accumulates 15, or until 10 usecs elapsed since the first one was sent.
 \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
 \end{itemize}
 
+\subparagraph{Notifications When Coalescing Parameters Change}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Notifications When Coalescing Parameters Change}
+
+When a device changes the coalescing parameters, it needs to check if the new notification conditions are met and send a used buffer notification if so.
+
+For example, \field{max_packets} = 15 for a device with a single transmit virtqueue.
+If the device sends 10 packets, then it receives a VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command with \field{max_packets} = 8, the device needs to immediately send a used buffer notification, if the notifications are not suppressed by the driver.
+
 \drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
 
 If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
@@ -1583,7 +1598,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
 
-A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
+A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.
 
 Upon reset, a device MUST initialize all coalescing parameters to 0.
 
-- 
2.34.1


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 related	[flat|nested] 8+ messages in thread

* [virtio-dev] Re: [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15  9:54 [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature Alvaro Karsz
@ 2023-02-15 11:42 ` Michael S. Tsirkin
  2023-02-15 13:41   ` [virtio-comment] " Alvaro Karsz
  2023-02-15 13:43 ` [virtio-comment] " David Edmondson
  1 sibling, 1 reply; 8+ messages in thread
From: Michael S. Tsirkin @ 2023-02-15 11:42 UTC (permalink / raw)
  To: Alvaro Karsz; +Cc: virtio-comment, virtio-dev, jasowang, hengqi, parav

On Wed, Feb 15, 2023 at 11:54:09AM +0200, Alvaro Karsz wrote:
> This patch makes several improvements to the notification coalescing
> feature, including:
> 
> - Consolidating virtio_net_ctrl_coal_tx and virtio_net_ctrl_coal_rx
>   into a single struct, virtio_net_ctrl_coal, as they are identical.
> - Emphasizing that the coalescing commands are best-effort.
> - Defining the behavior of coalescing with regards to delivering
>   notifications when a change occur.
> - Stating that the commands should apply to all the receive/transmit
>   virtqueues.
> - Stating that every receive/transmit virtqueue should count it's own
>   packets.
> - A new intro explaining the entire coalescing operation.
> 
> Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>


Looks good. Minor wording tweaks but mostly ready. Thanks!

> ---
> v2:
> 	- Add the last 2 points to the patch.
> 	- Rephrase the "commands are best-effort" note.
> 	- Replace "notification" with "used buffer notification" to be
> 	  more consistent.
> v3:
> 	- Add an intro explaining the entire coalescing operation.
> 
>  device-types/net/description.tex | 63 ++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 24 deletions(-)
> 
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 1741c79..2f4835e 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1514,15 +1514,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
>  send control commands for dynamically changing the coalescing parameters.
>  
> -\begin{lstlisting}
> -struct virtio_net_ctrl_coal_rx {
> -    le32 rx_max_packets;
> -    le32 rx_usecs;
> -};
> +\begin{note}
> +In general, these commands are best-effort: spurious notifications could still arrive.
> +\end{note}
>  
> -struct virtio_net_ctrl_coal_tx {
> -    le32 tx_max_packets;
> -    le32 tx_usecs;
> +\begin{lstlisting}
> +struct virtio_net_ctrl_coal {
> +    le32 max_packets;
> +    le32 usecs;
>  };
>  
>  #define VIRTIO_NET_CTRL_NOTF_COAL 6
> @@ -1532,49 +1531,65 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  Coalescing parameters:
>  \begin{itemize}
> -\item \field{rx_usecs}: Maximum number of usecs to delay a RX notification.
> -\item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
> -\item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
> -\item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
> +\item \field{usecs} for RX: Maximum number of usecs to delay a RX notification.
> +\item \field{usecs} for TX: Maximum number of usecs to delay a TX notification.
> +\item \field{max_packets} for RX: Maximum number of packets to receive before a RX notification.
> +\item \field{max_packets} for TX: Maximum number of packets to send before a TX notification.
>  \end{itemize}
>  
> -
>  The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
>  \begin{enumerate}
> -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
> -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{usecs} and \field{max_packets} parameters for all the transmit virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{usecs} and \field{max_packets} parameters for all the receive virtqueues.
>  \end{enumerate}
>  
> -\subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
> +\subparagraph{Operation}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Operation}
> +
> +The device sends a used buffer notification once the notification conditions are met, if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
> +
> +When the device has non-zero \field{usecs} and non-zero \field{max_packets}, it starts counting usecs and packets upon receiving/sending a packet.
> +The device counts packets and usecs for each receive virtqueue and transmit virtqueue separately.
> +In this case, the notification conditions are met when \field{usecs} usecs elapses, or upon sending/receiving \field{max_packets} packets, whichever happens first.
> +
> +When the device has \field{usecs} = 0 or \field{max_packets} = 0, the notification conditions are met after every packet received/sent.
> +
> +\subparagraph{RX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Example}
>  
>  If, for example:
>  \begin{itemize}
> -\item \field{rx_usecs} = 10.
> -\item \field{rx_max_packets} = 15.
> +\item \field{usecs} = 10.
> +\item \field{max_packets} = 15.
>  \end{itemize}
>  
> -The device will operate as follows:
> +A device with a single receive virtqueue will operate as follows:

I think this should be
"then a device"

and do not make it a new paragraph - what we have is continuation of the
example.

>  \begin{itemize}
>  \item The device will count received packets until it accumulates 15, or until 10 usecs elapsed since the first one was received.
>  \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
>  \end{itemize}
>  
> -\subparagraph{TX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / TX Notifications}
> +\subparagraph{TX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / TX Example}
>  
>  If, for example:
>  \begin{itemize}
> -\item \field{tx_usecs} = 10.
> -\item \field{tx_max_packets} = 15.
> +\item \field{usecs} = 10.
> +\item \field{max_packets} = 15.
>  \end{itemize}
>  
> -The device will operate as follows:
> +A device with a single transmit virtqueue will operate as follows:
>  


same

>  \begin{itemize}
>  \item The device will count sent packets until it accumulates 15, or until 10 usecs elapsed since the first one was sent.
>  \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
>  \end{itemize}
>  
> +\subparagraph{Notifications When Coalescing Parameters Change}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Notifications When Coalescing Parameters Change}
> +
> +When a device changes the coalescing parameters, it needs to check if the new notification conditions are met and send a used buffer notification if so.

actually it is the driver changing them no?
maybe better in a neutral voice:

	when the coalescing parameters of a device change ....


> +For example, \field{max_packets} = 15 for a device with a single transmit virtqueue.

the sentence ends abruptly. maybe merge with next one:
	a single transmit virtqueue: if the device ....

> +If the device sends 10 packets, then

then -> and afterwards

> it receives a
> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command with \field{max_packets} = 8,
> the device needs to immediately send a used buffer notification, if
> the notifications are not suppressed by the driver.

maybe here "then the notification condition is immediately considered to
be met; the device needs ... 

>  \drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>  
>  If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
> @@ -1583,7 +1598,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
>  
> -A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
> +A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.
>  
>  Upon reset, a device MUST initialize all coalescing parameters to 0.
>  
> -- 
> 2.34.1


---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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

* [virtio-comment] Re: [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15 11:42 ` [virtio-dev] " Michael S. Tsirkin
@ 2023-02-15 13:41   ` Alvaro Karsz
  0 siblings, 0 replies; 8+ messages in thread
From: Alvaro Karsz @ 2023-02-15 13:41 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtio-comment, virtio-dev, jasowang, hengqi, parav

Thanks.
I'll send v4 tomorrow.

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] 8+ messages in thread

* Re: [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15  9:54 [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature Alvaro Karsz
  2023-02-15 11:42 ` [virtio-dev] " Michael S. Tsirkin
@ 2023-02-15 13:43 ` David Edmondson
  2023-02-15 16:19   ` [virtio-dev] " Alvaro Karsz
  2023-02-15 16:45   ` [virtio-dev] " Alvaro Karsz
  1 sibling, 2 replies; 8+ messages in thread
From: David Edmondson @ 2023-02-15 13:43 UTC (permalink / raw)
  To: Alvaro Karsz; +Cc: virtio-dev, jasowang, mst, hengqi, parav, virtio-comment


On Wednesday, 2023-02-15 at 11:54:09 +02, Alvaro Karsz wrote:
> This patch makes several improvements to the notification coalescing
> feature, including:
>
> - Consolidating virtio_net_ctrl_coal_tx and virtio_net_ctrl_coal_rx
>   into a single struct, virtio_net_ctrl_coal, as they are identical.
> - Emphasizing that the coalescing commands are best-effort.
> - Defining the behavior of coalescing with regards to delivering
>   notifications when a change occur.
> - Stating that the commands should apply to all the receive/transmit
>   virtqueues.
> - Stating that every receive/transmit virtqueue should count it's own
>   packets.
> - A new intro explaining the entire coalescing operation.
>
> Signed-off-by: Alvaro Karsz <alvaro.karsz@solid-run.com>
> ---
> v2:
> 	- Add the last 2 points to the patch.
> 	- Rephrase the "commands are best-effort" note.
> 	- Replace "notification" with "used buffer notification" to be
> 	  more consistent.
> v3:
> 	- Add an intro explaining the entire coalescing operation.
>
>  device-types/net/description.tex | 63 ++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 24 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index 1741c79..2f4835e 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -1514,15 +1514,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
>  send control commands for dynamically changing the coalescing parameters.
>  
> -\begin{lstlisting}
> -struct virtio_net_ctrl_coal_rx {
> -    le32 rx_max_packets;
> -    le32 rx_usecs;
> -};
> +\begin{note}
> +In general, these commands are best-effort: spurious notifications could still arrive.
> +\end{note}

This is quite vague. How about:

The behaviour of the device in response to these commands is
best-effort: the device may generate notifications more frequently than
specified, for example after less than \field{max_packets} packets have
been processed or less than \field{usecs} usecs time has elapsed.

>  
> -struct virtio_net_ctrl_coal_tx {
> -    le32 tx_max_packets;
> -    le32 tx_usecs;
> +\begin{lstlisting}
> +struct virtio_net_ctrl_coal {
> +    le32 max_packets;
> +    le32 usecs;
>  };
>  
>  #define VIRTIO_NET_CTRL_NOTF_COAL 6
> @@ -1532,49 +1531,65 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  Coalescing parameters:
>  \begin{itemize}
> -\item \field{rx_usecs}: Maximum number of usecs to delay a RX notification.
> -\item \field{tx_usecs}: Maximum number of usecs to delay a TX notification.
> -\item \field{rx_max_packets}: Maximum number of packets to receive before a RX notification.
> -\item \field{tx_max_packets}: Maximum number of packets to send before a TX notification.
> +\item \field{usecs} for RX: Maximum number of usecs to delay a RX notification.
> +\item \field{usecs} for TX: Maximum number of usecs to delay a TX notification.
> +\item \field{max_packets} for RX: Maximum number of packets to receive before a RX notification.
> +\item \field{max_packets} for TX: Maximum number of packets to send before a TX notification.
>  \end{itemize}
>  
> -
>  The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
>  \begin{enumerate}
> -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and \field{tx_max_packets} parameters.
> -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{usecs} and \field{max_packets} parameters for all the transmit virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{usecs} and \field{max_packets} parameters for all the receive virtqueues.
>  \end{enumerate}
>  
> -\subparagraph{RX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Notifications}
> +\subparagraph{Operation}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Operation}
> +
> +The device sends a used buffer notification once the notification conditions are met, if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
> +
> +When the device has non-zero \field{usecs} and non-zero \field{max_packets}, it starts counting usecs and packets upon receiving/sending a packet.
> +The device counts packets and usecs for each receive virtqueue and transmit virtqueue separately.
> +In this case, the notification conditions are met when \field{usecs} usecs elapses, or upon sending/receiving \field{max_packets} packets, whichever happens first.
> +
> +When the device has \field{usecs} = 0 or \field{max_packets} = 0, the notification conditions are met after every packet received/sent.
> +
> +\subparagraph{RX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Example}
>  
>  If, for example:
>  \begin{itemize}
> -\item \field{rx_usecs} = 10.
> -\item \field{rx_max_packets} = 15.
> +\item \field{usecs} = 10.
> +\item \field{max_packets} = 15.
>  \end{itemize}
>  
> -The device will operate as follows:
> +A device with a single receive virtqueue will operate as follows:
>  
>  \begin{itemize}
>  \item The device will count received packets until it accumulates 15, or until 10 usecs elapsed since the first one was received.
>  \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
>  \end{itemize}
>  
> -\subparagraph{TX Notifications}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / TX Notifications}
> +\subparagraph{TX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / TX Example}
>  
>  If, for example:
>  \begin{itemize}
> -\item \field{tx_usecs} = 10.
> -\item \field{tx_max_packets} = 15.
> +\item \field{usecs} = 10.
> +\item \field{max_packets} = 15.
>  \end{itemize}
>  
> -The device will operate as follows:
> +A device with a single transmit virtqueue will operate as follows:
>  
>  \begin{itemize}
>  \item The device will count sent packets until it accumulates 15, or until 10 usecs elapsed since the first one was sent.
>  \item If the notifications are not suppressed by the driver, the device will send an used buffer notification, otherwise, the device will not send an used buffer notification as long as the notifications are suppressed.
>  \end{itemize}
>  
> +\subparagraph{Notifications When Coalescing Parameters Change}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / Notifications When Coalescing Parameters Change}
> +
> +When a device changes the coalescing parameters, it needs to check if the new notification conditions are met and send a used buffer notification if so.
> +
> +For example, \field{max_packets} = 15 for a device with a single transmit virtqueue.
> +If the device sends 10 packets, then it receives a VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command with \field{max_packets} = 8, the device needs to immediately send a used buffer notification, if the notifications are not suppressed by the driver.
> +
>  \drivernormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>  
>  If the VIRTIO_NET_F_NOTF_COAL feature has not been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_COAL commands.
> @@ -1583,7 +1598,7 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  A device SHOULD respond to the VIRTIO_NET_CTRL_NOTF_COAL commands with VIRTIO_NET_ERR if it was not able to change the parameters.
>  
> -A device SHOULD NOT send used buffer notifications to the driver, if the notifications are suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}, even if the coalescing counters expired.
> +A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.
>  
>  Upon reset, a device MUST initialize all coalescing parameters to 0.
-- 
Freedom is just a song by Wham!.

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] 8+ messages in thread

* [virtio-dev] Re: [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15 13:43 ` [virtio-comment] " David Edmondson
@ 2023-02-15 16:19   ` Alvaro Karsz
  2023-02-15 16:43     ` Parav Pandit
  2023-02-15 16:45   ` [virtio-dev] " Alvaro Karsz
  1 sibling, 1 reply; 8+ messages in thread
From: Alvaro Karsz @ 2023-02-15 16:19 UTC (permalink / raw)
  To: David Edmondson; +Cc: virtio-dev, jasowang, mst, hengqi, parav, virtio-comment

> > +\begin{note}
> > +In general, these commands are best-effort: spurious notifications could still arrive.
> > +\end{note}
>
> This is quite vague. How about:
>
> The behaviour of the device in response to these commands is
> best-effort: the device may generate notifications more frequently than
> specified, for example after less than \field{max_packets} packets have
> been processed or less than \field{usecs} usecs time has elapsed.
>
Looks good to me.
I'll add it to v4 if nobody objects.

Thanks.

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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

* RE: [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15 16:19   ` [virtio-dev] " Alvaro Karsz
@ 2023-02-15 16:43     ` Parav Pandit
  0 siblings, 0 replies; 8+ messages in thread
From: Parav Pandit @ 2023-02-15 16:43 UTC (permalink / raw)
  To: Alvaro Karsz, David Edmondson
  Cc: virtio-dev, jasowang, mst, hengqi, virtio-comment


> From: Alvaro Karsz <alvaro.karsz@solid-run.com>
> Sent: Wednesday, February 15, 2023 11:19 AM
> 
> > > +\begin{note}
> > > +In general, these commands are best-effort: spurious notifications could
> still arrive.
> > > +\end{note}
> >
> > This is quite vague. How about:
> >
> > The behaviour of the device in response to these commands is
> > best-effort: the device may generate notifications more frequently
> > than specified, for example after less than \field{max_packets}
> > packets have been processed or less than \field{usecs} usecs time has
> elapsed.
> >
> Looks good to me.
> I'll add it to v4 if nobody objects.

small improvement below.

s/more frequently/more or less frequently.

This is because when LSO or LRO is ongoing, packet count or timer may have reached, but user buffer notification wouldn’t occur, because used ring updates are not yet done. There is no partial completion. So, notification can be less frequent. Hence, the wording should be "more or less".

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

* [virtio-dev] Re: [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15 13:43 ` [virtio-comment] " David Edmondson
  2023-02-15 16:19   ` [virtio-dev] " Alvaro Karsz
@ 2023-02-15 16:45   ` Alvaro Karsz
  2023-02-15 16:56     ` David Edmondson
  1 sibling, 1 reply; 8+ messages in thread
From: Alvaro Karsz @ 2023-02-15 16:45 UTC (permalink / raw)
  To: David Edmondson; +Cc: virtio-dev, jasowang, mst, hengqi, parav, virtio-comment

> > +\begin{note}
> > +In general, these commands are best-effort: spurious notifications could still arrive.
> > +\end{note}
>
> This is quite vague. How about:
>
> The behaviour of the device in response to these commands is
> best-effort: the device may generate notifications more frequently than
> specified, for example after less than \field{max_packets} packets have
> been processed or less than \field{usecs} usecs time has elapsed.
>

On second thought, this may be a little confusing.
The device can send a notification before processing max_packets
packets, if usecs elapsed (and vice versa).
So it should be "less than \field{max_packets} packets have been
processed AND less than \field{usecs} usecs ..."

How about dropping the example?

---------------------------------------------------------------------
To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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

* Re: [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature
  2023-02-15 16:45   ` [virtio-dev] " Alvaro Karsz
@ 2023-02-15 16:56     ` David Edmondson
  0 siblings, 0 replies; 8+ messages in thread
From: David Edmondson @ 2023-02-15 16:56 UTC (permalink / raw)
  To: Alvaro Karsz; +Cc: virtio-dev, jasowang, mst, hengqi, parav, virtio-comment


On Wednesday, 2023-02-15 at 18:45:14 +02, Alvaro Karsz wrote:
>> > +\begin{note}
>> > +In general, these commands are best-effort: spurious notifications could still arrive.
>> > +\end{note}
>>
>> This is quite vague. How about:
>>
>> The behaviour of the device in response to these commands is
>> best-effort: the device may generate notifications more frequently than
>> specified, for example after less than \field{max_packets} packets have
>> been processed or less than \field{usecs} usecs time has elapsed.
>>
>
> On second thought, this may be a little confusing.
> The device can send a notification before processing max_packets
> packets, if usecs elapsed (and vice versa).
> So it should be "less than \field{max_packets} packets have been
> processed AND less than \field{usecs} usecs ..."
>
> How about dropping the example?

Given Parav's comment as well, drop the example, yes.
-- 
Don't worry, Mary, cause I'm taking care of Danny, and he's taking care of me.

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] 8+ messages in thread

end of thread, other threads:[~2023-02-15 16:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-15  9:54 [virtio-comment] [PATCH v3] virtio-net: Fix and update VIRTIO_NET_F_NOTF_COAL feature Alvaro Karsz
2023-02-15 11:42 ` [virtio-dev] " Michael S. Tsirkin
2023-02-15 13:41   ` [virtio-comment] " Alvaro Karsz
2023-02-15 13:43 ` [virtio-comment] " David Edmondson
2023-02-15 16:19   ` [virtio-dev] " Alvaro Karsz
2023-02-15 16:43     ` Parav Pandit
2023-02-15 16:45   ` [virtio-dev] " Alvaro Karsz
2023-02-15 16:56     ` David Edmondson

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.