[virtio-dev] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[virtio-dev] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

Currently, coalescing parameters are grouped for all transmit and receive
virtqueues. This patch supports setting or getting the parameters for a
specified virtqueue, and a typical application of this function is netdim[1].

When the traffic between virtqueues is unbalanced, for example, one virtqueue
is busy and another virtqueue is idle, then it will be very useful to
control coalescing parameters at the virtqueue granularity.

[1] https://docs.kernel.org/networking/net_dim.html

Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
---
This patch is on top of Alvaro's latest v7 patch: https://lists.oasis-open.org/archives/virtio-dev/202302/msg00431.html .

v9->v10:
       1. Remove the "global values". @Parav Pandit
       2. Avoid multiple interpretations of command behavior. @Alvaro Karsz

v8->v9:
       1. Declare the commands that can be sent for each feature. @Alvaro Karsz
       2. Add information about "global values" in the command's explanation. @Alvaro Karsz

v7->v8:
       1. Use "best-effort" in Alvaro's patch instead of "the device may set the parameter to a value close to 2". @Michael S . Tsirkin, @David Edmondson

v6->v7:
       1. Clarify the relationship of VIRTIO_NET_CTRL_NOTF_COAL_TX/RX_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET. @Alvaro Karsz, @Michael S. Tsirkin
       2. Remove the formula for vqn range. @Parav Pandit
       3. Some expressions are clearer. @Parav Pandit, @Michael S. Tsirkin

v5->v6:
       1. Explain that the device may set a different value than the one passed in by the driver. @David Edmondson

v4->v5:
       1. Add the correspondence between virtio_net_ctrl_coal and virtio_net_ctrl_coal_vq and control commands. @Michael S. Tsirkin
       2. Add read and write attributes for each field. @Michael S. Tsirkin
       3. A clearer description of how to set coalescing parameters for vq reset. @Michael S. Tsirkin
       4. Fix some syntax errors. @Michael S. Tsirkin, @David Edmondson

v3->v4:
       1. Include virtio_net_ctrl_coal in the virtio_net_ctrl_coal_vq structure. @Alvaro Karsz
       2. Add consideration of vq reset. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
       3. Avoid too many examples by giving a comprehensive example. @Michael S. Tsirkin
       4. Fix typos and streamline clarifications. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz

v2->v3:
       1. Add the netdim link. @Parav Pandit
       2. VIRTIO_NET_F_VQ_NOTF_COAL no longer depends on VIRTIO_NET_F_NOTF_COAL. @Michael S. Tsirkin, @Alvaro Karsz
       3. _VQ_GET is explained more. @Michael S. Tsirkin
       4. Add more examples to avoid misunderstandings. @Michael S. Tsirkin
       5. Clarify some statements. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
       6. Adjust the virtio_net_ctrl_coal_vq structure. @Michael S. Tsirkin
       7. Fix some typos. @Michael S. Tsirkin

v1->v2:
       1. Rename VIRTIO_NET_F_PERQUEUE_NOTF_COAL to VIRTIO_NET_F_VQ_NOTF_COAL. @Michael S. Tsirkin
       2. Use the \field{vqn} instead of the qid. @Michael S. Tsirkin
       3. Unify tx and rx control structures into one structure virtio_net_ctrl_coal_vq. @Michael S. Tsirkin
       4. Add a new control command VIRTIO_NET_CTRL_NOTF_COAL_VQ. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
       5. The special value 0xFFF is removed because VIRTIO_NET_CTRL_NOTF_COAL can be used. @Alvaro Karsz
       6. Clarify some special scenarios. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz

 device-types/net/description.tex | 88 ++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 11 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index e71e33b..b5d45e8 100644
--- a/device-types/net/description.tex
+++ b/device-types/net/description.tex
@@ -83,6 +83,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
 \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
     channel.
 
+\item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue notification coalescing.
+
 \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
 
 \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
@@ -139,6 +141,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
 \item[VIRTIO_NET_F_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
 \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
 \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
+\item[VIRTIO_NET_F_VQ_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
 \end{description}
 
 \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
@@ -1505,13 +1508,13 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 \paragraph{Notifications Coalescing}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
 
-If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
-send control commands for dynamically changing the coalescing parameters.
+If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, a driver can send
+commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET
+for notification coalescing.
 
-\begin{note}
-The behavior of the device in response to these commands is best-effort:
-the device may generate notifications more or less frequently than specified.
-\end{note}
+If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated, a driver can send
+commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
+for notification coalescing.
 
 \begin{lstlisting}
 struct virtio_net_ctrl_coal {
@@ -1519,25 +1522,74 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
     le32 max_usecs;
 };
 
+struct virtio_net_ctrl_coal_vq {
+    le16 vqn;
+    le16 reserved;
+    struct virtio_net_ctrl_coal coal;
+};
+
 #define VIRTIO_NET_CTRL_NOTF_COAL 6
  #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET  0
  #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
+ #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2
+ #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
 \end{lstlisting}
 
 Coalescing parameters:
 \begin{itemize}
+\item \field{vqn}: The virtqueue number of an enabled transmit or receive virtqueue.
 \item \field{max_usecs} for RX: Maximum number of microseconds to delay a RX notification.
 \item \field{max_usecs} for TX: Maximum number of microseconds 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:
+\field{reserved} is reserved and it is ignored by a device.
+
+Read/Write attributes for coalescing parameters:
+\begin{itemize}
+\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
+      and \field{max_packets} are write-only for a driver.
+\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, \field{vqn}, \field{reserved}, \field{max_usecs}
+      and \field{max_packets} are write-only for a driver.
+\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn} and \field{reserved} are write-only
+      for a driver, and, \field{max_usecs} and \field{max_packets} are read-only for the driver.
+\end{itemize}
+
+The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
 \begin{enumerate}
-\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{max_usecs} and \field{max_packets} parameters for all transmit virtqueues.
-\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{max_usecs} and \field{max_packets} parameters for all receive virtqueues.
+\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets} parameters for all transmit virtqueues.
+\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets} parameters for all receive virtqueues.
+\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets} parameters
+                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
+\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets} parameters
+                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
 \end{enumerate}
 
+\begin{note}
+The behavior of the device in response to these commands is best-effort:
+the device may generate notifications more or less frequently than specified.
+\end{note}
+
+If coalescing parameters are being set, the device applies the last coalescing parameters received for a
+virtqueue, regardless of the command used to set the parameters. Use the following command sequence
+with 2 pairs of virtqueues as an example:
+Each of the following commands sets \field{max_usecs} and \field{max_packets} parameters for virtqueues.
+\begin{itemize}
+\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing parameters for virtqueue0 and virtqueue2, and, virtqueue1 and virtqueue3 retain their previous parameter values.
+\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the values from command1.
+\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 0, the device responds with coalescing parameters of virtqueue0 set by command2.
+\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its previous values.
+\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing parameters for virtqueue1 and virtqueue3, and overrides the values set by command4.
+\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 1, the device responds with coalescing parameters of virtqueue1 set by command5.
+\end{itemize}
+
+Upon disabling and re-enabling the transmit virtqueue, the device will set the coalescing parameters of the virtqueue
+to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or to 0 if the command did not set any TX coalescing parameters.
+
+Upon disabling and re-enabling the receive virtqueue, the device will set the coalescing parameters of the virtqueue
+to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or to 0 if the command did not set any RX coalescing parameters.
+
 \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 and if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
@@ -1585,11 +1637,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
 
 \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.
+A driver MUST negotiate the VIRTIO_NET_F_NOTF_COAL feature before issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRIO_NET_CTRL_NOTF_COAL_RX_SET.
+
+A driver MUST negotiate the VIRTIO_NET_F_VQ_NOTF_COAL feature before issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRIO_NET_CTRL_NOTF_COAL_VQ_GET.
+
+A driver MUST ignore the values of coalescing parameters received from the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if a device responds with VIRTIO_NET_ERR.
 
 \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
 
