* [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes @ 2016-08-23 9:10 Frederico.Cadete-ext 2016-09-23 18:37 ` Thomas Monjalon 2016-09-27 2:42 ` Wu, Jingjing 0 siblings, 2 replies; 5+ messages in thread From: Frederico.Cadete-ext @ 2016-08-23 9:10 UTC (permalink / raw) To: dev; +Cc: frederico, Frederico Cadete From: Frederico Cadete <Frederico.Cadete-ext@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@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; } /* set to report FD ID by default */ -- 2.7.4 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes 2016-08-23 9:10 [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes Frederico.Cadete-ext @ 2016-09-23 18:37 ` Thomas Monjalon 2016-09-27 2:42 ` Wu, Jingjing 1 sibling, 0 replies; 5+ messages in thread From: Thomas Monjalon @ 2016-09-23 18:37 UTC (permalink / raw) To: dev; +Cc: Frederico.Cadete-ext, frederico, pablo.de.lara.guarch Anyone to review please? 2016-08-23 11:10, Frederico.Cadete-ext@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@oneaccess-net.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes 2016-08-23 9:10 [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes Frederico.Cadete-ext 2016-09-23 18:37 ` Thomas Monjalon @ 2016-09-27 2:42 ` Wu, Jingjing 2016-09-27 9:01 ` Frederico Cadete 1 sibling, 1 reply; 5+ messages in thread From: Wu, Jingjing @ 2016-09-27 2:42 UTC (permalink / raw) To: Frederico.Cadete-ext, dev; +Cc: frederico, De Lara Guarch, Pablo > -----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 <Frederico.Cadete-ext@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@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. 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. In MAC_VLAN or TUNNEL mode, why not just use pf. Maybe an optional field supporting on DPDK cmdline library is exactly what we Are waiting for :) Thanks Jingjing ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes 2016-09-27 2:42 ` Wu, Jingjing @ 2016-09-27 9:01 ` Frederico Cadete 2016-10-25 21:14 ` Thomas Monjalon 0 siblings, 1 reply; 5+ messages in thread From: Frederico Cadete @ 2016-09-27 9:01 UTC (permalink / raw) To: Wu, Jingjing; +Cc: Frederico.Cadete-ext, dev, De Lara Guarch, Pablo On Tue, Sep 27, 2016 at 4:42 AM, Wu, Jingjing <jingjing.wu@intel.com> 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 <Frederico.Cadete-ext@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@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes 2016-09-27 9:01 ` Frederico Cadete @ 2016-10-25 21:14 ` Thomas Monjalon 0 siblings, 0 replies; 5+ messages in thread From: Thomas Monjalon @ 2016-10-25 21:14 UTC (permalink / raw) To: Frederico Cadete, Frederico.Cadete-ext; +Cc: dev 2016-09-27 11:01, Frederico Cadete: > On Tue, Sep 27, 2016 at 4:42 AM, Wu, Jingjing <jingjing.wu@intel.com> wrote: > > 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 <Frederico.Cadete-ext@oneaccess-net.com> [...] > > > > 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 :/ Thanks Frederico. Your approach has been re-submitted and fixed by Wenzhuo: http://dpdk.org/patch/16679 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-25 21:14 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-23 9:10 [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes Frederico.Cadete-ext 2016-09-23 18:37 ` Thomas Monjalon 2016-09-27 2:42 ` Wu, Jingjing 2016-09-27 9:01 ` Frederico Cadete 2016-10-25 21:14 ` Thomas Monjalon
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).