From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by dpdk.org (Postfix) with ESMTP id AC33D2BAD for ; Tue, 27 Sep 2016 11:02:05 +0200 (CEST) Received: by mail-wm0-f66.google.com with SMTP id b4so78149wmb.2 for ; Tue, 27 Sep 2016 02:02:05 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cadete-eu.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=JT4VbqWoDE/pW1aGNu+VWN3OSEhnr8FZkrLWnl0YisE=; b=gWI2fo2Hg3kIUZJWi0ZBQESDRyiOe7DpUHdE1zjH8ebDtnym1NNSKvBAnHCMVcwS68 5+w2MtlzXSGcAUIoyOiWGvPF/KGRa50SsD5ebWaCPYp0Bv32JuE9YubeeakqQZh0zTLD 8UrOqpboPc8tsxsbcXtuyPZcz+xd6oHCMepPCOq11rMIKw7ftW93JmGCxKraP7+sVLKX mEZOrHZHHa5eIMJvliWIksFbqxI70l1O1TpvLl1GQ1giZvqvfh4mIAehwHea1W2xfsqB fsKecNMyhTsxtqRz6LIPjqnXecJT9Aqut2BGVKg3ImEKqfyHJ4p1kPXTEGPxpr/swjYR RgFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=JT4VbqWoDE/pW1aGNu+VWN3OSEhnr8FZkrLWnl0YisE=; b=hmPs1LyRTe3nRUeVbfNfx3zLCShEzKKxG7YTzFNXb8FFKWtyQAg+snFnRE6KQA3UZu iBLmGgiPHhMNoI1tEzrAsFMQnnugJJBdNCQCqQhOE+2TVg3LtfIL1w2pg/pyvlDe0dMs tL6k797tFlshCC2V66qjX5nGJo+q88JNZxRuThPV5RlpDdyzL5VZD0DW0ySDoPGcLkKv hODz8ye9KCrZK+ZOWHkMv5KEvKt4EsmAgyfazu3qLFe1U83iU46CinGC5GlLy7Jyhq1R nHhjItpUQ4Zm1opKymhroBaZ+BUqjBmAXNaiUxGgIjSggYMnYzFpYKefBeZB6O+/k15/ ujqg== X-Gm-Message-State: AA6/9Rmp97oJI05LFxuHPFhlUtPKVlQzSOb6AwNpCQjwGScEHi/Iff6/RAStEJlU7e/5tEZd2Orc5AwguPYmfw== X-Received: by 10.194.147.99 with SMTP id tj3mr21881463wjb.183.1474966925463; Tue, 27 Sep 2016 02:02:05 -0700 (PDT) MIME-Version: 1.0 Received: by 10.80.135.13 with HTTP; Tue, 27 Sep 2016 02:01:44 -0700 (PDT) X-Originating-IP: [85.234.217.146] In-Reply-To: <9BB6961774997848B5B42BEC655768F80E273098@SHSMSX103.ccr.corp.intel.com> References: <1471943445-20696-1-git-send-email-Frederico.Cadete-ext@oneaccess-net.com> <9BB6961774997848B5B42BEC655768F80E273098@SHSMSX103.ccr.corp.intel.com> From: Frederico Cadete Date: Tue, 27 Sep 2016 11:01:44 +0200 Message-ID: To: "Wu, Jingjing" Cc: "Frederico.Cadete-ext@oneaccess-net.com" , "dev@dpdk.org" , "De Lara Guarch, Pablo" Content-Type: text/plain; charset=UTF-8 X-Mailman-Approved-At: Tue, 27 Sep 2016 21:53:35 +0200 Subject: Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 27 Sep 2016 09:02:05 -0000 On Tue, Sep 27, 2016 at 4:42 AM, Wu, Jingjing wrote: > > >> -----Original Message----- >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Frederico.Cadete- >> ext@oneaccess-net.com >> Sent: Tuesday, August 23, 2016 5:11 PM >> To: dev@dpdk.org >> Cc: frederico@cadete.eu; Frederico Cadete >> Subject: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel >> modes >> >> From: Frederico Cadete >> >> The flow_director_filter commands has a pf|vf option for most modes >> except for MAC-VLAN and tunnel. On Intel NIC's these modes are not >> supported under virtualized environments. >> But the application was checking that this field was parsed for these cases, >> even though this token is not registered with the cmdline parser. >> >> This patch skips checking of this field for the commands that don't accept it. >> >> Signed-off-by: Frederico Cadete >> --- >> app/test-pmd/cmdline.c | 32 ++++++++++++++++++-------------- >> 1 file changed, 18 insertions(+), 14 deletions(-) >> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index >> f90befc..f516b1b 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -8502,24 +8502,28 @@ cmd_flow_director_filter_parsed(void >> *parsed_result, >> else >> entry.action.behavior = RTE_ETH_FDIR_ACCEPT; >> >> - if (!strcmp(res->pf_vf, "pf")) >> - entry.input.flow_ext.is_vf = 0; >> - else if (!strncmp(res->pf_vf, "vf", 2)) { >> - struct rte_eth_dev_info dev_info; >> + if (fdir_conf.mode != RTE_FDIR_MODE_PERFECT_MAC_VLAN && >> + fdir_conf.mode != RTE_FDIR_MODE_PERFECT_TUNNEL){ >> >> - memset(&dev_info, 0, sizeof(dev_info)); >> - rte_eth_dev_info_get(res->port_id, &dev_info); >> - errno = 0; >> - vf_id = strtoul(res->pf_vf + 2, &end, 10); >> - if (errno != 0 || *end != '\0' || vf_id >= dev_info.max_vfs) { >> + if (!strcmp(res->pf_vf, "pf")) >> + entry.input.flow_ext.is_vf = 0; >> + else if (!strncmp(res->pf_vf, "vf", 2)) { >> + struct rte_eth_dev_info dev_info; >> + >> + memset(&dev_info, 0, sizeof(dev_info)); >> + rte_eth_dev_info_get(res->port_id, &dev_info); >> + errno = 0; >> + vf_id = strtoul(res->pf_vf + 2, &end, 10); >> + if (errno != 0 || *end != '\0' || vf_id >= >> dev_info.max_vfs) { >> + printf("invalid parameter %s.\n", res->pf_vf); >> + return; >> + } >> + entry.input.flow_ext.is_vf = 1; >> + entry.input.flow_ext.dst_id = (uint16_t)vf_id; >> + } else { >> printf("invalid parameter %s.\n", res->pf_vf); >> return; >> } >> - entry.input.flow_ext.is_vf = 1; >> - entry.input.flow_ext.dst_id = (uint16_t)vf_id; >> - } else { >> - printf("invalid parameter %s.\n", res->pf_vf); >> - return; >> } > > Thanks for the patch. And thanks a lot for the review. > But with this change the field of pf_vf cannot omit either. > I think it still looks confused because it will allow any meaningless string. Sorry, I am not aware that it can be omitted. For MAC/VLAN and tunnel mode it does not and will not allow any meaningless string. At least that was my intention :) The cmdline parser expects "... flexbytes (flexbytes_value) (drop|fwd) queue ..." . This is what is documented [1] and the command's cmdline_parse_inst_t [2] matches this. If you put something in-between "(drop|fwd)" and "queue" it is rejected by the parser in librte_cmdline. > In MAC_VLAN or TUNNEL mode, why not just use pf. With the current code, because if you write that in the command, it is rejected by the parser :) Do you mean it would be preferable to make these commands always take such an argument, and only at the NIC driver check that it must equal PF for MAC_VLAN or TUNNEL mode? The command becomes a bit more complicated for the current intel NIC's, but as I understand it currently does not work anyway. Unless I'm missing something else. > > Maybe an optional field supporting on DPDK cmdline library is exactly what we > Are waiting for :) Laudable goal! My excuses but it's beyond my current skills and bandwith :/ Regards, Frederico [1] http://dpdk.org/browse/dpdk/tree/app/test-pmd/cmdline.c#n703 [2] http://dpdk.org/browse/dpdk/tree/app/test-pmd/cmdline.c#n8821 > > > Thanks > Jingjing