-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 MUST ignore \field{reserved}.
+
+A device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was not able to change the parameters.
+
+A device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with VIRTIO_NET_ERR if it was not able to change the parameters.
+
+A device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if the given virtqueue is disabled.
+
+After disabling and re-enabling a transmit/receive virtqueue, the device MUST set coalescing parameters of the virtqueue
+to those configured using the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET/VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the command
+did not configure TX/RX coalescing parameters, to 0.
 
 A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.
 
-- 
2.19.1.6.gb485710b


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

[virtio-dev] Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[David Edmondson]

On Wednesday, 2023-03-01 at 22:10:27 +08, Heng Qi wrote:
> Currently, coalescing parameters are grouped for all transmit and receive
> virtqueues. This patch supports setting or getting the parameters for a
> specified virtqueue, and a typical application of this function is netdim[1].
>
> When the traffic between virtqueues is unbalanced, for example, one virtqueue
> is busy and another virtqueue is idle, then it will be very useful to
> control coalescing parameters at the virtqueue granularity.
>
> [1] https://docs.kernel.org/networking/net_dim.html
>
> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> ---
> This patch is on top of Alvaro's latest v7 patch: https://lists.oasis-open.org/archives/virtio-dev/202302/msg00431.html .
>
> v9->v10:
>        1. Remove the "global values". @Parav Pandit
>        2. Avoid multiple interpretations of command behavior. @Alvaro Karsz
>
> v8->v9:
>        1. Declare the commands that can be sent for each feature. @Alvaro Karsz
>        2. Add information about "global values" in the command's explanation. @Alvaro Karsz
>
> v7->v8:
>        1. Use "best-effort" in Alvaro's patch instead of "the device may set the parameter to a value close to 2". @Michael S . Tsirkin, @David Edmondson
>
> v6->v7:
>        1. Clarify the relationship of VIRTIO_NET_CTRL_NOTF_COAL_TX/RX_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET. @Alvaro Karsz, @Michael S. Tsirkin
>        2. Remove the formula for vqn range. @Parav Pandit
>        3. Some expressions are clearer. @Parav Pandit, @Michael S. Tsirkin
>
> v5->v6:
>        1. Explain that the device may set a different value than the one passed in by the driver. @David Edmondson
>
> v4->v5:
>        1. Add the correspondence between virtio_net_ctrl_coal and virtio_net_ctrl_coal_vq and control commands. @Michael S. Tsirkin
>        2. Add read and write attributes for each field. @Michael S. Tsirkin
>        3. A clearer description of how to set coalescing parameters for vq reset. @Michael S. Tsirkin
>        4. Fix some syntax errors. @Michael S. Tsirkin, @David Edmondson
>
> v3->v4:
>        1. Include virtio_net_ctrl_coal in the virtio_net_ctrl_coal_vq structure. @Alvaro Karsz
>        2. Add consideration of vq reset. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>        3. Avoid too many examples by giving a comprehensive example. @Michael S. Tsirkin
>        4. Fix typos and streamline clarifications. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>
> v2->v3:
>        1. Add the netdim link. @Parav Pandit
>        2. VIRTIO_NET_F_VQ_NOTF_COAL no longer depends on VIRTIO_NET_F_NOTF_COAL. @Michael S. Tsirkin, @Alvaro Karsz
>        3. _VQ_GET is explained more. @Michael S. Tsirkin
>        4. Add more examples to avoid misunderstandings. @Michael S. Tsirkin
>        5. Clarify some statements. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>        6. Adjust the virtio_net_ctrl_coal_vq structure. @Michael S. Tsirkin
>        7. Fix some typos. @Michael S. Tsirkin
>
> v1->v2:
>        1. Rename VIRTIO_NET_F_PERQUEUE_NOTF_COAL to VIRTIO_NET_F_VQ_NOTF_COAL. @Michael S. Tsirkin
>        2. Use the \field{vqn} instead of the qid. @Michael S. Tsirkin
>        3. Unify tx and rx control structures into one structure virtio_net_ctrl_coal_vq. @Michael S. Tsirkin
>        4. Add a new control command VIRTIO_NET_CTRL_NOTF_COAL_VQ. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>        5. The special value 0xFFF is removed because VIRTIO_NET_CTRL_NOTF_COAL can be used. @Alvaro Karsz
>        6. Clarify some special scenarios. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>
>  device-types/net/description.tex | 88 ++++++++++++++++++++++++++++----
>  1 file changed, 77 insertions(+), 11 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index e71e33b..b5d45e8 100644
> --- a/device-types/net/description.tex
> +++ b/device-types/net/description.tex
> @@ -83,6 +83,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>  \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>      channel.
>  
> +\item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue notification coalescing.
> +
>  \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>  
>  \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -139,6 +141,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
>  \item[VIRTIO_NET_F_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
>  \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
>  \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
> +\item[VIRTIO_NET_F_VQ_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
>  \end{description}
>  
>  \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -1505,13 +1508,13 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  \paragraph{Notifications Coalescing}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>  
> -If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
> -send control commands for dynamically changing the coalescing parameters.
> +If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, a driver can send
> +commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET
> +for notification coalescing.

"to change the notification coalescing parameters."

>  
> -\begin{note}
> -The behavior of the device in response to these commands is best-effort:
> -the device may generate notifications more or less frequently than specified.
> -\end{note}
> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated, a driver can send
> +commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
> +for notification coalescing.

"to change the per-virtqueue notification coalescing parameters."

>  
>  \begin{lstlisting}
>  struct virtio_net_ctrl_coal {
> @@ -1519,25 +1522,74 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>      le32 max_usecs;
>  };
>  
> +struct virtio_net_ctrl_coal_vq {
> +    le16 vqn;
> +    le16 reserved;
> +    struct virtio_net_ctrl_coal coal;
> +};
> +
>  #define VIRTIO_NET_CTRL_NOTF_COAL 6
>   #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET  0
>   #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
>  \end{lstlisting}
>  
>  Coalescing parameters:
>  \begin{itemize}
> +\item \field{vqn}: The virtqueue number of an enabled transmit or receive virtqueue.
>  \item \field{max_usecs} for RX: Maximum number of microseconds to delay a RX notification.
>  \item \field{max_usecs} for TX: Maximum number of microseconds 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:
> +\field{reserved} is reserved and it is ignored by a device.
> +
> +Read/Write attributes for coalescing parameters:
> +\begin{itemize}
> +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
> +      and \field{max_packets} are write-only for a driver.
> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, \field{vqn}, \field{reserved}, \field{max_usecs}
> +      and \field{max_packets} are write-only for a driver.
> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn} and \field{reserved} are write-only
> +      for a driver, and, \field{max_usecs} and \field{max_packets} are read-only for the driver.
> +\end{itemize}
> +
> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
>  \begin{enumerate}
> -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{max_usecs} and \field{max_packets} parameters for all transmit virtqueues.
> -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{max_usecs} and \field{max_packets} parameters for all receive virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets} parameters for all transmit virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets} parameters for all receive virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets} parameters
> +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets} parameters
> +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.

Should this now be "whose index is \field{vqn}"?

