DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set
@ 2020-05-21 15:10 Ferruh Yigit
  2020-05-21 15:37 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Ferruh Yigit @ 2020-05-21 15:10 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: dev, Ferruh Yigit, stable, Xi Zhang

During MTU set (kni_change_mtu) sample application setup queues, which
can free and re-allocate queues.
Meanwhile sample application keeps continues in Rx/Tx burst calls in
different threads, which may cause crash during queue setup.

Pausing application Rx/Tx calls before MTU set and starts it back
afterwards.

Bugzilla ID: 482
Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Rasesh Mody <rmody@marvell.com>
Cc: Xi Zhang <xix.zhang@intel.com>
---
 examples/kni/main.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/examples/kni/main.c b/examples/kni/main.c
index 7a927a50c0..80dd0353e7 100644
--- a/examples/kni/main.c
+++ b/examples/kni/main.c
@@ -768,9 +768,8 @@ monitor_all_ports_link_status(void *arg)
 	return NULL;
 }
 
-/* Callback for request of changing MTU */
 static int
-kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
+kni_change_mtu_(uint16_t port_id, unsigned int new_mtu)
 {
 	int ret;
 	uint16_t nb_rxd = NB_RXD;
@@ -851,6 +850,19 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
 	return 0;
 }
 
+/* Callback for request of changing MTU */
+static int
+kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
+{
+	int ret;
+
+	rte_atomic32_inc(&kni_pause);
+	ret =  kni_change_mtu_(port_id, new_mtu);
+	rte_atomic32_dec(&kni_pause);
+
+	return ret;
+}
+
 /* Callback for request of configuring network interface up/down */
 static int
 kni_config_network_interface(uint16_t port_id, uint8_t if_up)
-- 
2.25.4


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

