From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id D49C0F94 for ; Fri, 21 Apr 2017 03:17:55 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Apr 2017 18:17:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,227,1488873600"; d="scan'208";a="92416744" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga006.fm.intel.com with ESMTP; 20 Apr 2017 18:17:53 -0700 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 20 Apr 2017 18:17:53 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 20 Apr 2017 18:17:53 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.246]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.117]) with mapi id 14.03.0319.002; Fri, 21 Apr 2017 09:17:51 +0800 From: "Lu, Wenzhuo" To: Olivier MATZ CC: "dev@dpdk.org" Thread-Topic: CLI parsing issue Thread-Index: AdK5rZEyXVh4UIJASg6apLd75x49UP//hlCA//5wZeA= Date: Fri, 21 Apr 2017 01:17:50 +0000 Message-ID: <6A0DE07E22DDAD4C9103DF62FEBC09093B59B412@shsmsx102.ccr.corp.intel.com> References: <6A0DE07E22DDAD4C9103DF62FEBC09093B59B248@shsmsx102.ccr.corp.intel.com> <20170420105455.02ccc25f@glumotte.dev.6wind.com> In-Reply-To: <20170420105455.02ccc25f@glumotte.dev.6wind.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: Fri, 21 Apr 2017 01:17:56 -0000 Hi Olivier, > -----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 >=20 > Hi Wenzhuo, >=20 > 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.b= uf, 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 b= uf */ > > } 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(res= ult.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++; > > > > >=20 > 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.=20 "testpmd> set tx loopback 0 on Invalid port 116" Whatever the input port id is, it's taken as 116 after parsing the CLI. Interesting, this issue is triggered by this patch, after I added a new CL= I, the "set tx loopback ..." is not working. commit 22e6545fd02cab42332acd716b8921dd0aab3ad9 Author: Wenzhuo Lu Date: Fri Feb 24 11:24:35 2017 +0800 app/testpmd: set TC strict link priority mode I checked the implement of CLI parsing. The implementation is going through all the instances in main_ctx to see wh= ich instance can match the string we typed. If typing "set tx loopback 0 on", it matches cmd_set_tx_loopback, and the p= arsing result is, $2 =3D {set =3D "set\000tx loopback 0 off \n", '\000' , = tx =3D "tx\000loopback 0 off \n", '\000' ,=20 loopback =3D "loopback\000\060 off \n", '\000' , port_= id =3D 0 '\000',=09 on_off =3D "off\000\n", '\000' } Till now, everything is fine. Then the parsing is not stopped, it's going on to check if the string can m= atch any of the left instances. When checking cmd_strict_link_prio, althoug= h it 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' ,=20 loopback =3D "loopback\000\060 off \n", '\000' , port_= id =3D 116 't',=20 on_off =3D "x\000loopback 0 off \n", '\000' } You see, now the port id and on_off both are wrong. Port_id points to char = 't' of "tx loopback ...". So it's always 116, the ASCII of 't'. >=20 >=20 > Thanks, > Olivier