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

[virtio-dev] [PATCH v14] 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>
Reviewed-by: Parav Pandit <parav@nvidia.com>
---
v13->v14:
       1. Modifications to sentence structure descriptions.

v12->v13:
       1. Added normative statement that driver must not send invalid \field{vqn}. @Cornelia Huck
       2. Some minor modifications. @Cornelia Huck

v11->v12:
       1. Use virtqueue having vqn N instead of virtqueueN. @Parav Pandit
       2. Some minor modifications. @Parav Pandit, @Michael S . Tsirkin
       3. Add Parav's Reviewed-by tag (Thanks!).

v10->v11:
       1. Describe read and write attributes for the structure. @Parav Pandit
       2. Some minor modifications. @Parav Pandit

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 | 83 ++++++++++++++++++++++++++++----
 1 file changed, 73 insertions(+), 10 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index e71e33b..ee48a57 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}
@@ -1506,12 +1509,12 @@ \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.
+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, the driver can
+send commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
+for virtqueue notification coalescing.
 
 \begin{lstlisting}
 struct virtio_net_ctrl_coal {
@@ -1519,25 +1522,63 @@ \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 the 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, the structure virtio_net_ctrl_coal is write-only for the driver.
+\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, the structure virtio_net_ctrl_coal_vq is write-only for the driver.
+\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn} and \field{reserved} are write-only
+      for the driver, and the structure virtio_net_ctrl_coal is read-only for the driver.
+\end{itemize}
+
+The class VIRTIO_NET_CTRL_NOTF_COAL has the following 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}
 
+The device may generate notifications more or less frequently than specified by set commands of the VIRTIO_NET_CTRL_NOTF_COAL class.
+
+If coalescing parameters are being set, the device applies the last coalescing parameters set for a
+virtqueue, regardless of the command used to set the parameters. Use the following command sequence
+with two 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 virtqueues having vqn 0 and vqn 2. Virtqueues having vqn 1 and vqn 3 retain their previous parameters.
+\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 0 sets coalescing parameters for virtqueue having vqn 0. Virtqueue having vqn 2 retains the parameters from command1.
+\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 0, the device responds with coalescing parameters of vqn 0 set by command2.
+\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 1 sets coalescing parameters for virtqueue having vqn 1. Virtqueue having vqn 3 retains its previous parameters.
+\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing parameters for virtqueues having vqn 1 and vqn 3, and overrides the parameters set by command4.
+\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 1, the device responds with coalescing parameters of vqn 1 set by command5.
+\end{itemize}
+
 \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 +1626,33 @@ \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.
+The driver MUST set \field{vqn} to the virtqueue number of an enabled transmit or receive virtqueue.
+
+The driver MUST have negotiated the VIRTIO_NET_F_NOTF_COAL feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET.
+
+The driver MUST have negotiated the VIRTIO_NET_F_VQ_NOTF_COAL feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET.
+
+The driver MUST ignore the values of coalescing parameters received from the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if the 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.
+The device MUST ignore \field{reserved}.
+
+The 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.
+
+The 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.
+
+The 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 designated virtqueue is not an enabled transmit or receive virtqueue.
+
+Upon disabling and re-enabling a transmit virtqueue, the device MUST set the coalescing parameters of the virtqueue
+to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or, if the driver did not set any TX coalescing parameters, to 0.
+
+Upon disabling and re-enabling a receive virtqueue, the device MUST set the coalescing parameters of the virtqueue
+to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set any RX coalescing parameters, to 0.
+
+The behavior of the device in response to set commands of the VIRTIO_NET_CTRL_NOTF_COAL class is best-effort:
+the device MAY generate notifications more or less frequently than specified.
 
 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

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

[Heng Qi]

Hi all! If after reviewing it you feel it meets the necessary 
requirements, I kindly request a vote.

Thanks a lot!

Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166

