* [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).