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

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

å 2023/2/8 äå10:43, Parav Pandit åé:
> > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Sent: Tuesday, February 7, 2023 9:25 PM
> >
> > On Wed, 8 Feb 2023 02:20:27 +0000, Parav Pandit <parav@nvidia.com> wrote:
> > > > From: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > Sent: Tuesday, February 7, 2023 8:46 PM
> > > >
> > > > On Tue, 7 Feb 2023 09:06:19 -0500, "Michael S. Tsirkin"
> > > > <mst@redhat.com>
> > > > wrote:
> > > > > On Tue, Feb 07, 2023 at 07:16:34PM +0800, Heng Qi wrote:
> > > > > > 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.
> > > > > ethtool does not support this though, does it? what's the plan?
> > > >
> > > > Although it can be done, I think it is difficult to let users use
> > > > ethtool to modify the parameters of each queue.
> > > >
> > > > At present ethtool supports adaptive-rx/adaptive-tx. This is that
> > > > the driver automatically adapted to the appropriate parameter.
> > > > Generally, it is implemented using netdim in the driver. At this time, this
> > interface is needed.
> > > > > > Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
> > > > > > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > > > > What I dislike about this interface is that if
> > > > > VIRTIO_NET_F_PERQUEUE_NOTF_COAL is negotiated, then in the
> > common
> > > > case
> > > > > of same parameters for all queues driver has to issue multiple
> > > > > commands.
> > > > > I can see either a special vq index (0xffff ?) or a special
> > > > > command used to set it for all queues.
> > > > Although the structure is very similar, in fact, adding a new
> > > > command may be clearer.
> > > O(N) loop is not that bad when user want to issue change it per device, as this
> > is something done very often.
> > > Once per VQ is supported, driver may just use the default net-dim to have
> > best out of box experience, whenever device supports it.
> >
> >
> > So you mean, we only support the parameters based on Per-Queue. My
> > original idea is to support two methods at the same time.
> Yes, if we want to support both the modes, that better to have such command.
> Because otherwise the device implementation is bit clumsy which needs to do the guess work.

I think whether it is a per-queue parameters or a per-device parameters, 
the latter should override the first, because we didn't make too many 
guesses about which is more important.

> Better to avoid such complexity in device around implicit and confusing behavior.
>
> I see two options.
> 1. Just have per VQ params. Software has the full knowledge of in which it is operating, and state remains at software level.
> This effectively achieves both the mode.
>
> 2. Have a mode cmd,
> Mode = (a) per device or (b) per VQ (c) disable
> After the mode is set, driver can set per device or per VQ.

I find this more clear.

Thanks.