在 2023/3/23 下午11:24, 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>
> Reviewed-by: Parav Pandit <parav@nvidia.com>
> ---
> v13->v14:
>         1. Modifications to sentence structure descriptions.
>
> v12->v13:
>         1. Added normative statement that driver must not send invalid \field{vqn}. @Cornelia Huck
>         2. Some minor modifications. @Cornelia Huck
>
> v11->v12:
>         1. Use virtqueue having vqn N instead of virtqueueN. @Parav Pandit
>         2. Some minor modifications. @Parav Pandit, @Michael S . Tsirkin
>         3. Add Parav's Reviewed-by tag (Thanks!).
>
> v10->v11:
>         1. Describe read and write attributes for the structure. @Parav Pandit
>         2. Some minor modifications. @Parav Pandit
>
> 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 | 83 ++++++++++++++++++++++++++++----
>   1 file changed, 73 insertions(+), 10 deletions(-)
>
> diff --git a/device-types/net/description.tex b/device-types/net/description.tex
> index e71e33b..ee48a57 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}
> @@ -1506,12 +1509,12 @@ \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.
> +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, the driver can
> +send commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET
> +for virtqueue notification coalescing.
>   
>   \begin{lstlisting}
>   struct virtio_net_ctrl_coal {
> @@ -1519,25 +1522,63 @@ \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 the 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, the structure virtio_net_ctrl_coal is write-only for the driver.
> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, the structure virtio_net_ctrl_coal_vq is write-only for the driver.
> +\item For the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET, \field{vqn} and \field{reserved} are write-only
> +      for the driver, and the structure virtio_net_ctrl_coal is read-only for the driver.
> +\end{itemize}
> +
> +The class VIRTIO_NET_CTRL_NOTF_COAL has the following 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}
>   
> +The device may generate notifications more or less frequently than specified by set commands of the VIRTIO_NET_CTRL_NOTF_COAL class.
> +
> +If coalescing parameters are being set, the device applies the last coalescing parameters set for a
> +virtqueue, regardless of the command used to set the parameters. Use the following command sequence
> +with two 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 virtqueues having vqn 0 and vqn 2. Virtqueues having vqn 1 and vqn 3 retain their previous parameters.
> +\item Command2: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 0 sets coalescing parameters for virtqueue having vqn 0. Virtqueue having vqn 2 retains the parameters from command1.
> +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 0, the device responds with coalescing parameters of vqn 0 set by command2.
> +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} = 1 sets coalescing parameters for virtqueue having vqn 1. Virtqueue having vqn 3 retains its previous parameters.
> +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing parameters for virtqueues having vqn 1 and vqn 3, and overrides the parameters set by command4.
> +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} = 1, the device responds with coalescing parameters of vqn 1 set by command5.
> +\end{itemize}
> +
>   \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 +1626,33 @@ \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.
> +The driver MUST set \field{vqn} to the virtqueue number of an enabled transmit or receive virtqueue.
> +
> +The driver MUST have negotiated the VIRTIO_NET_F_NOTF_COAL feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRTIO_NET_CTRL_NOTF_COAL_RX_SET.
> +
> +The driver MUST have negotiated the VIRTIO_NET_F_VQ_NOTF_COAL feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET.
> +
> +The driver MUST ignore the values of coalescing parameters received from the VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command if the 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.
> +The device MUST ignore \field{reserved}.
> +
> +The 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.
> +
> +The 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.
> +
> +The 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 designated virtqueue is not an enabled transmit or receive virtqueue.
> +
> +Upon disabling and re-enabling a transmit virtqueue, the device MUST set the coalescing parameters of the virtqueue
> +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command, or, if the driver did not set any TX coalescing parameters, to 0.
> +
> +Upon disabling and re-enabling a receive virtqueue, the device MUST set the coalescing parameters of the virtqueue
> +to those configured through the VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command, or, if the driver did not set any RX coalescing parameters, to 0.
> +
> +The behavior of the device in response to set commands of the VIRTIO_NET_CTRL_NOTF_COAL class is best-effort:
> +the device MAY generate notifications more or less frequently than specified.
>   
>   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

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

[Halil Pasic]

On Thu, 23 Mar 2023 23:24:22 +0800
Heng Qi <hengqi@linux.alibaba.com> wrote:

> +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.

Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
negotiated, and queue_select != queue_notify_data, is vqn supposed
to contain queue_notify_data or the number/index that is used for
queue_select (I'm talking about the PCI transport case)?

Regards,
Halil

---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Halil Pasic <pasic@linux.ibm.com>
> Sent: Tuesday, April 4, 2023 12:29 PM
> 
> On Thu, 23 Mar 2023 23:24:22 +0800
> Heng Qi <hengqi@linux.alibaba.com> wrote:
> 
> > +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.
> 
> Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> negotiated, and queue_select != queue_notify_data, is vqn supposed to contain
> queue_notify_data or the number/index that is used for queue_select (I'm
> talking about the PCI transport case)?
Vqn has zero relation to notification config data feature and featue bit.
It is the real vqn enabled via queue_select.

Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Tue, Apr 04, 2023 at 04:32:07PM +0000, Parav Pandit wrote:
> 
> > From: Halil Pasic <pasic@linux.ibm.com>
> > Sent: Tuesday, April 4, 2023 12:29 PM
> > 
> > On Thu, 23 Mar 2023 23:24:22 +0800
> > Heng Qi <hengqi@linux.alibaba.com> wrote:
> > 
> > > +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.
> > 
> > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > negotiated, and queue_select != queue_notify_data, is vqn supposed to contain
> > queue_notify_data or the number/index that is used for queue_select (I'm
> > talking about the PCI transport case)?
> Vqn has zero relation to notification config data feature and featue bit.
> It is the real vqn enabled via queue_select.
> 
> Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.

vqn here is the index. queue_select is also the index.

Inside notifications-le.c we have:
le32 {
        vqn : 16;
        next_off : 15;
        next_wrap : 1;
};

vqn here is queue_notify_data.


-- 
MST


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
> On Tue, Apr 04, 2023 at 04:32:07PM +0000, Parav Pandit wrote:
>>
>>> From: Halil Pasic <pasic@linux.ibm.com>
>>> Sent: Tuesday, April 4, 2023 12:29 PM
>>>
>>> On Thu, 23 Mar 2023 23:24:22 +0800
>>> Heng Qi <hengqi@linux.alibaba.com> wrote:
>>>
>>>> +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.
>>>
>>> Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
>>> negotiated, and queue_select != queue_notify_data, is vqn supposed to contain
>>> queue_notify_data or the number/index that is used for queue_select (I'm
>>> talking about the PCI transport case)?
>> Vqn has zero relation to notification config data feature and featue bit.
>> It is the real vqn enabled via queue_select.
>>
>> Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.
> 
> vqn here is the index. queue_select is also the index.
> 
Yes to both. No plan to rename them.

> Inside notifications-le.c we have:
> le32 {
>          vqn : 16;
>          next_off : 15;
>          next_wrap : 1;
> };
> 
> vqn here is queue_notify_data.
> 
vqn in above unnamed structure is contain
a. either vq index if CONFIG_DATA is not negotiated
or
b. it contains queue_notifiy_data if CONFIG_DATA is negotiated

Therefore, instead of naming it as vqn, renaming it to vq_notify_id 
crisply describe what it is for.

And not some vqn n stands for notification, but "d" of data is dropped 
somehow.

A notification identifier contains depending on negotiated feature bit.

> 

---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Tue, Apr 04, 2023 at 03:39:21PM -0400, Parav Pandit wrote:
> 
> 
> On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
> > On Tue, Apr 04, 2023 at 04:32:07PM +0000, Parav Pandit wrote:
> > > 
> > > > From: Halil Pasic <pasic@linux.ibm.com>
> > > > Sent: Tuesday, April 4, 2023 12:29 PM
> > > > 
> > > > On Thu, 23 Mar 2023 23:24:22 +0800
> > > > Heng Qi <hengqi@linux.alibaba.com> wrote:
> > > > 
> > > > > +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.
> > > > 
> > > > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > > > negotiated, and queue_select != queue_notify_data, is vqn supposed to contain
> > > > queue_notify_data or the number/index that is used for queue_select (I'm
> > > > talking about the PCI transport case)?
> > > Vqn has zero relation to notification config data feature and featue bit.
> > > It is the real vqn enabled via queue_select.
> > > 
> > > Once the vqn is renamed to vq_notify_id, we won't have this confusion anymore.
> > 
> > vqn here is the index. queue_select is also the index.
> > 
> Yes to both. No plan to rename them.
> 
> > Inside notifications-le.c we have:
> > le32 {
> >          vqn : 16;
> >          next_off : 15;
> >          next_wrap : 1;
> > };
> > 
> > vqn here is queue_notify_data.
> > 
> vqn in above unnamed structure is contain
> a. either vq index if CONFIG_DATA is not negotiated
> or
> b. it contains queue_notifiy_data if CONFIG_DATA is negotiated
> 
> Therefore, instead of naming it as vqn, renaming it to vq_notify_id crisply
> describe what it is for.
>
> And not some vqn n stands for notification, but "d" of data is dropped
> somehow.

vqnd then? for virt queue notification data?


> A notification identifier contains depending on negotiated feature bit.

it's not necessarily an identifier. can be e.g. just 0 for all vqs.
whatever the device needs.

> > 


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Parav Pandit]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Tuesday, April 4, 2023 4:14 PM
> 
> On Tue, Apr 04, 2023 at 03:39:21PM -0400, Parav Pandit wrote:
> >
> >
> > On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
> > > On Tue, Apr 04, 2023 at 04:32:07PM +0000, Parav Pandit wrote:
> > > >
> > > > > From: Halil Pasic <pasic@linux.ibm.com>
> > > > > Sent: Tuesday, April 4, 2023 12:29 PM
> > > > >
> > > > > On Thu, 23 Mar 2023 23:24:22 +0800 Heng Qi
> > > > > <hengqi@linux.alibaba.com> wrote:
> > > > >
> > > > > > +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.
> > > > >
> > > > > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > > > > negotiated, and queue_select != queue_notify_data, is vqn
> > > > > supposed to contain queue_notify_data or the number/index that
> > > > > is used for queue_select (I'm talking about the PCI transport case)?
> > > > Vqn has zero relation to notification config data feature and featue bit.
> > > > It is the real vqn enabled via queue_select.
> > > >
> > > > Once the vqn is renamed to vq_notify_id, we won't have this confusion
> anymore.
> > >
> > > vqn here is the index. queue_select is also the index.
> > >
> > Yes to both. No plan to rename them.
> >
> > > Inside notifications-le.c we have:
> > > le32 {
> > >          vqn : 16;
> > >          next_off : 15;
> > >          next_wrap : 1;
> > > };
> > >
> > > vqn here is queue_notify_data.
> > >
> > vqn in above unnamed structure is contain a. either vq index if
> > CONFIG_DATA is not negotiated or b. it contains queue_notifiy_data if
> > CONFIG_DATA is negotiated
> >
> > Therefore, instead of naming it as vqn, renaming it to vq_notify_id
> > crisply describe what it is for.
> >
> > And not some vqn n stands for notification, but "d" of data is dropped
> > somehow.
> 
> vqnd then? for virt queue notification data?
> 
"id" identifies the queue as opposed to "data" identifying a queue.

> 
> > A notification identifier contains depending on negotiated feature bit.
> 
> it's not necessarily an identifier. can be e.g. just 0 for all vqs.
> whatever the device needs.
For driver its just an id, content doesn't matter.

---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Tue, Apr 04, 2023 at 08:50:43PM +0000, Parav Pandit wrote:
> 
> 
> > From: Michael S. Tsirkin <mst@redhat.com>
> > Sent: Tuesday, April 4, 2023 4:14 PM
> > 
> > On Tue, Apr 04, 2023 at 03:39:21PM -0400, Parav Pandit wrote:
> > >
> > >
> > > On 4/4/2023 1:44 PM, Michael S. Tsirkin wrote:
> > > > On Tue, Apr 04, 2023 at 04:32:07PM +0000, Parav Pandit wrote:
> > > > >
> > > > > > From: Halil Pasic <pasic@linux.ibm.com>
> > > > > > Sent: Tuesday, April 4, 2023 12:29 PM
> > > > > >
> > > > > > On Thu, 23 Mar 2023 23:24:22 +0800 Heng Qi
> > > > > > <hengqi@linux.alibaba.com> wrote:
> > > > > >
> > > > > > > +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.
> > > > > >
> > > > > > Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> > > > > > negotiated, and queue_select != queue_notify_data, is vqn
> > > > > > supposed to contain queue_notify_data or the number/index that
> > > > > > is used for queue_select (I'm talking about the PCI transport case)?
> > > > > Vqn has zero relation to notification config data feature and featue bit.
> > > > > It is the real vqn enabled via queue_select.
> > > > >
> > > > > Once the vqn is renamed to vq_notify_id, we won't have this confusion
> > anymore.
> > > >
> > > > vqn here is the index. queue_select is also the index.
> > > >
> > > Yes to both. No plan to rename them.
> > >
> > > > Inside notifications-le.c we have:
> > > > le32 {
> > > >          vqn : 16;
> > > >          next_off : 15;
> > > >          next_wrap : 1;
> > > > };
> > > >
> > > > vqn here is queue_notify_data.
> > > >
> > > vqn in above unnamed structure is contain a. either vq index if
> > > CONFIG_DATA is not negotiated or b. it contains queue_notifiy_data if
> > > CONFIG_DATA is negotiated
> > >
> > > Therefore, instead of naming it as vqn, renaming it to vq_notify_id
> > > crisply describe what it is for.
> > >
> > > And not some vqn n stands for notification, but "d" of data is dropped
> > > somehow.
> > 
> > vqnd then? for virt queue notification data?
> > 
> "id" identifies the queue as opposed to "data" identifying a queue.
> > 
> > > A notification identifier contains depending on negotiated feature bit.
> > 
> > it's not necessarily an identifier. can be e.g. just 0 for all vqs.
> > whatever the device needs.
> For driver its just an id, content doesn't matter.

No, this value might or might not be somehow related to the vq but it
does not have to identify it. So it's some data, not an id.  Let's try
to use words for what they mean in english not try to come up with our
own language.

-- 
MST


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Halil Pasic]

On Wed, 5 Apr 2023 05:12:12 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> > > it's not necessarily an identifier. can be e.g. just 0 for all vqs.
> > > whatever the device needs.  
> > For driver its just an id, content doesn't matter.  
> 
> No, this value might or might not be somehow related to the vq but it
> does not have to identify it. So it's some data, not an id.  Let's try
> to use words for what they mean in english not try to come up with our
> own language.

I agree with Michael wholeheartedly, we should try to use words for what
they mean in English, especially in Computer Science English, and even
try to pick the most fitting and least ambiguous option if multiple
options are possible.

In that spirit I would say that "queue_notify_data" is actually a magic
cookie. https://en.wikipedia.org/wiki/Magic_cookie A possible name
or abbreviation would be vqn_cookie like "a virtqueue
notification cookie" (I'm not sure about mixing in the direction, but
this is only about driver -> device notifications). 