>  \end{enumerate}
>  
> +\begin{note}
> +The behavior of the device in response to these commands is best-effort:
> +the device may generate notifications more or less frequently than specified.
> +\end{note}
> +
> +If coalescing parameters are being set, the device applies the last coalescing parameters received for a
> +virtqueue, regardless of the command used to set the parameters. Use the following command sequence
> +with 2 pairs of virtqueues as an example:
> +Each of the following commands sets \field{max_usecs} and \field{max_packets} parameters for virtqueues.
> +\begin{itemize}
> +\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing parameters for virtqueue0 and virtqueue2, and, virtqueue1 and virtqueue3 retain their previous parameter values.
> +\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the values from command1.
> +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 0, the device responds with coalescing parameters of virtqueue0 set by command2.
> +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its previous values.
> +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing parameters for virtqueue1 and virtqueue3, and overrides the values set by command4.
> +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 1, the device responds with coalescing parameters of virtqueue1 set by command5.
> +\end{itemize}
> +
> +Upon disabling and re-enabling the transmit virtqueue, the device will set the coalescing parameters of the virtqueue
> +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or to 0 if the command did not set any TX coalescing parameters.
> +
> +Upon disabling and re-enabling the receive virtqueue, the device will set the coalescing parameters of the virtqueue
> +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or to 0 if the command did not set any RX coalescing parameters.
> +
>  \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 and if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
> @@ -1585,11 +1637,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>  
>  \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.
> +A driver MUST negotiate the VIRTIO_NET_F_NOTF_COAL feature before issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRIO_NET_CTRL_NOTF_COAL_RX_SET.
> +
> +A driver MUST negotiate the VIRTIO_NET_F_VQ_NOTF_COAL feature before issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRIO_NET_CTRL_NOTF_COAL_VQ_GET.
> +
> +A driver MUST ignore the values of coalescing parameters received from the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if a device responds with VIRTIO_NET_ERR.
>  
>  \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>  
> -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 MUST ignore \field{reserved}.
> +
> +A device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was not able to change the parameters.
> +
> +A device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with VIRTIO_NET_ERR if it was not able to change the parameters.
> +
> +A device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if the given virtqueue is disabled.
> +
> +After disabling and re-enabling a transmit/receive virtqueue, the device MUST set coalescing parameters of the virtqueue
> +to those configured using the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET/VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the command
> +did not configure TX/RX coalescing parameters, to 0.
>  
>  A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.
-- 
Welcome to Conditioning.

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

[virtio-dev] Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/3/2 下午7:36, David Edmondson 写道:
> On Wednesday, 2023-03-01 at 22:10:27 +08, Heng Qi wrote:
>> Currently, coalescing parameters are grouped for all transmit and receive
>> virtqueues. This patch supports setting or getting the parameters for a
>> specified virtqueue, and a typical application of this function is netdim[1].
>>
>> When the traffic between virtqueues is unbalanced, for example, one virtqueue
>> is busy and another virtqueue is idle, then it will be very useful to
>> control coalescing parameters at the virtqueue granularity.
>>
>> [1] https://docs.kernel.org/networking/net_dim.html
>>
>> Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
>> Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
>> ---
>> This patch is on top of Alvaro's latest v7 patch: https://lists.oasis-open.org/archives/virtio-dev/202302/msg00431.html .
>>
>> v9->v10:
>>         1. Remove the "global values". @Parav Pandit
>>         2. Avoid multiple interpretations of command behavior. @Alvaro Karsz
>>
>> v8->v9:
>>         1. Declare the commands that can be sent for each feature. @Alvaro Karsz
>>         2. Add information about "global values" in the command's explanation. @Alvaro Karsz
>>
>> v7->v8:
>>         1. Use "best-effort" in Alvaro's patch instead of "the device may set the parameter to a value close to 2". @Michael S . Tsirkin, @David Edmondson
>>
>> v6->v7:
>>         1. Clarify the relationship of VIRTIO_NET_CTRL_NOTF_COAL_TX/RX_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET. @Alvaro Karsz, @Michael S. Tsirkin
>>         2. Remove the formula for vqn range. @Parav Pandit
>>         3. Some expressions are clearer. @Parav Pandit, @Michael S. Tsirkin
>>
>> v5->v6:
>>         1. Explain that the device may set a different value than the one passed in by the driver. @David Edmondson
>>
>> v4->v5:
>>         1. Add the correspondence between virtio_net_ctrl_coal and virtio_net_ctrl_coal_vq and control commands. @Michael S. Tsirkin
>>         2. Add read and write attributes for each field. @Michael S. Tsirkin
>>         3. A clearer description of how to set coalescing parameters for vq reset. @Michael S. Tsirkin
>>         4. Fix some syntax errors. @Michael S. Tsirkin, @David Edmondson
>>
>> v3->v4:
>>         1. Include virtio_net_ctrl_coal in the virtio_net_ctrl_coal_vq structure. @Alvaro Karsz
>>         2. Add consideration of vq reset. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>>         3. Avoid too many examples by giving a comprehensive example. @Michael S. Tsirkin
>>         4. Fix typos and streamline clarifications. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>>
>> v2->v3:
>>         1. Add the netdim link. @Parav Pandit
>>         2. VIRTIO_NET_F_VQ_NOTF_COAL no longer depends on VIRTIO_NET_F_NOTF_COAL. @Michael S. Tsirkin, @Alvaro Karsz
>>         3. _VQ_GET is explained more. @Michael S. Tsirkin
>>         4. Add more examples to avoid misunderstandings. @Michael S. Tsirkin
>>         5. Clarify some statements. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>>         6. Adjust the virtio_net_ctrl_coal_vq structure. @Michael S. Tsirkin
>>         7. Fix some typos. @Michael S. Tsirkin
>>
>> v1->v2:
>>         1. Rename VIRTIO_NET_F_PERQUEUE_NOTF_COAL to VIRTIO_NET_F_VQ_NOTF_COAL. @Michael S. Tsirkin
>>         2. Use the \field{vqn} instead of the qid. @Michael S. Tsirkin
>>         3. Unify tx and rx control structures into one structure virtio_net_ctrl_coal_vq. @Michael S. Tsirkin
>>         4. Add a new control command VIRTIO_NET_CTRL_NOTF_COAL_VQ. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>>         5. The special value 0xFFF is removed because VIRTIO_NET_CTRL_NOTF_COAL can be used. @Alvaro Karsz
>>         6. Clarify some special scenarios. @Michael S. Tsirkin, @Parav Pandit, @Alvaro Karsz
>>
>>   device-types/net/description.tex | 88 ++++++++++++++++++++++++++++----
>>   1 file changed, 77 insertions(+), 11 deletions(-)
>>
>> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
>> index e71e33b..b5d45e8 100644
>> --- a/device-types/net/description.tex
>> +++ b/device-types/net/description.tex
>> @@ -83,6 +83,8 @@ \subsection{Feature bits}\label{sec:Device Types / Network Device / Feature bits
>>   \item[VIRTIO_NET_F_CTRL_MAC_ADDR(23)] Set MAC address through control
>>       channel.
>>   
>> +\item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue notification coalescing.
>> +
>>   \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>>   
>>   \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
>> @@ -139,6 +141,7 @@ \subsubsection{Feature bit requirements}\label{sec:Device Types / Network Device
>>   \item[VIRTIO_NET_F_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
>>   \item[VIRTIO_NET_F_RSC_EXT] Requires VIRTIO_NET_F_HOST_TSO4 or VIRTIO_NET_F_HOST_TSO6.
>>   \item[VIRTIO_NET_F_RSS] Requires VIRTIO_NET_F_CTRL_VQ.
>> +\item[VIRTIO_NET_F_VQ_NOTF_COAL] Requires VIRTIO_NET_F_CTRL_VQ.
>>   \end{description}
>>   
>>   \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types / Network Device / Feature bits / Legacy Interface: Feature bits}
>> @@ -1505,13 +1508,13 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>   
>>   \paragraph{Notifications Coalescing}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>>   
>> -If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, the driver can
>> -send control commands for dynamically changing the coalescing parameters.
>> +If the VIRTIO_NET_F_NOTF_COAL feature is negotiated, a driver can send
>> +commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET
>> +for notification coalescing.
> "to change the notification coalescing parameters."

Maybe I just want to express here that feature A can send commands B and C.

Alvaro, what's your opinion?

>
>>   
>> -\begin{note}
>> -The behavior of the device in response to these commands is best-effort:
>> -the device may generate notifications more or less frequently than specified.
>> -\end{note}
>> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated, a driver can send
>> +commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
>> +for notification coalescing.
> "to change the per-virtqueue notification coalescing parameters."
>
>>   
>>   \begin{lstlisting}
>>   struct virtio_net_ctrl_coal {
>> @@ -1519,25 +1522,74 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>       le32 max_usecs;
>>   };
>>   
>> +struct virtio_net_ctrl_coal_vq {
>> +    le16 vqn;
>> +    le16 reserved;
>> +    struct virtio_net_ctrl_coal coal;
>> +};
>> +
>>   #define VIRTIO_NET_CTRL_NOTF_COAL 6
>>    #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET  0
>>    #define VIRTIO_NET_CTRL_NOTF_COAL_RX_SET 1
>> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 2
>> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 3
>>   \end{lstlisting}
>>   
>>   Coalescing parameters:
>>   \begin{itemize}
>> +\item \field{vqn}: The virtqueue number of an enabled transmit or receive virtqueue.
>>   \item \field{max_usecs} for RX: Maximum number of microseconds to delay a RX notification.
>>   \item \field{max_usecs} for TX: Maximum number of microseconds 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:
>> +\field{reserved} is reserved and it is ignored by a device.
>> +
>> +Read/Write attributes for coalescing parameters:
>> +\begin{itemize}
>> +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
>> +      and \field{max_packets} are write-only for a driver.
>> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, \field{vqn}, \field{reserved}, \field{max_usecs}
>> +      and \field{max_packets} are write-only for a driver.
>> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn} and \field{reserved} are write-only
>> +      for a driver, and, \field{max_usecs} and \field{max_packets} are read-only for the driver.
>> +\end{itemize}
>> +
>> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
>>   \begin{enumerate}
>> -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{max_usecs} and \field{max_packets} parameters for all transmit virtqueues.
>> -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{max_usecs} and \field{max_packets} parameters for all receive virtqueues.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets} parameters for all transmit virtqueues.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets} parameters for all receive virtqueues.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets} parameters
>> +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets} parameters
>> +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
> Should this now be "whose index is \field{vqn}"?

