On Mon, Jan 29, 2018 at 08:07:23AM -0800, Stephen Hemminger wrote:
> On Mon, 29 Jan 2018 12:13:11 +0100
> Wolfgang Bumiller <w.bumil...@proxmox.com> wrote:
>
> > The 'parse_action_control()' helper advances the argument
> > pointers to past its parsed action already, so don't
> > advance it further in 'act_parse_polic()'.
> >
> > Fixes: e67aba559581 ("tc: actions: add helpers to parse and print control
> > actions")
> > Signed-off-by: Wolfgang Bumiller <w.bumil...@proxmox.com>
> > ---
> > Basically parse_action_control() silently added a NEXT_ARG() while the
> > cases before didn't have one. Not sure whether the goto is okay
> > style-wise, let me know if you prefer some other solution.
> >
> > Example for triggering this:
> > Specifying a 'flowid X' after a `police ... drop` will skip the 'flowid'
> > and error with "What is X"
> >
> > $ tc filter add dev eth0 parent ffff: basic police rate 13371337bps burst
> > 1337b mtu 64kb drop flowid :1
> > What is ":1"?
>
> Thank you for the patch. It is a real problem, and your patch addresses it.
> I just don't like jumping around in the the argument parsing with goto's.
> There was a similar problem recently, and the better fix was to fix the
> semantics
> of the parsing function to not do the extra implicit NEXT_ARG in the parsing
> logic.
> There is less likely to be future problems if all parsing functions leave the
> with the same argument location.
>
> Please try that and resubmit.
Actually I must apologize. Apparently I did not properly look at the
master branch but just the v4.13.0 tag. In master your requested
behavior already seems to be implemented. (There's even a commit with
the same `Fixes` tag by Michal Privoznik (3572e01a090).)
Sorry for the noise.
