Re: [dpdk-dev] [PATCH] testpmd: fix fdir command on MAC and tunnel modes

[Thomas Monjalon <thomas.monjalon@6wind.com>]

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