Have we decided yet, I see in Parav's thread it seems to be still under 
discussion? Michael should we be using index?

Thanks.

>
>>   \end{enumerate}
>>   
>> +\begin{note}
>> +The behavior of the device in response to these commands is best-effort:
>> +the device may generate notifications more or less frequently than specified.
>> +\end{note}
>> +
>> +If coalescing parameters are being set, the device applies the last coalescing parameters received for a
>> +virtqueue, regardless of the command used to set the parameters. Use the following command sequence
>> +with 2 pairs of virtqueues as an example:
>> +Each of the following commands sets \field{max_usecs} and \field{max_packets} parameters for virtqueues.
>> +\begin{itemize}
>> +\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing parameters for virtqueue0 and virtqueue2, and, virtqueue1 and virtqueue3 retain their previous parameter values.
>> +\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the values from command1.
>> +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 0, the device responds with coalescing parameters of virtqueue0 set by command2.
>> +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its previous values.
>> +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing parameters for virtqueue1 and virtqueue3, and overrides the values set by command4.
>> +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 1, the device responds with coalescing parameters of virtqueue1 set by command5.
>> +\end{itemize}
>> +
>> +Upon disabling and re-enabling the transmit virtqueue, the device will set the coalescing parameters of the virtqueue
>> +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or to 0 if the command did not set any TX coalescing parameters.
>> +
>> +Upon disabling and re-enabling the receive virtqueue, the device will set the coalescing parameters of the virtqueue
>> +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or to 0 if the command did not set any RX coalescing parameters.
>> +
>>   \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 and if the notifications are not suppressed as explained in \ref{sec:Basic Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
>> @@ -1585,11 +1637,25 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types / Network Device / Devi
>>   
>>   \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.
>> +A driver MUST negotiate the VIRTIO_NET_F_NOTF_COAL feature before issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRIO_NET_CTRL_NOTF_COAL_RX_SET.
>> +
>> +A driver MUST negotiate the VIRTIO_NET_F_VQ_NOTF_COAL feature before issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRIO_NET_CTRL_NOTF_COAL_VQ_GET.
>> +
>> +A driver MUST ignore the values of coalescing parameters received from the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if a device responds with VIRTIO_NET_ERR.
>>   
>>   \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing}
>>   
>> -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 MUST ignore \field{reserved}.
>> +
>> +A device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it was not able to change the parameters.
>> +
>> +A device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with VIRTIO_NET_ERR if it was not able to change the parameters.
>> +
>> +A device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if the given virtqueue is disabled.
>> +
>> +After disabling and re-enabling a transmit/receive virtqueue, the device MUST set coalescing parameters of the virtqueue
>> +to those configured using the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET/VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the command
>> +did not configure TX/RX coalescing parameters, to 0.
>>   
>>   A device SHOULD NOT send used buffer notifications to the driver if the notifications are suppressed, even if the notification conditions are met.


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

[virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Heng Qi <hengqi@linux.alibaba.com>
> Sent: Wednesday, March 1, 2023 9:10 AM

> Currently, coalescing parameters are grouped for all transmit and receive
> virtqueues. This patch supports setting or getting the parameters for a specified
> virtqueue, and a typical application of this function is netdim[1].
> 
> When the traffic between virtqueues is unbalanced, for example, one
> virtqueue is busy and another virtqueue is idle, then it will be very useful to
> control coalescing parameters at the virtqueue granularity.
> 

[..]
> +Read/Write attributes for coalescing parameters:
> +\begin{itemize}
> +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
> +      and \field{max_packets} are write-only for a driver.
> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, \field{vqn},
> \field{reserved}, \field{max_usecs}
> +      and \field{max_packets} are write-only for a driver.
> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn}
> and \field{reserved} are write-only
> +      for a driver, and, \field{max_usecs} and \field{max_packets} are read-only
> for the driver.
> +\end{itemize}
> +
Maybe I missed the conversation while I was sick.
I remember we discussed that instead of mentioning each individual field, better to describe the whole structure being read-only or write-only.

> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
>  \begin{enumerate}
> -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{max_usecs} and
> \field{max_packets} parameters for all transmit virtqueues.
> -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{max_usecs} and
> \field{max_packets} parameters for all receive virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure
> virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets}
> parameters for all transmit virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure
> virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets}
> parameters for all receive virtqueues.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure
> virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets}
> parameters
> +                                        for an enabled transmit/receive virtqueue whose
> number is \field{vqn}.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure
> virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets}
> parameters
> +                                        for an enabled transmit/receive virtqueue whose
> number is \field{vqn}.
>  \end{enumerate}
> 
> +\begin{note}
> +The behavior of the device in response to these commands is best-effort:
> +the device may generate notifications more or less frequently than specified.
> +\end{note}
> +
Michael,
Why this should be a note? Shouldn't it be a normative line in the device requirements?

> +If coalescing parameters are being set, the device applies the last
> +coalescing parameters received for a virtqueue, regardless of the
s/received/set to match the if condition wording and regardless sentence after.

> +command used to set the parameters. Use the following command sequence
> with 2 pairs of virtqueues as an example:
> +Each of the following commands sets \field{max_usecs} and
> \field{max_packets} parameters for virtqueues.
> +\begin{itemize}
> +\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing
> parameters for virtqueue0 and virtqueue2, and, virtqueue1 and virtqueue3
> retain their previous parameter values.
> +\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =
> 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the values
> from command1.
> +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =
> 0, the device responds with coalescing parameters of virtqueue0 set by
> command2.
> +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =
> 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its previous
> values.
> +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing
> parameters for virtqueue1 and virtqueue3, and overrides the values set by
> command4.
> +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =
> 1, the device responds with coalescing parameters of virtqueue1 set by
> command5.
> +\end{itemize}
> +
> +Upon disabling and re-enabling the transmit virtqueue, the device will
> +set the coalescing parameters of the virtqueue to those configured through
> the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or to 0 if the command
> did not set any TX coalescing parameters.
> +
> +Upon disabling and re-enabling the receive virtqueue, the device will
> +set the coalescing parameters of the virtqueue to those configured through
> the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or to 0 if the command
> did not set any RX coalescing parameters.
> +
Looks good, however you have well covered in the device normative statements.
So possibly it can be removed from here.

