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

[David Edmondson <david.edmondson@oracle.com>]

On Wednesday, 2023-02-22 at 04:13:06 UTC, Parav Pandit wrote:
>> From: Heng Qi <hengqi@linux.alibaba.com>
>> Sent: Tuesday, February 21, 2023 10:22 PM
>
>> +\item[VIRTIO_NET_F_VQ_NOTF_COAL(52)] Device supports virtqueue
>> notification coalescing.
>> +
> s/notification/notifications 
> should be plural as multiple notifications are coalesced like the below description of VIRTIO_NET_F_NOTF_COAL.

Using the plural sounds weird, and isn't it implied by "coalescing"? If
there were not multiple, there would be nothing to coalesce :-)

>>  \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} @@ -1508,6
>> +1511,14 @@ \subsubsection{Control Virtqueue}\label{sec:Device Types /
>> Network Device / Devi  If the VIRTIO_NET_F_NOTF_COAL feature is negotiated,
>> the driver can  send control commands for dynamically changing the coalescing
>> parameters.
>> 
>> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated:
>> +\begin{itemize}
>> +\item a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command
>> to set coalescing parameters of a given
>> +      enabled transmit/receive virtqueue.
>> +\item a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command
>> to a device, and the device responds with
>> +      coalescing parameters of a given enabled transmit/receive virtqueue.
>> +\end{itemize}
>> +
>>  \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.
>> @@ -1519,25 +1530,76 @@ \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}
>> 
>> +The VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands use the
>> virtio_net_ctrl_coal
>> +structure to set \field{max_usecs} and \field{max_packets} for all
>> transmit/receive virtqueues.
>> +
>> +The VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command uses the
>> +virtio_net_ctrl_coal_vq structure to set \field{max_usecs} and
>> \field{max_packets} for the supplied virtqueue number \field{vqn}.
>> +
>> +The VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command gets the values of
>> +\field{max_usecs} and \field{max_packets} of the specified virtqueue
>> +from the device by setting \field{vqn} in the virtio_net_ctrl_coal_vq structure.
>> +
>
>> +# 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},
>
> Virtio spec is using vq number and vq index terminology interchangeably.
> For example, a new patch about admin vq, uses aq_start_index.
>
> MMIO device has vq_index register too.
> I am inclined to vq_index given current state of spec and new additions by Michael.
>
> Michael,
> Can you please suggest vqn or vq_index to use?
>
>> \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 a driver.
> Remove this trailing "for a driver", it is a duplicate.
>
>> +\end{itemize}
>> +
>>  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{vqn} points to an enabled transmit/receive virtqueue, and its value
>> satisfies $ 0 \leq vqn < max_virtqueue_pairs \ast 2 $.
>> +
> This calculation description is not needed here. It is covered somewhere else.
>
>> +\field{reserved} is reserved and it is ignored by a device.
>> +
>> +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_VQ_SET: 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: the device returns 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_TX_SET: have the same effect as the
>> VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command repeated for each
>> virtqueue of transmitq1\ldots transmitqN.
>> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: have the same effect as the
>> VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command repeated for each
>> virtqueue of receiveq1\ldots receiveqN.
>>  \end{enumerate}
>> 
>> +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. For example with 2 pairs of virtqueues:
>> +# Command sequence
>> +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}
>> +
>>  \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}.
>> @@ -1549,6 +1611,16 @@ \subsubsection{Control
>> Virtqueue}\label{sec:Device Types / Network Device / Devi
>> 
>>  When the device has \field{max_usecs} = 0 or \field{max_packets} = 0, the
>> notification conditions are met after every packet received/sent.
>> 
>> +When a device receives a command of the VIRTIO_NET_CTRL_NOTF_COAL
>> class
>> +to set a coalescing parameter, it may set the parameter to a value close to a
>> power of 2. For example:
>> +If the device receives \field{max_usecs} = 7 from the
>> VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command, it may set
>> \field{max_usecs} = 8 for a given enabled virtqueue.
>> +
>> +When the device receives the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands, it saves the values of
>> +coalescing parameters as global values, and the
>> VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command does not change the global
>> values. If the device is reset, the global values will be set to 0.
>> +
>> +When a virtqueue is reset, its coalescing parameters are set to the global
>> values.
>> +
> A VQ reset operation disables (or destroys) the VQ in the device.
> Hence, a vq under reset doesn't have any valid parameters.
> Therefore, above wording should be,
>
> When a virtqueue is enabled after it is reset, its coalescing
> parameters are set to global values as configured by
> VIRTIO_NET_CTRL_NOTF_COAL_TX_SET or VIRTIO_NET_CTRL_NOTF_COAL_RX_SET.
>
>>  \subparagraph{RX Example}\label{sec:Device Types / Network Device / Device
>> Operation / Control Virtqueue / Notifications Coalescing / RX Example}
>> 
>>  If, for example:
>> @@ -1585,11 +1657,29 @@ \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.
>> +If neither the VIRTIO_NET_F_NOTF_COAL nor the
>> VIRTIO_NET_F_VQ_NOTF_COAL
>> +feature has been negotiated, the driver MUST NOT issue
>> VIRTIO_NET_CTRL_NOTF_COAL commands.
>> +
>> +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 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.
>> +
>> +The VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
>> +VIRTIO_NET_CTRL_NOTF_COAL_RX_SET commands set coalescing parameters
>> for all transmit/receive virtqueues respectively and values of coalescing
>> parameters are recorded as global values by a device.
>> +The device MUST set the global values of coalescing parameters to 0 after
>> being reset.
>> +The VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command sets the coalescing
>> parameters for a given enabled virtqueue without changing the global values.
>> +
>> +After disabling and re-enabling a virtqueue, the device MUST revert coalescing
>> parameters of the virtqueue to the global values.
>> +
>> +A device MAY set the coalescing parameter to a value close to a power of 2
>> value.
>> +
>> +A device MUST ignore \field{reserved}.
>> 
>>  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
-- 
Tonight I think I'll walk alone, I'll find my soul as I go home.

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/