[PATCH net v3 0/2] virtio-net: unbreak vq resizing if vq coalescing is not supported

[PATCH net v3 0/2] virtio-net: unbreak vq resizing if vq coalescing is not supported

[Heng Qi]

Currently, if the driver does not negotiate the vq coalescing feature
but supports vq resize, the vq resize action, which could have been
successfully executed, is interrupted due to the failure in configuring
the vq coalescing parameters. This issue needs to be fixed.

Changelog
=========
v2->v3:
  - Break out the feature check and the fix into separate patches.

v1->v2:
  - Rephrase the subject.
  - Put the feature check inside the virtnet_send_{r,t}x_ctrl_coal_vq_cmd.

Heng Qi (2):
  virtio-net: check feature before configuring the vq coalescing command
  virtio-net: unbreak vq resizing when coalescing is not negotiated

 drivers/net/virtio_net.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

-- 
2.32.0.3.g01195cf9f

[PATCH net v3 1/2] virtio-net: check feature before configuring the vq coalescing command

[Heng Qi]

Virtio spec says:

	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.

So we add the feature negotiation check to
virtnet_send_{r,t}x_ctrl_coal_vq_cmd as a basis for the next bugfix patch.

Suggested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Heng Qi <hengqi@linux.alibaba.com>
---
v2->v3:
  - Break out the feature check and the fix into separate patches.

 drivers/net/virtio_net.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 0383a3e136d6..b1176be8fcfd 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)
-- 
2.32.0.3.g01195cf9f

[PATCH net v3 2/2] virtio-net: unbreak vq resizing when coalescing is not negotiated

[Heng Qi]

Don't break the resize action if the vq coalescing feature
named VIRTIO_NET_F_VQ_NOTF_COAL is not negotiated.

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>
---
v2->v3:
  - Break out the feature check and the fix into separate patches.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b1176be8fcfd..2b566d893ea3 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3749,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;
 		}
 
@@ -3764,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;
 		}
 	}
-- 
2.32.0.3.g01195cf9f

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

[Michael S. Tsirkin]

On Thu, Aug 01, 2024 at 08:27:39PM +0800, Heng Qi wrote:
> Don't break the resize action if the vq coalescing feature
> named VIRTIO_NET_F_VQ_NOTF_COAL is not negotiated.
> 
> 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>
> ---
> v2->v3:
>   - Break out the feature check and the fix into separate patches.
> 
> 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 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index b1176be8fcfd..2b566d893ea3 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3749,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;
>  		}
>  
> @@ -3764,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;


This needs a comment.


>  		}
>  	}
> -- 
> 2.32.0.3.g01195cf9f

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

[Heng Qi]

On Thu, 1 Aug 2024 08:30:44 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Aug 01, 2024 at 08:27:39PM +0800, Heng Qi wrote:
> > Don't break the resize action if the vq coalescing feature
> > named VIRTIO_NET_F_VQ_NOTF_COAL is not negotiated.
> > 
> > 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>
> > ---
> > v2->v3:
> >   - Break out the feature check and the fix into separate patches.
> > 
> > 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 | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index b1176be8fcfd..2b566d893ea3 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -3749,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;
> >  		}
> >  
> > @@ -3764,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;
> 
> 
> This needs a comment.
> 

Since both the patch and the comment are small, I will send out the next version
soon and hope to get the understanding of the netdev maintainers.

Thanks.

> 
> >  		}
> >  	}
> > -- 
> > 2.32.0.3.g01195cf9f
>