From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: References: <20230222140632.10253-1-hengqi@linux.alibaba.com> <3e0d0bd0-b3e8-1616-7fd6-8a4a5a35e6db@linux.alibaba.com> From: David Edmondson Subject: Re: [virtio-comment] Re: [PATCH v7] virtio-net: support the virtqueue coalescing moderation Date: Thu, 23 Feb 2023 11:43:29 +0000 In-reply-to: <3e0d0bd0-b3e8-1616-7fd6-8a4a5a35e6db@linux.alibaba.com> Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable To: Heng Qi Cc: virtio-dev@lists.oasis-open.org, "Michael S . Tsirkin" , Parav Pandit , Alvaro Karsz , Jason Wang , Xuan Zhuo , Cornelia Huck , virtio-comment@lists.oasis-open.org List-ID: On Thursday, 2023-02-23 at 18:52:14 +08, Heng Qi wrote: > Hi, David. > > =E5=9C=A8 2023/2/23 =E4=B8=8B=E5=8D=886:05, David Edmondson =E5=86=99=E9= =81=93: >> On Wednesday, 2023-02-22 at 22:06:32 +08, Heng Qi wrote: >>> Currently, coalescing parameters are grouped for all transmit and recei= ve >>> virtqueues. This patch supports setting or getting the parameters for a >>> specified virtqueue, and a typical application of this function is netd= im[1]. >>> >>> When the traffic between virtqueues is unbalanced, for example, one vir= tqueue >>> 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 >>> Reviewed-by: Xuan Zhuo >>> --- >>> This patch is on top of Alvaro's latest v7 patch: https://lists.oasis-o= pen.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. Tsirki= n >>> 2. Remove formula for vqn range. @Parav Pandit >>> 3. Some expressions are clearer. @Parav Pandit, @Michael S. Tsi= rkin >>> >>> v5->v6: >>> 1. Explain that the device may set a different value than the o= ne 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 =3D 1, .pkts =3D NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 8, .pkts =3D NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 64, .pkts =3D NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 128, .pkts =3D NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 256, .pkts =3D NET_DIM_DEFAULT_RX_CQ_PKTS_FROM_EQE,} \ > } > > #define NET_DIM_RX_CQE_PROFILES { \ > {.usec =3D 2, .pkts =3D 256,}, \ > {.usec =3D 8, .pkts =3D 128,}, \ > {.usec =3D 16, .pkts =3D 64,}, \ > {.usec =3D 32, .pkts =3D 64,}, \ > {.usec =3D 64, .pkts =3D 64,} \ > } > > #define NET_DIM_TX_EQE_PROFILES { \ > {.usec =3D 1, .pkts =3D NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 8, .pkts =3D NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 32, .pkts =3D NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 64, .pkts =3D NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,}, \ > {.usec =3D 128, .pkts =3D NET_DIM_DEFAULT_TX_CQ_PKTS_FROM_EQE,} \ > } > > #define NET_DIM_TX_CQE_PROFILES { \ > {.usec =3D 5, .pkts =3D 128,}, \ > {.usec =3D 8, .pkts =3D 64,}, \ > {.usec =3D 16, .pkts =3D 32,}, \ > {.usec =3D 32, .pkts =3D 32,}, \ > {.usec =3D 64, .pkts =3D 32,} \ > } > " > [1]=C2=A0 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} =3D 7 from the > VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command, it may set \field{max_usecs} > =3D 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. The value chosen by the device can be retrieved using the VIRTIO_NET_CTRL_NOTF_VQ_GET command. >> - 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 virt= io_net_ctrl_coal_vq and control commands. @Michael S. Tsirkin >>> 2. Add read and write attributes for each field. @Michael S. Ts= irkin >>> 3. A clearer description of how to set coalescing parameters fo= r vq reset. @Michael S. Tsirkin >>> 4. Fix some syntax errors. @Michael S. Tsirkin, @David Edmondso= n >>> >>> 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 P= andit, @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. Ts= irkin >>> 5. Clarify some statements. @Michael S. Tsirkin, @Parav Pandit,= @Alvaro Karsz >>> 6. Adjust the virtio_net_ctrl_coal_vq structure. @Michael S. Ts= irkin >>> 7. Fix some typos. @Michael S. Tsirkin >>> >>> v1->v2: >>> 1. Rename VIRTIO_NET_F_PERQUEUE_NOTF_COAL to VIRTIO_NET_F_VQ_NO= TF_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. @Mic= hael S. Tsirkin, @Parav Pandit, @Alvaro Karsz >>> 5. The special value 0xFFF is removed because VIRTIO_NET_CTRL_N= OTF_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/descri= ption.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 / Ne= twork 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 coale= scing. >>> \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:Devi= ce 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 paramet= ers. >>> +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 virtqu= eue. >>> +\end{itemize} >>> + >>> \begin{note} >>> The behavior of the device in response to these commands is best-effo= rt: >>> the device may generate notifications more or less frequently than sp= ecified. >>> @@ -1519,25 +1530,76 @@ \subsubsection{Control Virtqueue}\label{sec:Dev= ice 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 virt= queue 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 sett= ing \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_CTR= L_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} an= d \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 rece= ive virtqueue. >>> \item \field{max_usecs} for RX: Maximum number of microseconds to del= ay a RX notification. >>> \item \field{max_usecs} for TX: Maximum number of microseconds to del= ay a TX notification. >>> \item \field{max_packets} for RX: Maximum number of packets to receiv= e before a RX notification. >>> \item \field{max_packets} for TX: Maximum number of packets to send b= efore 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 \fie= ld{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 who= se number is \field{vqn}. >>> +\item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: have the same effect of settin= g coalescing parameters as the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command rep= eated for >>> + each virtqueue of transmitq1\l= dots transmitqN. >>> +\item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: have the same effect of settin= g coalescing parameters as the VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command rep= eated for >>> + each virtqueue of receiveq1\ld= ots 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 e= xample with 2 pairs of virtqueues: >>> +# Command sequence >>> +Each of the following commands sets \field{max_usecs} and \field{max_p= ackets} 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} =3D = 0 sets coalescing parameters for virtqueue0, and virtqueue2 retains the val= ues from command1. >>> +\item Command3: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =3D = 0, the device responds with coalescing parameters of virtqueue0 set by comm= and2. >>> +\item Command4: VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with \field{vqn} =3D = 1 sets coalescing parameters for virtqueue1, and virtqueue3 retains its pre= vious values. >>> +\item Command5: VIRTIO_NET_CTRL_NOTF_COAL_TX_SET sets coalescing param= eters for virtqueue1 and virtqueue3, and overrides the values set by comman= d4. >>> +\item Command6: VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET with \field{vqn} =3D = 1, the device responds with coalescing parameters of virtqueue1 set by comm= and5. >>> +\end{itemize} >>> + >>> \subparagraph{Operation}\label{sec:Device Types / Network Device / De= vice 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:Devi= ce Types / Network Device / Devi >>> When the device has \field{max_usecs} =3D 0 or >>> \field{max_packets} =3D 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} =3D 7 from the VIRTIO_NET_CTR= L_NOTF_COAL_VQ_SET command, it may set \field{max_usecs} =3D 8 for a given = enabled virtqueue. >>> + >>> +When the device receives the VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and VIRT= IO_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 para= meters are set to global values. >>> + >>> \subparagraph{RX Example}\label{sec:Device Types / Network Device / D= evice Operation / Control Virtqueue / Notifications Coalescing / RX Example= } >>> If, for example: >>> @@ -1585,11 +1656,29 @@ \subsubsection{Control Virtqueue}\label{sec:Dev= ice 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_COA= L feature >>> +has been negotiated, the driver MUST NOT issue VIRTIO_NET_CTRL_NOTF_CO= AL 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 VIR= TIO_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_N= ET_CTRL_NOTF_COAL_VQ_GET commands with VIRTIO_NET_ERR if the given virtqueu= e 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 record= ed as global values by a device. >>> +The device MUST set the global values of coalescing parameters to 0 af= ter being reset. >>> +The VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command sets the coalescing param= eters for a given enabled virtqueue without changing the global values. >>> + >>> +After disabling and re-enabling a virtqueue, the device MUST revert co= alescing 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-l= ists > Committee: https://www.oasis-open.org/committees/virtio/ > Join OASIS: https://www.oasis-open.org/join/ --=20 Come down, come talk to me.