From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 7AC1C2C38 for ; Mon, 24 Apr 2017 03:49:43 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga105.jf.intel.com with ESMTP; 23 Apr 2017 18:49:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,242,1488873600"; d="scan'208";a="91313980" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga005.jf.intel.com with ESMTP; 23 Apr 2017 18:49:40 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 23 Apr 2017 18:49:41 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 23 Apr 2017 18:49:41 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.246]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.193]) with mapi id 14.03.0319.002; Mon, 24 Apr 2017 09:49:39 +0800 From: "Lu, Wenzhuo" To: 'Olivier MATZ' CC: "'dev@dpdk.org'" Thread-Topic: CLI parsing issue Thread-Index: AdK5rZEyXVh4UIJASg6apLd75x49UP//hlCA//5wZeD/+BgCMA== Date: Mon, 24 Apr 2017 01:49:38 +0000 Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC09093B59BE9C@shsmsx102.ccr.corp.intel.com> References: <6A0DE07E22DDAD4C9103DF62FEBC09093B59B248@shsmsx102.ccr.corp.intel.com> <20170420105455.02ccc25f@glumotte.dev.6wind.com> <6A0DE07E22DDAD4C9103DF62FEBC09093B59B412@shsmsx102.ccr.corp.intel.com> In-Reply-To: <6A0DE07E22DDAD4C9103DF62FEBC09093B59B412@shsmsx102.ccr.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] CLI parsing issue 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: , X-List-Received-Date: Mon, 24 Apr 2017 01:49:44 -0000 Hi Olivier, > -----Original Message----- > From: Lu, Wenzhuo > Sent: Friday, April 21, 2017 9:18 AM > To: Olivier MATZ > Cc: dev@dpdk.org > Subject: RE: CLI parsing issue >=20 > Hi Olivier, >=20 > > -----Original Message----- > > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > > Sent: Thursday, April 20, 2017 4:55 PM > > To: Lu, Wenzhuo > > Cc: dev@dpdk.org > > Subject: Re: CLI parsing issue > > > > Hi Wenzhuo, > > > > On Thu, 20 Apr 2017 08:36:38 +0000, "Lu, Wenzhuo" > > > > wrote: > > > Hi Olivier, > > > I met a problem thar the parsing result of CLI is wrong. > > > I checked this function, cmdline_parse. It will check the CLI > > > instances one by one. Even if an instance is matched, the parsing > > > will not stop for ambiguous check. Seems the following check may > > > change the parsing result of the previous one, > > > /* fully parsed */ > > > tok =3D match_inst(inst, buf, 0, > > > result.buf, sizeof(result.buf), > > > > > > &dyn_tokens); > > > > > > > > > Is it better to use a temporary validate for match_inst and only > > > store the result when it matches, so the previous result has no > > > chance to be changed? Like bellow, > > > > > > > > > diff --git a/lib/librte_cmdline/cmdline_parse.c > > > b/lib/librte_cmdline/cmdline_parse.c > > > index 763c286..663efd1 100644 > > > --- a/lib/librte_cmdline/cmdline_parse.c > > > +++ b/lib/librte_cmdline/cmdline_parse.c > > > @@ -259,6 +259,7 @@ > > > char buf[CMDLINE_PARSE_RESULT_BUFSIZE]; > > > long double align; /* strong alignment constraint for= buf */ > > > } result; > > > + char tmp_buf[CMDLINE_PARSE_RESULT_BUFSIZE]; > > > cmdline_parse_token_hdr_t > > *dyn_tokens[CMDLINE_PARSE_DYNAMIC_TOKENS]; > > > void (*f)(void *, struct cmdline *, void *) =3D NULL; > > > void *data =3D NULL; > > > @@ -321,7 +322,7 @@ > > > debug_printf("INST %d\n", inst_num); > > > > > > /* fully parsed */ > > > - tok =3D match_inst(inst, buf, 0, result.buf, sizeof(r= esult.buf), > > > + tok =3D match_inst(inst, buf, 0, tmp_buf, > > > + sizeof(tmp_buf), > > > &dyn_tokens); > > > > > > if (tok > 0) /* we matched at least one token */ @@ > > > -329,6 +330,8 @@ > > > > > > else if (!tok) { > > > debug_printf("INST fully parsed\n"); > > > + memcpy(result.buf, tmp_buf, > > > + CMDLINE_PARSE_RESULT_BUFSIZE); > > > /* skip spaces */ > > > while (isblank2(*curbuf)) { > > > curbuf++; > > > > > > > > > > At first glance, I think your patch doesn't hurt. Do you have an > > example code that triggers the issue? > Sorry, I didn't show you the issue we met. > It's easy to reproduce on 17.05 RC1. > "testpmd> set tx loopback 0 on > Invalid port 116" > Whatever the input port id is, it's taken as 116 after parsing the CLI. >=20 > Interesting, this issue is triggered by this patch, after I added a new = CLI, the > "set tx loopback ..." is not working. >=20 > commit 22e6545fd02cab42332acd716b8921dd0aab3ad9 > Author: Wenzhuo Lu > Date: Fri Feb 24 11:24:35 2017 +0800 >=20 > app/testpmd: set TC strict link priority mode >=20 > I checked the implement of CLI parsing. > The implementation is going through all the instances in main_ctx to see > which instance can match the string we typed. > If typing "set tx loopback 0 on", it matches cmd_set_tx_loopback, and the > parsing result is, > $2 =3D {set =3D "set\000tx loopback 0 off \n", '\000' = , tx =3D > "tx\000loopback 0 off \n", '\000' , > loopback =3D "loopback\000\060 off \n", '\000' , > port_id =3D 0 '\000', > on_off =3D "off\000\n", '\000' } >=20 > Till now, everything is fine. > Then the parsing is not stopped, it's going on to check if the string can= match > any of the left instances. When checking cmd_strict_link_prio, although i= t > doesn't match, the parsing result is changed to, > $1 =3D {set =3D "set\000tx loopback 0 off \n", '\000' = , tx =3D > "tx\000loopback 0 off \n", '\000' , > loopback =3D "loopback\000\060 off \n", '\000' , > port_id =3D 116 't', > on_off =3D "x\000loopback 0 off \n", '\000' } >=20 > You see, now the port id and on_off both are wrong. Port_id points to cha= r > 't' of "tx loopback ...". So it's always 116, the ASCII of 't'. Any news? Shall I send a patch?=20