DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH] net/netvsc: fix mtu_set in netvsc devices
@ 2024-06-28 16:35 Alexander Skorichenko
  2024-06-30 15:40 ` Stephen Hemminger
  2024-07-07  2:11 ` Ferruh Yigit
  0 siblings, 2 replies; 8+ messages in thread
From: Alexander Skorichenko @ 2024-06-28 16:35 UTC (permalink / raw)
  To: samandrew, ferruh.yigit, andrew.rybchenko
  Cc: longli, weh, dev, Alexander Skorichenko

Prevent segfault in hn_reinit() caused by changing the MTU for
an incompletely initialized device.

Signed-off-by: Alexander Skorichenko <askorichenko@netgate.com>
---
 drivers/net/netvsc/hn_ethdev.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/netvsc/hn_ethdev.c b/drivers/net/netvsc/hn_ethdev.c
index b8a32832d7..f8cb05a118 100644
--- a/drivers/net/netvsc/hn_ethdev.c
+++ b/drivers/net/netvsc/hn_ethdev.c
@@ -1127,8 +1127,10 @@ hn_reinit(struct rte_eth_dev *dev, uint16_t mtu)
 	int i, ret = 0;
 
 	/* Point primary queues at new primary channel */
-	rxqs[0]->chan = hv->channels[0];
-	txqs[0]->chan = hv->channels[0];
+	if (rxqs[0]) {
+		rxqs[0]->chan = hv->channels[0];
+		txqs[0]->chan = hv->channels[0];
+	}
 
 	ret = hn_attach(hv, mtu);
 	if (ret)
@@ -1140,10 +1142,12 @@ hn_reinit(struct rte_eth_dev *dev, uint16_t mtu)
 		return ret;
 
 	/* Point any additional queues at new subchannels */
-	for (i = 1; i < dev->data->nb_rx_queues; i++)
-		rxqs[i]->chan = hv->channels[i];
-	for (i = 1; i < dev->data->nb_tx_queues; i++)
-		txqs[i]->chan = hv->channels[i];
+	if (rxqs[0]) {
+		for (i = 1; i < dev->data->nb_rx_queues; i++)
+			rxqs[i]->chan = hv->channels[i];
+		for (i = 1; i < dev->data->nb_tx_queues; i++)
+			txqs[i]->chan = hv->channels[i];
+	}
 
 	return ret;
 }
-- 
2.34.1


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

* Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-06-28 16:35 [PATCH] net/netvsc: fix mtu_set in netvsc devices Alexander Skorichenko
@ 2024-06-30 15:40 ` Stephen Hemminger
  2024-07-02  8:49   ` Alexander Skorichenko
  2024-07-07  2:11 ` Ferruh Yigit
  1 sibling, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2024-06-30 15:40 UTC (permalink / raw)
  To: Alexander Skorichenko
  Cc: samandrew, ferruh.yigit, andrew.rybchenko, longli, weh, dev

On Fri, 28 Jun 2024 18:35:03 +0200
Alexander Skorichenko <askorichenko@netgate.com> wrote:

> Prevent segfault in hn_reinit() caused by changing the MTU for
> an incompletely initialized device.
> 
> Signed-off-by: Alexander Skorichenko <askorichenko@netgate.com>

How do you get in that state?
Maybe the init code should set up these pointers and avoid the problem.

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

* Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-06-30 15:40 ` Stephen Hemminger
@ 2024-07-02  8:49   ` Alexander Skorichenko
  2024-07-03  0:35     ` Long Li
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Skorichenko @ 2024-07-02  8:49 UTC (permalink / raw)
  To: stephen
  Cc: samandrew, ferruh.yigit, andrew.rybchenko, longli, Wei Hu, dev,
	Matthew Smith

[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]

Hello,
The failure happens when running under VPP. In terms of dpdk calls the
following sequence of events occurs for a NetVSC device:

Durung device probing stage of VPP
   1. eth_hn_dev_init() memory for a single primary rx_queue is allocated
            dev->data->rx_queues[0] = 0, allocated, but not set yet
            no allocation happened for tx_queues[i] and non-primary