And then 

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

could become something like

struct vq_notif_data {
        union {
		le16 vqn_cookie;
		le16 vq_index;
	};
        le16 {
		next_off : 15;
        	next_wrap : 1;
	};
};

BTW since "identifier" and "unique identifier" are not the same, in my
opinion "identifier" is still viable, if we explain that it is called
identifier because the idea behind that field is to be used to identify
the queue, but that there is in fact no requirement on selectivity let
alone uniqueness.


Regards,
Halil 

 

---------------------------------------------------------------------
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] Re: [virtio-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Tue, Apr 04, 2023 at 06:29:25PM +0200, Halil Pasic wrote:
> On Thu, 23 Mar 2023 23:24:22 +0800
> Heng Qi <hengqi@linux.alibaba.com> wrote:
> 
> > +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.
> 
> Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> negotiated, and queue_select != queue_notify_data, is vqn supposed
> to contain queue_notify_data or the number/index that is used for
> queue_select (I'm talking about the PCI transport case)?
> 
> Regards,
> Halil

I think it's the vq number as it says here, this is not transport specific.

-- 
MST


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/4/5 上午12:29, Halil Pasic 写道:
> On Thu, 23 Mar 2023 23:24:22 +0800
> Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>> +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.
> Just to be on the safe side: VIRTIO_F_NOTIF_CONFIG_DATA has been
> negotiated, and queue_select != queue_notify_data, is vqn supposed
> to contain queue_notify_data or the number/index that is used for
> queue_select (I'm talking about the PCI transport case)?

Queue event notification is based on transport, as we know, this should 
be discussed in the virtio transport chapter.
Virtqueue coalescing moderation uses the same vqn field as 
notifications, but this has a completely different meaning.
For example, when VIRTIO_F_NOTIF_CONFIG_DATA is negotiated, vqn uses the 
identifier (which Parav is doing) represented by q_notify_data,
such as virtqueue index/number or offset, etc.
vqn here does not belong to the PCI configuration space, because now we 
have involved controlq, which is used to set the non-configuration space.

Thanks.


>
> Regards,
> Halil


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Cornelia Huck]

On Thu, Mar 23 2023, Heng Qi <hengqi@linux.alibaba.com> wrote:

> Hi all! If after reviewing it you feel it meets the necessary 
> requirements, I kindly request a vote.
>
> Thanks a lot!
>
> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166

So this has been voted upon and accepted, but we have further discussion
downthread.

Should we merge the voted-upon version now and do further changes on
top, or wait for the discussion to conclude first so that we can merge
all of it in one go?


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Heng Qi]

在 2023/4/11 下午4:05, Cornelia Huck 写道:
> On Thu, Mar 23 2023, Heng Qi <hengqi@linux.alibaba.com> wrote:
>
>> Hi all! If after reviewing it you feel it meets the necessary
>> requirements, I kindly request a vote.
>>
>> Thanks a lot!
>>
>> Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166
> So this has been voted upon and accepted, but we have further discussion
> downthread.
>
> Should we merge the voted-upon version now and do further changes on
> top, or wait for the discussion to conclude first so that we can merge
> all of it in one go?
Hi, Cornelia.

According to Parav's excellent patch set work on "rename" I tracked and 
Halil and Michael's discussion,
the discussion is about vqn-> vq_notify_id, queue_notify_data-> 
queue_notify_id, virtqueue number->virtqueue index in the *Notifications 
chapter*;
in *MMIO Device Register Layout chapter*, QueueNum->QueueSize.
Thanks Parav for his respectable work and the reviewer's comments, which 
made the spec clearer.

We left the virtqueue number/index of RSS and virtqueue coalescing (this 
work) parts unchanged as suggested by Halil.
Please correct me if my trace missed something.

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-dev] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Tue, Apr 11, 2023 at 10:05:11AM +0200, Cornelia Huck wrote:
> On Thu, Mar 23 2023, Heng Qi <hengqi@linux.alibaba.com> wrote:
> 
> > Hi all! If after reviewing it you feel it meets the necessary 
> > requirements, I kindly request a vote.
> >
> > Thanks a lot!
> >
> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166
> 
> So this has been voted upon and accepted, but we have further discussion
> downthread.
> 
> Should we merge the voted-upon version now and do further changes on
> top, or wait for the discussion to conclude first so that we can merge
> all of it in one go?

