[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CACGkMEuCGPWJVG0knTnG-1YxbACZLXLTrFMqt=eKiV-K_B_8FA@mail.gmail.com>
Date: Tue, 6 Jun 2023 10:03:19 +0800
From: Jason Wang <jasowang@...hat.com>
To: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
Cc: Brett Creeley <brett.creeley@....com>, shannon.nelson@....com,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
virtualization@...ts.linux-foundation.org,
alvaro.karsz@...id-run.com, pabeni@...hat.com, kuba@...nel.org,
edumazet@...gle.com, davem@...emloft.net, mst@...hat.com
Subject: Re: [RFC PATCH net] virtio_net: Prevent napi_weight changes with
VIRTIO_NET_F_NOTF_COAL support
On Tue, Jun 6, 2023 at 9:57 AM Xuan Zhuo <xuanzhuo@...ux.alibaba.com> wrote:
>
> On Mon, 5 Jun 2023 14:02:36 -0700, Brett Creeley <brett.creeley@....com> wrote:
> > Commit 699b045a8e43 ("net: virtio_net: notifications coalescing
> > support") added support for VIRTIO_NET_F_NOTF_COAL. The get_coalesce
> > call made changes to report "1" in tx_max_coalesced_frames if
> > VIRTIO_NET_F_NOTF_COAL is not supported and napi.weight is non-zero.
> > However, the napi_weight value could still be changed by the
> > set_coalesce call regardless of whether or not the device supports
> > VIRTIO_NET_F_NOTF_COAL.
> >
> > It seems like the tx_max_coalesced_frames value should not control more
> > than 1 thing (i.e. napi_weight and the device's tx_max_packets). So, fix
> > this by only allowing the napi_weight change if VIRTIO_NET_F_NOTF_COAL
> > is not supported by the virtio device.
>
>
> @Jason I wonder should we keep this function to change the napi weight by the
> coalesec command.
I think so, explained in another thread.
Thanks
>
> Thanks.
>
> >
> > It wasn't clear to me if this was the intended behavior, so that's why
> > I'm sending this as an RFC patch initially. Based on the feedback, I
> > will resubmit as an official patch.
> >
> > Fixes: 699b045a8e43 ("net: virtio_net: notifications coalescing support")
> > Signed-off-by: Brett Creeley <brett.creeley@....com>
> > ---
> > drivers/net/virtio_net.c | 24 +++++++++++++-----------
> > 1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 486b5849033d..e28387866909 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -2990,19 +2990,21 @@ static int virtnet_set_coalesce(struct net_device *dev,
> > int ret, i, napi_weight;
> > bool update_napi = false;
> >
> > - /* Can't change NAPI weight if the link is up */
> > - napi_weight = ec->tx_max_coalesced_frames ? NAPI_POLL_WEIGHT : 0;
> > - if (napi_weight ^ vi->sq[0].napi.weight) {
> > - if (dev->flags & IFF_UP)
> > - return -EBUSY;
> > - else
> > - update_napi = true;
> > - }
> > -
> > - if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL))
> > + if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_NOTF_COAL)) {
> > ret = virtnet_send_notf_coal_cmds(vi, ec);
> > - else
> > + } else {
> > + /* Can't change NAPI weight if the link is up */
> > + napi_weight = ec->tx_max_coalesced_frames ?
> > + NAPI_POLL_WEIGHT : 0;
> > + if (napi_weight ^ vi->sq[0].napi.weight) {
> > + if (dev->flags & IFF_UP)
> > + return -EBUSY;
> > + else
> > + update_napi = true;
> > + }
> > +
> > ret = virtnet_coal_params_supported(ec);
> > + }
> >
> > if (ret)
> > return ret;
> > --
> > 2.17.1
> >
>
Powered by blists - more mailing lists