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

[Parav Pandit <parav@nvidia.com>]

> From: Michael S. Tsirkin <mst@redhat.com>
> Sent: Wednesday, February 8, 2023 4:56 AM
> 
> On Wed, Feb 08, 2023 at 10:20:08AM +0800, Heng Qi wrote:
> >
> >
> > å 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.
> 
> Not that is a bad way to figure out what is supported. E.g. driver can't
> reasonably probe a ton of commands just to check there's compatibility for
> migration. If it's optional pls gate by feature bit or some such.

There is no need to issue ton of commands.
One command can return lot of capabilities.
But it is different discussion. Not applicable here.


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

> > 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.
> > >