virtualization.lists.linux-foundation.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] virtio-net: close() to follow mirror of open()
@ 2023-02-02  5:00 Parav Pandit via Virtualization
  2023-02-02  5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit via Virtualization
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Parav Pandit via Virtualization @ 2023-02-02  5:00 UTC (permalink / raw)
  To: mst, jasowang, davem, kuba, netdev
  Cc: hawk, daniel, ast, virtualization, edumazet, bpf, pabeni

Hi,

This two small patches improves ndo_close() callback to follow
the mirror sequence of ndo_open() callback. This improves the code auditing
and also ensure that xdp rxq info is not unregistered while NAPI on
RXQ is ongoing.

Please review.

Patch summary:
patch-1 ensures that xdp rq info is unregistered after rq napi is disabled
patch-2 keeps the mirror sequence for close() be mirror of open()

Parav Pandit (2):
  virtio-net: Keep stop() to follow mirror sequence of open()
  virtio-net: Maintain reverse cleanup order

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

-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence of open()
  2023-02-02  5:00 [PATCH 0/2] virtio-net: close() to follow mirror of open() Parav Pandit via Virtualization
@ 2023-02-02  5:00 ` Parav Pandit via Virtualization
  2023-02-02 12:23   ` Jiri Pirko
  2023-02-02  5:00 ` [PATCH 2/2] virtio-net: Maintain reverse cleanup order Parav Pandit
  2023-02-02 10:56 ` [PATCH 0/2] virtio-net: close() to follow mirror of open() Michael S. Tsirkin
  2 siblings, 1 reply; 9+ messages in thread
From: Parav Pandit via Virtualization @ 2023-02-02  5:00 UTC (permalink / raw)
  To: mst, jasowang, davem, kuba, netdev
  Cc: hawk, daniel, ast, virtualization, edumazet, bpf, pabeni

Cited commit in fixes tag frees rxq xdp info while RQ NAPI is
still enabled and packet processing may be ongoing.

Follow the mirror sequence of open() in the stop() callback.
This ensures that when rxq info is unregistered, no rx
packet processing is ongoing.

Fixes: 754b8a21a96d ("virtio_net: setup xdp_rxq_info")
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 7e1a98430190..b7d0b54c3bb0 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2279,8 +2279,8 @@ static int virtnet_close(struct net_device *dev)
 	cancel_delayed_work_sync(&vi->refill);
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
-		xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
 		napi_disable(&vi->rq[i].napi);
+		xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
 		virtnet_napi_tx_disable(&vi->sq[i].napi);
 	}
 
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* [PATCH 2/2] virtio-net: Maintain reverse cleanup order
  2023-02-02  5:00 [PATCH 0/2] virtio-net: close() to follow mirror of open() Parav Pandit via Virtualization
  2023-02-02  5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit via Virtualization
@ 2023-02-02  5:00 ` Parav Pandit
  2023-02-02 12:26   ` Jiri Pirko
  2023-02-02 10:56 ` [PATCH 0/2] virtio-net: close() to follow mirror of open() Michael S. Tsirkin
  2 siblings, 1 reply; 9+ messages in thread
From: Parav Pandit @ 2023-02-02  5:00 UTC (permalink / raw)
  To: mst, jasowang, davem, kuba, netdev
  Cc: hawk, daniel, ast, virtualization, edumazet, bpf, pabeni

To easily audit the code, better to keep the device stop()
sequence to be mirror of the device open() sequence.

Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 drivers/net/virtio_net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index b7d0b54c3bb0..1f8168e0f64d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2279,9 +2279,9 @@ static int virtnet_close(struct net_device *dev)
 	cancel_delayed_work_sync(&vi->refill);
 
 	for (i = 0; i < vi->max_queue_pairs; i++) {
+		virtnet_napi_tx_disable(&vi->sq[i].napi);
 		napi_disable(&vi->rq[i].napi);
 		xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
-		virtnet_napi_tx_disable(&vi->sq[i].napi);
 	}
 
 	return 0;
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 0/2] virtio-net: close() to follow mirror of open()
  2023-02-02  5:00 [PATCH 0/2] virtio-net: close() to follow mirror of open() Parav Pandit via Virtualization
  2023-02-02  5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit via Virtualization
  2023-02-02  5:00 ` [PATCH 2/2] virtio-net: Maintain reverse cleanup order Parav Pandit
@ 2023-02-02 10:56 ` Michael S. Tsirkin
  2 siblings, 0 replies; 9+ messages in thread
From: Michael S. Tsirkin @ 2023-02-02 10:56 UTC (permalink / raw)
  To: Parav Pandit
  Cc: hawk, daniel, netdev, ast, virtualization, edumazet, kuba, bpf,
	pabeni, davem

On Thu, Feb 02, 2023 at 07:00:36AM +0200, Parav Pandit wrote:
> Hi,
> 
> This two small patches improves ndo_close() callback to follow
> the mirror sequence of ndo_open() callback. This improves the code auditing
> and also ensure that xdp rxq info is not unregistered while NAPI on
> RXQ is ongoing.


Acked-by: Michael S. Tsirkin <mst@redhat.com>

I'm guessing -net and 1/2 for stable?

> Please review.
> 
> Patch summary:
> patch-1 ensures that xdp rq info is unregistered after rq napi is disabled
> patch-2 keeps the mirror sequence for close() be mirror of open()
> 
> Parav Pandit (2):
>   virtio-net: Keep stop() to follow mirror sequence of open()
>   virtio-net: Maintain reverse cleanup order
> 
>  drivers/net/virtio_net.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> -- 
> 2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence of open()
  2023-02-02  5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit via Virtualization
@ 2023-02-02 12:23   ` Jiri Pirko
  0 siblings, 0 replies; 9+ messages in thread
