Xuan Zhuo
2023-Jun-06 01:52 UTC
[RFC PATCH net] virtio_net: Prevent napi_weight changes with VIRTIO_NET_F_NOTF_COAL support
On Mon, 5 Jun 2023 14:02:36 -0700, Brett Creeley <brett.creeley at amd.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. 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 at amd.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 >
Jason Wang
2023-Jun-06 02:03 UTC
[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 at linux.alibaba.com> wrote:> > On Mon, 5 Jun 2023 14:02:36 -0700, Brett Creeley <brett.creeley at amd.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 at amd.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 > > >