>  \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 and if the notifications are not suppressed as explained in \ref{sec:Basic
> Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
> @@ -1585,11 +1637,25 @@ \subsubsection{Control
> Virtqueue}\label{sec:Device Types / Network Device / Devi
> 
>  \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.
> +A driver MUST negotiate the VIRTIO_NET_F_NOTF_COAL feature before
> issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> VIRIO_NET_CTRL_NOTF_COAL_RX_SET.
> +
> +A driver MUST negotiate the VIRTIO_NET_F_VQ_NOTF_COAL feature before
> issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and
> VIRIO_NET_CTRL_NOTF_COAL_VQ_GET.
> +
> +A driver MUST ignore the values of coalescing parameters received from the
> VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if a device responds with
> VIRTIO_NET_ERR.
> 
>  \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types /
> Network Device / Device Operation / Control Virtqueue / Notifications
> Coalescing}
> 
> -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 MUST ignore \field{reserved}.
> +
> +A device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it
> was not able to change the parameters.
> +
> +A device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
> command with VIRTIO_NET_ERR if it was not able to change the parameters.
> +
> +A device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and
> VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if
> the given virtqueue is disabled.
> +
> +After disabling and re-enabling a transmit/receive virtqueue, the
> +device MUST set coalescing parameters of the virtqueue to those
> +configured using the
> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET/VIRTIO_NET_CTRL_NOTF_COAL_RX_S
> ET command, or, if the command did not configure TX/RX coalescing
> parameters, to 0.
> 
>  A device SHOULD NOT send used buffer notifications to the driver if the
> notifications are suppressed, even if the notification conditions are met.
> 
> --
> 2.19.1.6.gb485710b

Rest looks good to me.


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

[virtio-dev] Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Thu, Mar 02, 2023 at 08:46:03PM +0800, Heng Qi wrote:
> Have we decided yet, I see in Parav's thread it seems to be still under
> discussion? Michael should we be using index?

I don't know at this point. Maybe wait a bit more.

-- 
MST


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

[virtio-dev] Re: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Thu, Mar 02, 2023 at 02:48:44PM +0000, Parav Pandit wrote:
> 
> 
> > From: Heng Qi <hengqi@linux.alibaba.com>
> > Sent: Wednesday, March 1, 2023 9:10 AM
> 
> > Currently, coalescing parameters are grouped for all transmit and receive
> > virtqueues. This patch supports setting or getting the parameters for a specified
> > virtqueue, and a typical application of this function is netdim[1].
> > 
> > When the traffic between virtqueues is unbalanced, for example, one
> > virtqueue is busy and another virtqueue is idle, then it will be very useful to
> > control coalescing parameters at the virtqueue granularity.
> > 
> 
> [..]
> > +Read/Write attributes for coalescing parameters:
> > +\begin{itemize}
> > +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
> > +      and \field{max_packets} are write-only for a driver.
> > +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, \field{vqn},
> > \field{reserved}, \field{max_usecs}
> > +      and \field{max_packets} are write-only for a driver.
> > +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn}
> > and \field{reserved} are write-only
> > +      for a driver, and, \field{max_usecs} and \field{max_packets} are read-only
> > for the driver.
> > +\end{itemize}
> > +
> Maybe I missed the conversation while I was sick.
> I remember we discussed that instead of mentioning each individual field, better to describe the whole structure being read-only or write-only.

we can't, for GET part is RO part is WO.

> > +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> >  \begin{enumerate}
> > -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{max_usecs} and
> > \field{max_packets} parameters for all transmit virtqueues.
> > -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{max_usecs} and
> > \field{max_packets} parameters for all receive virtqueues.
> > +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure
> > virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets}
> > parameters for all transmit virtqueues.
> > +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure
> > virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets}
> > parameters for all receive virtqueues.
> > +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure
> > virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets}
> > parameters
> > +                                        for an enabled transmit/receive virtqueue whose
> > number is \field{vqn}.
> > +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure
> > virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets}
> > parameters
> > +                                        for an enabled transmit/receive virtqueue whose
> > number is \field{vqn}.
> >  \end{enumerate}
> > 
> > +\begin{note}
> > +The behavior of the device in response to these commands is best-effort:
> > +the device may generate notifications more or less frequently than specified.
> > +\end{note}
> > +
> Michael,
> Why this should be a note? Shouldn't it be a normative line in the device requirements?

the second part with
"may" should be upper case and used in a normative section, true.
We can also have a note with the best-effort part, just avoid "may".

> > +If coalescing parameters are being set, the device applies the last
> > +coalescing parameters received for a virtqueue, regardless of the
> s/received/set to match the if condition wording and regardless sentence after.
> 
> > +command used to set the parameters. Use the following command sequence
> > with 2 pairs of virtqueues as an example:
> > +Each of the following commands sets \field{max_usecs} and
> > \field{max_packets} parameters for virtqueues.
> > +\begin{itemize}
> > +\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing
> > parameters for virtqueue0 and virtqueue2, and, virtqueue1 and virtqueue3
> > retain their previous parameter values.
> > +\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =
> > 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the values
> > from command1.
> > +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =
> > 0, the device responds with coalescing parameters of virtqueue0 set by
> > command2.
> > +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =
> > 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its previous
> > values.
> > +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing
> > parameters for virtqueue1 and virtqueue3, and overrides the values set by
> > command4.
> > +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =
> > 1, the device responds with coalescing parameters of virtqueue1 set by
> > command5.
> > +\end{itemize}
> > +
> > +Upon disabling and re-enabling the transmit virtqueue, the device will
> > +set the coalescing parameters of the virtqueue to those configured through
> > the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or to 0 if the command
> > did not set any TX coalescing parameters.
> > +
> > +Upon disabling and re-enabling the receive virtqueue, the device will
> > +set the coalescing parameters of the virtqueue to those configured through
> > the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or to 0 if the command
> > did not set any RX coalescing parameters.
> > +
> Looks good, however you have well covered in the device normative statements.
> So possibly it can be removed from here.

I think it's ok - more readable.