From: Jiri Pirko @ 2023-02-02 12:23 UTC (permalink / raw)
  To: Parav Pandit
  Cc: hawk, daniel, mst, netdev, ast, virtualization, edumazet, kuba,
	bpf, pabeni, davem

Thu, Feb 02, 2023 at 06:00:37AM CET, parav@nvidia.com wrote:
>Cited commit in fixes tag frees rxq xdp info while RQ NAPI is
>still enabled and packet processing may be ongoing.
>
>Follow the mirror sequence of open() in the stop() callback.
>This ensures that when rxq info is unregistered, no rx
>packet processing is ongoing.
>
>Fixes: 754b8a21a96d ("virtio_net: setup xdp_rxq_info")
>Signed-off-by: Parav Pandit <parav@nvidia.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 2/2] virtio-net: Maintain reverse cleanup order
  2023-02-02  5:00 ` [PATCH 2/2] virtio-net: Maintain reverse cleanup order Parav Pandit
@ 2023-02-02 12:26   ` Jiri Pirko
  2023-02-02 15:10     ` Parav Pandit via Virtualization
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Pirko @ 2023-02-02 12:26 UTC (permalink / raw)
  To: Parav Pandit
  Cc: hawk, daniel, mst, netdev, ast, virtualization, edumazet, kuba,
	bpf, pabeni, davem

Thu, Feb 02, 2023 at 06:00:38AM CET, parav@nvidia.com wrote:
>To easily audit the code, better to keep the device stop()
>sequence to be mirror of the device open() sequence.
>
>Signed-off-by: Parav Pandit <parav@nvidia.com>

Reviewed-by: Jiri Pirko <jiri@nvidia.com>

If this is not fixing bug (which I believe is the case), you should
target it to net-next ([patch net-next] ..).


>---
> drivers/net/virtio_net.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>index b7d0b54c3bb0..1f8168e0f64d 100644
>--- a/drivers/net/virtio_net.c
>+++ b/drivers/net/virtio_net.c
>@@ -2279,9 +2279,9 @@ static int virtnet_close(struct net_device *dev)
> 	cancel_delayed_work_sync(&vi->refill);
> 
> 	for (i = 0; i < vi->max_queue_pairs; i++) {
>+		virtnet_napi_tx_disable(&vi->sq[i].napi);
> 		napi_disable(&vi->rq[i].napi);
> 		xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
>-		virtnet_napi_tx_disable(&vi->sq[i].napi);
> 	}
> 
> 	return 0;
>-- 
>2.26.2
>
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* RE: [PATCH 2/2] virtio-net: Maintain reverse cleanup order
  2023-02-02 12:26   ` Jiri Pirko
@ 2023-02-02 15:10     ` Parav Pandit via Virtualization
  2023-02-02 15:46       ` Jiri Pirko
  0 siblings, 1 reply; 9+ messages in thread
From: Parav Pandit via Virtualization @ 2023-02-02 15:10 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: hawk, daniel, mst, netdev, ast, virtualization, edumazet, kuba,
	bpf, pabeni, davem


> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, February 2, 2023 7:26 AM
> 
> Thu, Feb 02, 2023 at 06:00:38AM CET, parav@nvidia.com wrote:
> >To easily audit the code, better to keep the device stop() sequence to
> >be mirror of the device open() sequence.
> >
> >Signed-off-by: Parav Pandit <parav@nvidia.com>
> 
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> 
> If this is not fixing bug (which I believe is the case), you should target it to net-
> next ([patch net-next] ..).
> 
Yes. Right. First one was fix for net-rc, second was for net-next. And 2nd depends on the first to avoid merge conflicts.
So, I was unsure how to handle it.
Can you please suggest?


