From: Frederico Cadete <frederico@cadete.eu>
To: "Wu, Jingjing" <jingjing.wu@intel.com>
Cc: "Frederico.Cadete-ext@oneaccess-net.com"
<Frederico.Cadete-ext@oneaccess-net.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>
Subject: Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes
Date: Tue, 27 Sep 2016 11:01:44 +0200 [thread overview]
Message-ID: <CAL5ScNVyW=UU9A_X6BJ+BA53bkae=4W-OLqO31FHQgEynNHmsw@mail.gmail.com> (raw)
In-Reply-To: <9BB6961774997848B5B42BEC655768F80E273098@SHSMSX103.ccr.corp.intel.com>
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
next prev parent reply other threads:[~2016-09-27 9:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-23 9:10 Frederico.Cadete-ext
2016-09-23 18:37 ` Thomas Monjalon
2016-09-27 2:42 ` Wu, Jingjing
2016-09-27 9:01 ` Frederico Cadete [this message]
2016-10-25 21:14 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAL5ScNVyW=UU9A_X6BJ+BA53bkae=4W-OLqO31FHQgEynNHmsw@mail.gmail.com' \
--to=frederico@cadete.eu \
--cc=Frederico.Cadete-ext@oneaccess-net.com \
--cc=dev@dpdk.org \
--cc=jingjing.wu@intel.com \
--cc=pablo.de.lara.guarch@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).