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

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

On Wed, 31 Jul 2024 08:14:43 -0400, "Michael S. Tsirkin" <mst@redhat.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@linux.alibaba.com>
> > Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>
> > Acked-by: Eugenio Pé rez <eperezma@redhat.com>
> > Acked-by: Jason Wang <jasowang@redhat.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
>