> 
> >---
> > drivers/net/virtio_net.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
> >b7d0b54c3bb0..1f8168e0f64d 100644
> >--- a/drivers/net/virtio_net.c
> >+++ b/drivers/net/virtio_net.c
> >@@ -2279,9 +2279,9 @@ static int virtnet_close(struct net_device *dev)
> > 	cancel_delayed_work_sync(&vi->refill);
> >
> > 	for (i = 0; i < vi->max_queue_pairs; i++) {
> >+		virtnet_napi_tx_disable(&vi->sq[i].napi);
> > 		napi_disable(&vi->rq[i].napi);
> > 		xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
> >-		virtnet_napi_tx_disable(&vi->sq[i].napi);
> > 	}
> >
> > 	return 0;
> >--
> >2.26.2
> >
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* Re: [PATCH 2/2] virtio-net: Maintain reverse cleanup order
  2023-02-02 15:10     ` Parav Pandit via Virtualization
@ 2023-02-02 15:46       ` Jiri Pirko
  2023-02-02 16:33         ` Parav Pandit via Virtualization
  0 siblings, 1 reply; 9+ messages in thread
From: Jiri Pirko @ 2023-02-02 15:46 UTC (permalink / raw)
  To: Parav Pandit
  Cc: hawk, daniel, mst, netdev, ast, virtualization, edumazet, kuba,
	bpf, pabeni, davem

Thu, Feb 02, 2023 at 04:10:56PM CET, parav@nvidia.com wrote:
>
>> From: Jiri Pirko <jiri@resnulli.us>
>> Sent: Thursday, February 2, 2023 7:26 AM
>> 
>> Thu, Feb 02, 2023 at 06:00:38AM CET, parav@nvidia.com wrote:
>> >To easily audit the code, better to keep the device stop() sequence to
>> >be mirror of the device open() sequence.
>> >
>> >Signed-off-by: Parav Pandit <parav@nvidia.com>
>> 
>> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
>> 
>> If this is not fixing bug (which I believe is the case), you should target it to net-
>> next ([patch net-next] ..).
>> 
>Yes. Right. First one was fix for net-rc, second was for net-next. And 2nd depends on the first to avoid merge conflicts.
>So, I was unsure how to handle it.
>Can you please suggest?

1) Send the fix to -net
2) Wait until -net is merged into -net-next
3) Send the second patch to -net-next

>
>
>> 
>> >---
>> > drivers/net/virtio_net.c | 2 +-
>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> >diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index
>> >b7d0b54c3bb0..1f8168e0f64d 100644
>> >--- a/drivers/net/virtio_net.c
>> >+++ b/drivers/net/virtio_net.c
>> >@@ -2279,9 +2279,9 @@ static int virtnet_close(struct net_device *dev)
>> > 	cancel_delayed_work_sync(&vi->refill);
>> >
>> > 	for (i = 0; i < vi->max_queue_pairs; i++) {
>> >+		virtnet_napi_tx_disable(&vi->sq[i].napi);
>> > 		napi_disable(&vi->rq[i].napi);
>> > 		xdp_rxq_info_unreg(&vi->rq[i].xdp_rxq);
>> >-		virtnet_napi_tx_disable(&vi->sq[i].napi);
>> > 	}
>> >
>> > 	return 0;
>> >--
>> >2.26.2
>> >
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

* RE: [PATCH 2/2] virtio-net: Maintain reverse cleanup order
  2023-02-02 15:46       ` Jiri Pirko
@ 2023-02-02 16:33         ` Parav Pandit via Virtualization
  0 siblings, 0 replies; 9+ messages in thread
From: Parav Pandit via Virtualization @ 2023-02-02 16:33 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: hawk, daniel, mst, netdev, ast, virtualization, edumazet, kuba,
	bpf, pabeni, davem


> From: Jiri Pirko <jiri@resnulli.us>
> Sent: Thursday, February 2, 2023 10:47 AM
> 
> Thu, Feb 02, 2023 at 04:10:56PM CET, parav@nvidia.com wrote:
> >
> >> From: Jiri Pirko <jiri@resnulli.us>
> >> Sent: Thursday, February 2, 2023 7:26 AM
> >>
> >> Thu, Feb 02, 2023 at 06:00:38AM CET, parav@nvidia.com wrote:
> >> >To easily audit the code, better to keep the device stop() sequence
> >> >to be mirror of the device open() sequence.
> >> >
> >> >Signed-off-by: Parav Pandit <parav@nvidia.com>
> >>
> >> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> >>
> >> If this is not fixing bug (which I believe is the case), you should
> >> target it to net- next ([patch net-next] ..).
> >>
> >Yes. Right. First one was fix for net-rc, second was for net-next. And 2nd
> depends on the first to avoid merge conflicts.
> >So, I was unsure how to handle it.
> >Can you please suggest?
> 
> 1) Send the fix to -net
> 2) Wait until -net is merged into -net-next
> 3) Send the second patch to -net-next

Got it. Thanks.

Dave, Jakub,
Please drop this series.
I am sending one by one to net and net-next.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

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

end of thread, other threads:[~2023-02-02 16:33 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02  5:00 [PATCH 0/2] virtio-net: close() to follow mirror of open() Parav Pandit via Virtualization
2023-02-02  5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit via Virtualization
2023-02-02 12:23   ` Jiri Pirko
2023-02-02  5:00 ` [PATCH 2/2] virtio-net: Maintain reverse cleanup order Parav Pandit
2023-02-02 12:26   ` Jiri Pirko
2023-02-02 15:10     ` Parav Pandit via Virtualization
2023-02-02 15:46       ` Jiri Pirko
2023-02-02 16:33         ` Parav Pandit via Virtualization
2023-02-02 10:56 ` [PATCH 0/2] virtio-net: close() to follow mirror of open() 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).