rx_queues[i]

During device setup stage of VPP from VPP's own generic dpdk_device_setup():
   2.  rte_eth_dev_set_mtu()
            currently it segfaults in hn_reinit() when trying to reach into
            dev->data->rx_queues[i], dev->data->tx_queues[i]
   3.  rte_eth_tx_queue_setup()
            dev->data->tx_queues[i] are being allocated and set
   4.  rte_eth_rx_queue_setup()
            dev->data->rx_queues[i] get allocated (i > 0) and set

So rx_queues[0] could be set in step 1, but rx_queues[i] and tx_queues[i]
are still NULL.
Allocating all the remaining rx/tx queues in step 1 would prevent the
crash, but then in steps 3-4 would go through releasing and allocating all
of the queues again.

Another comment regarding the uniform condition introduced in hn_reinit()
in the patch:
        if (dev->data->rx_queues[0] != NULL) ...
Because of the difference between rx/tx queues described above, it's
probably safer to extend the condition to check both rx and tx separately
        if (dev->data->rx_queues[0] != NULL && dev->data->tx_queues[0] !=
NULL)

- Alexander Skorichenko


On Sun, Jun 30, 2024 at 5:40 PM Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Fri, 28 Jun 2024 18:35:03 +0200
> Alexander Skorichenko <askorichenko@netgate.com> wrote:
>
> > Prevent segfault in hn_reinit() caused by changing the MTU for
> > an incompletely initialized device.
> >
> > Signed-off-by: Alexander Skorichenko <askorichenko@netgate.com>
>
> How do you get in that state?
> Maybe the init code should set up these pointers and avoid the problem.
>

