app/testpmd: remove useless check

Message ID 20221012142915.553392-1-yuanx.wang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Andrew Rybchenko
Headers
Series app/testpmd: remove useless check |

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/github-robot: build success github build: passed
ci/checkpatch success coding style OK

Commit Message

Wang, YuanX Oct. 12, 2022, 2:29 p.m. UTC
  Protocol header sequence checking is supported in the ethdev library,
the application does not need to do it again.

Coverity issue: 381396
Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")

Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
---
 app/test-pmd/cmdline.c    | 6 ++----
 app/test-pmd/parameters.c | 2 +-
 app/test-pmd/testpmd.h    | 2 +-
 3 files changed, 4 insertions(+), 6 deletions(-)
  

Comments

Singh, Aman Deep Oct. 14, 2022, 9:37 a.m. UTC | #1
Thanks for the patch Yuan.

On 10/12/2022 7:59 PM, Yuan Wang wrote:
> Protocol header sequence checking is supported in the ethdev library,
> the application does not need to do it again.

I would like to rephrase the patch title to make it more specific.
Like "remove useless check" to "remove unused parameter in rx hdr split"
or something like that, to specify the patch.

> Coverity issue: 381396
> Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
>
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>

> ---
>   app/test-pmd/cmdline.c    | 6 ++----
>   app/test-pmd/parameters.c | 2 +-
>   app/test-pmd/testpmd.h    | 2 +-
>   3 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 17be2de402..29e4b2329b 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -3440,7 +3440,7 @@ get_ptype(char *value)
>   
>   unsigned int
>   parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
> -				unsigned int *parsed_items, int check_hdrs_sequence)
> +				unsigned int *parsed_items)
>   {
>   	unsigned int nb_item;
>   	char *cur;
> @@ -3462,8 +3462,6 @@ parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
>   		fprintf(stderr, "Number of %s = %u > %u (maximum items)\n",
>   			item_name, nb_item + 1, max_items);
>   	free(str2);
> -	if (!check_hdrs_sequence)
> -		return nb_item;
>   	return nb_item;
>   }
>   
> @@ -3854,7 +3852,7 @@ cmd_set_rxhdrs_parsed(void *parsed_result,
>   
>   	res = parsed_result;
>   	nb_segs = parse_hdrs_list(res->values, "segment hdrs",
> -				  MAX_SEGS_BUFFER_SPLIT, seg_hdrs, 0);
> +				  MAX_SEGS_BUFFER_SPLIT, seg_hdrs);
>   	if (nb_segs > 0)
>   		set_rx_pkt_hdrs(seg_hdrs, nb_segs);
>   	cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1);
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
> index ff760460ec..5b305c833c 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -1272,7 +1272,7 @@ launch_args_parse(int argc, char** argv)
>   				nb_segs = parse_hdrs_list
>   						(optarg, "rxpkt segments",
>   						MAX_SEGS_BUFFER_SPLIT,
> -						seg_hdrs, 0);
> +						seg_hdrs);
>   				if (nb_segs > 0)
>   					set_rx_pkt_hdrs(seg_hdrs, nb_segs);
>   				else
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index e65be323b8..0b99339df8 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -854,7 +854,7 @@ unsigned int parse_item_list(const char *str, const char *item_name,
>   			unsigned int *parsed_items, int check_unique_values);
>   unsigned int parse_hdrs_list(const char *str, const char *item_name,
>   			unsigned int max_item,
> -			unsigned int *parsed_items, int check_unique_values);
> +			unsigned int *parsed_items);
>   void launch_args_parse(int argc, char** argv);
>   void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue);
>   void cmdline_read_from_file(const char *filename);
  
Wang, YuanX Oct. 14, 2022, 9:52 a.m. UTC | #2
Hi Singh,

> -----Original Message-----
> From: Singh, Aman Deep <aman.deep.singh@intel.com>
> Sent: Friday, October 14, 2022 5:37 PM
> To: Wang, YuanX <yuanx.wang@intel.com>; Zhang, Yuying
> <yuying.zhang@intel.com>
> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi
> <yaqi.tang@intel.com>
> Subject: Re: [PATCH] app/testpmd: remove useless check
> 
> Thanks for the patch Yuan.
> 
> On 10/12/2022 7:59 PM, Yuan Wang wrote:
> > Protocol header sequence checking is supported in the ethdev library,
> > the application does not need to do it again.
> 
> I would like to rephrase the patch title to make it more specific.
> Like "remove useless check" to "remove unused parameter in rx hdr split"
> or something like that, to specify the patch.

Thank you for the correction, please feel free to fix when merging.

Thanks,
Yuan

