diff mbox

[dpdk-dev,v2,2/2] app/testpmd: fix invalid txq number setting

Message ID 1515557660-36763-3-git-send-email-wei.dai@intel.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Wei Dai Jan. 10, 2018, 4:14 a.m. UTC 
  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(-)

Comments

Qiming Yang Jan. 10, 2018, 6:37 a.m. UTC | #1 
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
Wei Dai Jan. 10, 2018, 8:50 a.m. UTC | #2 
> -----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
diff mbox

Patch

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;