[-- Attachment #2: Type: text/html, Size: 2656 bytes --]

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

* RE: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-07-02  8:49   ` Alexander Skorichenko
@ 2024-07-03  0:35     ` Long Li
  2024-07-03 10:13       ` Alexander Skorichenko
  0 siblings, 1 reply; 8+ messages in thread
From: Long Li @ 2024-07-03  0:35 UTC (permalink / raw)
  To: Alexander Skorichenko, stephen, Sam Andrew
  Cc: ferruh.yigit, andrew.rybchenko, Wei Hu, dev, Matthew Smith

[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]

Thank you, Alexander.

The patch looks good to me. With this patch, do you see other problems with VPP?

I'd like to get a review from @Sam Andrew<mailto:samandrew@microsoft.com>.

Acked-by: Long Li <longli@microsoft.com>


From: Alexander Skorichenko <askorichenko@netgate.com>
Sent: Tuesday, July 2, 2024 1:49 AM
To: stephen@networkplumber.org
Cc: Sam Andrew <samandrew@microsoft.com>; ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; Long Li <longli@microsoft.com>; Wei Hu <weh@microsoft.com>; dev@dpdk.org; Matthew Smith <mgsmith@netgate.com>
Subject: Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

You don't often get email from askorichenko@netgate.com<mailto:askorichenko@netgate.com>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
Hello,
The failure happens when running under VPP. In terms of dpdk calls the following sequence of events occurs for a NetVSC device:

Durung device probing stage of VPP
   1. eth_hn_dev_init() memory for a single primary rx_queue is allocated
            dev->data->rx_queues[0] = 0, allocated, but not set yet
            no allocation happened for tx_queues[i] and non-primary rx_queues[i]

During device setup stage of VPP from VPP's own generic dpdk_device_setup():
   2.  rte_eth_dev_set_mtu()
            currently it segfaults in hn_reinit() when trying to reach into
            dev->data->rx_queues[i], dev->data->tx_queues[i]
   3.  rte_eth_tx_queue_setup()
            dev->data->tx_queues[i] are being allocated and set
   4.  rte_eth_rx_queue_setup()
            dev->data->rx_queues[i] get allocated (i > 0) and set

So rx_queues[0] could be set in step 1, but rx_queues[i] and tx_queues[i] are still NULL.
Allocating all the remaining rx/tx queues in step 1 would prevent the crash, but then in steps 3-4 would go through releasing and allocating all of the queues again.

Another comment regarding the uniform condition introduced in hn_reinit() in the patch:
        if (dev->data->rx_queues[0] != NULL) ...
Because of the difference between rx/tx queues described above, it's probably safer to extend the condition to check both rx and tx separately
        if (dev->data->rx_queues[0] != NULL && dev->data->tx_queues[0] != NULL)

- Alexander Skorichenko


[-- Attachment #2: Type: text/html, Size: 7026 bytes --]

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

* Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-07-03  0:35     ` Long Li
@ 2024-07-03 10:13       ` Alexander Skorichenko
  2024-07-03 22:50         ` [EXTERNAL] " Sam Andrew
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Skorichenko @ 2024-07-03 10:13 UTC (permalink / raw)
  To: Long Li; +Cc: stephen, Sam Andrew, ferruh.yigit, andrew.rybchenko, Wei Hu, dev

[-- Attachment #1: Type: text/plain, Size: 502 bytes --]

Thanks.

>> With this patch, do you see other problems with VPP?
No further problems were noticed and MTU can be set to custom values.

- Alexander Skorichenko

On Wed, Jul 3, 2024 at 2:35 AM Long Li <longli@microsoft.com> wrote:

> Thank you, Alexander.
>
>
>
> The patch looks good to me. With this patch, do you see other problems
> with VPP?
>
>
>
> I’d like to get a review from @Sam Andrew <samandrew@microsoft.com>.
>
>
>
> Acked-by: Long Li <longli@microsoft.com>
>

[-- Attachment #2: Type: text/html, Size: 1487 bytes --]

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

* RE: [EXTERNAL] Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-07-03 10:13       ` Alexander Skorichenko
@ 2024-07-03 22:50         ` Sam Andrew
  2024-07-03 22:57           ` Long Li
  0 siblings, 1 reply; 8+ messages in thread
From: Sam Andrew @ 2024-07-03 22:50 UTC (permalink / raw)
  To: Alexander Skorichenko, Long Li
  Cc: stephen, ferruh.yigit, andrew.rybchenko, Wei Hu, dev

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

Thank you so much, Alexander. I’ve reviewed the patch. It looks good to me as well.

Reviewed-by: Sam Andrew <samandrew@microsoft.com<mailto:samandrew@microsoft.com>>


From: Alexander Skorichenko <askorichenko@netgate.com>
Sent: Wednesday, July 3, 2024 3:14 AM
To: Long Li <longli@microsoft.com>
Cc: stephen@networkplumber.org; Sam Andrew <samandrew@microsoft.com>; ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; Wei Hu <weh@microsoft.com>; dev@dpdk.org
Subject: [EXTERNAL] Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

You don't often get email from askorichenko@netgate.com<mailto:askorichenko@netgate.com>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
Thanks.
>> With this patch, do you see other problems with VPP?
No further problems were noticed and MTU can be set to custom values.

- Alexander Skorichenko

On Wed, Jul 3, 2024 at 2:35 AM Long Li <longli@microsoft.com<mailto:longli@microsoft.com>> wrote:
Thank you, Alexander.

The patch looks good to me. With this patch, do you see other problems with VPP?

I’d like to get a review from @Sam Andrew<mailto:samandrew@microsoft.com>.

Acked-by: Long Li <longli@microsoft.com<mailto:longli@microsoft.com>>

[-- Attachment #2: Type: text/html, Size: 5933 bytes --]

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

* RE: [EXTERNAL] Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-07-03 22:50         ` [EXTERNAL] " Sam Andrew
@ 2024-07-03 22:57           ` Long Li
  0 siblings, 0 replies; 8+ messages in thread
From: Long Li @ 2024-07-03 22:57 UTC (permalink / raw)
  To: Sam Andrew, Alexander Skorichenko
  Cc: stephen, ferruh.yigit, andrew.rybchenko, Wei Hu, dev

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

Fixes: 45c83603087e (“net/netvsc: support MTU set”)
Cc: stable@dpdk.org<mailto:stable@dpdk.org>

From: Sam Andrew <samandrew@microsoft.com>
Sent: Wednesday, July 3, 2024 3:50 PM
To: Alexander Skorichenko <askorichenko@netgate.com>; Long Li <longli@microsoft.com>
Cc: stephen@networkplumber.org; ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; Wei Hu <weh@microsoft.com>; dev@dpdk.org
Subject: RE: [EXTERNAL] Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

Thank you so much, Alexander. I’ve reviewed the patch. It looks good to me as well.

Reviewed-by: Sam Andrew <samandrew@microsoft.com<mailto:samandrew@microsoft.com>>


From: Alexander Skorichenko <askorichenko@netgate.com<mailto:askorichenko@netgate.com>>
Sent: Wednesday, July 3, 2024 3:14 AM
To: Long Li <longli@microsoft.com<mailto:longli@microsoft.com>>
Cc: stephen@networkplumber.org<mailto:stephen@networkplumber.org>; Sam Andrew <samandrew@microsoft.com<mailto:samandrew@microsoft.com>>; ferruh.yigit@amd.com<mailto:ferruh.yigit@amd.com>; andrew.rybchenko@oktetlabs.ru<mailto:andrew.rybchenko@oktetlabs.ru>; Wei Hu <weh@microsoft.com<mailto:weh@microsoft.com>>; dev@dpdk.org<mailto:dev@dpdk.org>
Subject: [EXTERNAL] Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices

You don't often get email from askorichenko@netgate.com<mailto:askorichenko@netgate.com>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification>
Thanks.
>> With this patch, do you see other problems with VPP?
No further problems were noticed and MTU can be set to custom values.

- Alexander Skorichenko

On Wed, Jul 3, 2024 at 2:35 AM Long Li <longli@microsoft.com<mailto:longli@microsoft.com>> wrote:
Thank you, Alexander.

The patch looks good to me. With this patch, do you see other problems with VPP?

I’d like to get a review from @Sam Andrew<mailto:samandrew@microsoft.com>.

Acked-by: Long Li <longli@microsoft.com<mailto:longli@microsoft.com>>

[-- Attachment #2: Type: text/html, Size: 7550 bytes --]

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

* Re: [PATCH] net/netvsc: fix mtu_set in netvsc devices
  2024-06-28 16:35 [PATCH] net/netvsc: fix mtu_set in netvsc devices Alexander Skorichenko
  2024-06-30 15:40 ` Stephen Hemminger
@ 2024-07-07  2:11 ` Ferruh Yigit
  1 sibling, 0 replies; 8+ messages in thread
From: Ferruh Yigit @ 2024-07-07  2:11 UTC (permalink / raw)
  To: Alexander Skorichenko, samandrew, andrew.rybchenko; +Cc: longli, weh, dev

On 6/28/2024 5:35 PM, Alexander Skorichenko wrote:
> Prevent segfault in hn_reinit() caused by changing the MTU for
> an incompletely initialized device.
> 
> Signed-off-by: Alexander Skorichenko <askorichenko@netgate.com>
>

Acked-by: Long Li <longli@microsoft.com>
Reviewed-by: Sam Andrew <samandrew@microsoft.com>

  Fixes: 45c83603087e (“net/netvsc: support MTU set”)
  Cc: stable@dpdk.org


Applied to dpdk-next-net/main, thanks.

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

end of thread, other threads:[~2024-07-07  2:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-28 16:35 [PATCH] net/netvsc: fix mtu_set in netvsc devices Alexander Skorichenko
2024-06-30 15:40 ` Stephen Hemminger
2024-07-02  8:49   ` Alexander Skorichenko
2024-07-03  0:35     ` Long Li
2024-07-03 10:13       ` Alexander Skorichenko
2024-07-03 22:50         ` [EXTERNAL] " Sam Andrew
2024-07-03 22:57           ` Long Li
2024-07-07  2:11 ` Ferruh Yigit

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).