DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
       [not found] <1503378222-23306-1-git-send-email-han.li1@zte.com.cn>
@ 2017-09-04 14:11 ` Rybalchenko, Kirill
  2017-09-07  8:44   ` Wu, Jingjing
  2017-10-09  4:16 ` Ferruh Yigit
  1 sibling, 1 reply; 6+ messages in thread
From: Rybalchenko, Kirill @ 2017-09-04 14:11 UTC (permalink / raw)
  To: Li Han, Wu, Jingjing; +Cc: dev

Hi Han,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Li Han
> Sent: Tuesday 22 August 2017 06:04
> To: Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org; Li Han <han.li1@zte.com.cn>
> Subject: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
> 
> in parse_ringnuma_config/parse_portnuma_config functions,port_id should
> less than RTE_MAX_ETHPORTS,but port_id_is_invalid check assumes that
> port_id may be 255.
> 
> Signed-off-by: Li Han <han.li1@zte.com.cn>
> ---
>  app/test-pmd/parameters.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> 2f7f70f..0c97ba4 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -424,7 +424,8 @@
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
> +			port_id == (portid_t)RTE_PORT_ALL) {
In this case the message "printf("Invalid port %d\n", port_id)" will be omitted and there will be no way 
To figure out which port id caused the problem

>  			printf("Valid port range is [0");
>  			RTE_ETH_FOREACH_DEV(pid)
>  				printf(", %d", pid);
> @@ -483,7 +484,8 @@
>  				return -1;
>  		}
>  		port_id = (uint8_t)int_fld[FLD_PORT];
> -		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> +		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
> +			port_id == (portid_t)RTE_PORT_ALL) {
The same here

>  			printf("Valid port range is [0");
>  			RTE_ETH_FOREACH_DEV(pid)
>  				printf(", %d", pid);
> --
> 1.8.3.1
> 

Thanks,
Kirill

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

* Re: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
  2017-09-04 14:11 ` [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters Rybalchenko, Kirill
@ 2017-09-07  8:44   ` Wu, Jingjing
  0 siblings, 0 replies; 6+ messages in thread
From: Wu, Jingjing @ 2017-09-07  8:44 UTC (permalink / raw)
  To: Rybalchenko, Kirill, Li Han; +Cc: dev



> -----Original Message-----
> From: Rybalchenko, Kirill
> Sent: Monday, September 4, 2017 10:12 PM
> To: Li Han <han.li1@zte.com.cn>; Wu, Jingjing <jingjing.wu@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
> 
> Hi Han,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Li Han
> > Sent: Tuesday 22 August 2017 06:04
> > To: Wu, Jingjing <jingjing.wu@intel.com>
> > Cc: dev@dpdk.org; Li Han <han.li1@zte.com.cn>
> > Subject: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
> >
> > in parse_ringnuma_config/parse_portnuma_config functions,port_id should
> > less than RTE_MAX_ETHPORTS,but port_id_is_invalid check assumes that
> > port_id may be 255.
> >
> > Signed-off-by: Li Han <han.li1@zte.com.cn>
> > ---
> >  app/test-pmd/parameters.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> > 2f7f70f..0c97ba4 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -424,7 +424,8 @@
> >  				return -1;
> >  		}
> >  		port_id = (uint8_t)int_fld[FLD_PORT];
> > -		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
> > +		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
> > +			port_id == (portid_t)RTE_PORT_ALL) {
> In this case the message "printf("Invalid port %d\n", port_id)" will be omitted and there
> will be no way
> To figure out which port id caused the problem
Why? If the port_id is invalid and not RTE_PORT_ALL, the "printf("Invalid port %d\n", port_id)" should be print.

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

* Re: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
       [not found] <1503378222-23306-1-git-send-email-han.li1@zte.com.cn>
  2017-09-04 14:11 ` [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters Rybalchenko, Kirill
@ 2017-10-09  4:16 ` Ferruh Yigit
  2017-10-09  4:57   ` Ferruh Yigit
  1 sibling, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2017-10-09  4:16 UTC (permalink / raw)
  To: Li Han, jingjing.wu; +Cc: dev

On 8/22/2017 6:03 AM, Li Han wrote:
> in parse_ringnuma_config/parse_portnuma_config functions,port_id
> should less than RTE_MAX_ETHPORTS,but port_id_is_invalid check
> assumes that port_id may be 255.

testpmd using RTE_PORT_ALL [1], which is in valid port_id range, as
special meaning [2] making things tricky.

Only above 255 is no more valid since port_id is not 16bits, it should
be 65535.

Except from above detail,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>


And if you are familiar with this code, above a few lines there is a
"int_fld[i] > 255" check, is the intention to check port_id limit there?
If so this is no more valid check, and would you mind sending a patch to
fix if you have time?

Thanks,
ferruh


[1]
#define RTE_PORT_ALL            (~(portid_t)0x0)


[2]
Meaning all enabled ethdev ports, like:
start_port(RTE_PORT_ALL);

> 
> Signed-off-by: Li Han <han.li1@zte.com.cn>

<...>

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

* Re: [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
  2017-10-09  4:16 ` Ferruh Yigit
@ 2017-10-09  4:57   ` Ferruh Yigit
  0 siblings, 0 replies; 6+ messages in thread
From: Ferruh Yigit @ 2017-10-09  4:57 UTC (permalink / raw)
  To: Li Han, jingjing.wu; +Cc: dev

On 10/9/2017 5:16 AM, Ferruh Yigit wrote:
> On 8/22/2017 6:03 AM, Li Han wrote:
>> in parse_ringnuma_config/parse_portnuma_config functions,port_id
>> should less than RTE_MAX_ETHPORTS,but port_id_is_invalid check
>> assumes that port_id may be 255.>>
>> Signed-off-by: Li Han <han.li1@zte.com.cn>

> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

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

Welcome Li Han!

(Fixed mentioned 255 note in commit log while applying, also fixed local
port_id storage size while applying)

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

* [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
@ 2017-08-22  2:31 Li Han
  0 siblings, 0 replies; 6+ messages in thread
From: Li Han @ 2017-08-22  2:31 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, Li Han

in parse_ringnuma_config/parse_portnuma_config functions,port_id
should less than RTE_MAX_ETHPORTS,but port_id_is_invalid check
assumes that port_id may be 255.

Signed-off-by: Li Han <han.li1@zte.com.cn>
---
 app/test-pmd/parameters.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 app/test-pmd/parameters.c

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
old mode 100644
new mode 100755
index 2f7f70f..99a5340
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -424,7 +424,8 @@
 				return -1;
 		}
 		port_id = (uint8_t)int_fld[FLD_PORT];
-		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		      port_id == (portid_t)RTE_PORT_ALL) {
 			printf("Valid port range is [0");
 			RTE_ETH_FOREACH_DEV(pid)
 				printf(", %d", pid);
@@ -483,7 +484,8 @@
 				return -1;
 		}
 		port_id = (uint8_t)int_fld[FLD_PORT];
-		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		      port_id == (portid_t)RTE_PORT_ALL) {
 			printf("Valid port range is [0");
 			RTE_ETH_FOREACH_DEV(pid)
 				printf(", %d", pid);
-- 
1.8.3.1

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

* [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters
@ 2017-08-21  8:11 Li Han
  0 siblings, 0 replies; 6+ messages in thread
From: Li Han @ 2017-08-21  8:11 UTC (permalink / raw)
  To: jingjing.wu; +Cc: dev, Li Han

in parse_ringnuma_config/parse_portnuma_config functions,port_id
should less than RTE_MAX_ETHPORTS,but port_id_is_invalid check
assumes that port_id may be 255.

Signed-off-by: Li Han <han.li1@zte.com.cn>
---
 app/test-pmd/parameters.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
 mode change 100644 => 100755 app/test-pmd/parameters.c

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
old mode 100644
new mode 100755
index 2f7f70f..99a5340
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -424,7 +424,8 @@
 				return -1;
 		}
 		port_id = (uint8_t)int_fld[FLD_PORT];
-		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		      port_id == (portid_t)RTE_PORT_ALL) {
 			printf("Valid port range is [0");
 			RTE_ETH_FOREACH_DEV(pid)
 				printf(", %d", pid);
@@ -483,7 +484,8 @@
 				return -1;
 		}
 		port_id = (uint8_t)int_fld[FLD_PORT];
-		if (port_id_is_invalid(port_id, ENABLED_WARN)) {
+		if (port_id_is_invalid(port_id, ENABLED_WARN) ||
+		      port_id == (portid_t)RTE_PORT_ALL) {
 			printf("Valid port range is [0");
 			RTE_ETH_FOREACH_DEV(pid)
 				printf(", %d", pid);
-- 
1.8.3.1

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

end of thread, other threads:[~2017-10-09  4:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1503378222-23306-1-git-send-email-han.li1@zte.com.cn>
2017-09-04 14:11 ` [dpdk-dev] [PATCH] app/testpmd:fix invalid port id parameters Rybalchenko, Kirill
2017-09-07  8:44   ` Wu, Jingjing
2017-10-09  4:16 ` Ferruh Yigit
2017-10-09  4:57   ` Ferruh Yigit
2017-08-22  2:31 Li Han
  -- strict thread matches above, loose matches on Subject: below --
2017-08-21  8:11 Li Han

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