DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/2] app/testpmd: fix invalid rxq and txq nubmer setting
@ 2018-01-08 13:02 Wei Dai
  2018-01-08 13:02 ` [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
                   ` (2 more replies)
  0 siblings, 3 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-08 13:02 UTC (permalink / raw)
  To: wenzhuo.lu, jingjing.wu, yuan.peng; +Cc: dev, stable, Wei Dai

If an invlaid number of RX or TX queues is configured from testpmd
command like "port config all rxq number" or "port config all txq number".
The global variable rxq or txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of
RX or TX queues is 4, testpmd will crash after running commands
"port config all rxq 5", "port config all txq 5" and "start" in sequence.

These 2 patches reserve the last correct rxq and txq, if an invalid input
is detected, it is restored to the backup value to avoid crash.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>

Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c |  2 ++
 app/test-pmd/testpmd.c | 19 +++++++++++++++----
 app/test-pmd/testpmd.h |  3 +++
 3 files changed, 20 insertions(+), 4 deletions(-)

-- 
2.7.5

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

* [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-08 13:02 [dpdk-dev] [PATCH 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
@ 2018-01-08 13:02 ` Wei Dai
  2018-01-08 20:05   ` Ananyev, Konstantin
  2018-01-08 13:02 ` [dpdk-dev] [PATCH 2/2] app/testpmd: fix invalid txq " Wei Dai
  2018-01-10  4:14 ` [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2 siblings, 1 reply; 32+ messages in thread
From: Wei Dai @ 2018-01-08 13:02 UTC (permalink / raw)
  To: wenzhuo.lu, jingjing.wu, yuan.peng; +Cc: dev, stable, Wei Dai

If an invalid RX queue is configured from testpmd command
like "port config all rxq number", the global variable rxq
is updated by this invalid value. It may cause testpmd crash.
This patch restores its last correct value when an invalid
rxq number configured is detected.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 app/test-pmd/cmdline.c |  1 +
 app/test-pmd/testpmd.c | 10 ++++++++--
 app/test-pmd/testpmd.h |  2 ++
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f71d963..3f3986c 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1501,6 +1501,7 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		nb_rxq_bak = nb_rxq;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 47e145c..5939c88 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -188,6 +188,8 @@ uint8_t dcb_test = 0;
 queueid_t nb_rxq = 1; /**< Number of RX queues per port. */
 queueid_t nb_txq = 1; /**< Number of TX queues per port. */
 
+queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX queues */
+
 /*
  * Configurable number of RX/TX ring descriptors.
  */
@@ -708,10 +710,14 @@ init_fwd_streams(void)
 		port = &ports[pid];
 		if (nb_rxq > port->dev_info.max_rx_queues) {
 			printf("Fail: nb_rxq(%d) is greater than "
-				"max_rx_queues(%d)\n", nb_rxq,
-				port->dev_info.max_rx_queues);
+				"max_rx_queues(%d), restore to backup "
+				"rxq number(%d)\n", nb_rxq,
+				port->dev_info.max_rx_queues,
+				nb_rxq_bak);
+			nb_rxq = nb_rxq_bak;
 			return -1;
 		}
+		nb_rxq_bak = nb_rxq;
 		if (nb_txq > port->dev_info.max_tx_queues) {
 			printf("Fail: nb_txq(%d) is greater than "
 				"max_tx_queues(%d)\n", nb_txq,
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 4d7f27c..84246f7 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -391,6 +391,8 @@ extern uint64_t rss_hf;
 extern queueid_t nb_rxq;
 extern queueid_t nb_txq;
 
+extern queueid_t nb_rxq_bak;
+
 extern uint16_t nb_rxd;
 extern uint16_t nb_txd;
 
-- 
2.7.5

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

* [dpdk-dev] [PATCH 2/2] app/testpmd: fix invalid txq number setting
  2018-01-08 13:02 [dpdk-dev] [PATCH 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2018-01-08 13:02 ` [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-08 13:02 ` Wei Dai
  2018-01-10  4:14 ` [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-08 13:02 UTC (permalink / raw)
  To: wenzhuo.lu, jingjing.wu, yuan.peng; +Cc: dev, stable, Wei Dai

If an invalid TX queue is configured from testpmd command
like "port config all txq number", the global variable txq
is updated by this invalid value. It may cause testpmd crash.
This patch restores its last correct value when an invalid
txq number configured is detected.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 app/test-pmd/cmdline.c | 1 +
 app/test-pmd/testpmd.c | 9 +++++++--
 app/test-pmd/testpmd.h | 1 +
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 3f3986c..b3ca850 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1509,6 +1509,7 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		nb_txq_bak = nb_txq;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 5939c88..894e49f 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,7 @@ queueid_t nb_rxq = 1; /**< Number of RX queues per port. */
 queueid_t nb_txq = 1; /**< Number of TX queues per port. */
 
 queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX queues */
+queueid_t nb_txq_bak = 1; /**< Backup of last correct number of TX queues */
 
 /*
  * Configurable number of RX/TX ring descriptors.
@@ -720,10 +721,14 @@ init_fwd_streams(void)
 		nb_rxq_bak = nb_rxq;
 		if (nb_txq > port->dev_info.max_tx_queues) {
 			printf("Fail: nb_txq(%d) is greater than "
-				"max_tx_queues(%d)\n", nb_txq,
-				port->dev_info.max_tx_queues);
+				"max_tx_queues(%d), restore to backup "
+				"txq number(%d)\n", nb_txq,
+				port->dev_info.max_tx_queues,
+				nb_txq_bak);
+			nb_txq = nb_txq_bak;
 			return -1;
 		}
+		nb_txq_bak = nb_txq;
 		if (numa_support) {
 			if (port_numa[pid] != NUMA_NO_CONFIG)
 				port->socket_id = port_numa[pid];
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 84246f7..7480c1a 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -392,6 +392,7 @@ extern queueid_t nb_rxq;
 extern queueid_t nb_txq;
 
 extern queueid_t nb_rxq_bak;
+extern queueid_t nb_txq_bak;
 
 extern uint16_t nb_rxd;
 extern uint16_t nb_txd;
-- 
2.7.5

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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-08 13:02 ` [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-08 20:05   ` Ananyev, Konstantin
  2018-01-10  1:33     ` Dai, Wei
  0 siblings, 1 reply; 32+ messages in thread
From: Ananyev, Konstantin @ 2018-01-08 20:05 UTC (permalink / raw)
  To: Dai, Wei, Lu, Wenzhuo, Wu, Jingjing, Peng, Yuan; +Cc: dev, stable, Dai,  Wei



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> Sent: Monday, January 8, 2018 1:03 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
> 
> If an invalid RX queue is configured from testpmd command
> like "port config all rxq number", the global variable rxq
> is updated by this invalid value. It may cause testpmd crash.
> This patch restores its last correct value when an invalid
> rxq number configured is detected.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  app/test-pmd/cmdline.c |  1 +
>  app/test-pmd/testpmd.c | 10 ++++++++--
>  app/test-pmd/testpmd.h |  2 ++
>  3 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index f71d963..3f3986c 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1501,6 +1501,7 @@ cmd_config_rx_tx_parsed(void *parsed_result,
>  			printf("Warning: Either rx or tx queues should be non zero\n");
>  			return;
>  		}
> +		nb_rxq_bak = nb_rxq;
>  		nb_rxq = res->value;
>  	}
>  	else if (!strcmp(res->name, "txq")) {
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 47e145c..5939c88 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -188,6 +188,8 @@ uint8_t dcb_test = 0;
>  queueid_t nb_rxq = 1; /**< Number of RX queues per port. */
>  queueid_t nb_txq = 1; /**< Number of TX queues per port. */
> 
> +queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX queues */
> +
>  /*
>   * Configurable number of RX/TX ring descriptors.
>   */
> @@ -708,10 +710,14 @@ init_fwd_streams(void)
>  		port = &ports[pid];
>  		if (nb_rxq > port->dev_info.max_rx_queues) {

Why not to add that check in the function handler for " port config ... rxq ..." command itself?
In that case you wouldn't need nb_rxq_bak at all.
Konstantin

>  			printf("Fail: nb_rxq(%d) is greater than "
> -				"max_rx_queues(%d)\n", nb_rxq,
> -				port->dev_info.max_rx_queues);
> +				"max_rx_queues(%d), restore to backup "
> +				"rxq number(%d)\n", nb_rxq,
> +				port->dev_info.max_rx_queues,
> +				nb_rxq_bak);
> +			nb_rxq = nb_rxq_bak;
>  			return -1;
>  		}
> +		nb_rxq_bak = nb_rxq;
>  		if (nb_txq > port->dev_info.max_tx_queues) {
>  			printf("Fail: nb_txq(%d) is greater than "
>  				"max_tx_queues(%d)\n", nb_txq,
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 4d7f27c..84246f7 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -391,6 +391,8 @@ extern uint64_t rss_hf;
>  extern queueid_t nb_rxq;
>  extern queueid_t nb_txq;
> 
> +extern queueid_t nb_rxq_bak;
> +
>  extern uint16_t nb_rxd;
>  extern uint16_t nb_txd;
> 
> --
> 2.7.5

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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-08 20:05   ` Ananyev, Konstantin
@ 2018-01-10  1:33     ` Dai, Wei
  2018-01-10  1:54       ` Ananyev, Konstantin
  0 siblings, 1 reply; 32+ messages in thread
From: Dai, Wei @ 2018-01-10  1:33 UTC (permalink / raw)
  To: Ananyev, Konstantin, Lu, Wenzhuo, Wu, Jingjing, Peng, Yuan; +Cc: dev, stable

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Tuesday, January 9, 2018 4:06 AM
> To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number
> setting
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > Sent: Monday, January 8, 2018 1:03 PM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number
> > setting
> >
> > If an invalid RX queue is configured from testpmd command like "port
> > config all rxq number", the global variable rxq is updated by this
> > invalid value. It may cause testpmd crash.
> > This patch restores its last correct value when an invalid rxq number
> > configured is detected.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >  app/test-pmd/cmdline.c |  1 +
> >  app/test-pmd/testpmd.c | 10 ++++++++--  app/test-pmd/testpmd.h |
> 2
> > ++
> >  3 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > f71d963..3f3986c 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -1501,6 +1501,7 @@ cmd_config_rx_tx_parsed(void *parsed_result,
> >  			printf("Warning: Either rx or tx queues should be non zero\n");
> >  			return;
> >  		}
> > +		nb_rxq_bak = nb_rxq;
> >  		nb_rxq = res->value;
> >  	}
> >  	else if (!strcmp(res->name, "txq")) { diff --git
> > a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > 47e145c..5939c88 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -188,6 +188,8 @@ uint8_t dcb_test = 0;  queueid_t nb_rxq = 1; /**<
> > Number of RX queues per port. */  queueid_t nb_txq = 1; /**< Number of
> > TX queues per port. */
> >
> > +queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX
> > +queues */
> > +
> >  /*
> >   * Configurable number of RX/TX ring descriptors.
> >   */
> > @@ -708,10 +710,14 @@ init_fwd_streams(void)
> >  		port = &ports[pid];
> >  		if (nb_rxq > port->dev_info.max_rx_queues) {
> 
> Why not to add that check in the function handler for " port config ... rxq ..."
> command itself?
> In that case you wouldn't need nb_rxq_bak at all.
> Konstantin

Thanks for your feedback, Konstantin.
I see your point. If that check is added in the function handler of command line,
same loop code will be repeated.

There are 2 ways to change nb_rxq:
1. with --rxq in the command line to start testpmd app to overwrite the default value 1
2. with "port config all rxq number" in the testpmd run time command line

How can your method correct an invalid input with --rxq in the command to start testpm app ?

By the way,  my v1 patch has a bug in case of more than 1 port with different maximum number of Rx queues.
For example,  the max_rx_queues of port 0 is 8 and that of port 1 is 4.
The previous valid rxq=4, if "port config all rxq 5" is run,
The nb_rxq_bak is updated to 5,  nb_rxq can't be restored to last correct value 4 for port 1.

I will fix bug in my v2 patch.

 
> 
> >  			printf("Fail: nb_rxq(%d) is greater than "
> > -				"max_rx_queues(%d)\n", nb_rxq,
> > -				port->dev_info.max_rx_queues);
> > +				"max_rx_queues(%d), restore to backup "
> > +				"rxq number(%d)\n", nb_rxq,
> > +				port->dev_info.max_rx_queues,
> > +				nb_rxq_bak);
> > +			nb_rxq = nb_rxq_bak;
> >  			return -1;
> >  		}
> > +		nb_rxq_bak = nb_rxq;
> >  		if (nb_txq > port->dev_info.max_tx_queues) {
> >  			printf("Fail: nb_txq(%d) is greater than "
> >  				"max_tx_queues(%d)\n", nb_txq,
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 4d7f27c..84246f7 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -391,6 +391,8 @@ extern uint64_t rss_hf;  extern queueid_t nb_rxq;
> > extern queueid_t nb_txq;
> >
> > +extern queueid_t nb_rxq_bak;
> > +
> >  extern uint16_t nb_rxd;
> >  extern uint16_t nb_txd;
> >
> > --
> > 2.7.5

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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-10  1:33     ` Dai, Wei
@ 2018-01-10  1:54       ` Ananyev, Konstantin
  2018-01-10  6:00         ` Dai, Wei
  0 siblings, 1 reply; 32+ messages in thread
From: Ananyev, Konstantin @ 2018-01-10  1:54 UTC (permalink / raw)
  To: Dai, Wei, Lu, Wenzhuo, Wu, Jingjing, Peng, Yuan; +Cc: dev, stable



> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, January 10, 2018 1:34 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Peng, Yuan <yuan.peng@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
> 
> > -----Original Message-----
> > From: Ananyev, Konstantin
> > Sent: Tuesday, January 9, 2018 4:06 AM
> > To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> > Wu, Jingjing <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: RE: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number
> > setting
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > > Sent: Monday, January 8, 2018 1:03 PM
> > > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > > <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> > > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > > Subject: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number
> > > setting
> > >
> > > If an invalid RX queue is configured from testpmd command like "port
> > > config all rxq number", the global variable rxq is updated by this
> > > invalid value. It may cause testpmd crash.
> > > This patch restores its last correct value when an invalid rxq number
> > > configured is detected.
> > >
> > > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > > ---
> > >  app/test-pmd/cmdline.c |  1 +
> > >  app/test-pmd/testpmd.c | 10 ++++++++--  app/test-pmd/testpmd.h |
> > 2
> > > ++
> > >  3 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > f71d963..3f3986c 100644
> > > --- a/app/test-pmd/cmdline.c
> > > +++ b/app/test-pmd/cmdline.c
> > > @@ -1501,6 +1501,7 @@ cmd_config_rx_tx_parsed(void *parsed_result,
> > >  			printf("Warning: Either rx or tx queues should be non zero\n");
> > >  			return;
> > >  		}
> > > +		nb_rxq_bak = nb_rxq;
> > >  		nb_rxq = res->value;
> > >  	}
> > >  	else if (!strcmp(res->name, "txq")) { diff --git
> > > a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > 47e145c..5939c88 100644
> > > --- a/app/test-pmd/testpmd.c
> > > +++ b/app/test-pmd/testpmd.c
> > > @@ -188,6 +188,8 @@ uint8_t dcb_test = 0;  queueid_t nb_rxq = 1; /**<
> > > Number of RX queues per port. */  queueid_t nb_txq = 1; /**< Number of
> > > TX queues per port. */
> > >
> > > +queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX
> > > +queues */
> > > +
> > >  /*
> > >   * Configurable number of RX/TX ring descriptors.
> > >   */
> > > @@ -708,10 +710,14 @@ init_fwd_streams(void)
> > >  		port = &ports[pid];
> > >  		if (nb_rxq > port->dev_info.max_rx_queues) {
> >
> > Why not to add that check in the function handler for " port config ... rxq ..."
> > command itself?
> > In that case you wouldn't need nb_rxq_bak at all.
> > Konstantin
> 
> Thanks for your feedback, Konstantin.
> I see your point. If that check is added in the function handler of command line,
> same loop code will be repeated.
> 
> There are 2 ways to change nb_rxq:
> 1. with --rxq in the command line to start testpmd app to overwrite the default value 1
> 2. with "port config all rxq number" in the testpmd run time command line
> 
> How can your method correct an invalid input with --rxq in the command to start testpm app ?

Have a separate function: test_queue_num(port, qnum) and call it from both places?
Konstantin 

> 
> By the way,  my v1 patch has a bug in case of more than 1 port with different maximum number of Rx queues.
> For example,  the max_rx_queues of port 0 is 8 and that of port 1 is 4.
> The previous valid rxq=4, if "port config all rxq 5" is run,
> The nb_rxq_bak is updated to 5,  nb_rxq can't be restored to last correct value 4 for port 1.
> 
> I will fix bug in my v2 patch.
> 
> 
> >
> > >  			printf("Fail: nb_rxq(%d) is greater than "
> > > -				"max_rx_queues(%d)\n", nb_rxq,
> > > -				port->dev_info.max_rx_queues);
> > > +				"max_rx_queues(%d), restore to backup "
> > > +				"rxq number(%d)\n", nb_rxq,
> > > +				port->dev_info.max_rx_queues,
> > > +				nb_rxq_bak);
> > > +			nb_rxq = nb_rxq_bak;
> > >  			return -1;
> > >  		}
> > > +		nb_rxq_bak = nb_rxq;
> > >  		if (nb_txq > port->dev_info.max_tx_queues) {
> > >  			printf("Fail: nb_txq(%d) is greater than "
> > >  				"max_tx_queues(%d)\n", nb_txq,
> > > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > > 4d7f27c..84246f7 100644
> > > --- a/app/test-pmd/testpmd.h
> > > +++ b/app/test-pmd/testpmd.h
> > > @@ -391,6 +391,8 @@ extern uint64_t rss_hf;  extern queueid_t nb_rxq;
> > > extern queueid_t nb_txq;
> > >
> > > +extern queueid_t nb_rxq_bak;
> > > +
> > >  extern uint16_t nb_rxd;
> > >  extern uint16_t nb_txd;
> > >
> > > --
> > > 2.7.5

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

* [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting
  2018-01-08 13:02 [dpdk-dev] [PATCH 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2018-01-08 13:02 ` [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
  2018-01-08 13:02 ` [dpdk-dev] [PATCH 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-10  4:14 ` Wei Dai
  2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
                     ` (2 more replies)
  2 siblings, 3 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-10  4:14 UTC (permalink / raw)
  To: wenzhuo.lu, jingjing.wu, yuan.peng, konstantin.ananyev
  Cc: dev, stable, Wei Dai

If an invlaid number of RX or TX queues is configured from testpmd
command like "port config all rxq number" or "port config all txq number".
The global variable rxq or txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of
RX or TX queues is 4, testpmd will crash after running commands
"port config all rxq 5", "port config all txq 5" and "start" in sequence.

These 2 patches reserve the last correct rxq and txq, if an invalid input
is detected, it is restored to the backup value to avoid crash.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>

---
v2: fix a bug in v1

Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c |  4 ++++
 app/test-pmd/testpmd.c | 23 +++++++++++++++++++----
 app/test-pmd/testpmd.h |  3 +++
 3 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.7.5

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

* [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-10  4:14 ` [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
@ 2018-01-10  4:14   ` Wei Dai
  2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq " Wei Dai
  2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-10  4:14 UTC (permalink / raw)
  To: wenzhuo.lu, jingjing.wu, yuan.peng, konstantin.ananyev
  Cc: dev, stable, Wei Dai

If an invalid RX queue is configured from testpmd command
like "port config all rxq number", the global variable rxq
is updated by this invalid value. It may cause testpmd crash.
This patch restores its last correct value when an invalid
rxq number configured is detected.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 app/test-pmd/cmdline.c |  2 ++
 app/test-pmd/testpmd.c | 13 +++++++++++--
 app/test-pmd/testpmd.h |  2 ++
 3 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5b2e2ef..a5a1d57 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1518,6 +1518,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		/* bakcup last correct nb_rxq */
+		nb_rxq_bak = nb_rxq;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9414d0e..efafc24 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -189,6 +189,8 @@ uint8_t dcb_test = 0;
 queueid_t nb_rxq = 1; /**< Number of RX queues per port. */
 queueid_t nb_txq = 1; /**< Number of TX queues per port. */
 
+queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX queues */
+
 /*
  * Configurable number of RX/TX ring descriptors.
  */
@@ -709,8 +711,12 @@ init_fwd_streams(void)
 		port = &ports[pid];
 		if (nb_rxq > port->dev_info.max_rx_queues) {
 			printf("Fail: nb_rxq(%d) is greater than "
-				"max_rx_queues(%d)\n", nb_rxq,
-				port->dev_info.max_rx_queues);
+				"max_rx_queues(%d), restored to backup "
+				"rxq number(%d)\n", nb_rxq,
+				port->dev_info.max_rx_queues,
+				nb_rxq_bak);
+			/* restored to last corrcect nb_rxq */
+			nb_rxq = nb_rxq_bak;
 			return -1;
 		}
 		if (nb_txq > port->dev_info.max_tx_queues) {
@@ -738,6 +744,9 @@ init_fwd_streams(void)
 		}
 	}
 
+	/* backup the correct nb_rxq */
+	nb_rxq_bak = nb_rxq;
+
 	q = RTE_MAX(nb_rxq, nb_txq);
 	if (q == 0) {
 		printf("Fail: Cannot allocate fwd streams as number of queues is 0\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2a266fd..6f7932d 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -392,6 +392,8 @@ extern uint64_t rss_hf;
 extern queueid_t nb_rxq;
 extern queueid_t nb_txq;
 
+extern queueid_t nb_rxq_bak;
+
 extern uint16_t nb_rxd;
 extern uint16_t nb_txd;
 
-- 
2.7.5

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

* [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq number setting
  2018-01-10  4:14 ` [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-10  4:14   ` Wei Dai
  2018-01-10  6:37     ` Yang, Qiming
  2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2 siblings, 1 reply; 32+ messages in thread
From: Wei Dai @ 2018-01-10  4:14 UTC (permalink / raw)
  To: wenzhuo.lu, jingjing.wu, yuan.peng, konstantin.ananyev
  Cc: dev, stable, Wei Dai

If an invalid TX queue is configured from testpmd command
like "port config all txq number", the global variable txq
is updated by this invalid value. It may cause testpmd crash.
This patch restores its last correct value when an invalid
txq number configured is detected.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 app/test-pmd/cmdline.c |  2 ++
 app/test-pmd/testpmd.c | 12 +++++++++---
 app/test-pmd/testpmd.h |  1 +
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index a5a1d57..26dd81a 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		/* bakcup last correct nb_txq */
+		nb_txq_bak = nb_txq;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index efafc24..8b49d96 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -190,6 +190,7 @@ queueid_t nb_rxq = 1; /**< Number of RX queues per port. */
 queueid_t nb_txq = 1; /**< Number of TX queues per port. */
 
 queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX queues */
+queueid_t nb_txq_bak = 1; /**< Backup of last correct number of TX queues */
 
 /*
  * Configurable number of RX/TX ring descriptors.
@@ -721,8 +722,12 @@ init_fwd_streams(void)
 		}
 		if (nb_txq > port->dev_info.max_tx_queues) {
 			printf("Fail: nb_txq(%d) is greater than "
-				"max_tx_queues(%d)\n", nb_txq,
-				port->dev_info.max_tx_queues);
+				"max_tx_queues(%d), restored to backup "
+				"txq number(%d)\n", nb_txq,
+				port->dev_info.max_tx_queues,
+				nb_txq_bak);
+			/* restored to last correct nb_txq */
+			nb_txq = nb_txq_bak;
 			return -1;
 		}
 		if (numa_support) {
@@ -744,8 +749,9 @@ init_fwd_streams(void)
 		}
 	}
 
-	/* backup the correct nb_rxq */
+	/* backup the correct nb_rxq and nb_txq */
 	nb_rxq_bak = nb_rxq;
+	nb_txq_bak = nb_txq;
 
 	q = RTE_MAX(nb_rxq, nb_txq);
 	if (q == 0) {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 6f7932d..bca93c1 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -393,6 +393,7 @@ extern queueid_t nb_rxq;
 extern queueid_t nb_txq;
 
 extern queueid_t nb_rxq_bak;
+extern queueid_t nb_txq_bak;
 
 extern uint16_t nb_rxd;
 extern uint16_t nb_txd;
-- 
2.7.5

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

* Re: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-10  1:54       ` Ananyev, Konstantin
@ 2018-01-10  6:00         ` Dai, Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Dai, Wei @ 2018-01-10  6:00 UTC (permalink / raw)
  To: Ananyev, Konstantin, Lu, Wenzhuo, Wu, Jingjing, Peng, Yuan; +Cc: dev, stable



> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, January 10, 2018 9:55 AM
> To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number
> setting
> 
> 
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Wednesday, January 10, 2018 1:34 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo
> > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Peng,
> > Yuan <yuan.peng@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq
> > number setting
> >
> > > -----Original Message-----
> > > From: Ananyev, Konstantin
> > > Sent: Tuesday, January 9, 2018 4:06 AM
> > > To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo
> > > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Peng,
> > > Yuan <yuan.peng@intel.com>
> > > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > > Subject: RE: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq
> > > number setting
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > > > Sent: Monday, January 8, 2018 1:03 PM
> > > > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > > > <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>
> > > > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > > > Subject: [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq
> > > > number setting
> > > >
> > > > If an invalid RX queue is configured from testpmd command like
> > > > "port config all rxq number", the global variable rxq is updated
> > > > by this invalid value. It may cause testpmd crash.
> > > > This patch restores its last correct value when an invalid rxq
> > > > number configured is detected.
> > > >
> > > > Fixes: ce8d561418d4 ("app/testpmd: add port configuration
> > > > settings")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > > > ---
> > > >  app/test-pmd/cmdline.c |  1 +
> > > >  app/test-pmd/testpmd.c | 10 ++++++++--  app/test-pmd/testpmd.h
> |
> > > 2
> > > > ++
> > > >  3 files changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > > > f71d963..3f3986c 100644
> > > > --- a/app/test-pmd/cmdline.c
> > > > +++ b/app/test-pmd/cmdline.c
> > > > @@ -1501,6 +1501,7 @@ cmd_config_rx_tx_parsed(void
> *parsed_result,
> > > >  			printf("Warning: Either rx or tx queues should be non
> zero\n");
> > > >  			return;
> > > >  		}
> > > > +		nb_rxq_bak = nb_rxq;
> > > >  		nb_rxq = res->value;
> > > >  	}
> > > >  	else if (!strcmp(res->name, "txq")) { diff --git
> > > > a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > > > 47e145c..5939c88 100644
> > > > --- a/app/test-pmd/testpmd.c
> > > > +++ b/app/test-pmd/testpmd.c
> > > > @@ -188,6 +188,8 @@ uint8_t dcb_test = 0;  queueid_t nb_rxq = 1;
> > > > /**< Number of RX queues per port. */  queueid_t nb_txq = 1; /**<
> > > > Number of TX queues per port. */
> > > >
> > > > +queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of
> > > > +RX queues */
> > > > +
> > > >  /*
> > > >   * Configurable number of RX/TX ring descriptors.
> > > >   */
> > > > @@ -708,10 +710,14 @@ init_fwd_streams(void)
> > > >  		port = &ports[pid];
> > > >  		if (nb_rxq > port->dev_info.max_rx_queues) {
> > >
> > > Why not to add that check in the function handler for " port config ...
> rxq ..."
> > > command itself?
> > > In that case you wouldn't need nb_rxq_bak at all.
> > > Konstantin
> >
> > Thanks for your feedback, Konstantin.
> > I see your point. If that check is added in the function handler of
> > command line, same loop code will be repeated.
> >
> > There are 2 ways to change nb_rxq:
> > 1. with --rxq in the command line to start testpmd app to overwrite
> > the default value 1 2. with "port config all rxq number" in the
> > testpmd run time command line
> >
> > How can your method correct an invalid input with --rxq in the command
> to start testpm app ?
> 
> Have a separate function: test_queue_num(port, qnum) and call it from both
> places?
> Konstantin
> 

This nb_rxq is a common setting for all ports.
I can follow your guide in my v3 patch. 
Thanks

> >
> > By the way,  my v1 patch has a bug in case of more than 1 port with
> different maximum number of Rx queues.
> > For example,  the max_rx_queues of port 0 is 8 and that of port 1 is 4.
> > The previous valid rxq=4, if "port config all rxq 5" is run, The
> > nb_rxq_bak is updated to 5,  nb_rxq can't be restored to last correct value
> 4 for port 1.
> >
> > I will fix bug in my v2 patch.
> >
> >
> > >
> > > >  			printf("Fail: nb_rxq(%d) is greater than "
> > > > -				"max_rx_queues(%d)\n", nb_rxq,
> > > > -				port->dev_info.max_rx_queues);
> > > > +				"max_rx_queues(%d), restore to backup "
> > > > +				"rxq number(%d)\n", nb_rxq,
> > > > +				port->dev_info.max_rx_queues,
> > > > +				nb_rxq_bak);
> > > > +			nb_rxq = nb_rxq_bak;
> > > >  			return -1;
> > > >  		}
> > > > +		nb_rxq_bak = nb_rxq;
> > > >  		if (nb_txq > port->dev_info.max_tx_queues) {
> > > >  			printf("Fail: nb_txq(%d) is greater than "
> > > >  				"max_tx_queues(%d)\n", nb_txq, diff --git
> > > > a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > > > 4d7f27c..84246f7 100644
> > > > --- a/app/test-pmd/testpmd.h
> > > > +++ b/app/test-pmd/testpmd.h
> > > > @@ -391,6 +391,8 @@ extern uint64_t rss_hf;  extern queueid_t
> > > > nb_rxq; extern queueid_t nb_txq;
> > > >
> > > > +extern queueid_t nb_rxq_bak;
> > > > +
> > > >  extern uint16_t nb_rxd;
> > > >  extern uint16_t nb_txd;
> > > >
> > > > --
> > > > 2.7.5

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

* Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq number setting
  2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-10  6:37     ` Yang, Qiming
  2018-01-10  8:50       ` Dai, Wei
  0 siblings, 1 reply; 32+ messages in thread
From: Yang, Qiming @ 2018-01-10  6:37 UTC (permalink / raw)
  To: Dai, Wei, Lu, Wenzhuo, Wu, Jingjing, Peng, Yuan, Ananyev, Konstantin
  Cc: dev, stable, Dai,  Wei

I think the name bak is a little bit confused, what do you think just use nd_txq_backup/nd_rxq_backup?
And I think it's no need to break the patch into two patch, them fix the same thing and the code amount are not large.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> Sent: Wednesday, January 10, 2018 12:14 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq number setting
> 
> If an invalid TX queue is configured from testpmd command like "port config all
> txq number", the global variable txq is updated by this invalid value. It may cause
> testpmd crash.
> This patch restores its last correct value when an invalid txq number configured
> is detected.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> ---
>  app/test-pmd/cmdline.c |  2 ++
>  app/test-pmd/testpmd.c | 12 +++++++++---  app/test-pmd/testpmd.h |  1 +
>  3 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> a5a1d57..26dd81a 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
>  			printf("Warning: Either rx or tx queues should be non
> zero\n");
>  			return;
>  		}
> +		/* bakcup last correct nb_txq */
> +		nb_txq_bak = nb_txq;
>  		nb_txq = res->value;
>  	}
>  	else if (!strcmp(res->name, "rxd")) {
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> efafc24..8b49d96 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -190,6 +190,7 @@ queueid_t nb_rxq = 1; /**< Number of RX queues per
> port. */  queueid_t nb_txq = 1; /**< Number of TX queues per port. */
> 
>  queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX queues */
> +queueid_t nb_txq_bak = 1; /**< Backup of last correct number of TX
> +queues */
> 
>  /*
>   * Configurable number of RX/TX ring descriptors.
> @@ -721,8 +722,12 @@ init_fwd_streams(void)
>  		}
>  		if (nb_txq > port->dev_info.max_tx_queues) {
>  			printf("Fail: nb_txq(%d) is greater than "
> -				"max_tx_queues(%d)\n", nb_txq,
> -				port->dev_info.max_tx_queues);
> +				"max_tx_queues(%d), restored to backup "
> +				"txq number(%d)\n", nb_txq,
> +				port->dev_info.max_tx_queues,
> +				nb_txq_bak);
> +			/* restored to last correct nb_txq */
> +			nb_txq = nb_txq_bak;
>  			return -1;
>  		}
>  		if (numa_support) {
> @@ -744,8 +749,9 @@ init_fwd_streams(void)
>  		}
>  	}
> 
> -	/* backup the correct nb_rxq */
> +	/* backup the correct nb_rxq and nb_txq */
>  	nb_rxq_bak = nb_rxq;
> +	nb_txq_bak = nb_txq;
> 
>  	q = RTE_MAX(nb_rxq, nb_txq);
>  	if (q == 0) {
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 6f7932d..bca93c1 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -393,6 +393,7 @@ extern queueid_t nb_rxq;  extern queueid_t nb_txq;
> 
>  extern queueid_t nb_rxq_bak;
> +extern queueid_t nb_txq_bak;
> 
>  extern uint16_t nb_rxd;
>  extern uint16_t nb_txd;
> --
> 2.7.5

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

* [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting
  2018-01-10  4:14 ` [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
  2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-10  8:40   ` Wei Dai
  2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
                       ` (3 more replies)
  2 siblings, 4 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-10  8:40 UTC (permalink / raw)
  To: konstantin.ananyev, qiming.yang, yuan.peng, wenzhuo.lu, jingjing.wu
  Cc: dev, stable, Wei Dai

If an invlaid number of RX or TX queues is configured from testpmd
command like "port config all rxq number" or "port config all txq number".
The global variable rxq or txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of
RX or TX queues is 4, testpmd will crash after running commands
"port config all rxq 5", "port config all txq 5" and "start" in sequence.

These 2 patches reserve the last correct rxq and txq, if an invalid input
is detected, it is restored to the backup value to avoid crash.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>

---
v3: follow the guide from Konstantin to use functions to check 
    input rxq and txq instead of usage of new added global variables.

v2: fix a bug in v1


Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c    |  4 ++
 app/test-pmd/parameters.c | 13 ++++---
 app/test-pmd/testpmd.c    | 94 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  5 +++
 4 files changed, 110 insertions(+), 6 deletions(-)

-- 
2.7.5

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

* [dpdk-dev] [PATCH v3 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
@ 2018-01-10  8:40     ` Wei Dai
  2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix invalid txq " Wei Dai
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-10  8:40 UTC (permalink / raw)
  To: konstantin.ananyev, qiming.yang, yuan.peng, wenzhuo.lu, jingjing.wu
  Cc: dev, stable, Wei Dai

If an invalid RX queue is configured from testpmd command
like "port config all rxq number", the global variable rxq
is updated by this invalid value. It may cause testpmd crash.
This patch restores its last correct value when an invalid
rxq number configured is detected.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  7 ++++---
 app/test-pmd/testpmd.c    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  3 +++
 4 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5b2e2ef..f0623b1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1518,6 +1518,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_rxq(res->value) != 0)
+			return;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 304b98d..eac1826 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -536,6 +536,7 @@ launch_args_parse(int argc, char** argv)
 	int n, opt;
 	char **argvopt;
 	int opt_idx;
+	portid_t pid;
 	enum { TX, RX };
 
 	static struct option lgopts[] = {
@@ -922,12 +923,12 @@ launch_args_parse(int argc, char** argv)
 				rss_hf = ETH_RSS_UDP;
 			if (!strcmp(lgopts[opt_idx].name, "rxq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_rxq((queueid_t)n) == 0)
 					nb_rxq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "rxq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_nb_rxq(&pid));
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9414d0e..1203b17 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -540,6 +540,53 @@ check_socket_id(const unsigned int socket_id)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of RX queues.
+ * *pid return the port id which has mimumal value of
+ * max_rx_queues in all ports.
+ */
+
+queueid_t
+get_allowed_nb_rxq(portid_t *pid)
+{
+	queueid_t allowed_max_rxq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_rx_queues < allowed_max_rxq) {
+			allowed_max_rxq = dev_info.max_rx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_rxq;
+}
+
+/*
+ * Check input rxq is valid or not.
+ * If input rxq is not greater than any of maximum number
+ * of RX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_rxq(queueid_t rxq)
+{
+	queueid_t allowed_max_rxq;
+	portid_t pid;
+
+	allowed_max_rxq = get_allowed_nb_rxq(&pid);
+	if (rxq > allowed_max_rxq) {
+		printf("Fail: input rxq (%u) can't be greater "
+		       "than max_rx_queues (%u) of port %u\n",
+		       rxq,
+		       allowed_max_rxq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2a266fd..1e38f43 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -700,6 +700,9 @@ enum print_warning {
 int port_id_is_invalid(portid_t port_id, enum print_warning warning);
 int new_socket_id(unsigned int socket_id);
 
+queueid_t get_allowed_nb_rxq(portid_t *pid);
+int check_nb_rxq(queueid_t rxq);
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
-- 
2.7.5

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

* [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix invalid txq number setting
  2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-10  8:40     ` Wei Dai
  2018-01-10  9:58     ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Ananyev, Konstantin
  2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
  3 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-10  8:40 UTC (permalink / raw)
  To: konstantin.ananyev, qiming.yang, yuan.peng, wenzhuo.lu, jingjing.wu
  Cc: dev, stable, Wei Dai

If an invalid TX queue is configured from testpmd command
like "port config all txq number", the global variable txq
is updated by this invalid value. It may cause testpmd crash.
This patch restores its last correct value when an invalid
txq number configured is detected.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  6 +++---
 app/test-pmd/testpmd.c    | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  2 ++
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f0623b1..6619cb8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_txq(res->value) != 0)
+			return;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index eac1826..6b5925d 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -932,12 +932,12 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_txq((queueid_t)n) == 0)
 					nb_txq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "txq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_nb_txq(&pid));
 			}
 			if (!nb_rxq && !nb_txq) {
 				rte_exit(EXIT_FAILURE, "Either rx or tx queues should "
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1203b17..fb8bb48 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -587,6 +587,53 @@ check_nb_rxq(queueid_t rxq)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of TX queues.
+ * *pid return the port id which has mimumal value of
+ * max_tx_queues in all ports.
+ */
+
+queueid_t
+get_allowed_nb_txq(portid_t *pid)
+{
+	queueid_t allowed_max_txq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_tx_queues < allowed_max_txq) {
+			allowed_max_txq = dev_info.max_tx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_txq;
+}
+
+/*
+ * Check input txq is valid or not.
+ * If input txq is not greater than any of maximum number
+ * of TX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_txq(queueid_t txq)
+{
+	queueid_t allowed_max_txq;
+	portid_t pid;
+
+	allowed_max_txq = get_allowed_nb_txq(&pid);
+	if (txq > allowed_max_txq) {
+		printf("Fail: input txq (%u) can't be greater "
+		       "than max_tx_queues (%u) of port %u\n",
+		       txq,
+		       allowed_max_txq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 1e38f43..b848364 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -702,6 +702,8 @@ int new_socket_id(unsigned int socket_id);
 
 queueid_t get_allowed_nb_rxq(portid_t *pid);
 int check_nb_rxq(queueid_t rxq);
+queueid_t get_allowed_nb_txq(portid_t *pid);
+int check_nb_txq(queueid_t txq);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
-- 
2.7.5

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

* Re: [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq number setting
  2018-01-10  6:37     ` Yang, Qiming
@ 2018-01-10  8:50       ` Dai, Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Dai, Wei @ 2018-01-10  8:50 UTC (permalink / raw)
  To: Yang, Qiming, Lu, Wenzhuo, Wu, Jingjing, Peng, Yuan, Ananyev, Konstantin
  Cc: dev, stable

> -----Original Message-----
> From: Yang, Qiming
> Sent: Wednesday, January 10, 2018 2:38 PM
> To: Dai, Wei <wei.dai@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: RE: [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq number
> setting
> 
> I think the name bak is a little bit confused, what do you think just use
> nd_txq_backup/nd_rxq_backup?
> And I think it's no need to break the patch into two patch, them fix the same
> thing and the code amount are not large.

I will follow Konstantin's guide to give v3 patch set.
By the way, I think 2 patches are much clearer and keep each very simple 
for others to review and for maintainer's convenience.

> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wei Dai
> > Sent: Wednesday, January 10, 2018 12:14 PM
> > To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> > <jingjing.wu@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Ananyev,
> > Konstantin <konstantin.ananyev@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq number
> > setting
> >
> > If an invalid TX queue is configured from testpmd command like "port
> > config all txq number", the global variable txq is updated by this
> > invalid value. It may cause testpmd crash.
> > This patch restores its last correct value when an invalid txq number
> > configured is detected.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> > ---
> >  app/test-pmd/cmdline.c |  2 ++
> >  app/test-pmd/testpmd.c | 12 +++++++++---  app/test-pmd/testpmd.h |
> 1
> > +
> >  3 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > a5a1d57..26dd81a 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
> >  			printf("Warning: Either rx or tx queues should be non zero\n");
> >  			return;
> >  		}
> > +		/* bakcup last correct nb_txq */
> > +		nb_txq_bak = nb_txq;
> >  		nb_txq = res->value;
> >  	}
> >  	else if (!strcmp(res->name, "rxd")) { diff --git
> > a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > efafc24..8b49d96 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -190,6 +190,7 @@ queueid_t nb_rxq = 1; /**< Number of RX queues
> per
> > port. */  queueid_t nb_txq = 1; /**< Number of TX queues per port. */
> >
> >  queueid_t nb_rxq_bak = 1; /**< Backup of last correct number of RX
> > queues */
> > +queueid_t nb_txq_bak = 1; /**< Backup of last correct number of TX
> > +queues */
> >
> >  /*
> >   * Configurable number of RX/TX ring descriptors.
> > @@ -721,8 +722,12 @@ init_fwd_streams(void)
> >  		}
> >  		if (nb_txq > port->dev_info.max_tx_queues) {
> >  			printf("Fail: nb_txq(%d) is greater than "
> > -				"max_tx_queues(%d)\n", nb_txq,
> > -				port->dev_info.max_tx_queues);
> > +				"max_tx_queues(%d), restored to backup "
> > +				"txq number(%d)\n", nb_txq,
> > +				port->dev_info.max_tx_queues,
> > +				nb_txq_bak);
> > +			/* restored to last correct nb_txq */
> > +			nb_txq = nb_txq_bak;
> >  			return -1;
> >  		}
> >  		if (numa_support) {
> > @@ -744,8 +749,9 @@ init_fwd_streams(void)
> >  		}
> >  	}
> >
> > -	/* backup the correct nb_rxq */
> > +	/* backup the correct nb_rxq and nb_txq */
> >  	nb_rxq_bak = nb_rxq;
> > +	nb_txq_bak = nb_txq;
> >
> >  	q = RTE_MAX(nb_rxq, nb_txq);
> >  	if (q == 0) {
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > 6f7932d..bca93c1 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -393,6 +393,7 @@ extern queueid_t nb_rxq;  extern queueid_t
> nb_txq;
> >
> >  extern queueid_t nb_rxq_bak;
> > +extern queueid_t nb_txq_bak;
> >
> >  extern uint16_t nb_rxd;
> >  extern uint16_t nb_txd;
> > --
> > 2.7.5

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

* Re: [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting
  2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
  2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
  2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-10  9:58     ` Ananyev, Konstantin
  2018-01-11  1:21       ` Dai, Wei
  2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
  3 siblings, 1 reply; 32+ messages in thread
From: Ananyev, Konstantin @ 2018-01-10  9:58 UTC (permalink / raw)
  To: Dai, Wei, Yang, Qiming, Peng, Yuan, Lu, Wenzhuo, Wu, Jingjing; +Cc: dev, stable



> -----Original Message-----
> From: Dai, Wei
> Sent: Wednesday, January 10, 2018 8:41 AM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Peng, Yuan <yuan.peng@intel.com>;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting
> 
> If an invlaid number of RX or TX queues is configured from testpmd
> command like "port config all rxq number" or "port config all txq number".
> The global variable rxq or txq is updated by the invalid input.
> This can cause testpmd crash. For example, if the maximum number of
> RX or TX queues is 4, testpmd will crash after running commands
> "port config all rxq 5", "port config all txq 5" and "start" in sequence.
> 
> These 2 patches reserve the last correct rxq and txq, if an invalid input
> is detected, it is restored to the backup value to avoid crash.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> 
> ---

The code looks good to me, just seems some misspellings in the comments (mimumal).
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

> v3: follow the guide from Konstantin to use functions to check
>     input rxq and txq instead of usage of new added global variables.
> 
> v2: fix a bug in v1
> 
> 
> Wei Dai (2):
>   app/testpmd: fix invalid rxq number setting
>   app/testpmd: fix invalid txq number setting
> 
>  app/test-pmd/cmdline.c    |  4 ++
>  app/test-pmd/parameters.c | 13 ++++---
>  app/test-pmd/testpmd.c    | 94 +++++++++++++++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h    |  5 +++
>  4 files changed, 110 insertions(+), 6 deletions(-)
> 
> --
> 2.7.5

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

* Re: [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting
  2018-01-10  9:58     ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Ananyev, Konstantin
@ 2018-01-11  1:21       ` Dai, Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Dai, Wei @ 2018-01-11  1:21 UTC (permalink / raw)
  To: Ananyev, Konstantin, Yang, Qiming, Peng, Yuan, Lu, Wenzhuo, Wu, Jingjing
  Cc: dev, stable


> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Wednesday, January 10, 2018 5:59 PM
> To: Dai, Wei <wei.dai@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> Peng, Yuan <yuan.peng@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer
> setting
> 
> 
> 
> > -----Original Message-----
> > From: Dai, Wei
> > Sent: Wednesday, January 10, 2018 8:41 AM
> > To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Lu,
> Wenzhuo
> > <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> > Subject: [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer
> > setting
> >
> > If an invlaid number of RX or TX queues is configured from testpmd
> > command like "port config all rxq number" or "port config all txq number".
> > The global variable rxq or txq is updated by the invalid input.
> > This can cause testpmd crash. For example, if the maximum number of RX
> > or TX queues is 4, testpmd will crash after running commands "port
> > config all rxq 5", "port config all txq 5" and "start" in sequence.
> >
> > These 2 patches reserve the last correct rxq and txq, if an invalid
> > input is detected, it is restored to the backup value to avoid crash.
> >
> > Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Wei Dai <wei.dai@intel.com>
> >
> > ---
> 
> The code looks good to me, just seems some misspellings in the comments
> (mimumal).
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Thanks, Konstantin.
I'll correct these misspellings in my v4 patch set.

> 
> > v3: follow the guide from Konstantin to use functions to check
> >     input rxq and txq instead of usage of new added global variables.
> >
> > v2: fix a bug in v1
> >
> >
> > Wei Dai (2):
> >   app/testpmd: fix invalid rxq number setting
> >   app/testpmd: fix invalid txq number setting
> >
> >  app/test-pmd/cmdline.c    |  4 ++
> >  app/test-pmd/parameters.c | 13 ++++---
> >  app/test-pmd/testpmd.c    | 94
> +++++++++++++++++++++++++++++++++++++++++++++++
> >  app/test-pmd/testpmd.h    |  5 +++
> >  4 files changed, 110 insertions(+), 6 deletions(-)
> >
> > --
> > 2.7.5

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

* [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
                       ` (2 preceding siblings ...)
  2018-01-10  9:58     ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Ananyev, Konstantin
@ 2018-01-11  4:58     ` Wei Dai
  2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
                         ` (3 more replies)
  3 siblings, 4 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-11  4:58 UTC (permalink / raw)
  To: konstantin.ananyev, qiming.yang, yuan.peng, wenzhuo.lu, jingjing.wu
  Cc: dev, stable, Wei Dai

If an invlaid number of RX or TX queues is configured from testpmd
command like "port config all rxq number" or "port config all txq number".
or from --rxq and --txq in the command to start testpmd. The global variable
nb_rxq or nb_txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of
RX or TX queues is 4, testpmd will crash after running commands
"port config all rxq 5", "port config all txq 5" and "start" in sequence.

With these 2 patches, if an invalid input is detected, it is refused and
testpmd keeps last correct values of  nb_rxq and nb_txq.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

---
v4: update git log message and rename 2 new added functions

v3: follow the guide from Konstantin to use functions to check
    input rxq and txq instead of usage of new added global variables.

v2: fix a bug in v1



Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c    |  4 +++
 app/test-pmd/parameters.c | 13 +++----
 app/test-pmd/testpmd.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  5 +++
 4 files changed, 108 insertions(+), 6 deletions(-)

-- 
2.7.5

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

* [dpdk-dev] [PATCH v4 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
@ 2018-01-11  4:58       ` Wei Dai
  2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix invalid txq " Wei Dai
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-11  4:58 UTC (permalink / raw)
  To: konstantin.ananyev, qiming.yang, yuan.peng, wenzhuo.lu, jingjing.wu
  Cc: dev, stable, Wei Dai

If an invalid number of RX queues is configured from testpmd run-time
command like "port config all rxq number" or from --rxq in the command
to start testpmd, the global variable nb_rxq is updated by this invalid
value without this patch. It may cause testpmd crash. This patch refuses
invalid rxq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  7 ++++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  3 +++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5b2e2ef..f0623b1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1518,6 +1518,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_rxq(res->value) != 0)
+			return;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 304b98d..c46e734 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -536,6 +536,7 @@ launch_args_parse(int argc, char** argv)
 	int n, opt;
 	char **argvopt;
 	int opt_idx;
+	portid_t pid;
 	enum { TX, RX };
 
 	static struct option lgopts[] = {
@@ -922,12 +923,12 @@ launch_args_parse(int argc, char** argv)
 				rss_hf = ETH_RSS_UDP;
 			if (!strcmp(lgopts[opt_idx].name, "rxq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_rxq((queueid_t)n) == 0)
 					nb_rxq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "rxq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_rxq(&pid));
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9414d0e..979c55c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -540,6 +540,52 @@ check_socket_id(const unsigned int socket_id)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of RX queues.
+ * *pid return the port id which has mimumal value of
+ * max_rx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_rxq(portid_t *pid)
+{
+	queueid_t allowed_max_rxq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_rx_queues < allowed_max_rxq) {
+			allowed_max_rxq = dev_info.max_rx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_rxq;
+}
+
+/*
+ * Check input rxq is valid or not.
+ * If input rxq is not greater than any of maximum number
+ * of RX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_rxq(queueid_t rxq)
+{
+	queueid_t allowed_max_rxq;
+	portid_t pid;
+
+	allowed_max_rxq = get_allowed_max_nb_rxq(&pid);
+	if (rxq > allowed_max_rxq) {
+		printf("Fail: input rxq (%u) can't be greater "
+		       "than max_rx_queues (%u) of port %u\n",
+		       rxq,
+		       allowed_max_rxq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2a266fd..e815509 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -700,6 +700,9 @@ enum print_warning {
 int port_id_is_invalid(portid_t port_id, enum print_warning warning);
 int new_socket_id(unsigned int socket_id);
 
+queueid_t get_allowed_max_nb_rxq(portid_t *pid);
+int check_nb_rxq(queueid_t rxq);
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
-- 
2.7.5

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

* [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix invalid txq number setting
  2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
  2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-11  4:58       ` Wei Dai
  2018-01-12  5:39       ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
  2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
  3 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-11  4:58 UTC (permalink / raw)
  To: konstantin.ananyev, qiming.yang, yuan.peng, wenzhuo.lu, jingjing.wu
  Cc: dev, stable, Wei Dai

If an invalid number of TX queues is configured from testpmd run-time
command like "port config all txq number" or from --txq in the command
to start testpmd, the global variable nb_txq is updated by this invalid
value without this patch. It may cause testpmd crash. This patch refuses
invalid txq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  6 +++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  2 ++
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f0623b1..6619cb8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_txq(res->value) != 0)
+			return;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index c46e734..ca2af65 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -932,12 +932,12 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_txq((queueid_t)n) == 0)
 					nb_txq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "txq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_txq(&pid));
 			}
 			if (!nb_rxq && !nb_txq) {
 				rte_exit(EXIT_FAILURE, "Either rx or tx queues should "
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 979c55c..bf7fd3b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -586,6 +586,52 @@ check_nb_rxq(queueid_t rxq)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of TX queues.
+ * *pid return the port id which has mimumal value of
+ * max_tx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_txq(portid_t *pid)
+{
+	queueid_t allowed_max_txq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_tx_queues < allowed_max_txq) {
+			allowed_max_txq = dev_info.max_tx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_txq;
+}
+
+/*
+ * Check input txq is valid or not.
+ * If input txq is not greater than any of maximum number
+ * of TX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_txq(queueid_t txq)
+{
+	queueid_t allowed_max_txq;
+	portid_t pid;
+
+	allowed_max_txq = get_allowed_max_nb_txq(&pid);
+	if (txq > allowed_max_txq) {
+		printf("Fail: input txq (%u) can't be greater "
+		       "than max_tx_queues (%u) of port %u\n",
+		       txq,
+		       allowed_max_txq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e815509..fe40892 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -702,6 +702,8 @@ int new_socket_id(unsigned int socket_id);
 
 queueid_t get_allowed_max_nb_rxq(portid_t *pid);
 int check_nb_rxq(queueid_t rxq);
+queueid_t get_allowed_max_nb_txq(portid_t *pid);
+int check_nb_txq(queueid_t txq);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
-- 
2.7.5

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

* Re: [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
  2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
  2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-12  5:39       ` Peng, Yuan
  2018-01-12  6:05         ` Dai, Wei
  2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
  3 siblings, 1 reply; 32+ messages in thread
From: Peng, Yuan @ 2018-01-12  5:39 UTC (permalink / raw)
  To: Dai, Wei, Ananyev, Konstantin, Yang, Qiming, Lu, Wenzhuo, Wu, Jingjing
  Cc: dev, stable

Hi Wei,

There is a build error applied your patches to the latest DPDK version.
/root/dpdk/app/test-pmd/testpmd.c: In function 'check_nb_rxq':
/root/dpdk/app/test-pmd/testpmd.c:579:3: error: 'pid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   printf("Fail: input rxq (%u) can't be greater "
   ^
/root/dpdk/app/test-pmd/testpmd.c: In function 'check_nb_txq':
/root/dpdk/app/test-pmd/testpmd.c:625:3: error: 'pid' may be used uninitialized in this function [-Werror=maybe-uninitialized]
   printf("Fail: input txq (%u) can't be greater "
   ^
My gcc verison is gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)

Could you help to check it?

Thanks.
Yuan.

-----Original Message-----
From: Dai, Wei 
Sent: Thursday, January 11, 2018 12:58 PM
To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
Subject: [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings

If an invlaid number of RX or TX queues is configured from testpmd command like "port config all rxq number" or "port config all txq number".
or from --rxq and --txq in the command to start testpmd. The global variable nb_rxq or nb_txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of RX or TX queues is 4, testpmd will crash after running commands "port config all rxq 5", "port config all txq 5" and "start" in sequence.

With these 2 patches, if an invalid input is detected, it is refused and testpmd keeps last correct values of  nb_rxq and nb_txq.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

---
v4: update git log message and rename 2 new added functions

v3: follow the guide from Konstantin to use functions to check
    input rxq and txq instead of usage of new added global variables.

v2: fix a bug in v1



Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c    |  4 +++
 app/test-pmd/parameters.c | 13 +++----
 app/test-pmd/testpmd.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  5 +++
 4 files changed, 108 insertions(+), 6 deletions(-)

--
2.7.5

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

* Re: [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-12  5:39       ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
@ 2018-01-12  6:05         ` Dai, Wei
  0 siblings, 0 replies; 32+ messages in thread
From: Dai, Wei @ 2018-01-12  6:05 UTC (permalink / raw)
  To: Peng, Yuan, Ananyev, Konstantin, Yang, Qiming, Lu, Wenzhuo, Wu, Jingjing
  Cc: dev, stable

Hi, Yuan
I can build dpdk with my patches successfully in my Fedora 24 with gcc 6.3.1 20161221 (Red Hat 6.3.1-1)
Anyway, I will make v5 patch to address the problem you report and correct typos reported by Konstantin.

Thanks

> -----Original Message-----
> From: Peng, Yuan
> Sent: Friday, January 12, 2018 1:40 PM
> To: Dai, Wei <wei.dai@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Yang, Qiming <qiming.yang@intel.com>;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer
> settings
> 
> Hi Wei,
> 
> There is a build error applied your patches to the latest DPDK version.
> /root/dpdk/app/test-pmd/testpmd.c: In function 'check_nb_rxq':
> /root/dpdk/app/test-pmd/testpmd.c:579:3: error: 'pid' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>    printf("Fail: input rxq (%u) can't be greater "
>    ^
> /root/dpdk/app/test-pmd/testpmd.c: In function 'check_nb_txq':
> /root/dpdk/app/test-pmd/testpmd.c:625:3: error: 'pid' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>    printf("Fail: input txq (%u) can't be greater "
>    ^
> My gcc verison is gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)
> 
> Could you help to check it?
> 
> Thanks.
> Yuan.
> 
> -----Original Message-----
> From: Dai, Wei
> Sent: Thursday, January 11, 2018 12:58 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Peng, Yuan <yuan.peng@intel.com>; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Dai, Wei <wei.dai@intel.com>
> Subject: [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
> 
> If an invlaid number of RX or TX queues is configured from testpmd
> command like "port config all rxq number" or "port config all txq number".
> or from --rxq and --txq in the command to start testpmd. The global variable
> nb_rxq or nb_txq is updated by the invalid input.
> This can cause testpmd crash. For example, if the maximum number of RX or
> TX queues is 4, testpmd will crash after running commands "port config all
> rxq 5", "port config all txq 5" and "start" in sequence.
> 
> With these 2 patches, if an invalid input is detected, it is refused and testpmd
> keeps last correct values of  nb_rxq and nb_txq.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> 
> ---
> v4: update git log message and rename 2 new added functions
> 
> v3: follow the guide from Konstantin to use functions to check
>     input rxq and txq instead of usage of new added global variables.
> 
> v2: fix a bug in v1
> 
> 
> 
> Wei Dai (2):
>   app/testpmd: fix invalid rxq number setting
>   app/testpmd: fix invalid txq number setting
> 
>  app/test-pmd/cmdline.c    |  4 +++
>  app/test-pmd/parameters.c | 13 +++----
>  app/test-pmd/testpmd.c    | 92
> +++++++++++++++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h    |  5 +++
>  4 files changed, 108 insertions(+), 6 deletions(-)
> 
> --
> 2.7.5

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

* [dpdk-dev] [PATCH v5 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
                         ` (2 preceding siblings ...)
  2018-01-12  5:39       ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
@ 2018-01-12  8:10       ` Wei Dai
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
                           ` (3 more replies)
  3 siblings, 4 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-12  8:10 UTC (permalink / raw)
  To: yuan.peng, konstantin.ananyev, wenzhuo.lu, beilei.xing
  Cc: dev, Wei Dai, stable

If an invlaid number of RX or TX queues is configured from testpmd
command like "port config all rxq number" or "port config all txq number".
or from --rxq and --txq in the command to start testpmd. The global variable
nb_rxq or nb_txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of
RX or TX queues is 4, testpmd will crash after running commands
"port config all rxq 5", "port config all txq 5" and "start" in sequence.

With these 2 patches, if an invalid input is detected, it is refused and
testpmd keeps last correct values of  nb_rxq and nb_txq.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

---
v5: fix building failure with -Werror=maybe-uninitialized by gcc 5.3.1
    fix typo error

v4: update git log message and rename 2 new added functions

v3: follow the guide from Konstantin to use functions to check
    input rxq and txq instead of usage of new added global variables.

v2: fix a bug in v1


Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c    |  4 +++
 app/test-pmd/parameters.c | 13 +++----
 app/test-pmd/testpmd.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  5 +++
 4 files changed, 108 insertions(+), 6 deletions(-)

-- 
2.7.5

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

* [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
@ 2018-01-12  8:10         ` Wei Dai
  2018-01-12  9:09           ` Peng, Yuan
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix invalid txq " Wei Dai
                           ` (2 subsequent siblings)
  3 siblings, 1 reply; 32+ messages in thread
From: Wei Dai @ 2018-01-12  8:10 UTC (permalink / raw)
  To: yuan.peng, konstantin.ananyev, wenzhuo.lu, beilei.xing
  Cc: dev, Wei Dai, stable

If an invalid number of RX queues is configured from testpmd run-time
command like "port config all rxq number" or from --rxq in the command
to start testpmd, the global variable nb_rxq is updated by this invalid
value without this patch. It may cause testpmd crash. This patch refuses
invalid rxq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  7 ++++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  3 +++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5b2e2ef..f0623b1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1518,6 +1518,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_rxq(res->value) != 0)
+			return;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 304b98d..c46e734 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -536,6 +536,7 @@ launch_args_parse(int argc, char** argv)
 	int n, opt;
 	char **argvopt;
 	int opt_idx;
+	portid_t pid;
 	enum { TX, RX };
 
 	static struct option lgopts[] = {
@@ -922,12 +923,12 @@ launch_args_parse(int argc, char** argv)
 				rss_hf = ETH_RSS_UDP;
 			if (!strcmp(lgopts[opt_idx].name, "rxq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_rxq((queueid_t)n) == 0)
 					nb_rxq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "rxq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_rxq(&pid));
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9414d0e..4e8667d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -540,6 +540,52 @@ check_socket_id(const unsigned int socket_id)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of RX queues.
+ * *pid return the port id which has minimal value of
+ * max_rx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_rxq(portid_t *pid)
+{
+	queueid_t allowed_max_rxq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_rx_queues < allowed_max_rxq) {
+			allowed_max_rxq = dev_info.max_rx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_rxq;
+}
+
+/*
+ * Check input rxq is valid or not.
+ * If input rxq is not greater than any of maximum number
+ * of RX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_rxq(queueid_t rxq)
+{
+	queueid_t allowed_max_rxq;
+	portid_t pid = 0;
+
+	allowed_max_rxq = get_allowed_max_nb_rxq(&pid);
+	if (rxq > allowed_max_rxq) {
+		printf("Fail: input rxq (%u) can't be greater "
+		       "than max_rx_queues (%u) of port %u\n",
+		       rxq,
+		       allowed_max_rxq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2a266fd..e815509 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -700,6 +700,9 @@ enum print_warning {
 int port_id_is_invalid(portid_t port_id, enum print_warning warning);
 int new_socket_id(unsigned int socket_id);
 
+queueid_t get_allowed_max_nb_rxq(portid_t *pid);
+int check_nb_rxq(queueid_t rxq);
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
-- 
2.7.5

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

* [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix invalid txq number setting
  2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-12  8:10         ` Wei Dai
  2018-01-12  9:12           ` Peng, Yuan
  2018-01-12  9:09         ` [dpdk-dev] [PATCH v5 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
  2018-01-12 11:31         ` [dpdk-dev] [PATCH v6 " Wei Dai
  3 siblings, 1 reply; 32+ messages in thread
From: Wei Dai @ 2018-01-12  8:10 UTC (permalink / raw)
  To: yuan.peng, konstantin.ananyev, wenzhuo.lu, beilei.xing
  Cc: dev, Wei Dai, stable

If an invalid number of TX queues is configured from testpmd run-time
command like "port config all txq number" or from --txq in the command
to start testpmd, the global variable nb_txq is updated by this invalid
value without this patch. It may cause testpmd crash. This patch refuses
invalid txq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  6 +++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  2 ++
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f0623b1..6619cb8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_txq(res->value) != 0)
+			return;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index c46e734..ca2af65 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -932,12 +932,12 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_txq((queueid_t)n) == 0)
 					nb_txq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "txq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_txq(&pid));
 			}
 			if (!nb_rxq && !nb_txq) {
 				rte_exit(EXIT_FAILURE, "Either rx or tx queues should "
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4e8667d..493e028 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -586,6 +586,52 @@ check_nb_rxq(queueid_t rxq)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of TX queues.
+ * *pid return the port id which has minimal value of
+ * max_tx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_txq(portid_t *pid)
+{
+	queueid_t allowed_max_txq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_tx_queues < allowed_max_txq) {
+			allowed_max_txq = dev_info.max_tx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_txq;
+}
+
+/*
+ * Check input txq is valid or not.
+ * If input txq is not greater than any of maximum number
+ * of TX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_txq(queueid_t txq)
+{
+	queueid_t allowed_max_txq;
+	portid_t pid = 0;
+
+	allowed_max_txq = get_allowed_max_nb_txq(&pid);
+	if (txq > allowed_max_txq) {
+		printf("Fail: input txq (%u) can't be greater "
+		       "than max_tx_queues (%u) of port %u\n",
+		       txq,
+		       allowed_max_txq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e815509..fe40892 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -702,6 +702,8 @@ int new_socket_id(unsigned int socket_id);
 
 queueid_t get_allowed_max_nb_rxq(portid_t *pid);
 int check_nb_rxq(queueid_t rxq);
+queueid_t get_allowed_max_nb_txq(portid_t *pid);
+int check_nb_txq(queueid_t txq);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
-- 
2.7.5

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

* Re: [dpdk-dev] [PATCH v5 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-12  9:09         ` Peng, Yuan
  2018-01-12 11:31         ` [dpdk-dev] [PATCH v6 " Wei Dai
  3 siblings, 0 replies; 32+ messages in thread
From: Peng, Yuan @ 2018-01-12  9:09 UTC (permalink / raw)
  To: Dai, Wei, Ananyev, Konstantin, Lu, Wenzhuo, Xing, Beilei
  Cc: dev, stable, Peng, Yuan

Tested-by: Peng Yuan<yuan.peng@intel.com>

- Tested Branch: master
- Tested commit 6c7001480ac6356ff0a4995f3ed495ed9c866061
- OS: 4.5.5-300.fc24.x86_64
- GCC: gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)
- CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
- NIC: Intel Corporation Device Fortville [8086:1572]
- Default x86_64-native-linuxapp-gcc configuration
- Prerequisites:
- Total 1 cases, 1 passed, 0 failed

- Case: 
./usertools/dpdk-devbind.py -b igb_uio 05:00:0
 echo 1 >/sys/bus/pci/devices/0000:05:00.0/max_vfs
 ./usertools/dpdk-devbind.py -b vfio-pci 05:02.0
 pf:
 ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 4 -w 05:00.0,queue-num-per-vf=4 --file-prefix=test1 --socket-mem 1024,1024 - -I
 vf:
 ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 4 -w 05:02.0 --file-prefix=test2 --socket-mem 1024,1024 - -i --rxq=4 --txq=4
 EAL: Detected 88 lcore(s)
 EAL: Probing VFIO support...
 EAL: VFIO support initialized
 EAL: PCI device 0000:05:02.0 on NUMA socket 0
 EAL: probe driver: 8086:154c net_i40e_vf
 EAL: using IOMMU type 1 (Type 1)
 Interactive-mode selected
 USER1: create a new mbuf pool <mbuf_pool_socket_0>: n=171456, size=2176, socket=0
 USER1: create a new mbuf pool <mbuf_pool_socket_1>: n=171456, size=2176, socket=1
 Configuring Port 0 (socket 0)
 Port 0: 7E:AC:58:44:3C:94
 Checking link statuses...
 Done
 testpmd> port stop all
 Stopping ports...
 Checking link statuses...
 Done
 testpmd> port config all txq 5
 Fail: input txq (5) can't be greater than max_tx_queues (4) of port 0
 testpmd> port config all rxq 5
 Fail: input rxq (5) can't be greater than max_rx_queues (4) of port 0
 testpmd> port start all
 Port 0: 5A:19:E4:5C:A3:C7
 Checking link statuses...
 Done
 testpmd> show port info all
 Current number of RX queues: 4
 Max possible RX queues: 4
 Current number of TX queues: 4
 Max possible TX queues: 4

there is no core dump, and the actual queue number doesn't change.
The case passed.

-----Original Message-----
From: Dai, Wei 
Sent: Friday, January 12, 2018 4:10 PM
To: Peng, Yuan <yuan.peng@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
Subject: [PATCH v5 0/2] app/testpmd: fix invalid rxq and txq nubmer settings

If an invlaid number of RX or TX queues is configured from testpmd command like "port config all rxq number" or "port config all txq number".
or from --rxq and --txq in the command to start testpmd. The global variable nb_rxq or nb_txq is updated by the invalid input.
This can cause testpmd crash. For example, if the maximum number of RX or TX queues is 4, testpmd will crash after running commands "port config all rxq 5", "port config all txq 5" and "start" in sequence.

With these 2 patches, if an invalid input is detected, it is refused and testpmd keeps last correct values of  nb_rxq and nb_txq.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

---
v5: fix building failure with -Werror=maybe-uninitialized by gcc 5.3.1
    fix typo error

v4: update git log message and rename 2 new added functions

v3: follow the guide from Konstantin to use functions to check
    input rxq and txq instead of usage of new added global variables.

v2: fix a bug in v1


Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c    |  4 +++
 app/test-pmd/parameters.c | 13 +++----
 app/test-pmd/testpmd.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  5 +++
 4 files changed, 108 insertions(+), 6 deletions(-)

--
2.7.5

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

* Re: [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-12  9:09           ` Peng, Yuan
  0 siblings, 0 replies; 32+ messages in thread
From: Peng, Yuan @ 2018-01-12  9:09 UTC (permalink / raw)
  To: Dai, Wei, Ananyev, Konstantin, Lu, Wenzhuo, Xing, Beilei; +Cc: dev, stable

Tested-by: Peng Yuan<yuan.peng@intel.com>

- Tested Branch: master
- Tested commit 6c7001480ac6356ff0a4995f3ed495ed9c866061
- OS: 4.5.5-300.fc24.x86_64
- GCC: gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)
- CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
- NIC: Intel Corporation Device Fortville [8086:1572]
- Default x86_64-native-linuxapp-gcc configuration
- Prerequisites:
- Total 1 cases, 1 passed, 0 failed

- Case: 
./usertools/dpdk-devbind.py -b igb_uio 05:00:0  echo 1 >/sys/bus/pci/devices/0000:05:00.0/max_vfs
 ./usertools/dpdk-devbind.py -b vfio-pci 05:02.0
 pf:
 ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 4 -w 05:00.0,queue-num-per-vf=4 --file-prefix=test1 --socket-mem 1024,1024 - -I
 vf:
 ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 4 -w 05:02.0 --file-prefix=test2 --socket-mem 1024,1024 - -i --rxq=4 --txq=4
 EAL: Detected 88 lcore(s)
 EAL: Probing VFIO support...
 EAL: VFIO support initialized
 EAL: PCI device 0000:05:02.0 on NUMA socket 0
 EAL: probe driver: 8086:154c net_i40e_vf
 EAL: using IOMMU type 1 (Type 1)
 Interactive-mode selected
 USER1: create a new mbuf pool <mbuf_pool_socket_0>: n=171456, size=2176, socket=0
 USER1: create a new mbuf pool <mbuf_pool_socket_1>: n=171456, size=2176, socket=1  Configuring Port 0 (socket 0)  Port 0: 7E:AC:58:44:3C:94  Checking link statuses...
 Done
 testpmd> port stop all
 Stopping ports...
 Checking link statuses...
 Done
 testpmd> port config all txq 5
 Fail: input txq (5) can't be greater than max_tx_queues (4) of port 0  testpmd> port config all rxq 5
 Fail: input rxq (5) can't be greater than max_rx_queues (4) of port 0  testpmd> port start all  Port 0: 5A:19:E4:5C:A3:C7  Checking link statuses...
 Done
 testpmd> show port info all
 Current number of RX queues: 4
 Max possible RX queues: 4
 Current number of TX queues: 4
 Max possible TX queues: 4

there is no core dump, and the actual queue number doesn't change.
The case passed.


-----Original Message-----
From: Dai, Wei 
Sent: Friday, January 12, 2018 4:10 PM
To: Peng, Yuan <yuan.peng@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
Subject: [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting

If an invalid number of RX queues is configured from testpmd run-time command like "port config all rxq number" or from --rxq in the command to start testpmd, the global variable nb_rxq is updated by this invalid value without this patch. It may cause testpmd crash. This patch refuses invalid rxq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  7 ++++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  3 +++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index 5b2e2ef..f0623b1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1518,6 +1518,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_rxq(res->value) != 0)
+			return;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index 304b98d..c46e734 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -536,6 +536,7 @@ launch_args_parse(int argc, char** argv)
 	int n, opt;
 	char **argvopt;
 	int opt_idx;
+	portid_t pid;
 	enum { TX, RX };
 
 	static struct option lgopts[] = {
@@ -922,12 +923,12 @@ launch_args_parse(int argc, char** argv)
 				rss_hf = ETH_RSS_UDP;
 			if (!strcmp(lgopts[opt_idx].name, "rxq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_rxq((queueid_t)n) == 0)
 					nb_rxq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "rxq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_rxq(&pid));
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 9414d0e..4e8667d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -540,6 +540,52 @@ check_socket_id(const unsigned int socket_id)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of RX queues.
+ * *pid return the port id which has minimal value of
+ * max_rx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_rxq(portid_t *pid)
+{
+	queueid_t allowed_max_rxq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_rx_queues < allowed_max_rxq) {
+			allowed_max_rxq = dev_info.max_rx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_rxq;
+}
+
+/*
+ * Check input rxq is valid or not.
+ * If input rxq is not greater than any of maximum number
+ * of RX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_rxq(queueid_t rxq)
+{
+	queueid_t allowed_max_rxq;
+	portid_t pid = 0;
+
+	allowed_max_rxq = get_allowed_max_nb_rxq(&pid);
+	if (rxq > allowed_max_rxq) {
+		printf("Fail: input rxq (%u) can't be greater "
+		       "than max_rx_queues (%u) of port %u\n",
+		       rxq,
+		       allowed_max_rxq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index 2a266fd..e815509 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -700,6 +700,9 @@ enum print_warning {  int port_id_is_invalid(portid_t port_id, enum print_warning warning);  int new_socket_id(unsigned int socket_id);
 
+queueid_t get_allowed_max_nb_rxq(portid_t *pid); int 
+check_nb_rxq(queueid_t rxq);
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
--
2.7.5

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

* Re: [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix invalid txq number setting
  2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-12  9:12           ` Peng, Yuan
  0 siblings, 0 replies; 32+ messages in thread
From: Peng, Yuan @ 2018-01-12  9:12 UTC (permalink / raw)
  To: Dai, Wei, Ananyev, Konstantin, Lu, Wenzhuo, Xing, Beilei; +Cc: dev, stable

Tested-by: Peng Yuan<yuan.peng@intel.com>

- Tested Branch: master
- Tested commit 6c7001480ac6356ff0a4995f3ed495ed9c866061
- OS: 4.5.5-300.fc24.x86_64
- GCC: gcc version 5.3.1 20151207 (Red Hat 5.3.1-2) (GCC)
- CPU: Intel(R) Xeon(R) CPU E5-2699 v4 @ 2.20GHz
- NIC: Intel Corporation Device Fortville [8086:1572]
- Default x86_64-native-linuxapp-gcc configuration
- Prerequisites:
- Total 1 cases, 1 passed, 0 failed

- Case: 
./usertools/dpdk-devbind.py -b igb_uio 05:00:0  echo 1 >/sys/bus/pci/devices/0000:05:00.0/max_vfs
 ./usertools/dpdk-devbind.py -b vfio-pci 05:02.0
 pf:
 ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 4 -w 05:00.0,queue-num-per-vf=4 --file-prefix=test1 --socket-mem 1024,1024 - -I
 vf:
 ./x86_64-native-linuxapp-gcc/app/testpmd -c f -n 4 -w 05:02.0 --file-prefix=test2 --socket-mem 1024,1024 - -i --rxq=4 --txq=4
 EAL: Detected 88 lcore(s)
 EAL: Probing VFIO support...
 EAL: VFIO support initialized
 EAL: PCI device 0000:05:02.0 on NUMA socket 0
 EAL: probe driver: 8086:154c net_i40e_vf
 EAL: using IOMMU type 1 (Type 1)
 Interactive-mode selected
 USER1: create a new mbuf pool <mbuf_pool_socket_0>: n=171456, size=2176, socket=0
 USER1: create a new mbuf pool <mbuf_pool_socket_1>: n=171456, size=2176, socket=1  Configuring Port 0 (socket 0)  Port 0: 7E:AC:58:44:3C:94  Checking link statuses...
 Done
 testpmd> port stop all
 Stopping ports...
 Checking link statuses...
 Done
 testpmd> port config all txq 5
 Fail: input txq (5) can't be greater than max_tx_queues (4) of port 0  testpmd> port config all rxq 5
 Fail: input rxq (5) can't be greater than max_rx_queues (4) of port 0  testpmd> port start all  Port 0: 5A:19:E4:5C:A3:C7  Checking link statuses...
 Done
 testpmd> show port info all
 Current number of RX queues: 4
 Max possible RX queues: 4
 Current number of TX queues: 4
 Max possible TX queues: 4

there is no core dump, and the actual queue number doesn't change.
The case passed.


-----Original Message-----
From: Dai, Wei 
Sent: Friday, January 12, 2018 4:10 PM
To: Peng, Yuan <yuan.peng@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Xing, Beilei <beilei.xing@intel.com>
Cc: dev@dpdk.org; Dai, Wei <wei.dai@intel.com>; stable@dpdk.org
Subject: [PATCH v5 2/2] app/testpmd: fix invalid txq number setting

If an invalid number of TX queues is configured from testpmd run-time command like "port config all txq number" or from --txq in the command to start testpmd, the global variable nb_txq is updated by this invalid value without this patch. It may cause testpmd crash. This patch refuses invalid txq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  6 +++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  2 ++
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index f0623b1..6619cb8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_txq(res->value) != 0)
+			return;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index c46e734..ca2af65 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -932,12 +932,12 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_txq((queueid_t)n) == 0)
 					nb_txq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "txq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_txq(&pid));
 			}
 			if (!nb_rxq && !nb_txq) {
 				rte_exit(EXIT_FAILURE, "Either rx or tx queues should "
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index 4e8667d..493e028 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -586,6 +586,52 @@ check_nb_rxq(queueid_t rxq)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of TX queues.
+ * *pid return the port id which has minimal value of
+ * max_tx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_txq(portid_t *pid)
+{
+	queueid_t allowed_max_txq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_tx_queues < allowed_max_txq) {
+			allowed_max_txq = dev_info.max_tx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_txq;
+}
+
+/*
+ * Check input txq is valid or not.
+ * If input txq is not greater than any of maximum number
+ * of TX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_txq(queueid_t txq)
+{
+	queueid_t allowed_max_txq;
+	portid_t pid = 0;
+
+	allowed_max_txq = get_allowed_max_nb_txq(&pid);
+	if (txq > allowed_max_txq) {
+		printf("Fail: input txq (%u) can't be greater "
+		       "than max_tx_queues (%u) of port %u\n",
+		       txq,
+		       allowed_max_txq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index e815509..fe40892 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -702,6 +702,8 @@ int new_socket_id(unsigned int socket_id);
 
 queueid_t get_allowed_max_nb_rxq(portid_t *pid);  int check_nb_rxq(queueid_t rxq);
+queueid_t get_allowed_max_nb_txq(portid_t *pid); int 
+check_nb_txq(queueid_t txq);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
--
2.7.5

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

* [dpdk-dev] [PATCH v6 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
                           ` (2 preceding siblings ...)
  2018-01-12  9:09         ` [dpdk-dev] [PATCH v5 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
@ 2018-01-12 11:31         ` Wei Dai
  2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
                             ` (2 more replies)
  3 siblings, 3 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-12 11:31 UTC (permalink / raw)
  To: ferruh.yigit, yuan.peng, konstantin.ananyev, wenzhuo.lu, jingjing.wu
  Cc: dev, Wei Dai, stable

If an invlaid number of RX or TX queues is configured from testpmd
command like "port config all rxq number" or "port config all txq
number" or from --rxq and --txq in the command to start testpmd.
The global variable nb_rxq or nb_txq is updated by the invalid
input. This can cause testpmd crash. For example, if the maximum
number of RX or TX queues is 4, testpmd will crash after running
commands "port config all rxq 5", "port config all txq 5" and
"start" in sequence.
With these 2 patches, if an invalid input is detected, it is refused
and testpmd keeps last correct values of  nb_rxq and nb_txq.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Yuan Peng <yuan.peng@intel.com>

---
v6: same as v5. only correct patchwork mail format error
    for maintainer's convenience.

v5: fix building failure with -Werror=maybe-uninitialized by gcc 5.3.1
    fix typo error

v4: update git log message and rename 2 new added functions

v3: follow the guide from Konstantin to use functions to check
    input rxq and txq instead of usage of new added global variables.

v2: fix a bug in v1

Wei Dai (2):
  app/testpmd: fix invalid rxq number setting
  app/testpmd: fix invalid txq number setting

 app/test-pmd/cmdline.c    |  4 +++
 app/test-pmd/parameters.c | 13 +++----
 app/test-pmd/testpmd.c    | 92 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  5 +++
 4 files changed, 108 insertions(+), 6 deletions(-)

-- 
2.7.5

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

* [dpdk-dev] [PATCH v6 1/2] app/testpmd: fix invalid rxq number setting
  2018-01-12 11:31         ` [dpdk-dev] [PATCH v6 " Wei Dai
@ 2018-01-12 11:31           ` Wei Dai
  2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: fix invalid txq " Wei Dai
  2018-01-17 14:06           ` [dpdk-dev] [PATCH v6 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Thomas Monjalon
  2 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-12 11:31 UTC (permalink / raw)
  To: ferruh.yigit, yuan.peng, konstantin.ananyev, wenzhuo.lu, jingjing.wu
  Cc: dev, Wei Dai, stable

If an invalid number of RX queues is configured from testpmd run-time
command like "port config all rxq number" or from --rxq in the command
to start testpmd, the global variable nb_rxq is updated by this invalid
value without this patch. It may cause testpmd crash. This patch refuses
invalid rxq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Yuan Peng <yuan.peng@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  7 ++++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  3 +++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 5b2e2ef..f0623b1 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1518,6 +1518,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_rxq(res->value) != 0)
+			return;
 		nb_rxq = res->value;
 	}
 	else if (!strcmp(res->name, "txq")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 304b98d..c46e734 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -536,6 +536,7 @@ launch_args_parse(int argc, char** argv)
 	int n, opt;
 	char **argvopt;
 	int opt_idx;
+	portid_t pid;
 	enum { TX, RX };
 
 	static struct option lgopts[] = {
@@ -922,12 +923,12 @@ launch_args_parse(int argc, char** argv)
 				rss_hf = ETH_RSS_UDP;
 			if (!strcmp(lgopts[opt_idx].name, "rxq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_rxq((queueid_t)n) == 0)
 					nb_rxq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "rxq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_rxq(&pid));
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9414d0e..4e8667d 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -540,6 +540,52 @@ check_socket_id(const unsigned int socket_id)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of RX queues.
+ * *pid return the port id which has minimal value of
+ * max_rx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_rxq(portid_t *pid)
+{
+	queueid_t allowed_max_rxq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_rx_queues < allowed_max_rxq) {
+			allowed_max_rxq = dev_info.max_rx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_rxq;
+}
+
+/*
+ * Check input rxq is valid or not.
+ * If input rxq is not greater than any of maximum number
+ * of RX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_rxq(queueid_t rxq)
+{
+	queueid_t allowed_max_rxq;
+	portid_t pid = 0;
+
+	allowed_max_rxq = get_allowed_max_nb_rxq(&pid);
+	if (rxq > allowed_max_rxq) {
+		printf("Fail: input rxq (%u) can't be greater "
+		       "than max_rx_queues (%u) of port %u\n",
+		       rxq,
+		       allowed_max_rxq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2a266fd..e815509 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -700,6 +700,9 @@ enum print_warning {
 int port_id_is_invalid(portid_t port_id, enum print_warning warning);
 int new_socket_id(unsigned int socket_id);
 
+queueid_t get_allowed_max_nb_rxq(portid_t *pid);
+int check_nb_rxq(queueid_t rxq);
+
 /*
  * Work-around of a compilation error with ICC on invocations of the
  * rte_be_to_cpu_16() function.
-- 
2.7.5

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

* [dpdk-dev] [PATCH v6 2/2] app/testpmd: fix invalid txq number setting
  2018-01-12 11:31         ` [dpdk-dev] [PATCH v6 " Wei Dai
  2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
@ 2018-01-12 11:31           ` Wei Dai
  2018-01-17 14:06           ` [dpdk-dev] [PATCH v6 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Thomas Monjalon
  2 siblings, 0 replies; 32+ messages in thread
From: Wei Dai @ 2018-01-12 11:31 UTC (permalink / raw)
  To: ferruh.yigit, yuan.peng, konstantin.ananyev, wenzhuo.lu, jingjing.wu
  Cc: dev, Wei Dai, stable

If an invalid number of TX queues is configured from testpmd run-time
command like "port config all txq number" or from --txq in the command
to start testpmd, the global variable nb_txq is updated by this invalid
value without this patch. It may cause testpmd crash. This patch refuses
invalid txq setting and keeps its last correct value.

Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
Cc: stable@dpdk.org

Signed-off-by: Wei Dai <wei.dai@intel.com>
Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Tested-by: Yuan Peng <yuan.peng@intel.com>
---
 app/test-pmd/cmdline.c    |  2 ++
 app/test-pmd/parameters.c |  6 +++---
 app/test-pmd/testpmd.c    | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  2 ++
 4 files changed, 53 insertions(+), 3 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f0623b1..6619cb8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -1527,6 +1527,8 @@ cmd_config_rx_tx_parsed(void *parsed_result,
 			printf("Warning: Either rx or tx queues should be non zero\n");
 			return;
 		}
+		if (check_nb_txq(res->value) != 0)
+			return;
 		nb_txq = res->value;
 	}
 	else if (!strcmp(res->name, "rxd")) {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index c46e734..ca2af65 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -932,12 +932,12 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "txq")) {
 				n = atoi(optarg);
-				if (n >= 0 && n <= (int) MAX_QUEUE_ID)
+				if (n >= 0 && check_nb_txq((queueid_t)n) == 0)
 					nb_txq = (queueid_t) n;
 				else
 					rte_exit(EXIT_FAILURE, "txq %d invalid - must be"
-						  " >= 0 && <= %d\n", n,
-						  (int) MAX_QUEUE_ID);
+						  " >= 0 && <= %u\n", n,
+						  get_allowed_max_nb_txq(&pid));
 			}
 			if (!nb_rxq && !nb_txq) {
 				rte_exit(EXIT_FAILURE, "Either rx or tx queues should "
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4e8667d..493e028 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -586,6 +586,52 @@ check_nb_rxq(queueid_t rxq)
 	return 0;
 }
 
+/*
+ * Get the allowed maximum number of TX queues.
+ * *pid return the port id which has minimal value of
+ * max_tx_queues in all ports.
+ */
+queueid_t
+get_allowed_max_nb_txq(portid_t *pid)
+{
+	queueid_t allowed_max_txq = MAX_QUEUE_ID;
+	portid_t pi;
+	struct rte_eth_dev_info dev_info;
+
+	RTE_ETH_FOREACH_DEV(pi) {
+		rte_eth_dev_info_get(pi, &dev_info);
+		if (dev_info.max_tx_queues < allowed_max_txq) {
+			allowed_max_txq = dev_info.max_tx_queues;
+			*pid = pi;
+		}
+	}
+	return allowed_max_txq;
+}
+
+/*
+ * Check input txq is valid or not.
+ * If input txq is not greater than any of maximum number
+ * of TX queues of all ports, it is valid.
+ * if valid, return 0, else return -1
+ */
+int
+check_nb_txq(queueid_t txq)
+{
+	queueid_t allowed_max_txq;
+	portid_t pid = 0;
+
+	allowed_max_txq = get_allowed_max_nb_txq(&pid);
+	if (txq > allowed_max_txq) {
+		printf("Fail: input txq (%u) can't be greater "
+		       "than max_tx_queues (%u) of port %u\n",
+		       txq,
+		       allowed_max_txq,
+		       pid);
+		return -1;
+	}
+	return 0;
+}
+
 static void
 init_config(void)
 {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e815509..fe40892 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -702,6 +702,8 @@ int new_socket_id(unsigned int socket_id);
 
 queueid_t get_allowed_max_nb_rxq(portid_t *pid);
 int check_nb_rxq(queueid_t rxq);
+queueid_t get_allowed_max_nb_txq(portid_t *pid);
+int check_nb_txq(queueid_t txq);
 
 /*
  * Work-around of a compilation error with ICC on invocations of the
-- 
2.7.5

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

* Re: [dpdk-dev] [PATCH v6 0/2] app/testpmd: fix invalid rxq and txq nubmer settings
  2018-01-12 11:31         ` [dpdk-dev] [PATCH v6 " Wei Dai
  2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
  2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: fix invalid txq " Wei Dai
@ 2018-01-17 14:06           ` Thomas Monjalon
  2 siblings, 0 replies; 32+ messages in thread
From: Thomas Monjalon @ 2018-01-17 14:06 UTC (permalink / raw)
  To: Wei Dai
  Cc: dev, ferruh.yigit, yuan.peng, konstantin.ananyev, wenzhuo.lu,
	jingjing.wu, stable

12/01/2018 12:31, Wei Dai:
> If an invlaid number of RX or TX queues is configured from testpmd
> command like "port config all rxq number" or "port config all txq
> number" or from --rxq and --txq in the command to start testpmd.
> The global variable nb_rxq or nb_txq is updated by the invalid
> input. This can cause testpmd crash. For example, if the maximum
> number of RX or TX queues is 4, testpmd will crash after running
> commands "port config all rxq 5", "port config all txq 5" and
> "start" in sequence.
> With these 2 patches, if an invalid input is detected, it is refused
> and testpmd keeps last correct values of  nb_rxq and nb_txq.
> 
> Fixes: ce8d561418d4 ("app/testpmd: add port configuration settings")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Wei Dai <wei.dai@intel.com>
> Aced-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Tested-by: Yuan Peng <yuan.peng@intel.com>

Applied, thanks

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

end of thread, other threads:[~2018-01-17 14:06 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-08 13:02 [dpdk-dev] [PATCH 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
2018-01-08 13:02 ` [dpdk-dev] [PATCH 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
2018-01-08 20:05   ` Ananyev, Konstantin
2018-01-10  1:33     ` Dai, Wei
2018-01-10  1:54       ` Ananyev, Konstantin
2018-01-10  6:00         ` Dai, Wei
2018-01-08 13:02 ` [dpdk-dev] [PATCH 2/2] app/testpmd: fix invalid txq " Wei Dai
2018-01-10  4:14 ` [dpdk-dev] [PATCH v2 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
2018-01-10  4:14   ` [dpdk-dev] [PATCH v2 2/2] app/testpmd: fix invalid txq " Wei Dai
2018-01-10  6:37     ` Yang, Qiming
2018-01-10  8:50       ` Dai, Wei
2018-01-10  8:40   ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Wei Dai
2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
2018-01-10  8:40     ` [dpdk-dev] [PATCH v3 2/2] app/testpmd: fix invalid txq " Wei Dai
2018-01-10  9:58     ` [dpdk-dev] [PATCH v3 0/2] app/testpmd: fix invalid rxq and txq nubmer setting Ananyev, Konstantin
2018-01-11  1:21       ` Dai, Wei
2018-01-11  4:58     ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Wei Dai
2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
2018-01-11  4:58       ` [dpdk-dev] [PATCH v4 2/2] app/testpmd: fix invalid txq " Wei Dai
2018-01-12  5:39       ` [dpdk-dev] [PATCH v4 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
2018-01-12  6:05         ` Dai, Wei
2018-01-12  8:10       ` [dpdk-dev] [PATCH v5 " Wei Dai
2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
2018-01-12  9:09           ` Peng, Yuan
2018-01-12  8:10         ` [dpdk-dev] [PATCH v5 2/2] app/testpmd: fix invalid txq " Wei Dai
2018-01-12  9:12           ` Peng, Yuan
2018-01-12  9:09         ` [dpdk-dev] [PATCH v5 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Peng, Yuan
2018-01-12 11:31         ` [dpdk-dev] [PATCH v6 " Wei Dai
2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 1/2] app/testpmd: fix invalid rxq number setting Wei Dai
2018-01-12 11:31           ` [dpdk-dev] [PATCH v6 2/2] app/testpmd: fix invalid txq " Wei Dai
2018-01-17 14:06           ` [dpdk-dev] [PATCH v6 0/2] app/testpmd: fix invalid rxq and txq nubmer settings Thomas Monjalon

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