* Re: [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set
  2020-05-21 15:10 [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set Ferruh Yigit
@ 2020-05-21 15:37 ` Thomas Monjalon
  2020-05-21 15:46   ` Ferruh Yigit
  2020-05-21 17:32 ` [dpdk-dev] [EXT] " Rasesh Mody
  2020-05-22  2:17 ` [dpdk-dev] " Zhang, XiX
  2 siblings, 1 reply; 7+ messages in thread
From: Thomas Monjalon @ 2020-05-21 15:37 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Rasesh Mody, dev, stable, Xi Zhang

21/05/2020 17:10, Ferruh Yigit:
> During MTU set (kni_change_mtu) sample application setup queues, which
> can free and re-allocate queues.
> Meanwhile sample application keeps continues in Rx/Tx burst calls in
> different threads, which may cause crash during queue setup.
> 
> Pausing application Rx/Tx calls before MTU set and starts it back
> afterwards.
> 
> Bugzilla ID: 482
> Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> -/* Callback for request of changing MTU */
>  static int
> -kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
> +kni_change_mtu_(uint16_t port_id, unsigned int new_mtu)
[...]
> +/* Callback for request of changing MTU */
> +static int
> +kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
> +{
> +	int ret;
> +
> +	rte_atomic32_inc(&kni_pause);
> +	ret =  kni_change_mtu_(port_id, new_mtu);
> +	rte_atomic32_dec(&kni_pause);
> +
> +	return ret;
> +}

Why creating a new function which is called only once?



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

* Re: [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set
  2020-05-21 15:37 ` Thomas Monjalon
@ 2020-05-21 15:46   ` Ferruh Yigit
  2020-05-21 20:29     ` Thomas Monjalon
  0 siblings, 1 reply; 7+ messages in thread
From: Ferruh Yigit @ 2020-05-21 15:46 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: Rasesh Mody, dev, stable, Xi Zhang

On 5/21/2020 4:37 PM, Thomas Monjalon wrote:
> 21/05/2020 17:10, Ferruh Yigit:
>> During MTU set (kni_change_mtu) sample application setup queues, which
>> can free and re-allocate queues.
>> Meanwhile sample application keeps continues in Rx/Tx burst calls in
>> different threads, which may cause crash during queue setup.
>>
>> Pausing application Rx/Tx calls before MTU set and starts it back
>> afterwards.
>>
>> Bugzilla ID: 482
>> Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
>> -/* Callback for request of changing MTU */
>>  static int
>> -kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
>> +kni_change_mtu_(uint16_t port_id, unsigned int new_mtu)
> [...]
>> +/* Callback for request of changing MTU */
>> +static int
>> +kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
>> +{
>> +	int ret;
>> +
>> +	rte_atomic32_inc(&kni_pause);
>> +	ret =  kni_change_mtu_(port_id, new_mtu);
>> +	rte_atomic32_dec(&kni_pause);
>> +
>> +	return ret;
>> +}
> 
> Why creating a new function which is called only once?
> 

Just wrapping the existing one with stop/start the forwarding.

These can be added into the existing function but there are many exit points in
the function, so it will create more clutter and it is more error prone. I think
this way more simple and clear.

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

* Re: [dpdk-dev] [EXT] [PATCH] examples/kni: fix crash during MTU set
  2020-05-21 15:10 [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set Ferruh Yigit
  2020-05-21 15:37 ` Thomas Monjalon
@ 2020-05-21 17:32 ` Rasesh Mody
  2020-05-24 15:50   ` Thomas Monjalon
  2020-05-22  2:17 ` [dpdk-dev] " Zhang, XiX
  2 siblings, 1 reply; 7+ messages in thread
From: Rasesh Mody @ 2020-05-21 17:32 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, stable, Xi Zhang

>From: Ferruh Yigit <ferruh.yigit@intel.com>
>Sent: Thursday, May 21, 2020 8:11 AM
>
>External Email
>
>----------------------------------------------------------------------
>During MTU set (kni_change_mtu) sample application setup queues, which
>can free and re-allocate queues.
>Meanwhile sample application keeps continues in Rx/Tx burst calls in different
>threads, which may cause crash during queue setup.
>
>Pausing application Rx/Tx calls before MTU set and starts it back afterwards.
>
>Bugzilla ID: 482
>Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
>Cc: stable@dpdk.org
>
>Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>---

Acked-by: Rasesh Mody <rmody@marvell.com>


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

* Re: [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set
  2020-05-21 15:46   ` Ferruh Yigit
@ 2020-05-21 20:29     ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2020-05-21 20:29 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: Rasesh Mody, dev, stable, Xi Zhang

21/05/2020 17:46, Ferruh Yigit:
> On 5/21/2020 4:37 PM, Thomas Monjalon wrote:
> > 21/05/2020 17:10, Ferruh Yigit:
> >> During MTU set (kni_change_mtu) sample application setup queues, which
> >> can free and re-allocate queues.
> >> Meanwhile sample application keeps continues in Rx/Tx burst calls in
> >> different threads, which may cause crash during queue setup.
> >>
> >> Pausing application Rx/Tx calls before MTU set and starts it back
> >> afterwards.
> >>
> >> Bugzilla ID: 482
> >> Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> >> -/* Callback for request of changing MTU */
> >>  static int
> >> -kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
> >> +kni_change_mtu_(uint16_t port_id, unsigned int new_mtu)
> > [...]
> >> +/* Callback for request of changing MTU */
> >> +static int
> >> +kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
> >> +{
> >> +	int ret;
> >> +
> >> +	rte_atomic32_inc(&kni_pause);
> >> +	ret =  kni_change_mtu_(port_id, new_mtu);
> >> +	rte_atomic32_dec(&kni_pause);
> >> +
> >> +	return ret;
> >> +}
> > 
> > Why creating a new function which is called only once?
> > 
> 
> Just wrapping the existing one with stop/start the forwarding.
> 
> These can be added into the existing function but there are many exit points in
> the function, so it will create more clutter and it is more error prone. I think
> this way more simple and clear.

OK to go on the safe side at this stage of the release :-)



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

* Re: [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set
  2020-05-21 15:10 [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set Ferruh Yigit
  2020-05-21 15:37 ` Thomas Monjalon
  2020-05-21 17:32 ` [dpdk-dev] [EXT] " Rasesh Mody
@ 2020-05-22  2:17 ` Zhang, XiX
  2 siblings, 0 replies; 7+ messages in thread
From: Zhang, XiX @ 2020-05-22  2:17 UTC (permalink / raw)
  To: Yigit, Ferruh, Rasesh Mody; +Cc: dev, stable

Tested-by:zhang,xi <xix.zhang@intel.com>

-----Original Message-----
From: Yigit, Ferruh 
Sent: Thursday, May 21, 2020 11:11 PM
To: Rasesh Mody <rmody@marvell.com>
Cc: dev@dpdk.org; Yigit, Ferruh <ferruh.yigit@intel.com>; stable@dpdk.org; Zhang, XiX <xix.zhang@intel.com>
Subject: [PATCH] examples/kni: fix crash during MTU set

During MTU set (kni_change_mtu) sample application setup queues, which can free and re-allocate queues.
Meanwhile sample application keeps continues in Rx/Tx burst calls in different threads, which may cause crash during queue setup.

Pausing application Rx/Tx calls before MTU set and starts it back afterwards.

Bugzilla ID: 482
Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: Rasesh Mody <rmody@marvell.com>
Cc: Xi Zhang <xix.zhang@intel.com>
---
 examples/kni/main.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/examples/kni/main.c b/examples/kni/main.c index 7a927a50c0..80dd0353e7 100644
--- a/examples/kni/main.c
+++ b/examples/kni/main.c
@@ -768,9 +768,8 @@ monitor_all_ports_link_status(void *arg)
 	return NULL;
 }
 
-/* Callback for request of changing MTU */  static int -kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
+kni_change_mtu_(uint16_t port_id, unsigned int new_mtu)
 {
 	int ret;
 	uint16_t nb_rxd = NB_RXD;
@@ -851,6 +850,19 @@ kni_change_mtu(uint16_t port_id, unsigned int new_mtu)
 	return 0;
 }
 
+/* Callback for request of changing MTU */ static int 
+kni_change_mtu(uint16_t port_id, unsigned int new_mtu) {
+	int ret;
+
+	rte_atomic32_inc(&kni_pause);
+	ret =  kni_change_mtu_(port_id, new_mtu);
+	rte_atomic32_dec(&kni_pause);
+
+	return ret;
+}
+
 /* Callback for request of configuring network interface up/down */  static int  kni_config_network_interface(uint16_t port_id, uint8_t if_up)
--
2.25.4


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

* Re: [dpdk-dev] [EXT] [PATCH] examples/kni: fix crash during MTU set
  2020-05-21 17:32 ` [dpdk-dev] [EXT] " Rasesh Mody
@ 2020-05-24 15:50   ` Thomas Monjalon
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2020-05-24 15:50 UTC (permalink / raw)
  To: Ferruh Yigit; +Cc: dev, stable, Xi Zhang, Rasesh Mody

21/05/2020 19:32, Rasesh Mody:
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> >During MTU set (kni_change_mtu) sample application setup queues, which
> >can free and re-allocate queues.
> >Meanwhile sample application keeps continues in Rx/Tx burst calls in different
> >threads, which may cause crash during queue setup.
> >
> >Pausing application Rx/Tx calls before MTU set and starts it back afterwards.
> >
> >Bugzilla ID: 482
> >Fixes: a26b116749a3 ("examples/kni: fix MTU change to setup Tx queue")
> >Cc: stable@dpdk.org
> >
> >Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> Acked-by: Rasesh Mody <rmody@marvell.com>

Applied, thanks



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

end of thread, other threads:[~2020-05-24 15:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 15:10 [dpdk-dev] [PATCH] examples/kni: fix crash during MTU set Ferruh Yigit
2020-05-21 15:37 ` Thomas Monjalon
2020-05-21 15:46   ` Ferruh Yigit
2020-05-21 20:29     ` Thomas Monjalon
2020-05-21 17:32 ` [dpdk-dev] [EXT] " Rasesh Mody
2020-05-24 15:50   ` Thomas Monjalon
2020-05-22  2:17 ` [dpdk-dev] " Zhang, XiX

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