> >  \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 and if the notifications are not suppressed as explained in \ref{sec:Basic
> > Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
> > @@ -1585,11 +1637,25 @@ \subsubsection{Control
> > Virtqueue}\label{sec:Device Types / Network Device / Devi
> > 
> >  \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.
> > +A driver MUST negotiate the VIRTIO_NET_F_NOTF_COAL feature before
> > issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > VIRIO_NET_CTRL_NOTF_COAL_RX_SET.
> > +
> > +A driver MUST negotiate the VIRTIO_NET_F_VQ_NOTF_COAL feature before
> > issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and
> > VIRIO_NET_CTRL_NOTF_COAL_VQ_GET.
> > +
> > +A driver MUST ignore the values of coalescing parameters received from the
> > VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if a device responds with
> > VIRTIO_NET_ERR.
> > 
> >  \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types /
> > Network Device / Device Operation / Control Virtqueue / Notifications
> > Coalescing}
> > 
> > -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 MUST ignore \field{reserved}.
> > +
> > +A device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it
> > was not able to change the parameters.
> > +
> > +A device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
> > command with VIRTIO_NET_ERR if it was not able to change the parameters.
> > +
> > +A device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and
> > VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if
> > the given virtqueue is disabled.
> > +
> > +After disabling and re-enabling a transmit/receive virtqueue, the
> > +device MUST set coalescing parameters of the virtqueue to those
> > +configured using the
> > VIRTIO_NET_CTRL_NOTF_COAL_TX_SET/VIRTIO_NET_CTRL_NOTF_COAL_RX_S
> > ET command, or, if the command did not configure TX/RX coalescing
> > parameters, to 0.
> > 
> >  A device SHOULD NOT send used buffer notifications to the driver if the
> > notifications are suppressed, even if the notification conditions are met.
> > 
> > --
> > 2.19.1.6.gb485710b
> 
> Rest looks good to me.


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

Re: [virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/3/2 下午10:48, Parav Pandit 写道:
>
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Wednesday, March 1, 2023 9:10 AM
>> Currently, coalescing parameters are grouped for all transmit and receive
>> virtqueues. This patch supports setting or getting the parameters for a specified
>> virtqueue, and a typical application of this function is netdim[1].
>>
>> When the traffic between virtqueues is unbalanced, for example, one
>> virtqueue is busy and another virtqueue is idle, then it will be very useful to
>> control coalescing parameters at the virtqueue granularity.
>>
> [..]
>> +Read/Write attributes for coalescing parameters:
>> +\begin{itemize}
>> +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
>> +      and \field{max_packets} are write-only for a driver.
>> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, \field{vqn},
>> \field{reserved}, \field{max_usecs}
>> +      and \field{max_packets} are write-only for a driver.
>> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn}
>> and \field{reserved} are write-only
>> +      for a driver, and, \field{max_usecs} and \field{max_packets} are read-only
>> for the driver.
>> +\end{itemize}
>> +
> Maybe I missed the conversation while I was sick.

I hope you'll be all right very soon.

> I remember we discussed that instead of mentioning each individual field, better to describe the whole structure being read-only or write-only.

Consider the following scenarios:
1. A read-only field of the structure virtio_net_ctrl_coal is extended 
for CTRL_NOTF_COAL_RX/TX_SET to get some extra information
2. \field{reserved} of the structure virtio_net_ctrl_coal_vq is reused 
by CTRL_NOTF_COAL_VQ_SET, and changed from write-only to read-only
These cases still need to unpack the structure to describe it with 
fields, and if we now use field descriptions, we can be satisfied with 
minimal changes.

>> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
>>   \begin{enumerate}
>> -\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{max_usecs} and
>> \field{max_packets} parameters for all transmit virtqueues.
>> -\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{max_usecs} and
>> \field{max_packets} parameters for all receive virtqueues.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: use the structure
>> virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets}
>> parameters for all transmit virtqueues.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: use the structure
>> virtio_net_ctrl_coal to set the \field{max_usecs} and \field{max_packets}
>> parameters for all receive virtqueues.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: use the structure
>> virtio_net_ctrl_coal_vq to set the \field{max_usecs} and \field{max_packets}
>> parameters
>> +                                        for an enabled transmit/receive virtqueue whose
>> number is \field{vqn}.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: use the structure
>> virtio_net_ctrl_coal_vq to get the \field{max_usecs} and \field{max_packets}
>> parameters
>> +                                        for an enabled transmit/receive virtqueue whose
>> number is \field{vqn}.
>>   \end{enumerate}
>>
>> +\begin{note}
>> +The behavior of the device in response to these commands is best-effort:
>> +the device may generate notifications more or less frequently than specified.
>> +\end{note}
>> +
> Michael,
> Why this should be a note? Shouldn't it be a normative line in the device requirements?
>
>> +If coalescing parameters are being set, the device applies the last
>> +coalescing parameters received for a virtqueue, regardless of the
> s/received/set to match the if condition wording and regardless sentence after.

Ok. I'll modify it.

>
>> +command used to set the parameters. Use the following command sequence
>> with 2 pairs of virtqueues as an example:
>> +Each of the following commands sets \field{max_usecs} and
>> \field{max_packets} parameters for virtqueues.
>> +\begin{itemize}
>> +\item Command1: VIRTIO_NET_CTRL_NOTF_COAL_RX_SET sets coalescing
>> parameters for virtqueue0 and virtqueue2, and, virtqueue1 and virtqueue3
>> retain their previous parameter values.
>> +\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =
>> 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the values
>> from command1.
>> +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =
>> 0, the device responds with coalescing parameters of virtqueue0 set by
>> command2.
>> +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =
>> 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its previous
>> values.
>> +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing
>> parameters for virtqueue1 and virtqueue3, and overrides the values set by
>> command4.
>> +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =
>> 1, the device responds with coalescing parameters of virtqueue1 set by
>> command5.
>> +\end{itemize}
>> +
>> +Upon disabling and re-enabling the transmit virtqueue, the device will
>> +set the coalescing parameters of the virtqueue to those configured through
>> the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or to 0 if the command
>> did not set any TX coalescing parameters.
>> +
>> +Upon disabling and re-enabling the receive virtqueue, the device will
>> +set the coalescing parameters of the virtqueue to those configured through
>> the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or to 0 if the command
>> did not set any RX coalescing parameters.
>> +
> Looks good, however you have well covered in the device normative statements.
> So possibly it can be removed from here.

I tend to keep this, as we have done in the past, we can have normative 
descriptions and the corresponding non-normative descriptions.

Thanks.