> 
> > Coverity issue: 381396
> > Fixes: 52e2e7edcf48 ("app/testpmd: add protocol-based buffer split")
> >
> > Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> 
> > ---
> >   app/test-pmd/cmdline.c    | 6 ++----
> >   app/test-pmd/parameters.c | 2 +-
> >   app/test-pmd/testpmd.h    | 2 +-
> >   3 files changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> > 17be2de402..29e4b2329b 100644
> > --- a/app/test-pmd/cmdline.c
> > +++ b/app/test-pmd/cmdline.c
> > @@ -3440,7 +3440,7 @@ get_ptype(char *value)
> >
> >   unsigned int
> >   parse_hdrs_list(const char *str, const char *item_name, unsigned int
> max_items,
> > -				unsigned int *parsed_items, int
> check_hdrs_sequence)
> > +				unsigned int *parsed_items)
> >   {
> >   	unsigned int nb_item;
> >   	char *cur;
> > @@ -3462,8 +3462,6 @@ parse_hdrs_list(const char *str, const char
> *item_name, unsigned int max_items,
> >   		fprintf(stderr, "Number of %s = %u > %u (maximum
> items)\n",
> >   			item_name, nb_item + 1, max_items);
> >   	free(str2);
> > -	if (!check_hdrs_sequence)
> > -		return nb_item;
> >   	return nb_item;
> >   }
> >
> > @@ -3854,7 +3852,7 @@ cmd_set_rxhdrs_parsed(void *parsed_result,
> >
> >   	res = parsed_result;
> >   	nb_segs = parse_hdrs_list(res->values, "segment hdrs",
> > -				  MAX_SEGS_BUFFER_SPLIT, seg_hdrs, 0);
> > +				  MAX_SEGS_BUFFER_SPLIT, seg_hdrs);
> >   	if (nb_segs > 0)
> >   		set_rx_pkt_hdrs(seg_hdrs, nb_segs);
> >   	cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1); diff --git
> > a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> > ff760460ec..5b305c833c 100644
> > --- a/app/test-pmd/parameters.c
> > +++ b/app/test-pmd/parameters.c
> > @@ -1272,7 +1272,7 @@ launch_args_parse(int argc, char** argv)
> >   				nb_segs = parse_hdrs_list
> >   						(optarg, "rxpkt segments",
> >   						MAX_SEGS_BUFFER_SPLIT,
> > -						seg_hdrs, 0);
> > +						seg_hdrs);
> >   				if (nb_segs > 0)
> >   					set_rx_pkt_hdrs(seg_hdrs, nb_segs);
> >   				else
> > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> > e65be323b8..0b99339df8 100644
> > --- a/app/test-pmd/testpmd.h
> > +++ b/app/test-pmd/testpmd.h
> > @@ -854,7 +854,7 @@ unsigned int parse_item_list(const char *str, const
> char *item_name,
> >   			unsigned int *parsed_items, int
> check_unique_values);
> >   unsigned int parse_hdrs_list(const char *str, const char *item_name,
> >   			unsigned int max_item,
> > -			unsigned int *parsed_items, int
> check_unique_values);
> > +			unsigned int *parsed_items);
> >   void launch_args_parse(int argc, char** argv);
> >   void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t
> queue);
> >   void cmdline_read_from_file(const char *filename);
  
Andrew Rybchenko Oct. 17, 2022, 8:36 a.m. UTC | #3
On 10/14/22 12:52, Wang, YuanX wrote:
> Hi Singh,
> 
>> -----Original Message-----
>> From: Singh, Aman Deep <aman.deep.singh@intel.com>
>> Sent: Friday, October 14, 2022 5:37 PM
>> To: Wang, YuanX <yuanx.wang@intel.com>; Zhang, Yuying
>> <yuying.zhang@intel.com>
>> Cc: dev@dpdk.org; Ding, Xuan <xuan.ding@intel.com>; Tang, Yaqi
>> <yaqi.tang@intel.com>
>> Subject: Re: [PATCH] app/testpmd: remove useless check
>>
>> Thanks for the patch Yuan.
>>
>> On 10/12/2022 7:59 PM, Yuan Wang wrote:
>>> Protocol header sequence checking is supported in the ethdev library,
>>> the application does not need to do it again.
>>
>> I would like to rephrase the patch title to make it more specific.
>> Like "remove useless check" to "remove unused parameter in rx hdr split"
>> or something like that, to specify the patch.
> 
> Thank you for the correction, please feel free to fix when merging.
> 
> Thanks,
> Yuan

Done

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

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 17be2de402..29e4b2329b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3440,7 +3440,7 @@  get_ptype(char *value)
 
 unsigned int
 parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
-				unsigned int *parsed_items, int check_hdrs_sequence)
+				unsigned int *parsed_items)
 {
 	unsigned int nb_item;
 	char *cur;
@@ -3462,8 +3462,6 @@  parse_hdrs_list(const char *str, const char *item_name, unsigned int max_items,
 		fprintf(stderr, "Number of %s = %u > %u (maximum items)\n",
 			item_name, nb_item + 1, max_items);
 	free(str2);
-	if (!check_hdrs_sequence)
-		return nb_item;
 	return nb_item;
 }
 
@@ -3854,7 +3852,7 @@  cmd_set_rxhdrs_parsed(void *parsed_result,
 
 	res = parsed_result;
 	nb_segs = parse_hdrs_list(res->values, "segment hdrs",
-				  MAX_SEGS_BUFFER_SPLIT, seg_hdrs, 0);
+				  MAX_SEGS_BUFFER_SPLIT, seg_hdrs);
 	if (nb_segs > 0)
 		set_rx_pkt_hdrs(seg_hdrs, nb_segs);
 	cmd_reconfig_device_queue(RTE_PORT_ALL, 0, 1);
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index ff760460ec..5b305c833c 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -1272,7 +1272,7 @@  launch_args_parse(int argc, char** argv)
 				nb_segs = parse_hdrs_list
 						(optarg, "rxpkt segments",
 						MAX_SEGS_BUFFER_SPLIT,
-						seg_hdrs, 0);
+						seg_hdrs);
 				if (nb_segs > 0)
 					set_rx_pkt_hdrs(seg_hdrs, nb_segs);
 				else
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e65be323b8..0b99339df8 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -854,7 +854,7 @@  unsigned int parse_item_list(const char *str, const char *item_name,
 			unsigned int *parsed_items, int check_unique_values);
 unsigned int parse_hdrs_list(const char *str, const char *item_name,
 			unsigned int max_item,
-			unsigned int *parsed_items, int check_unique_values);
+			unsigned int *parsed_items);
 void launch_args_parse(int argc, char** argv);
 void cmd_reconfig_device_queue(portid_t id, uint8_t dev, uint8_t queue);
 void cmdline_read_from_file(const char *filename);