From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id BC07DA0353; Tue, 19 Nov 2019 20:18:45 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id AC862A69; Tue, 19 Nov 2019 20:18:44 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 85AFD9E4; Tue, 19 Nov 2019 20:18:42 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 19 Nov 2019 11:18:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,219,1571727600"; d="scan'208,217";a="237450443" Received: from unknown (HELO [10.241.225.118]) ([10.241.225.118]) by fmsmga002.fm.intel.com with ESMTP; 19 Nov 2019 11:18:40 -0800 To: Ferruh Yigit , David Marchand , dev@dpdk.org Cc: thomas@monjalon.net, stable@dpdk.org, Wenzhuo Lu , Jingjing Wu , Bernard Iremonger , Anatoly Burakov , Ranjit Menon References: <20191117171013.17373-1-david.marchand@redhat.com> <20191118153714.17610-1-david.marchand@redhat.com> <6d3650f1-4b00-9e58-c31b-cfb57d21bf60@intel.com> From: Pallavi Kadam Message-ID: Date: Tue, 19 Nov 2019 11:18:40 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <6d3650f1-4b00-9e58-c31b-cfb57d21bf60@intel.com> Content-Language: en-US Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: report invalid command line parameter X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 11/18/2019 8:18 AM, Ferruh Yigit wrote: > On 11/18/2019 3:37 PM, David Marchand wrote: >> We currently do not check that a non option string has been passed to >> testpmd. >> >> Example: >> $ ./master/app/testpmd --no-huge -m 512 --vdev net_null0 \ >> --vdev net_null1 -- -i nb-cores=2 --total-num-mbuf 2048 >> [...] >> testpmd> show config fwd >> io packet forwarding - ports=2 - cores=1 - streams=2 - NUMA support >> enabled, MP allocation mode: native >> Logical Core 1 (socket 0) forwards packets on 2 streams: >> RX P=0/Q=0 (socket 0) -> TX P=1/Q=0 (socket 0) peer=02:00:00:00:00:01 >> RX P=1/Q=0 (socket 0) -> TX P=0/Q=0 (socket 0) peer=02:00:00:00:00:00 >> >> Here nb-cores=2 is just ignored, while the (probably sleepy) user did not >> notice this. >> >> Validate that all strings passed to testpmd are part of a known option. >> >> After this patch: >> $ ./master/app/testpmd --no-huge -m 512 --vdev net_null0 \ >> --vdev net_null1 -- -i nb-cores=2 --total-num-mbuf 2048 >> [...] >> Invalid parameter: nb-cores=2 >> EAL: Error - exiting with code: 1 >> Cause: Command line incorrect >> >> While at it, when passing an unknown option, print the string that gets >> refused by getopt_long to help the user. >> >> Fixes: af75078fece3 ("first public release") >> Cc: stable@dpdk.org >> >> Signed-off-by: David Marchand >> Reviewed-by: Ferruh Yigit >> --- >> This seems a bit dangerous to take this kind of change this late. >> Some "working fine" scripts might now report failures from testpmd because >> of garbage in the command line. >> >> Sending the patch anyway to see what others think about it. >> >> Changelog since v1: >> - fixed example in commitlog, >> >> --- >> app/test-pmd/parameters.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c >> index deca7a6828..2e7a504415 100644 >> --- a/app/test-pmd/parameters.c >> +++ b/app/test-pmd/parameters.c >> @@ -1363,12 +1363,19 @@ launch_args_parse(int argc, char** argv) >> break; >> default: >> usage(argv[0]); >> + printf("Invalid option: %s\n", argv[optind]); >> rte_exit(EXIT_FAILURE, >> "Command line is incomplete or incorrect\n"); >> break; >> } >> } >> >> + if (optind != argc) { > I hope 'optind' works as expected [1] in Windows too (Anatoly verified the > FreeBSD one). > > @Pallavi, @Ranjit, Can you please confirm it? > > [1] > https://linux.die.net/man/3/optind > " > If there are no more option characters, getopt() returns -1. Then optind is the > index in argv of the first argv-element that is not an option. > " Verified on Windows. So far this change does not affect Windows code. Also, Windows does not support testpmd app yet.