From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f177.google.com (mail-wr0-f177.google.com [209.85.128.177]) by dpdk.org (Postfix) with ESMTP id 39F832904 for ; Mon, 24 Apr 2017 12:11:48 +0200 (CEST) Received: by mail-wr0-f177.google.com with SMTP id l50so14780438wrc.3 for ; Mon, 24 Apr 2017 03:11:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=XS8KeC8yZ8GhtZ71ePMqZITahja0pmwXOc3vZsKQwWs=; b=BkB4HbDCaVKSs/2x31wZ5qbn8WxdU+dMPavXTzToCm4s7LOmRnZ71ARrxK5aoBMQfP GcbNWn+qUue0JWCyiWkiT3sltdIf8MJQeWMLdGax36i2/Ocgnb2lUfmP2MeyTe4qqLOz cRlkrM58LHKe+a2hEZSA9u+YGsUCtC+lCu3HMvHECNjpPHi17CtALtgRuXkTGz0DSyPE J2+zOpRTkjaZAz1DGIbZzD9hZWnsnyF+KfiFV7WLYo+z0vYAkyj0h1bT2VgYVVmx/csv 3+H98CXoQ9EVZioGa+XZNzvWDEuX7yIt73BwBw1uqu40VJMb0JVRl2O8wlxukj1v9Vow PGtQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=XS8KeC8yZ8GhtZ71ePMqZITahja0pmwXOc3vZsKQwWs=; b=snsDs1bTTvg3zkqLnbXbEqDMLuTVyzdegRGQpXjHvuoXflwBqqqo/WVYxsNp3ZcB0I cm4Zq9iw6g3cPJPWN23fR2zinHN/iihahlsBoK6nZMZdKh+YlLNn41zHF1stNlcYASmd PaCqdcZ9LzB1+NJWg41wkIMW7ovSkIbe7VkETeVj3hZwguhCkke2600HJZ5g1zFHnqoe fktTVZXitaGReBEvWkVDNpgvrXlAeqNGT2CB7Jpj4y8D15bRw/2qeehdjP/w7NjXIHHb roUagKoHqKn5B4OWL7X4dUjJ/m9A/35EyYnGXposBMKXiwTMXReYAfKjGz6gLhJekxcz PC8Q== X-Gm-Message-State: AN3rC/4pcnqiTy4LOIvAEG1WdYATAfWNVUpcn6EHTRQld4QadYE8/TZ2 F7NXRjEcs/9WoIEghrQ= X-Received: by 10.223.133.97 with SMTP id 88mr5379950wrh.166.1493028707936; Mon, 24 Apr 2017 03:11:47 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id c16sm21761453wrb.56.2017.04.24.03.11.47 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 24 Apr 2017 03:11:47 -0700 (PDT) Date: Mon, 24 Apr 2017 12:11:45 +0200 From: Olivier Matz To: "Lu, Wenzhuo" Cc: "'dev@dpdk.org'" Message-ID: <20170424121145.243614b1@platinum> In-Reply-To: <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> <6A0DE07E22DDAD4C9103DF62FEBC09093B59BE9C@shsmsx102.ccr.corp.intel.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 10:11:48 -0000 Hi Wenzhuo, On Mon, 24 Apr 2017 01:49:38 +0000, "Lu, Wenzhuo" wrote: > 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 > > > > 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 > > > > > > 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 = 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 *) = NULL; > > > > void *data = NULL; > > > > @@ -321,7 +322,7 @@ > > > > debug_printf("INST %d\n", inst_num); > > > > > > > > /* fully parsed */ > > > > - tok = match_inst(inst, buf, 0, result.buf, sizeof(result.buf), > > > > + tok = 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. > > > > Interesting, this issue is triggered by this patch, after I added a new CLI, 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 > > 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 = {set = "set\000tx loopback 0 off \n", '\000' , tx = > > "tx\000loopback 0 off \n", '\000' , > > loopback = "loopback\000\060 off \n", '\000' , > > port_id = 0 '\000', > > on_off = "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 match > > any of the left instances. When checking cmd_strict_link_prio, although it > > doesn't match, the parsing result is changed to, > > $1 = {set = "set\000tx loopback 0 off \n", '\000' , tx = > > "tx\000loopback 0 off \n", '\000' , > > loopback = "loopback\000\060 off \n", '\000' , > > port_id = 116 't', > > on_off = "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'. > Any news? Shall I send a patch? I checked in detail, and you're right there is a bug here. Your proposed patch looks good, you can send it. Thank you for reporting it! You can add in the log: Fixes: af75078fece3 ("first public release") CC: stable@dpdk.org Regards, Olivier