OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-comment message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


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




å 2023/2/23 äå9:20, Michael S. Tsirkin åé:
On Thu, Feb 23, 2023 at 11:43:29AM +0000, David Edmondson wrote:
On Thursday, 2023-02-23 at 18:52:14 +08, Heng Qi wrote:
Hi, David.

å 2023/2/23 äå6:05, David Edmondson åé:
On Wednesday, 2023-02-22 at 22:06:32 +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 .

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 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
A couple of things about this:
- why say "a value close to a power of 2" - couldn't the device pick any
    value it chooses?
This is just a hint from the spec, it is "MAY", not "MUST" in the
conformance of the device, the device can still set any value it
receives.
Okay.

And, since "virtqueue notification coalescing" feature will be used in
the netdim [1] algorithm,
and the coalescing moderation of netdim is roughly as follows, so it
is allowed to give the hint in the spec:
"
#define NET_DIM_RX_EQE_PROFILES { \
{.usec = 1, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
{.usec = 8, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
{.usec = 64, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
{.usec = 128, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \
{.usec = 256, .pkts = NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,} \
}

#define NET_DIM_RX_CQE_PROFILES { \
{.usec = 2, .pkts = 256,}, \
{.usec = 8, .pkts = 128,}, \
{.usec = 16, .pkts = 64,}, \
{.usec = 32, .pkts = 64,}, \
{.usec = 64, .pkts = 64,} \
}

#define NET_DIM_TX_EQE_PROFILES { \
{.usec = 1, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
{.usec = 8, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
{.usec = 32, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
{.usec = 64, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \
{.usec = 128, .pkts = NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,} \
}

#define NET_DIM_TX_CQE_PROFILES { \
{.usec = 5, .pkts = 128,}, \
{.usec = 8, .pkts = 64,}, \
{.usec = 16, .pkts = 32,}, \
{.usec = 32, .pkts = 32,}, \
{.usec = 64, .pkts = 32,} \
}
"
[1]Â https://docs.kernel.org/networking/net_dim.html

- I think that we need to be more explicit that the values passed in the
    SET request may not be honoured exactly.
Yes, there are already examples in the current spec:
"
+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.
"
If you find this unclear, do you need more examples or clarification,
or do you have a better way?
Explicit is good:

When a device receives a command of the VIRTIO_NET_CTRL_NOTF_COAL class
to set a coalescing parameter it may choose to use a value different to
that specified in the command, for example a power of two value close to
the specified parameter.

Hmm. This clarification actually belongs in Alvaro's patch btw, no?
Basically with the best-effort sentence.


Indeed, considering my patch is on top of Alvaro's patch and this description in his patch seems to contain what we now want to express "the device may set a different value": âThe behavior of the device in response to these commands is best-effort: the device may generate notifications more or less frequently than specified.â

Maybe, do we need to add this to my patch?
"
When a device receives a command of class VIRTIO_NET_CTRL_NOTF_COAL to set or respond to a coalescing parameter, it behaves on a best-effort basis. For example: If a device receives \field{max_usecs} = 7 from a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command, it may set \field{max_usecs} = 8 for a given enabled virtqueue. Then the device receives the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, it may respond with \field{max_usecs} = 8 for the virtqueue.
"
If the example is dropped, there is no difference with Alvaro's expression, and we can delete the description about "close to a power of 2" in my patch.

Or, use the description below?
"
When a device receives a command of the VIRTIO_NET_CTRL_NOTF_COAL class to set a coalescing parameter, it may choose to use a value different to that specified in the command. The value chosen by the device can be retrieved using the VIRTIO_NET_CTRL_NOTF_VQ_GET command. For example: If a device receives \field{max_usecs} = 7 from a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command, it may set \field{max_usecs} = 8 for a given enabled virtqueue. Then the device receives the command VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET, it may respond with \field{max_usecs} = 8 for the virtqueue.
"
If we don't want the example of this description, we can also drop it and keep only the others.

Thanks :)


The value chosen by the device can be retrieved
using the VIRTIO_NET_CTRL_NOTF_VQ_GET command.
I do however feel that maybe we should not allow any value.
Values lower than what is specified are definitely ok.


- should the chosen value be returned in the SET call? (Not too fussed
    about this, though it may result in an implementation immediately
    calling GET after SET to see what actually happened.)
As you said, I think we can just call GET to view.

- the example which shows how the global and per-VQ set operations
    interact is reasonably worded ("the device responds with coalescing
    parameters of virtqueue1 set by command5"), so that seems okay.
Yeah.

Thanks.

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 structres 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 | 99 ++++++++++++++++++++++++++++++--
   1 file changed, 94 insertions(+), 5 deletions(-)

diff --git a/device-types/net/description.tex b/device-types/net/description.tex
index e71e33b..745e4d9 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}
@@ -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}, \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}
+
   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.
+
+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 of setting coalescing parameters 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 of setting coalescing parameters 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,15 @@ \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 enabled after virtqueue reset, its coalescing parameters are set to global values.
+
   \subparagraph{RX Example}\label{sec:Device Types / Network Device / Device Operation / Control Virtqueue / Notifications Coalescing / RX Example}
     If, for example:
@@ -1585,11 +1656,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.

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/
--
Come down, come talk to me.



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]