I think we should merge as is. vqn/idx/id are not new discussions
and have been going on before voting took place, so the TC
was informed. Parav is working on making tree-wide changes
around vq index applying this will not make his work
measureably harder.

-- 
MST


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Michael S. Tsirkin]

On Wed, Apr 05, 2023 at 04:54:44PM +0200, Halil Pasic wrote:
> On Wed, 5 Apr 2023 05:12:12 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > > > it's not necessarily an identifier. can be e.g. just 0 for all vqs.
> > > > whatever the device needs.  
> > > For driver its just an id, content doesn't matter.  
> > 
> > No, this value might or might not be somehow related to the vq but it
> > does not have to identify it. So it's some data, not an id.  Let's try
> > to use words for what they mean in english not try to come up with our
> > own language.
> 
> I agree with Michael wholeheartedly, we should try to use words for what
> they mean in English, especially in Computer Science English, and even
> try to pick the most fitting and least ambiguous option if multiple
> options are possible.
> 
> In that spirit I would say that "queue_notify_data" is actually a magic
> cookie. https://en.wikipedia.org/wiki/Magic_cookie A possible name
> or abbreviation would be vqn_cookie like "a virtqueue
> notification cookie" (I'm not sure about mixing in the direction, but
> this is only about driver -> device notifications). 
> 
> 
> And then 
> 
> le32 {
>         vqn : 16;
>         next_off : 15;
>         next_wrap : 1;
> };
> 
> could become something like
> 
> struct vq_notif_data {
>         union {
> 		le16 vqn_cookie;
> 		le16 vq_index;
> 	};
>         le16 {
> 		next_off : 15;
>         	next_wrap : 1;
> 	};
> };
> 
> BTW since "identifier" and "unique identifier" are not the same, in my
> opinion "identifier" is still viable, if we explain that it is called
> identifier because the idea behind that field is to be used to identify
> the queue, but that there is in fact no requirement on selectivity let
> alone uniqueness.
> 
> 
> Regards,
> Halil 
> 

