Re: [PATCH iproute2] police: don't skip parameters after actions

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 29 Jan 2018 12:13:11 +0100
Wolfgang Bumiller <w.bumiller@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.bumiller@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.