* [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
@ 2023-02-02 5:00 ` Parav Pandit
2023-02-02 12:23 ` Jiri Pirko
2023-02-02 5:00 ` [PATCH 2/2] virtio-net: Maintain reverse cleanup order Parav Pandit
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2023-02-02 5:00 UTC (permalink / raw)
To: mst, jasowang, davem, kuba, netdev
Cc: edumazet, pabeni, ast, daniel, hawk, virtualization, bpf,
Parav Pandit
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
^ permalink raw reply related [flat|nested] 10+ 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
@ 2023-02-02 12:23 ` Jiri Pirko
0 siblings, 0 replies; 10+ messages in thread
From: Jiri Pirko @ 2023-02-02 12:23 UTC (permalink / raw)
To: Parav Pandit
Cc: mst, jasowang, davem, kuba, netdev, edumazet, pabeni, ast, daniel,
hawk, virtualization, bpf
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>
^ permalink raw reply [flat|nested] 10+ 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
2023-02-02 5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit
@ 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
2023-02-02 18:40 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2023-02-02 5:00 UTC (permalink / raw)
To: mst, jasowang, davem, kuba, netdev
Cc: edumazet, pabeni, ast, daniel, hawk, virtualization, bpf,
Parav Pandit
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
^ permalink raw reply related [flat|nested] 10+ 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
0 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2023-02-02 12:26 UTC (permalink / raw)
To: Parav Pandit
Cc: mst, jasowang, davem, kuba, netdev, edumazet, pabeni, ast, daniel,
hawk, virtualization, bpf
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
>
^ permalink raw reply [flat|nested] 10+ 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
2023-02-02 15:46 ` Jiri Pirko
0 siblings, 1 reply; 10+ messages in thread
From: Parav Pandit @ 2023-02-02 15:10 UTC (permalink / raw)
To: Jiri Pirko
Cc: mst@redhat.com, jasowang@redhat.com, davem@davemloft.net,
kuba@kernel.org, netdev@vger.kernel.org, edumazet@google.com,
pabeni@redhat.com, ast@kernel.org, daniel@iogearbox.net,
hawk@kernel.org, virtualization@lists.linux-foundation.org,
bpf@vger.kernel.org
> 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
> >
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 2/2] virtio-net: Maintain reverse cleanup order
2023-02-02 15:10 ` Parav Pandit
@ 2023-02-02 15:46 ` Jiri Pirko
2023-02-02 16:33 ` Parav Pandit
0 siblings, 1 reply; 10+ messages in thread
From: Jiri Pirko @ 2023-02-02 15:46 UTC (permalink / raw)
To: Parav Pandit
Cc: mst@redhat.com, jasowang@redhat.com, davem@davemloft.net,
kuba@kernel.org, netdev@vger.kernel.org, edumazet@google.com,
pabeni@redhat.com, ast@kernel.org, daniel@iogearbox.net,
hawk@kernel.org, virtualization@lists.linux-foundation.org,
bpf@vger.kernel.org
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
>> >
^ permalink raw reply [flat|nested] 10+ 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
0 siblings, 0 replies; 10+ messages in thread
From: Parav Pandit @ 2023-02-02 16:33 UTC (permalink / raw)
To: Jiri Pirko
Cc: mst@redhat.com, jasowang@redhat.com, davem@davemloft.net,
kuba@kernel.org, netdev@vger.kernel.org, edumazet@google.com,
pabeni@redhat.com, ast@kernel.org, daniel@iogearbox.net,
hawk@kernel.org, virtualization@lists.linux-foundation.org,
bpf@vger.kernel.org
> 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.
^ permalink raw reply [flat|nested] 10+ 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
2023-02-02 5:00 ` [PATCH 1/2] virtio-net: Keep stop() to follow mirror sequence " Parav Pandit
2023-02-02 5:00 ` [PATCH 2/2] virtio-net: Maintain reverse cleanup order Parav Pandit
@ 2023-02-02 10:56 ` Michael S. Tsirkin
2023-02-02 18:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2023-02-02 10:56 UTC (permalink / raw)
To: Parav Pandit
Cc: jasowang, davem, kuba, netdev, edumazet, pabeni, ast, daniel,
hawk, virtualization, bpf
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
^ permalink raw reply [flat|nested] 10+ 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
` (2 preceding siblings ...)
2023-02-02 10:56 ` [PATCH 0/2] virtio-net: close() to follow mirror of open() Michael S. Tsirkin
@ 2023-02-02 18:40 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 10+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-02-02 18:40 UTC (permalink / raw)
To: Parav Pandit
Cc: mst, jasowang, davem, kuba, netdev, edumazet, pabeni, ast, daniel,
hawk, virtualization, bpf
Hello:
This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 2 Feb 2023 07:00:36 +0200 you 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.
>
> [...]
Here is the summary with links:
- [1/2] virtio-net: Keep stop() to follow mirror sequence of open()
https://git.kernel.org/netdev/net/c/63b114042d8a
- [2/2] virtio-net: Maintain reverse cleanup order
(no matching commit)
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 10+ messages in thread