On Tue, Sep 27, 2016 at 4:42 AM, Wu, Jingjing <jingjing.wu at intel.com> wrote: > > >> -----Original Message----- >> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Frederico.Cadete- >> ext at oneaccess-net.com >> Sent: Tuesday, August 23, 2016 5:11 PM >> To: dev at dpdk.org >> Cc: frederico at cadete.eu; Frederico Cadete >> Subject: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel >> modes >> >> From: Frederico Cadete <Frederico.Cadete-ext at oneaccess-net.com> >> >> 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 <Frederico.Cadete-ext at oneaccess-net.com> >> --- >> 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
