RE: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> From: Heng Qi > Sent: Friday, February 17, 2023 12:17 PM > Yes, but there seems to be such a situation: when the device is reactivated, > as > the specification says, all parameters are set to 0 (use parameters as the > default > configuration on the device). > When CTRL_COAL_SET and CTRL_COAL_VQ_SET are sent, the configuration is > updated (the parameters of each vq may be different, but the global parameter > configuration may be recorded), at this time, if vq is reset, should the > parameters be 0 or a recorded global parameters after it is re-enabled? Recorded global parameters should be fine to report on an enabled vq even if the COAL_VQ_SET was not done. This is because the global value reflects on the VQ. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
On Fri, Feb 17, 2023 at 04:12:34PM +, Parav Pandit wrote: > > > From: Michael S. Tsirkin > > Sent: Friday, February 17, 2023 6:35 AM > > > > We mention the device reset case, but nothing about VQ reset. > > > > > > I feel that no matter how we handle this, we break something. > > > > > > Having default coalescing values may collide with "Upon reset, a > > > device MUST initialize all coalescing parameters to 0." > > > > No this is after device reset. > > > > > We can say that VQ reset doesn't affect the "global parameters" and a > > > device reset does, but this collides with "Device Requirements: > > > Virtqueue Reset". > > > > Not really. > When the device resets, VQ objects are destroyed in the device. > So VQ's notifications parameters doesn't exists on device reset. > > And so the same case with VQ reset. > When a VQ is reset (disabled), VQ's notifications configuration is removed in > the device too. > Just like its desc ring and other addresses are invalid. Yes, but there seems to be such a situation: when the device is reactivated, as the specification says, all parameters are set to 0 (use parameters as the default configuration on the device). When CTRL_COAL_SET and CTRL_COAL_VQ_SET are sent, the configuration is updated (the parameters of each vq may be different, but the global parameter configuration may be recorded), at this time, if vq is reset, should the parameters be 0 or a recorded global parameters after it is re-enabled? > > > > In fact, resetting coalescing values after vq reset may be derived > > > from "Upon reset, a device MUST initialize all coalescing parameters > > > to 0". > > > This is consistent with "Device Requirements: Virtqueue Reset". > > > > > > I can add in my patch some clarifications. > > > > > > This will break the linux virtio_net ethtool implementation a little, > > > we'll need to fix it. > > > > Not good. I feel we must come up with spec that is backwards compatible. > > Hmm, maybe this is why Parav kept talking about modes. > > I did not realize at the time, sorry Parav. > > > > I still feel modes are not the best way to describe things so I propose > > this: > > - in addition to per vq parameters, device that supports global TX/RX > > commands and ring reset maintains two sets of default parameters: for RX > > and > > TX > > - existing commands change default and change all enabled vqs > > of the correct type (RX/TX) to the same value > > - vq reset changes a vq to the default > > - device reset changes defaults to 0 and changes all vqs to 0 > > > > note how defaults are only used for ring reset. is "vq reset parameter" > > a better name? I feel we will repeat "reset" too many times in a sentence > > if we > > call it that though. > > > > So fundamentally the only change is with RING_RESET, then default is not > > always 0, it can be set by the global command. > > True default is not zero when global are configured. > It is ok to report VQ parameters on GET command when globals are configured. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
RE: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> From: Michael S. Tsirkin > Sent: Friday, February 17, 2023 6:35 AM > > We mention the device reset case, but nothing about VQ reset. > > > > I feel that no matter how we handle this, we break something. > > > > Having default coalescing values may collide with "Upon reset, a > > device MUST initialize all coalescing parameters to 0." > > No this is after device reset. > > > We can say that VQ reset doesn't affect the "global parameters" and a > > device reset does, but this collides with "Device Requirements: > > Virtqueue Reset". > > Not really. When the device resets, VQ objects are destroyed in the device. So VQ's notifications parameters doesn't exists on device reset. And so the same case with VQ reset. When a VQ is reset (disabled), VQ's notifications configuration is removed in the device too. Just like its desc ring and other addresses are invalid. > > In fact, resetting coalescing values after vq reset may be derived > > from "Upon reset, a device MUST initialize all coalescing parameters > > to 0". > > This is consistent with "Device Requirements: Virtqueue Reset". > > > > I can add in my patch some clarifications. > > > > This will break the linux virtio_net ethtool implementation a little, > > we'll need to fix it. > > Not good. I feel we must come up with spec that is backwards compatible. > Hmm, maybe this is why Parav kept talking about modes. > I did not realize at the time, sorry Parav. > > I still feel modes are not the best way to describe things so I propose this: > - in addition to per vq parameters, device that supports global TX/RX > commands and ring reset maintains two sets of default parameters: for RX and > TX > - existing commands change default and change all enabled vqs > of the correct type (RX/TX) to the same value > - vq reset changes a vq to the default > - device reset changes defaults to 0 and changes all vqs to 0 > > note how defaults are only used for ring reset. is "vq reset parameter" > a better name? I feel we will repeat "reset" too many times in a sentence if > we > call it that though. > > So fundamentally the only change is with RING_RESET, then default is not > always 0, it can be set by the global command. True default is not zero when global are configured. It is ok to report VQ parameters on GET command when globals are configured. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
RE: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> From: Alvaro Karsz > Sent: Friday, February 17, 2023 7:18 AM > I wonder if some of that should be included in my patch. > Most of that is not relevant before introducing the per vq command. > So maybe I could just mention in the device conformance that: > "Coalescing parameters MUST/SHOULD persist a virtqueue reset." > For sure coalescing parameters must not persist on a vq reset in the device. VQ reset operation destroys or disables the VQ. Device should not have any obligation to maintain some parameter on the disabled/destroyed object. User likely disabled the VQ because user may want to have a fresh start for this VQ for same or different use. And hence, reconfiguring the new addresses, new vq size, new vq notifications parameters..
[virtio-dev] RE: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> From: Heng Qi
> Sent: Friday, February 17, 2023 12:24 AM
> I think there are two kinds of sequences:
> #Seq1:
> 1. vq is disabled (vq params are reset to max_packets = 0, max_usecs = 0) 2.
> VQ_SET command arrives with max_packets=10, max_usecs = 8 3. vq is enabled
> (vq params are max_packets = 0, max_usecs = 0) After #3 is done, vq params
> should be max_packets = 0, max_usecs = 0, because the device cannot apply
> parameters when vq is disabled.
> Other reason is that parameters determined based on the previous traffic
> information before vq is re-enabled, and it is not applicable now.
>From dev point of view, above is not a valid sequence.
VQ is the object whose lifecycle (create/delete/query/modify) is triggered by
the driver.
So step 2 cannot happen before step 3.
If driver does that, driver should get error code INVALID_VQ on step 2.
>
> #Seq1:
> 1. vq is disabled (vq params are reset to max_packets = 0, max_usecs = 0) 2.
> vq
> is enabled (vq params are max_packets = 0, max_usecs = 0) 3. VQ_SET
> command arrives with max_packets=10, max_usecs = 8 After 3 is done, vq
> params should be max_packets = 10, max_usecs = 8
>
> Thanks.
>
> >
> > > +
> > > +A device MAY NOT set the coalescing parameter to the exact same
> > > +value as the one passed in by a driver. For example, the value of
> \field{max_packets} set by the driver is 15, but the device MAY store a value
> that is a power of 2, that is, 16.
> > > +
> > > A device SHOULD NOT send used buffer notifications to the driver, if the
> notifications are suppressed as explained in \ref{sec:Basic Facilities of a
> Virtio
> Device / Virtqueues / Used Buffer Notification Suppression}, even if the
> coalescing counters expired.
> > >
> > > Upon reset, a device MUST initialize all coalescing parameters to 0.
> > > --
> > > 2.19.1.6.gb485710b
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> Not good. I feel we must come up with spec that is backwards compatible. > Hmm, maybe this is why Parav kept talking about modes. > I did not realize at the time, sorry Parav. > > I still feel modes are not the best way to describe things so I propose this: > - in addition to per vq parameters, device that supports global TX/RX > commands and ring reset maintains two sets of default parameters: for RX > and TX > - existing commands change default and change all enabled vqs > of the correct type (RX/TX) to the same value > - vq reset changes a vq to the default > - device reset changes defaults to 0 and changes all vqs to 0 > > note how defaults are only used for ring reset. is "vq reset parameter" > a better name? I feel we will repeat "reset" too many times in a > sentence if we call it that though. > > So fundamentally the only change is with RING_RESET, then > default is not always 0, it can be set by the global command. I like the idea, maintaining default coalescing parameters is compatible with vq reset. "After a queue has been reset by the driver, the device MUST NOT execute any requests from that virtqueue, or notify the driver for it. The device MUST reset any state of a virtqueue to the *default state*, including the available state and the used state." I wonder if some of that should be included in my patch. Most of that is not relevant before introducing the per vq command. So maybe I could just mention in the device conformance that: "Coalescing parameters MUST/SHOULD persist a virtqueue reset." And then Heng can edit/add on top of that. Now that the existing commands do more than just iterating through all the virtqueues, maybe we should be able to modify all virtqueues with VIRTIO_NET_F_VQ_NOTF_COAL. I think that using a special vqn is messy, we can split "le32 reserved" to "le16 flags" and "le16 reserved". Then we can have: - A flag to apply parameters to all receive VQs, vqn is ignored in this case. (same as VIRTIO_NET_CTRL_NOTF_COAL_RX_SET but without changing the default parameters). - A flag to apply parameters to all transmit VQs, vqn is ignored in this case. (same as VIRTIO_NET_CTRL_NOTF_COAL_TX_SET but without changing the default parameters). And maybe a flag to apply to all vqs (transmit + receive), I actually am not sure how useful this is.. - To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
On Fri, Feb 17, 2023 at 01:17:50PM +0200, Alvaro Karsz wrote:
> > > Yes, max_packets and max_usecs SHOULD be reset to 0.
> > > When the virtqueue is re-enabled, it means that notification conditions
> > > are met after each packet is sent/received.
> > >
> > > As described by Alvaro in "[PATCH v5] virtio-net: Fix and update
> > > VIRTIO_NET_F_NOTF_COAL feature":
> > > "+When the device has \field{max_usecs} = 0 or \field{max_packets} = 0,
> > > the notification conditions are met after every packet received/sent."
> >
> > Oh. Hmm.
> >
> > What Alvaro's patch does not describe is what happens when VQ is reset.
> >
> > Alvaro you said you have hardware implementing this right?
> > How does the command interact with vq reset?
> >
>
> The device doesn't offer VIRTIO_F_RING_RESET at the moment.
>
> >
> > > > What about VIRTIO_NET_CTRL_NOTF_COAL?
> > >
> > > I think it should be handled in the same way as the above VQ_SET, that
> > > is, reset the corresponding virtqeuue parameters to 0.
> >
> > sounds consistent. but the problem is, I don't think this is
> > how we previously behaved. and RING_RESET is out in 1.2.
> > So we need something compatible. I am sorry.
> >
> > I suspect that instead we can say that existing hardware has a default set
> > of
> > parameters for tx and rx. And global commands change that
> > besides changing all enabled VQs.
> >
> > That is a side effect beyond just changing all VQs.
> >
> > Hmm.
> > Alvaro?
> >
>
> This is indeed a good point.
> We mention the device reset case, but nothing about VQ reset.
>
> I feel that no matter how we handle this, we break something.
>
> Having default coalescing values may collide with "Upon reset, a
> device MUST initialize all coalescing parameters to 0."
No this is after device reset.
> We can say that VQ reset doesn't affect the "global parameters" and a
> device reset does, but this collides with "Device Requirements:
> Virtqueue Reset".
>
> In fact, resetting coalescing values after vq reset may be derived
> from "Upon reset, a device MUST initialize all coalescing parameters
> to 0".
> This is consistent with "Device Requirements: Virtqueue Reset".
>
> I can add in my patch some clarifications.
>
> This will break the linux virtio_net ethtool implementation a little,
> we'll need to fix it.
Not good. I feel we must come up with spec that is backwards compatible.
Hmm, maybe this is why Parav kept talking about modes.
I did not realize at the time, sorry Parav.
I still feel modes are not the best way to describe things so I propose this:
- in addition to per vq parameters, device that supports global TX/RX
commands and ring reset maintains two sets of default parameters: for RX and
TX
- existing commands change default and change all enabled vqs
of the correct type (RX/TX) to the same value
- vq reset changes a vq to the default
- device reset changes defaults to 0 and changes all vqs to 0
note how defaults are only used for ring reset. is "vq reset parameter"
a better name? I feel we will repeat "reset" too many times in a
sentence if we call it that though.
So fundamentally the only change is with RING_RESET, then
default is not always 0, it can be set by the global command.
--
MST
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
Re: [virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> > Yes, max_packets and max_usecs SHOULD be reset to 0.
> > When the virtqueue is re-enabled, it means that notification conditions are
> > met after each packet is sent/received.
> >
> > As described by Alvaro in "[PATCH v5] virtio-net: Fix and update
> > VIRTIO_NET_F_NOTF_COAL feature":
> > "+When the device has \field{max_usecs} = 0 or \field{max_packets} = 0, the
> > notification conditions are met after every packet received/sent."
>
> Oh. Hmm.
>
> What Alvaro's patch does not describe is what happens when VQ is reset.
>
> Alvaro you said you have hardware implementing this right?
> How does the command interact with vq reset?
>
The device doesn't offer VIRTIO_F_RING_RESET at the moment.
>
> > > What about VIRTIO_NET_CTRL_NOTF_COAL?
> >
> > I think it should be handled in the same way as the above VQ_SET, that is,
> > reset the corresponding virtqeuue parameters to 0.
>
> sounds consistent. but the problem is, I don't think this is
> how we previously behaved. and RING_RESET is out in 1.2.
> So we need something compatible. I am sorry.
>
> I suspect that instead we can say that existing hardware has a default set of
> parameters for tx and rx. And global commands change that
> besides changing all enabled VQs.
>
> That is a side effect beyond just changing all VQs.
>
> Hmm.
> Alvaro?
>
This is indeed a good point.
We mention the device reset case, but nothing about VQ reset.
I feel that no matter how we handle this, we break something.
Having default coalescing values may collide with "Upon reset, a
device MUST initialize all coalescing parameters to 0."
We can say that VQ reset doesn't affect the "global parameters" and a
device reset does, but this collides with "Device Requirements:
Virtqueue Reset".
In fact, resetting coalescing values after vq reset may be derived
from "Upon reset, a device MUST initialize all coalescing parameters
to 0".
This is consistent with "Device Requirements: Virtqueue Reset".
I can add in my patch some clarifications.
This will break the linux virtio_net ethtool implementation a little,
we'll need to fix it.
-
To unsubscribe, e-mail: virtio-dev-unsubscr...@lists.oasis-open.org
For additional commands, e-mail: virtio-dev-h...@lists.oasis-open.org
[virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
On Fri, Feb 17, 2023 at 01:24:04PM +0800, Heng Qi wrote:
> On Thu, Feb 16, 2023 at 11:17:48AM -0500, Michael S. Tsirkin wrote:
> > On Thu, Feb 16, 2023 at 10:43:01PM +0800, 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
> > > Reviewed-by: Xuan Zhuo
> > > ---
> > > 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
> > >
> > > content.tex | 130 ++--
> > > 1 file changed, 126 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/content.tex b/content.tex
> > > index e863709..7f99503 100644
> > > --- a/content.tex
> > > +++ b/content.tex
> > > @@ -3084,6 +3084,9 @@ \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 the virtqueue
> > > +notifications coalescing.
> > > +
> > > \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications
> > > coalescing.
> > >
> > > \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> > > @@ -3140,6 +3143,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}
> > > @@ -4501,8 +4505,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> > > Types / Network Device / Devi
> > > };
> > >
> > > #define VIRTIO_NET_CTRL_NOTF_COAL 6
> > > - #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> > > + #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:
> > > @@ -4514,12 +4521,67 @@ \subsubsection{Control
> > > Virtqueue}\label{sec:Device Types / Network Device / Devi
> > > \end{itemize}
> > >
> > >
> > > -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> > > +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> > > \begin{enumerate}
> > > \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and
> > > \field{tx_max_packets} parameters.
> > > \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and
> > > \field{rx_max_packets} parameters.
> > > +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: set the \field{max_packets} and
> > > \field{max_usecs} parameters for a enabled
> > > +transmit/receive virtqueue whose
> > > number is \field{vqn}.
> > > +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: the device gets the
> > > \field{max_packets} and \field{max_usecs} parameters of
> > > +a enabled transmit/receive
> > > virtqueue whose number is \field{vqn}, and then
> > > +
[virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
On Thu, Feb 16, 2023 at 10:43:01PM +0800, 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
> Reviewed-by: Xuan Zhuo
> ---
> 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
>
> content.tex | 130 ++--
> 1 file changed, 126 insertions(+), 4 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index e863709..7f99503 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3084,6 +3084,9 @@ \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 the virtqueue
> +notifications coalescing.
btw no "the" here. 1st time we mention it. and notification not
notifications - reduced relative here, uses the singular form.
> +
> \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>
> \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -3140,6 +3143,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}
> @@ -4501,8 +4505,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> Types / Network Device / Devi
> };
>
> #define VIRTIO_NET_CTRL_NOTF_COAL 6
> - #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> + #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:
> @@ -4514,12 +4521,67 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> Types / Network Device / Devi
> \end{itemize}
>
>
> -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> \begin{enumerate}
> \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and
> \field{tx_max_packets} parameters.
> \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and
> \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: set the \field{max_packets} and
> \field{max_usecs} parameters for a enabled
> +transmit/receive virtqueue whose
> number is \field{vqn}.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: the device gets the
> \field{max_packets} and \field{max_usecs} parameters of
> +a enabled transmit/receive virtqueue
> whose number is \field{vqn}, and then
> +responds them to the driver.
> \end{enumerate}
>
> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated:
> +1. a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command to set the
> coalescing
> + parameters of a enabled transmit/receive virtqueue.
> +2. a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command to a device,
> and the device
> + responds to the driver with the coalescing
[virtio-dev] RE: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
> From: Heng Qi
> Sent: Thursday, February 16, 2023 9:43 AM
[..]
> #define VIRTIO_NET_CTRL_NOTF_COAL 6
> - #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> + #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
Unrelated change. Please remove this hunk.
> #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:
> @@ -4514,12 +4521,67 @@ \subsubsection{Control
> Virtqueue}\label{sec:Device Types / Network Device / Devi \end{itemize}
>
>
> -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> \begin{enumerate}
> \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and
> \field{tx_max_packets} parameters.
> \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and
> \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: set the \field{max_packets}
> and \field{max_usecs} parameters for a enabled
> +transmit/receive virtqueue whose
> number is \field{vqn}.
In the spec we have mix of notion as vqn or vq index.
For example, recent aq patch from Michael refers to aq vq index.
Mmio has vq index register too for long time.
And some places we don't refer to index vs number. (pcie queue_select register).
We all understand that they refer to same.
But it is time to get consistency for new additions.
So lets agree on either one of the naming convention.
Michael,
I am inclined to name it as vq index given existings references and with your
aq vq index patch.
Do you have preference?
If its index, above should be changed from vqn/vq_index.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: the device gets the
> \field{max_packets} and \field{max_usecs} parameters of
> +a enabled transmit/receive virtqueue
> whose number is
> \field{vqn}, and then
> +responds them to the driver.
> \end{enumerate}
It is confusing line of "device gets".
It is better to write it as,
The device returns max_packets and max_usecs for the enabled transmit or
receive virtuque whose index is vq_index.
>
> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated:
> +1. a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command to
> set the coalescing
> + parameters of a enabled transmit/receive virtqueue.
> +2. a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command to a
> device, and the device
> + responds to the driver with the coalescing parameters of a enabled
> transmit/receive virtqueue.
> +
> +\begin{lstlisting}
> +struct virtio_net_ctrl_coal_vq {
> +le16 vqn;
> +le16 reserved;
> +le32 max_packets;
> +le32 max_usecs;
> +};
> +\end{lstlisting}
> +
> +Virtqueue coalescing parameters:
> +\begin{itemize}
> +\item \field{vqn}: The virtqueue number of the enabled transmit or receive
> virtqueue, excluding the control virtqueue.
Transmit and receive vq is self-explanatory. No need to mention control vq.
The word "excluding" doesn't fit here well given it refers to only single vq at
a time.
> +\item \field{max_packets}: The maximum number of packets sent/received by
> the specified virtqueue before a TX/RX notification.
> +\item \field{max_usecs}: The maximum number of TX/RX usecs that the
> specified virtqueue delays a TX/RX notification.
> +\end{itemize}
> +
> +\field{reserved} is reserved and it is ignored by the device.
> +
> +THe value of \field{vqn} satisfies $ 0 \leq vqn < max_virtqueue_pairs \ast 2
> $.
> +The conversion between \field{vqn} and transmitq/receiveq index:
> +\begin{itemize}
> +$ \lfloor vqn / 2 \rfloor $ is the index of the corresponding receiveq.
> +$ \lfloor (vqn / 2) + 1 \rfloor $ is the index of the corresponding tranmitq.
> +\end{itemize}
> +
No need to provide the math here, it is about enabled vq.
The spec somewhere else already describes what the valid index of enabled vq.
> +The VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command is the same as the
> +VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command repeated for each
> virtqueue of receiveq1\ldots receiveqN.
> +
> +The VIRTIO_NET_CTRL_NOTF_COAL_TX_SET command is the same as the
> +VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command repeated for each
> virtqueue of transmitq1\ldots transmitqN.
> +
> +If coalescing parameters are being set, the device applies the last
> +coalescing parameters received for a virtqueue, regardless of the command
> used to deliver the parameters. For example:
used to set the parameters.
> +# Command sequence 1:
> +1. VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command with $
> \field{max_packets}
> += 15 $ and $ \field{max_usecs} = 10 $ 2.
> +VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command with $ \field{vqn} = 0 $, $
> +\field{max_packets} = 12 $ and $ \field{max_usecs} = 8 $ After #2 command is
> applied by the device, the coalescing parameters of receiveq1 are $
> \field{max_packets} = 12 $ and $
[virtio-dev] Re: [PATCH v3] virtio-net: support the virtqueue coalescing moderation
On Thu, Feb 16, 2023 at 10:43:01PM +0800, 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
> Reviewed-by: Xuan Zhuo
> ---
> 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
>
> content.tex | 130 ++--
> 1 file changed, 126 insertions(+), 4 deletions(-)
>
> diff --git a/content.tex b/content.tex
> index e863709..7f99503 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -3084,6 +3084,9 @@ \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 the virtqueue
> +notifications coalescing.
> +
> \item[VIRTIO_NET_F_NOTF_COAL(53)] Device supports notifications coalescing.
>
> \item[VIRTIO_NET_F_GUEST_USO4 (54)] Driver can receive USOv4 packets.
> @@ -3140,6 +3143,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}
> @@ -4501,8 +4505,11 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> Types / Network Device / Devi
> };
>
> #define VIRTIO_NET_CTRL_NOTF_COAL 6
> - #define VIRTIO_NET_CTRL_NOTF_COAL_TX_SET 0
> + #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:
> @@ -4514,12 +4521,67 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> Types / Network Device / Devi
> \end{itemize}
>
>
> -The class VIRTIO_NET_CTRL_NOTF_COAL has 2 commands:
> +The class VIRTIO_NET_CTRL_NOTF_COAL has 4 commands:
> \begin{enumerate}
> \item VIRTIO_NET_CTRL_NOTF_COAL_TX_SET: set the \field{tx_usecs} and
> \field{tx_max_packets} parameters.
> \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and
> \field{rx_max_packets} parameters.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET: set the \field{max_packets} and
> \field{max_usecs} parameters for a enabled
> +transmit/receive virtqueue whose
> number is \field{vqn}.
> +\item VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET: the device gets the
> \field{max_packets} and \field{max_usecs} parameters of
> +a enabled transmit/receive virtqueue
> whose number is \field{vqn}, and then
> +responds them to the driver.
> \end{enumerate}
>
> +If the VIRTIO_NET_F_VQ_NOTF_COAL feature is negotiated:
> +1. a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET command to set the
> coalescing
> + parameters of a enabled transmit/receive virtqueue.
> +2. a driver can send a VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET command to a device,
> and the device
> + responds to the driver with the coalescing parameters of a enabled
> transmit/receive virtqueue.
> +
> +\begin{lstlisting}
> +struct virtio_net_ctrl_coal_vq {
> +le16 vqn;
>