>
>>   \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 and if the notifications are not suppressed as explained in \ref{sec:Basic
>> Facilities of a Virtio Device / Virtqueues / Used Buffer Notification Suppression}.
>> @@ -1585,11 +1637,25 @@ \subsubsection{Control
>> Virtqueue}\label{sec:Device Types / Network Device / Devi
>>
>>   \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.
>> +A driver MUST negotiate the VIRTIO_NET_F_NOTF_COAL feature before
>> issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> VIRIO_NET_CTRL_NOTF_COAL_RX_SET.
>> +
>> +A driver MUST negotiate the VIRTIO_NET_F_VQ_NOTF_COAL feature before
>> issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and
>> VIRIO_NET_CTRL_NOTF_COAL_VQ_GET.
>> +
>> +A driver MUST ignore the values of coalescing parameters received from the
>> VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if a device responds with
>> VIRTIO_NET_ERR.
>>
>>   \devicenormative{\subparagraph}{Notifications Coalescing}{Device Types /
>> Network Device / Device Operation / Control Virtqueue / Notifications
>> Coalescing}
>>
>> -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 MUST ignore \field{reserved}.
>> +
>> +A device SHOULD respond to VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands with VIRTIO_NET_ERR if it
>> was not able to change the parameters.
>> +
>> +A device MUST respond to the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
>> command with VIRTIO_NET_ERR if it was not able to change the parameters.
>> +
>> +A device MUST respond to VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and
>> VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if
>> the given virtqueue is disabled.
>> +
>> +After disabling and re-enabling a transmit/receive virtqueue, the
>> +device MUST set coalescing parameters of the virtqueue to those
>> +configured using the
>> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET/VIRTIO_NET_CTRL_NOTF_COAL_RX_S
>> ET command, or, if the command did not configure TX/RX coalescing
>> parameters, to 0.
>>
>>   A device SHOULD NOT send used buffer notifications to the driver if the
>> notifications are suppressed, even if the notification conditions are met.
>>
>> --
>> 2.19.1.6.gb485710b
> Rest looks good to me.
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org


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

Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
> > +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
> 
> Should this now be "whose index is \field{vqn}"?

Ugh.  I guess we'll have to fix the number/index mess in the spec
first. Parav you said you are looking into it?

-- 
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/

[virtio-dev] Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
> > +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
> 
> Should this now be "whose index is \field{vqn}"?

Ugh.  I guess we'll have to fix the number/index mess in the spec
first. Parav you said you are looking into it?

-- 
MST


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

[virtio-comment] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, March 2, 2023 6:35 PM
> > > +\begin{itemize}
> > > +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
> > > +      and \field{max_packets} are write-only for a driver.
> > > +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET,
> > > +\field{vqn},
> > > \field{reserved}, \field{max_usecs}
> > > +      and \field{max_packets} are write-only for a driver.
> > > +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
> \field{vqn}
> > > and \field{reserved} are write-only
> > > +      for a driver, and, \field{max_usecs} and \field{max_packets}
> > > + are read-only
> > > for the driver.
> > > +\end{itemize}
> > > +
> > Maybe I missed the conversation while I was sick.
> > I remember we discussed that instead of mentioning each individual field,
> better to describe the whole structure being read-only or write-only.
> 
> we can't, for GET part is RO part is WO.
> 
I am talking about the struct virtio_net_ctrl_coal.
This is RO for GET and WO for SET.
The text above can say struct virtio_net_ctrl_coal is RO or WO without dissecting its individual fields.

> > > +\begin{note}
> > > +The behavior of the device in response to these commands is best-effort:
> > > +the device may generate notifications more or less frequently than
> specified.
> > > +\end{note}
> > > +
> > Michael,
> > Why this should be a note? Shouldn't it be a normative line in the device
> requirements?
> 
> the second part with
> "may" should be upper case and used in a normative section, true.
> We can also have a note with the best-effort part, just avoid "may".
> 
Ok. In that case, just the normative line is good enough, as it avoids duplication.

Sorry for the late response, as returning back from travel and sickness.

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/

[virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Thursday, March 2, 2023 6:35 PM
> > > +\begin{itemize}
> > > +\item For commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
> > > VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, \field{max_usecs}
> > > +      and \field{max_packets} are write-only for a driver.
> > > +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET,
> > > +\field{vqn},
> > > \field{reserved}, \field{max_usecs}
> > > +      and \field{max_packets} are write-only for a driver.
> > > +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET,
> \field{vqn}
> > > and \field{reserved} are write-only
> > > +      for a driver, and, \field{max_usecs} and \field{max_packets}
> > > + are read-only
> > > for the driver.
> > > +\end{itemize}
> > > +
> > Maybe I missed the conversation while I was sick.
> > I remember we discussed that instead of mentioning each individual field,
> better to describe the whole structure being read-only or write-only.
> 
> we can't, for GET part is RO part is WO.
> 
I am talking about the struct virtio_net_ctrl_coal.
This is RO for GET and WO for SET.
The text above can say struct virtio_net_ctrl_coal is RO or WO without dissecting its individual fields.

> > > +\begin{note}
> > > +The behavior of the device in response to these commands is best-effort:
> > > +the device may generate notifications more or less frequently than
> specified.
> > > +\end{note}
> > > +
> > Michael,
> > Why this should be a note? Shouldn't it be a normative line in the device
> requirements?
> 
> the second part with
> "may" should be upper case and used in a normative section, true.
> We can also have a note with the best-effort part, just avoid "may".
> 
Ok. In that case, just the normative line is good enough, as it avoids duplication.

Sorry for the late response, as returning back from travel and sickness.

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

Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/3/7 上午6:57, Michael S. Tsirkin 写道:
> On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
>>> +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
>> Should this now be "whose index is \field{vqn}"?
> Ugh.  I guess we'll have to fix the number/index mess in the spec
> first. Parav you said you are looking into it?



# Where virtqueue number and virtqueue index are used.
   1. In the Virtqueue Configuration Section, use the virtqueue index: 
"Write the **virtqueue index** (first queue is 0) to queue_select."
   2. Both descriptions are used separately in the Notification Section.
       2.1 Here vqn is called virtqueue index:
             "When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, 
the driver sends an available buffer notification
               to the device by writing the **16-bit virtqueue index** 
of this virtqueue to the Queue Notify address.
               ...

               le32 {
                      vqn: 16;
                      next_off : 15;
                      next_wrap: 1;
              };
              ...

              If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the 
driver MUST use the queue_notify_data value instead of the **virtqueue 
index**."

       2.2 Here vqn is called virtqueue number:
             "When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, 
the Notification data contains the **Virtqueue number**.
             ...

             be32 {
                     vqn: 16;
                     next_off : 15;
                     next_wrap: 1;
              };
             ...

             When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, 
this notification involves sending the **virtqueue number** to the 
device (method depending on the transport).
             ...

             vqn -- **VQ number** to be notified."

# 0-based index and 0-based number are used respectively in the RSS Section:
1. "Field unclassified_queue contains the **0-based index** of the 
receive virtqueue to place unclassified packets in. Index 0 corresponds 
to receiveq1."
2. "use the result as the index in the indirection table to get 
**0-based number** of destination receiveq (value of 0 corresponds to 
receiveq1)."



\field{vqn} has been called '0-based virtqueue index' or '0-based 
virtqueue number',
I think both seem to be friendly to readers, so what are your options?

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/

[virtio-dev] Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/3/7 上午6:57, Michael S. Tsirkin 写道:
> On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
>>> +                                        for an enabled transmit/receive virtqueue whose number is \field{vqn}.
>> Should this now be "whose index is \field{vqn}"?
> Ugh.  I guess we'll have to fix the number/index mess in the spec
> first. Parav you said you are looking into it?



# Where virtqueue number and virtqueue index are used.
   1. In the Virtqueue Configuration Section, use the virtqueue index: 
"Write the **virtqueue index** (first queue is 0) to queue_select."
   2. Both descriptions are used separately in the Notification Section.
       2.1 Here vqn is called virtqueue index:
             "When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, 
the driver sends an available buffer notification
               to the device by writing the **16-bit virtqueue index** 
of this virtqueue to the Queue Notify address.
               ...

               le32 {
                      vqn: 16;
                      next_off : 15;
                      next_wrap: 1;
              };
              ...

              If VIRTIO_F_NOTIFICATION_DATA has not been negotiated, the 
driver MUST use the queue_notify_data value instead of the **virtqueue 
index**."

       2.2 Here vqn is called virtqueue number:
             "When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, 
the Notification data contains the **Virtqueue number**.
             ...

             be32 {
                     vqn: 16;
                     next_off : 15;
                     next_wrap: 1;
              };
             ...

             When VIRTIO_F_NOTIFICATION_DATA has not been negotiated, 
this notification involves sending the **virtqueue number** to the 
device (method depending on the transport).
             ...

             vqn -- **VQ number** to be notified."

# 0-based index and 0-based number are used respectively in the RSS Section:
1. "Field unclassified_queue contains the **0-based index** of the 
receive virtqueue to place unclassified packets in. Index 0 corresponds 
to receiveq1."
2. "use the result as the index in the indirection table to get 
**0-based number** of destination receiveq (value of 0 corresponds to 
receiveq1)."



\field{vqn} has been called '0-based virtqueue index' or '0-based 
virtqueue number',
I think both seem to be friendly to readers, so what are your options?

Thanks.

>


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

RE: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, March 6, 2023 5:57 PM
> 
> On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
> > > +                                        for an enabled transmit/receive virtqueue whose
> number is \field{vqn}.
> >
> > Should this now be "whose index is \field{vqn}"?
> 
> Ugh.  I guess we'll have to fix the number/index mess in the spec first. Parav
> you said you are looking into it?

Yes, I want to send v1 for " Rename queue index to queue number" series.
V1 will include,
 
a. the vqn changes for net device rss and 
b. other ccw changes that Cornelia requested.
c. extra note that you asked to add to mmio for referring the old naming convention

I guess we agree that it should be vqn.
Therefore, vq coalescing and aq series should progress with vqn terminology.
This way all 3 series can progress in parallel (aq, vq coalescing, current spec cleanup).

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/

[virtio-dev] RE: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Monday, March 6, 2023 5:57 PM
> 
> On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
> > > +                                        for an enabled transmit/receive virtqueue whose
> number is \field{vqn}.
> >
> > Should this now be "whose index is \field{vqn}"?
> 
> Ugh.  I guess we'll have to fix the number/index mess in the spec first. Parav
> you said you are looking into it?

Yes, I want to send v1 for " Rename queue index to queue number" series.
V1 will include,
 
a. the vqn changes for net device rss and 
b. other ccw changes that Cornelia requested.
c. extra note that you asked to add to mmio for referring the old naming convention

I guess we agree that it should be vqn.
Therefore, vq coalescing and aq series should progress with vqn terminology.
This way all 3 series can progress in parallel (aq, vq coalescing, current spec cleanup).

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

Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Wed, Mar 08, 2023 at 04:42:25PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, March 6, 2023 5:57 PM
> > 
> > On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
> > > > +                                        for an enabled transmit/receive virtqueue whose
> > number is \field{vqn}.
> > >
> > > Should this now be "whose index is \field{vqn}"?
> > 
> > Ugh.  I guess we'll have to fix the number/index mess in the spec first. Parav
> > you said you are looking into it?
> 
> Yes, I want to send v1 for " Rename queue index to queue number" series.
> V1 will include,
>  
> a. the vqn changes for net device rss and 
> b. other ccw changes that Cornelia requested.
> c. extra note that you asked to add to mmio for referring the old naming convention
> 
> I guess we agree that it should be vqn.
> Therefore, vq coalescing and aq series should progress with vqn terminology.
> This way all 3 series can progress in parallel (aq, vq coalescing, current spec cleanup).

Don't much care at this point but I think when I thought about this
deeply it seemed that yes, it is marginally better. Don't remember why
though ;)

-- 
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/

[virtio-dev] Re: [virtio-comment] [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Wed, Mar 08, 2023 at 04:42:25PM +0000, Parav Pandit wrote:
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Monday, March 6, 2023 5:57 PM
> > 
> > On Thu, Mar 02, 2023 at 11:36:18AM +0000, David Edmondson wrote:
> > > > +                                        for an enabled transmit/receive virtqueue whose
> > number is \field{vqn}.
> > >
> > > Should this now be "whose index is \field{vqn}"?
> > 
> > Ugh.  I guess we'll have to fix the number/index mess in the spec first. Parav
> > you said you are looking into it?
> 
> Yes, I want to send v1 for " Rename queue index to queue number" series.
> V1 will include,
>  
> a. the vqn changes for net device rss and 
> b. other ccw changes that Cornelia requested.
> c. extra note that you asked to add to mmio for referring the old naming convention
> 
> I guess we agree that it should be vqn.
> Therefore, vq coalescing and aq series should progress with vqn terminology.
> This way all 3 series can progress in parallel (aq, vq coalescing, current spec cleanup).

Don't much care at this point but I think when I thought about this
deeply it seemed that yes, it is marginally better. Don't remember why
though ;)

-- 
MST


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

[virtio-comment] RE: [virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
> Behalf Of Heng Qi
> Sent: Thursday, March 2, 2023 10:27 PM
> 
> > I remember we discussed that instead of mentioning each individual field,
> better to describe the whole structure being read-only or write-only.
> 
> Consider the following scenarios:
> 1. A read-only field of the structure virtio_net_ctrl_coal is extended for
> CTRL_NOTF_COAL_RX/TX_SET to get some extra information 
A set command cannot extend the struct virtio_net_ctrl_coal, particularly for read-only and partially for write-only.
This would mean that for the tiny number of bytes, an additional dma descriptor is to be allocated with read/write-only permissions.
It would be inefficient for the driver to do so for the SET command to have vqn as write-only, reserved as read-only, rest fields as write-only dma attributes.

As I think more, I think the whole set command fields should be read-only for device. Better to describe it this way instead of splitting individual fields.
This way driver can just do a single DMA allocation with read-only attributes for set cmd.

Get command doesn’t have any choice today the way CVQ is structured to it lives with the limitation.

> > Looks good, however you have well covered in the device normative
> statements.
> > So possibly it can be removed from here.
> 
> I tend to keep this, as we have done in the past, we can have normative
> descriptions and the corresponding non-normative descriptions.
> 
Ok. but please revisit if the description can be simpler text than the normative lines.

RE: [virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
> Behalf Of Heng Qi
> Sent: Thursday, March 2, 2023 10:27 PM
> 
> > I remember we discussed that instead of mentioning each individual field,
> better to describe the whole structure being read-only or write-only.
> 
> Consider the following scenarios:
> 1. A read-only field of the structure virtio_net_ctrl_coal is extended for
> CTRL_NOTF_COAL_RX/TX_SET to get some extra information 
A set command cannot extend the struct virtio_net_ctrl_coal, particularly for read-only and partially for write-only.
This would mean that for the tiny number of bytes, an additional dma descriptor is to be allocated with read/write-only permissions.
It would be inefficient for the driver to do so for the SET command to have vqn as write-only, reserved as read-only, rest fields as write-only dma attributes.

As I think more, I think the whole set command fields should be read-only for device. Better to describe it this way instead of splitting individual fields.
This way driver can just do a single DMA allocation with read-only attributes for set cmd.

Get command doesn’t have any choice today the way CVQ is structured to it lives with the limitation.

> > Looks good, however you have well covered in the device normative
> statements.
> > So possibly it can be removed from here.
> 
> I tend to keep this, as we have done in the past, we can have normative
> descriptions and the corresponding non-normative descriptions.
> 
Ok. but please revisit if the description can be simpler text than the normative lines.

Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/3/9 上午6:30, Parav Pandit 写道:
>> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
>> Behalf Of Heng Qi
>> Sent: Thursday, March 2, 2023 10:27 PM
>>
>>> I remember we discussed that instead of mentioning each individual field,
>> better to describe the whole structure being read-only or write-only.
>>
>> Consider the following scenarios:
>> 1. A read-only field of the structure virtio_net_ctrl_coal is extended for
>> CTRL_NOTF_COAL_RX/TX_SET to get some extra information
> A set command cannot extend the struct virtio_net_ctrl_coal, particularly for read-only and partially for write-only.
> This would mean that for the tiny number of bytes, an additional dma descriptor is to be allocated with read/write-only permissions.
> It would be inefficient for the driver to do so for the SET command to have vqn as write-only, reserved as read-only, rest fields as write-only dma attributes.
>
> As I think more, I think the whole set command fields should be read-only for device. Better to describe it this way instead of splitting individual fields.
> This way driver can just do a single DMA allocation with read-only attributes for set cmd.
>
> Get command doesn’t have any choice today the way CVQ is structured to it lives with the limitation.

I think it is reasonable and will be revised in the next version.

>
>>> Looks good, however you have well covered in the device normative
>> statements.
>>> So possibly it can be removed from here.
>> I tend to keep this, as we have done in the past, we can have normative
>> descriptions and the corresponding non-normative descriptions.
>>
> Ok. but please revisit if the description can be simpler text than the normative lines.

Ok.

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/

[virtio-dev] Re: [virtio-comment] RE: [virtio-dev] RE: [PATCH v10] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/3/9 上午6:30, Parav Pandit 写道:
>> From: virtio-dev@lists.oasis-open.org <virtio-dev@lists.oasis-open.org> On
>> Behalf Of Heng Qi
>> Sent: Thursday, March 2, 2023 10:27 PM
>>
>>> I remember we discussed that instead of mentioning each individual field,
>> better to describe the whole structure being read-only or write-only.
>>
>> Consider the following scenarios:
>> 1. A read-only field of the structure virtio_net_ctrl_coal is extended for
>> CTRL_NOTF_COAL_RX/TX_SET to get some extra information
> A set command cannot extend the struct virtio_net_ctrl_coal, particularly for read-only and partially for write-only.
> This would mean that for the tiny number of bytes, an additional dma descriptor is to be allocated with read/write-only permissions.
> It would be inefficient for the driver to do so for the SET command to have vqn as write-only, reserved as read-only, rest fields as write-only dma attributes.
>
> As I think more, I think the whole set command fields should be read-only for device. Better to describe it this way instead of splitting individual fields.
> This way driver can just do a single DMA allocation with read-only attributes for set cmd.
>
> Get command doesn’t have any choice today the way CVQ is structured to it lives with the limitation.

I think it is reasonable and will be revised in the next version.

>
>>> Looks good, however you have well covered in the device normative
>> statements.
>>> So possibly it can be removed from here.
>> I tend to keep this, as we have done in the past, we can have normative
>> descriptions and the corresponding non-normative descriptions.
>>
> Ok. but please revisit if the description can be simpler text than the normative lines.

Ok.

Thanks.



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