netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated
@ 2024-07-31 12:07 Heng Qi
  2024-07-31 12:14 ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Heng Qi @ 2024-07-31 12:07 UTC (permalink / raw)
  To: netdev, Michael S. Tsirkin, Jason Wang
  Cc: Jakub Kicinski, Xuan Zhuo, David S. Miller, Eric Dumazet,
	Paolo Abeni, virtualization, EugenioPérez

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;
 		}
 
@@ -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;
 		}
 	}
-- 
2.32.0.3.g01195cf9f


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated
  2024-07-31 12:07 [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated Heng Qi
@ 2024-07-31 12:14 ` Michael S. Tsirkin
  2024-07-31 12:25   ` Heng Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-07-31 12:14 UTC (permalink / raw)
  To: Heng Qi
  Cc: netdev, Jason Wang, Jakub Kicinski, Xuan Zhuo, David S. Miller,
	Eric Dumazet, Paolo Abeni, virtualization, EugenioPérez

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, we pretend it
was successful? Why?

> -- 
> 2.32.0.3.g01195cf9f


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated
  2024-07-31 12:14 ` Michael S. Tsirkin
@ 2024-07-31 12:25   ` Heng Qi
  2024-07-31 12:46     ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Heng Qi @ 2024-07-31 12:25 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Jason Wang, Jakub Kicinski, Xuan Zhuo, David S. Miller,
	Eric Dumazet, Paolo Abeni, virtualization, EugenioPérez

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
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated
  2024-07-31 12:25   ` Heng Qi
@ 2024-07-31 12:46     ` Michael S. Tsirkin
  2024-08-01  6:07       ` Heng Qi
  0 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-07-31 12:46 UTC (permalink / raw)
  To: Heng Qi
  Cc: netdev, Jason Wang, Jakub Kicinski, Xuan Zhuo, David S. Miller,
	Eric Dumazet, Paolo Abeni, virtualization, EugenioPérez

On Wed, Jul 31, 2024 at 08:25:23PM +0800, Heng Qi wrote:
> 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.


you mean it's a separate bugfix?

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated
  2024-07-31 12:46     ` Michael S. Tsirkin
@ 2024-08-01  6:07       ` Heng Qi
  2024-08-01  6:39         ` Michael S. Tsirkin
  0 siblings, 1 reply; 6+ messages in thread
From: Heng Qi @ 2024-08-01  6:07 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, Jason Wang, Jakub Kicinski, Xuan Zhuo, David S. Miller,
	Eric Dumazet, Paolo Abeni, virtualization, EugenioPérez

On Wed, 31 Jul 2024 08:46:42 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Jul 31, 2024 at 08:25:23PM +0800, Heng Qi wrote:
> > 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.
> 
> 
> you mean it's a separate bugfix?

Right.

Don't break resize when coalescing is not negotiated.

Thanks.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated
  2024-08-01  6:07       ` Heng Qi
@ 2024-08-01  6:39         ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2024-08-01  6:39 UTC (permalink / raw)
  To: Heng Qi
  Cc: netdev, Jason Wang, Jakub Kicinski, Xuan Zhuo, David S. Miller,
	Eric Dumazet, Paolo Abeni, virtualization, EugenioPérez

On Thu, Aug 01, 2024 at 02:07:43PM +0800, Heng Qi wrote:
> On Wed, 31 Jul 2024 08:46:42 -0400, "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, Jul 31, 2024 at 08:25:23PM +0800, Heng Qi wrote:
> > > 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.
> > 
> > 
> > you mean it's a separate bugfix?
> 
> Right.
> 
> Don't break resize when coalescing is not negotiated.
> 
> Thanks.

Let's make this a separate patch then, please.

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-08-01  6:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-31 12:07 [PATCH net v2] virtio-net: unbreak vq resizing when coalescing is not negotiated Heng Qi
2024-07-31 12:14 ` Michael S. Tsirkin
2024-07-31 12:25   ` Heng Qi
2024-07-31 12:46     ` Michael S. Tsirkin
2024-08-01  6:07       ` Heng Qi
2024-08-01  6:39         ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).