Well "identifier" seems to come from "identity" meaning "same" so I
think yes, it implies a 1:1 relationship.
And more importantly, it might not identify the queue, there is
in fact no requirement on how it is used at all.

-- 
MST


---------------------------------------------------------------------
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] [PATCH v14] virtio-net: support the virtqueue coalescing moderation

[Cornelia Huck]

On Tue, Apr 11 2023, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Apr 11, 2023 at 10:05:11AM +0200, Cornelia Huck wrote:
>> On Thu, Mar 23 2023, Heng Qi <hengqi@linux.alibaba.com> wrote:
>> 
>> > Hi all! If after reviewing it you feel it meets the necessary 
>> > requirements, I kindly request a vote.
>> >
>> > Thanks a lot!
>> >
>> > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/166
>> 
>> So this has been voted upon and accepted, but we have further discussion
>> downthread.
>> 
>> Should we merge the voted-upon version now and do further changes on
>> top, or wait for the discussion to conclude first so that we can merge
>> all of it in one go?
>
> I think we should merge as is. vqn/idx/id are not new discussions
> and have been going on before voting took place, so the TC
> was informed. Parav is working on making tree-wide changes
> around vq index applying this will not make his work
> measureably harder.

I had not looked at the discussion in detail and was unsure whether it
was covering something new... I went ahead and applied it.


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