Re: [virtio-comment] Re: [PATCH] virtio-net: support per-queue coalescing moderation

[Heng Qi <hengqi@linux.alibaba.com>]

å 2023/2/7 äå10:29, Michael S. Tsirkin åé:
> On Tue, Feb 07, 2023 at 12:51:26PM +0000, Parav Pandit wrote:
> > > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > Sent: Tuesday, February 7, 2023 6:50 AM
> > >
> > > On Tue, 7 Feb 2023 13:25:13 +0200, Alvaro Karsz <alvaro.karsz@solid-run.com>
> > > wrote:
> > > > Hi Heng,
> > > >
> > > > > Currently, the coalescing profile is directly applied to all queues.
> > > > > This patch supports configuring the parameters for a specified queue.
> > > > >
> > > > > 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 <hengqi@linux.alibaba.com>
> > > > > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > ---
> > > > >   content.tex | 49 ++++++++++++++++++++++++++++++++++++++++++-------
> > > > >   1 file changed, 42 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/content.tex b/content.tex index e863709..049c0e4 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_PERQUEUE_NOTF_COAL(52)] Device supports per-
> > > queue
> > > > > +       notifications coalescing.
> > > > > +
> > > > Since this feature allows us to change the coalescing parameters for
> > > > all the queues when rx/tx_qid = 0xFFFF, and since version 1.3 wasn't
> > > > released yet, maybe the "per-vq" functionality can be added to
> > > > VIRTIO_NET_F_NOTF_COAL instead of adding a new feature?
> > >
> > > According to my understanding, all the features of voting are formal. It can be
> > > used by the manufacturer.
> > >
> > > Of course, as far as I know, no manufacturer has used this feature for the time
> > > being. But I think we should add a new feature.
> > >
> > > Or other people have other ideas.
> > I believe we should treat it as fix and avoid a new feature bit as spec is not released, and it is very recent change.
> Arguably it's true.
> It will all be up to the committee vote of course ...
> To keep things a bit safer how about we just allow commands
> without qid instead of a special qid value?

Good idea, the new command seems to make compatibility easier to achieve.
An error can be returned when the old device does not recognize the new 
command.

> Also if we are going to change things how about adding a query command too?

Yes, we should.

Thanks.

> Also Alvaro what is your take on whether the gloabal version counts
> packets and time for all queues together or per queue? The spec
> does not make it clear ATM.