netdev - Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1722428723.505313-1-hengqi@linux.alibaba.com>
Date: Wed, 31 Jul 2024 20:25:23 +0800
From: Heng Qi <hengqi@...ux.alibaba.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: netdev@...r.kernel.org,
 Jason Wang <jasowang@...hat.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
 "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>,
 Paolo Abeni <pabeni@...hat.com>,
 virtualization@...ts.linux.dev,
 EugenioPérez <eperezma@...hat.com>
Subject: Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated

On Wed, 31 Jul 2024 08:14:43 -0400, "Michael S. Tsirkin" <mst@...hat.com> wrote:
> On Wed, Jul 31, 2024 at 08:07:17PM +0800, Heng Qi wrote:
> > >From the virtio spec:
> > 
> > 	The driver MUST have negotiated the VIRTIO_NET_F_VQ_NOTF_COAL
> > 	feature when issuing commands VIRTIO_NET_CTRL_NOTF_COAL_VQ_SET
> > 	and VIRTIO_NET_CTRL_NOTF_COAL_VQ_GET.
> > 
> > The driver must not send vq notification coalescing commands if
> > VIRTIO_NET_F_VQ_NOTF_COAL is not negotiated. This limitation of course
> > applies to vq resize.
> > 
> > Fixes: f61fe5f081cf ("virtio-net: fix the vq coalescing setting for vq resize")
> > Signed-off-by: Heng Qi <hengqi@...ux.alibaba.com>
> > Reviewed-by: Xuan Zhuo <xuanzhuo@...ux.alibaba.com>
> > Acked-by: Eugenio Pé rez <eperezma@...hat.com>
> > Acked-by: Jason Wang <jasowang@...hat.com>
> > ---
> > v1->v2:
> >  - Rephrase the subject.
> >  - Put the feature check inside the virtnet_send_{r,t}x_ctrl_coal_vq_cmd().
> > 
> >  drivers/net/virtio_net.c | 10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index 0383a3e136d6..2b566d893ea3 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -3658,6 +3658,9 @@ static int virtnet_send_rx_ctrl_coal_vq_cmd(struct virtnet_info *vi,
> >  {
> >  	int err;
> >  
> > +	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
> > +		return -EOPNOTSUPP;
> > +
> >  	err = virtnet_send_ctrl_coal_vq_cmd(vi, rxq2vq(queue),
> >  					    max_usecs, max_packets);
> >  	if (err)
> > @@ -3675,6 +3678,9 @@ static int virtnet_send_tx_ctrl_coal_vq_cmd(struct virtnet_info *vi,
> >  {
> >  	int err;
> >  
> > +	if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_VQ_NOTF_COAL))
> > +		return -EOPNOTSUPP;
> > +
> >  	err = virtnet_send_ctrl_coal_vq_cmd(vi, txq2vq(queue),
> >  					    max_usecs, max_packets);
> >  	if (err)
> > @@ -3743,7 +3749,7 @@ static int virtnet_set_ringparam(struct net_device *dev,
> >  			err = virtnet_send_tx_ctrl_coal_vq_cmd(vi, i,
> >  							       vi->intr_coal_tx.max_usecs,
> >  							       vi->intr_coal_tx.max_packets);
> > -			if (err)
> > +			if (err && err != -EOPNOTSUPP)
> >  				return err;
> >  		}
> >
> 
> 
> So far so good.
>   
> > @@ -3758,7 +3764,7 @@ static int virtnet_set_ringparam(struct net_device *dev,
> >  							       vi->intr_coal_rx.max_usecs,
> >  							       vi->intr_coal_rx.max_packets);
> >  			mutex_unlock(&vi->rq[i].dim_lock);
> > -			if (err)
> > +			if (err && err != -EOPNOTSUPP)
> >  				return err;
> >  		}
> >  	}
> 
> I don't get this one. If resize is not supported,

Here means that the *dim feature* is not supported, not the *resize* feature.

> we pretend it was successful? Why?

During a resize, if the dim feature is not supported, the driver does not
need to try to recover any coalescing values, since the device does not have
these parameters.
Therefore, the resize should continue without interruption.

Thanks.

> 
> > -- 
> > 2.32.0.3.g01195cf9f
>