[virtio-dev] Re: [PATCH v2] virtio-net: support the virtqueue coalescing moderation
On Sun, Feb 12, 2023 at 04:35:37AM -0500, Michael S. Tsirkin wrote: > On Sat, Feb 11, 2023 at 01:47:16PM +, Parav Pandit wrote: > > > > > > > From: Alvaro Karsz > > > Sent: Saturday, February 11, 2023 3:45 AM > > > > > > > Please add short description something like, > > > > > > > > When the driver prefers to use per virtqueue notifications coalescing, > > > > and if > > > queue group (transmit or receive) level notification coalescing is > > > enabled, driver > > > SHOULD first disable device level notification coalescing. > > > > Or it should be, > > > > > > > > > > I disagree here. > > > IMO "queue group level notification coalescing" is not something to > > > enable or > > > disable, but a shortcut to set all TX/RX queues at once. > > That short cut is the enable/disablement. > > > > > Why should the spec force a driver to "disable device level notification > > > coalescing" (I assume you mean send a > > > VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)? > > Yes. Because to have well defined behavior when sw configured both one > > after the another. > > > > > What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET > > > command, and then a single queue traffic increases? why should it zero the > > > parameters to all other queues? > > That is short transition when driver is switching over to per queue mode. > > This is fine to have short glitch. > > > > > I think that this should be discussed in the driver implementation stage, > > > not in > > > the spec. > > > > > There should be a clear guidance on how device should behave when both per > > q and per device are configured. > > > > > > Virtqueue level notifications coalescing, and device level > > > > notifications can be > > > enabled together. > > > > When both of them are enabled, per virtqueue notifications coalescing > > > > take > > > priority over queue group level. > > > > > > How do you enable Virtqueue level notifications coalescing? Why are they > > > different entities? > > Using the new command that has vqn in it. > > > > > I don't think that we should have priorities, but the last command should > > > be the > > > one that dictates the coalescing parameters. > > > > > Priority is applicable when driver has issued both the commands. Per tx/rx, > > and per vqn. > > > > > For example, let's look at vq0 (RX): > > > Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change > > > the parameters accordingly (all RX vqs should do the same). > > > Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, > > > vq0 changes the parameters accordingly (all RX vqs are still using the > > > "old" > > > parameters) Then device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 > > > changes the parameters accordingly (all RX vqs should do the same). > > In this example, per VQ were overridden with per device. > > Yes, so the last one is applicable, so priority of last one applies. > > > > We continue to refuse to add the mode, and hence need to supply these > > description of both the sequence on how device should behave. > > > > Sequence_1: > > 1. tx/rx group level > > 2. per vqn level > > When #2 is done, VQ's whose per vq level is configured, follows vqn, rest > > of the VQs follow #1. > > > > Sequence_2: > > 1. per vqn > > 2. tx/rx group level > > When #2 is done, group level overrides the per vqn parameters. > > Since there's apparently some room for misunderstanding, I think adding these > examples can't hurt. Ok, I'll handle this. > I would also be more specific and just use specific numbers in the > example, to avoid any ambiguity. Agree. Thanks. > > -- > MST - 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 v2] virtio-net: support the virtqueue coalescing moderation
On Sat, Feb 11, 2023 at 01:47:16PM +, Parav Pandit wrote: > > > > From: Alvaro Karsz > > Sent: Saturday, February 11, 2023 3:45 AM > > > > > Please add short description something like, > > > > > > When the driver prefers to use per virtqueue notifications coalescing, > > > and if > > queue group (transmit or receive) level notification coalescing is enabled, > > driver > > SHOULD first disable device level notification coalescing. > > > Or it should be, > > > > > > > I disagree here. > > IMO "queue group level notification coalescing" is not something to enable > > or > > disable, but a shortcut to set all TX/RX queues at once. > That short cut is the enable/disablement. > > > Why should the spec force a driver to "disable device level notification > > coalescing" (I assume you mean send a > > VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)? > Yes. Because to have well defined behavior when sw configured both one after > the another. > > > What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET > > command, and then a single queue traffic increases? why should it zero the > > parameters to all other queues? > That is short transition when driver is switching over to per queue mode. > This is fine to have short glitch. > > > I think that this should be discussed in the driver implementation stage, > > not in > > the spec. > > > There should be a clear guidance on how device should behave when both per q > and per device are configured. > > > > Virtqueue level notifications coalescing, and device level notifications > > > can be > > enabled together. > > > When both of them are enabled, per virtqueue notifications coalescing take > > priority over queue group level. > > > > How do you enable Virtqueue level notifications coalescing? Why are they > > different entities? > Using the new command that has vqn in it. > > > I don't think that we should have priorities, but the last command should > > be the > > one that dictates the coalescing parameters. > > > Priority is applicable when driver has issued both the commands. Per tx/rx, > and per vqn. > > > For example, let's look at vq0 (RX): > > Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change > > the parameters accordingly (all RX vqs should do the same). > > Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, > > vq0 changes the parameters accordingly (all RX vqs are still using the "old" > > parameters) Then device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 > > changes the parameters accordingly (all RX vqs should do the same). > In this example, per VQ were overridden with per device. > Yes, so the last one is applicable, so priority of last one applies. > > We continue to refuse to add the mode, and hence need to supply these > description of both the sequence on how device should behave. > > Sequence_1: > 1. tx/rx group level > 2. per vqn level > When #2 is done, VQ's whose per vq level is configured, follows vqn, rest of > the VQs follow #1. > > Sequence_2: > 1. per vqn > 2. tx/rx group level > When #2 is done, group level overrides the per vqn parameters. Adding examples of the two command sequences is due, I will do so in the next release. :) Thanks. - 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 v2] virtio-net: support the virtqueue coalescing moderation
On Fri, Feb 10, 2023 at 07:36:03PM +, Parav Pandit wrote:
>
>
> > From: Heng Qi
> > Sent: Friday, February 10, 2023 2:02 AM
> >
> > Currently, the coalescing profile is directly applied to all queues.
>
> Say it,
> Currently coalescing parameters are grouped for all transmit and receive
> virtqueues.
>
> > This patch supports setting or getting the parameters for a specified
> > queue, and
> > a typical application of this function is NetDIM.
> Many of us know the net dim.
> But if you prefer to mention it here, better to have the link for it.
>
> Please add pointer to it.
> [1] https://docs.kernel.org/networking/net_dim.html
>
> >
> > When the traffic between queues is unbalanced, for example, one queue is
> > busy and another queue is idle, then it will be very useful to control
> > coalescing
> > parameters at the queue granularity.
> >
> >
> > +If additionally VIRTIO_NET_F_VQ_NOTF_COAL is negotiated, the driver can
> > +send control commands to set or get the coalescing parameters of a
> Control command singular?
why? driver can send any number of commands e.g. to different vqs.
> > +specified virtqueue (excluding the control virtqueue).
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_ctrl_coal_vq {
> > +le32 max_packets;
> > +le32 usecs;
> > +le16 vqn;
> > +};
> > +
> Change that Michael suggest restructuring and under same class looks good to
> me.
>
> > +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ 7
> > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 0 #define
> > +VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 1 \end{lstlisting}
> > +
> > +Virtqueue coalescing parameters:
> > +\begin{itemize}
> > +\item \field{max_packets}: The maximum number of packets sent/received by
> > the
> > +specified virtqueue before a TX/RX notification.
> > +\item \field{usecs}: The maximum number of TX/RX usecs that the specified
> > +virtqueue delays a TX/RX notification.
> > +\item \field{vqn}: The virtqueue number of the specified virtqueue.
> > +\end{itemize}
> > +
> The virtqueue number of the enabled transmit or receive virtuqueue.
> This will simplify below description.
>
> > +The range of \filed{vqn} is between 0 and 0x inclusive, $ \lfloor
> > +vqn / 2 \rfloor $ is the index of the corresponding receiveq, and
> > +$\lfloor (vqn / 2) + 1 \rfloor $ is the corresponding tranmitq.
> > +
>
> Please add short description something like,
>
> When the driver prefers to use per virtqueue notifications coalescing, and if
> queue group (transmit or receive) level notification coalescing is enabled,
> driver SHOULD first disable device level notification coalescing.
> Or it should be,
>
> Virtqueue level notifications coalescing, and device level notifications can
> be enabled together.
> When both of them are enabled, per virtqueue notifications coalescing take
> priority over queue group level.
Note that neither of these reflects what I proposed.
I proposed explaining that VIRTIO_NET_CTRL_NOTF_COAL_TX_SET and
VIRTIO_NET_CTRL_NOTF_COAL_RX_SET have the same effect as
repeatedly calling VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET for all TX/RX vqs.
> With rests of the comments from Michael and Alvaro in progress, looks good.
-
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 v2] virtio-net: support the virtqueue coalescing moderation
On Sat, Feb 11, 2023 at 01:47:16PM +, Parav Pandit wrote: > > > > From: Alvaro Karsz > > Sent: Saturday, February 11, 2023 3:45 AM > > > > > Please add short description something like, > > > > > > When the driver prefers to use per virtqueue notifications coalescing, > > > and if > > queue group (transmit or receive) level notification coalescing is enabled, > > driver > > SHOULD first disable device level notification coalescing. > > > Or it should be, > > > > > > > I disagree here. > > IMO "queue group level notification coalescing" is not something to enable > > or > > disable, but a shortcut to set all TX/RX queues at once. > That short cut is the enable/disablement. > > > Why should the spec force a driver to "disable device level notification > > coalescing" (I assume you mean send a > > VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)? > Yes. Because to have well defined behavior when sw configured both one after > the another. > > > What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET > > command, and then a single queue traffic increases? why should it zero the > > parameters to all other queues? > That is short transition when driver is switching over to per queue mode. > This is fine to have short glitch. > > > I think that this should be discussed in the driver implementation stage, > > not in > > the spec. > > > There should be a clear guidance on how device should behave when both per q > and per device are configured. > > > > Virtqueue level notifications coalescing, and device level notifications > > > can be > > enabled together. > > > When both of them are enabled, per virtqueue notifications coalescing take > > priority over queue group level. > > > > How do you enable Virtqueue level notifications coalescing? Why are they > > different entities? > Using the new command that has vqn in it. > > > I don't think that we should have priorities, but the last command should > > be the > > one that dictates the coalescing parameters. > > > Priority is applicable when driver has issued both the commands. Per tx/rx, > and per vqn. > > > For example, let's look at vq0 (RX): > > Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change > > the parameters accordingly (all RX vqs should do the same). > > Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, > > vq0 changes the parameters accordingly (all RX vqs are still using the "old" > > parameters) Then device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 > > changes the parameters accordingly (all RX vqs should do the same). > In this example, per VQ were overridden with per device. > Yes, so the last one is applicable, so priority of last one applies. > > We continue to refuse to add the mode, and hence need to supply these > description of both the sequence on how device should behave. > > Sequence_1: > 1. tx/rx group level > 2. per vqn level > When #2 is done, VQ's whose per vq level is configured, follows vqn, rest of > the VQs follow #1. > > Sequence_2: > 1. per vqn > 2. tx/rx group level > When #2 is done, group level overrides the per vqn parameters. Since there's apparently some room for misunderstanding, I think adding these examples can't hurt. I would also be more specific and just use specific numbers in the example, to avoid any ambiguity. -- MST - 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 v2] virtio-net: support the virtqueue coalescing moderation
> From: Alvaro Karsz > Sent: Saturday, February 11, 2023 3:45 AM > > > Please add short description something like, > > > > When the driver prefers to use per virtqueue notifications coalescing, and > > if > queue group (transmit or receive) level notification coalescing is enabled, > driver > SHOULD first disable device level notification coalescing. > > Or it should be, > > > > I disagree here. > IMO "queue group level notification coalescing" is not something to enable or > disable, but a shortcut to set all TX/RX queues at once. That short cut is the enable/disablement. > Why should the spec force a driver to "disable device level notification > coalescing" (I assume you mean send a > VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)? Yes. Because to have well defined behavior when sw configured both one after the another. > What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET > command, and then a single queue traffic increases? why should it zero the > parameters to all other queues? That is short transition when driver is switching over to per queue mode. This is fine to have short glitch. > I think that this should be discussed in the driver implementation stage, not > in > the spec. > There should be a clear guidance on how device should behave when both per q and per device are configured. > > Virtqueue level notifications coalescing, and device level notifications > > can be > enabled together. > > When both of them are enabled, per virtqueue notifications coalescing take > priority over queue group level. > > How do you enable Virtqueue level notifications coalescing? Why are they > different entities? Using the new command that has vqn in it. > I don't think that we should have priorities, but the last command should be > the > one that dictates the coalescing parameters. > Priority is applicable when driver has issued both the commands. Per tx/rx, and per vqn. > For example, let's look at vq0 (RX): > Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change > the parameters accordingly (all RX vqs should do the same). > Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, > vq0 changes the parameters accordingly (all RX vqs are still using the "old" > parameters) Then device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 > changes the parameters accordingly (all RX vqs should do the same). In this example, per VQ were overridden with per device. Yes, so the last one is applicable, so priority of last one applies. We continue to refuse to add the mode, and hence need to supply these description of both the sequence on how device should behave. Sequence_1: 1. tx/rx group level 2. per vqn level When #2 is done, VQ's whose per vq level is configured, follows vqn, rest of the VQs follow #1. Sequence_2: 1. per vqn 2. tx/rx group level When #2 is done, group level overrides the per vqn parameters.
[virtio-dev] Re: [PATCH v2] virtio-net: support the virtqueue coalescing moderation
> > Please add short description something like, > > > > When the driver prefers to use per virtqueue notifications coalescing, and > > if queue group (transmit or receive) level notification coalescing is > > enabled, driver SHOULD first disable device level notification coalescing. > > Or it should be, > > > > I disagree here. > IMO "queue group level notification coalescing" is not something to > enable or disable, but a shortcut to set all TX/RX queues at once. > Why should the spec force a driver to "disable device level > notification coalescing" (I assume you mean send a > VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)? > What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET > command, and then a single queue traffic increases? why should it zero > the parameters to all other queues? > I think that this should be discussed in the driver implementation > stage, not in the spec. > > > Virtqueue level notifications coalescing, and device level notifications > > can be enabled together. > > When both of them are enabled, per virtqueue notifications coalescing take > > priority over queue group level. > > How do you enable Virtqueue level notifications coalescing? Why are > they different entities? > I don't think that we should have priorities, but the last command > should be the one that dictates the coalescing parameters. > > For example, let's look at vq0 (RX): > Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change > the parameters accordingly (all RX vqs should do the same). > Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, > vq0 changes the parameters accordingly (all RX vqs are still using the > "old" parameters) Opps, I missed a word in here, it should have been: Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, vq0 changes the parameters accordingly (all RX vqs EXCEPT FOR vq0 still use the previous parameters) - 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 v2] virtio-net: support the virtqueue coalescing moderation
> Please add short description something like, > > When the driver prefers to use per virtqueue notifications coalescing, and if > queue group (transmit or receive) level notification coalescing is enabled, > driver SHOULD first disable device level notification coalescing. > Or it should be, > I disagree here. IMO "queue group level notification coalescing" is not something to enable or disable, but a shortcut to set all TX/RX queues at once. Why should the spec force a driver to "disable device level notification coalescing" (I assume you mean send a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command with zeros)? What if the driver sends a VIRTIO_NET_CTRL_NOTF_COAL_[T/R]X_SET command, and then a single queue traffic increases? why should it zero the parameters to all other queues? I think that this should be discussed in the driver implementation stage, not in the spec. > Virtqueue level notifications coalescing, and device level notifications can > be enabled together. > When both of them are enabled, per virtqueue notifications coalescing take > priority over queue group level. How do you enable Virtqueue level notifications coalescing? Why are they different entities? I don't think that we should have priorities, but the last command should be the one that dictates the coalescing parameters. For example, let's look at vq0 (RX): Device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 should change the parameters accordingly (all RX vqs should do the same). Then device receives VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET with vqn = 0, vq0 changes the parameters accordingly (all RX vqs are still using the "old" parameters) Then device receives VIRTIO_NET_CTRL_NOTF_COAL_RX_SET, vq0 changes the parameters accordingly (all RX vqs should do the same). - 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 v2] virtio-net: support the virtqueue coalescing moderation
> From: Heng Qi
> Sent: Friday, February 10, 2023 2:02 AM
>
> Currently, the coalescing profile is directly applied to all queues.
Say it,
Currently coalescing parameters are grouped for all transmit and receive
virtqueues.
> This patch supports setting or getting the parameters for a specified queue,
> and
> a typical application of this function is NetDIM.
Many of us know the net dim.
But if you prefer to mention it here, better to have the link for it.
Please add pointer to it.
[1] https://docs.kernel.org/networking/net_dim.html
>
> When the traffic between queues is unbalanced, for example, one queue is
> busy and another queue is idle, then it will be very useful to control
> coalescing
> parameters at the queue granularity.
>
>
> +If additionally VIRTIO_NET_F_VQ_NOTF_COAL is negotiated, the driver can
> +send control commands to set or get the coalescing parameters of a
Control command singular?
> +specified virtqueue (excluding the control virtqueue).
> +
> +\begin{lstlisting}
> +struct virtio_net_ctrl_coal_vq {
> +le32 max_packets;
> +le32 usecs;
> +le16 vqn;
> +};
> +
Change that Michael suggest restructuring and under same class looks good to me.
> +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ 7
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 0 #define
> +VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 1 \end{lstlisting}
> +
> +Virtqueue coalescing parameters:
> +\begin{itemize}
> +\item \field{max_packets}: The maximum number of packets sent/received by
> the
> +specified virtqueue before a TX/RX notification.
> +\item \field{usecs}: The maximum number of TX/RX usecs that the specified
> +virtqueue delays a TX/RX notification.
> +\item \field{vqn}: The virtqueue number of the specified virtqueue.
> +\end{itemize}
> +
The virtqueue number of the enabled transmit or receive virtuqueue.
This will simplify below description.
> +The range of \filed{vqn} is between 0 and 0x inclusive, $ \lfloor
> +vqn / 2 \rfloor $ is the index of the corresponding receiveq, and
> +$\lfloor (vqn / 2) + 1 \rfloor $ is the corresponding tranmitq.
> +
Please add short description something like,
When the driver prefers to use per virtqueue notifications coalescing, and if
queue group (transmit or receive) level notification coalescing is enabled,
driver SHOULD first disable device level notification coalescing.
Or it should be,
Virtqueue level notifications coalescing, and device level notifications can be
enabled together.
When both of them are enabled, per virtqueue notifications coalescing take
priority over queue group level.
With rests of the comments from Michael and Alvaro in progress, looks good.
-
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 v2] virtio-net: support the virtqueue coalescing moderation
On Fri, Feb 10, 2023 at 11:30:40AM +0200, Alvaro Karsz wrote:
> > > \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 the VIRTIO_NET_CTRL_NOTF_COAL commands with
> > > VIRTIO_NET_ERR if it was not able to change the parameters or
> > > +was not able to find a virtqueue using the \field{vqn}.
> >
> > First please create multiple statements not a single long one.
> > Second vqn only exists for per vq commands so create a statement
> > just for that.
> > Also more explicit please. What does this mean? I suspect that vq was not
> > enabled?
> > Also, we MUST have vqn < max_virtqueue_pairs.
> >
>
> I believe that you meant vqn < max_virtqueue_pairs * 2
Yes thanks!
Just pls snip to just the text you qoute to just relevant parts -
I had to scroll like 300 lines to get to the relevant one :)
--
MST
-
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 v2] virtio-net: support the virtqueue coalescing moderation
> On Fri, Feb 10, 2023 at 03:01:30PM +0800, Heng Qi wrote:
> > Currently, the coalescing profile is directly applied to all queues.
> > This patch supports setting or getting the parameters for a specified queue,
> > and a typical application of this function is NetDIM.
> >
> > When the traffic between queues is unbalanced, for example, one queue
> > is busy and another queue is idle, then it will be very useful to
> > control coalescing parameters at the queue granularity.
> >
> > Signed-off-by: Heng Qi
> > Reviewed-by: Xuan Zhuo
>
>
> I like this generally, thanks! Language needs to be tightened a bit.
> > ---
> > 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 | 69 -
> > 1 file changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/content.tex b/content.tex
> > index e863709..2c497e1 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 and
> > VIRTIO_NET_F_NOTF_COAL.
> > \end{description}
> >
> > \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types /
> > Network Device / Feature bits / Legacy Interface: Feature bits}
> > @@ -4520,6 +4524,49 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> > Types / Network Device / Devi
> > \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and
> > \field{rx_max_packets} parameters.
> > \end{enumerate}
> >
> > +If additionally VIRTIO_NET_F_VQ_NOTF_COAL is negotiated, the driver can
> > send
> > +control commands to set or get the coalescing parameters of a specified
> > +virtqueue (excluding the control virtqueue).
> > +
> > +\begin{lstlisting}
> > +struct virtio_net_ctrl_coal_vq {
> > +le32 max_packets;
> > +le32 usecs;
> > +le16 vqn;
>
> Add le16 reserved here, so keep things aligned naturally.
> In fact if you want to support GET you need to re-order things
> since write buffers come before read buffers:
>
> +le16 vqn;
> +le16 reserved;
>
> +le32 max_packets;
> +le32 usecs;
>
> see below for more explanation.
>
>
>
> > +};
> > +
> > +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ 7
> > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 0
> > + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 1
> > +\end{lstlisting}
> > +
> > +Virtqueue coalescing parameters:
> > +\begin{itemize}
> > +\item \field{max_packets}: The maximum number of packets sent/received by
> > the
> > +specified virtqueue before a TX/RX notification.
> > +\item \field{usecs}: The maximum number of TX/RX usecs that the specified
> > +virtqueue delays a TX/RX notification.
> > +\item \field{vqn}: The virtqueue number of the specified virtqueue.
> > +\end{itemize}
> > +
> > +The range of \filed{vqn} is between 0 and 0x inclusive,
>
>
> No really because last vq is a cvq. Maybe just drop this?
>
> > $ \lfloor vqn / 2 \rfloor $
> > +is the index of the corresponding receiveq, and $\lfloor (vqn / 2) + 1
> > \rfloor $ is
> > +the corresponding tranmitq.
> > +
> > +The VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command is the same as calling
> > VIRTIO_NET_CTRL_NOTF_COAL_VQ
>
> you don't "call" commands. driver sends them, device receives them.
> But here you are talking about a command abstracty so I'd just drop
> a verb, or maybe "repeating".
> And "same" is inexact in that it's not the same - takes more time - it
> just has the same effect, or is equivalent to.
>
>
> > +for virtqueues corresponding to all receiveqs.
>
> receiveqs is confusing.
>
> Also elsewhere we use the language receiveq1\ldots receiveqn
> which seems clearer.
[virtio-dev] Re: [PATCH v2] virtio-net: support the virtqueue coalescing moderation
On Fri, Feb 10, 2023 at 03:01:30PM +0800, Heng Qi wrote:
> Currently, the coalescing profile is directly applied to all queues.
> This patch supports setting or getting the parameters for a specified queue,
> and a typical application of this function is NetDIM.
>
> When the traffic between queues is unbalanced, for example, one queue
> is busy and another queue is idle, then it will be very useful to
> control coalescing parameters at the queue granularity.
>
> Signed-off-by: Heng Qi
> Reviewed-by: Xuan Zhuo
I like this generally, thanks! Language needs to be tightened a bit.
> ---
> 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 | 69 -
> 1 file changed, 68 insertions(+), 1 deletion(-)
>
> diff --git a/content.tex b/content.tex
> index e863709..2c497e1 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 and
> VIRTIO_NET_F_NOTF_COAL.
> \end{description}
>
> \subsubsection{Legacy Interface: Feature bits}\label{sec:Device Types /
> Network Device / Feature bits / Legacy Interface: Feature bits}
> @@ -4520,6 +4524,49 @@ \subsubsection{Control Virtqueue}\label{sec:Device
> Types / Network Device / Devi
> \item VIRTIO_NET_CTRL_NOTF_COAL_RX_SET: set the \field{rx_usecs} and
> \field{rx_max_packets} parameters.
> \end{enumerate}
>
> +If additionally VIRTIO_NET_F_VQ_NOTF_COAL is negotiated, the driver can send
> +control commands to set or get the coalescing parameters of a specified
> +virtqueue (excluding the control virtqueue).
> +
> +\begin{lstlisting}
> +struct virtio_net_ctrl_coal_vq {
> +le32 max_packets;
> +le32 usecs;
> +le16 vqn;
Add le16 reserved here, so keep things aligned naturally.
In fact if you want to support GET you need to re-order things
since write buffers come before read buffers:
+le16 vqn;
+le16 reserved;
+le32 max_packets;
+le32 usecs;
see below for more explanation.
> +};
> +
> +#define VIRTIO_NET_CTRL_NOTF_COAL_VQ 7
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET 0
> + #define VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET 1
> +\end{lstlisting}
> +
> +Virtqueue coalescing parameters:
> +\begin{itemize}
> +\item \field{max_packets}: The maximum number of packets sent/received by the
> +specified virtqueue before a TX/RX notification.
> +\item \field{usecs}: The maximum number of TX/RX usecs that the specified
> +virtqueue delays a TX/RX notification.
> +\item \field{vqn}: The virtqueue number of the specified virtqueue.
> +\end{itemize}
> +
> +The range of \filed{vqn} is between 0 and 0x inclusive,
No really because last vq is a cvq. Maybe just drop this?
> $ \lfloor vqn / 2 \rfloor $
> +is the index of the corresponding receiveq, and $\lfloor (vqn / 2) + 1
> \rfloor $ is
> +the corresponding tranmitq.
> +
> +The VIRTIO_NET_CTRL_NOTF_COAL_RX_SET command is the same as calling
> VIRTIO_NET_CTRL_NOTF_COAL_VQ
you don't "call" commands. driver sends them, device receives them.
But here you are talking about a command abstracty so I'd just drop
a verb, or maybe "repeating".
And "same" is inexact in that it's not the same - takes more time - it
just has the same effect, or is equivalent to.
> +for virtqueues corresponding to all receiveqs.
receiveqs is confusing.
Also elsewhere we use the language receiveq1\ldots receiveqn
which seems clearer. Also you can not call VIRTIO_NET_CTRL_NOTF_COAL_VQ
for all vqs - it applies to one vq only. You mean each.
And virtqueues do not correspond to receiveqs - they
are receiveqs. If you like vqn corresponds to them.
Or better just "same as
