[PATCH iproute2 net] tc: remove duplicated NEXT_ARG_FWD() in parse_ct()

[PATCH iproute2 net] tc: remove duplicated NEXT_ARG_FWD() in parse_ct()

[Vlad Buslov]

Function parse_ct() manually calls NEXT_ARG_FWD() after
parse_action_control_dflt(). This is redundant because
parse_action_control_dflt() modifies argc and argv itself. Moreover, such
implementation parses out any following actions option. For example, adding
action ct with cookie errors:

$ sudo tc actions add action ct cookie 111111111111
Bad action type 111111111111
Usage: ... gact <ACTION> [RAND] [INDEX]
Where:  ACTION := reclassify | drop | continue | pass | pipe |
                  goto chain <CHAIN_INDEX> | jump <JUMP_COUNT>
        RAND := random <RANDTYPE> <ACTION> <VAL>
        RANDTYPE := netrand | determ
        VAL : = value not exceeding 10000
        JUMP_COUNT := Absolute jump from start of action list
        INDEX := index value used

With fix:

$ sudo tc actions add action ct cookie 111111111111
$ sudo tc actions list action ct
total acts 1

        action order 0: ct zone 0 pipe
         index 1 ref 1 bind 0
        cookie 111111111111

Fixes: c8a494314c40 ("tc: Introduce tc ct action")
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
 tc/m_ct.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tc/m_ct.c b/tc/m_ct.c
index 8589cb9a3c51..d79eb5e361ac 100644
--- a/tc/m_ct.c
+++ b/tc/m_ct.c
@@ -316,7 +316,6 @@ parse_ct(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
 
 	parse_action_control_dflt(&argc, &argv, &sel.action, false,
 				  TC_ACT_PIPE);
-	NEXT_ARG_FWD();
 
 	addattr16(n, MAX_MSG, TCA_CT_ACTION, ct_action);
 	addattr_l(n, MAX_MSG, TCA_CT_PARMS, &sel, sizeof(sel));
-- 
2.21.0

Re: [PATCH iproute2 net] tc: remove duplicated NEXT_ARG_FWD() in parse_ct()

[Marcelo Ricardo Leitner]

On Tue, Oct 29, 2019 at 07:53:46PM +0200, Vlad Buslov wrote:
> Function parse_ct() manually calls NEXT_ARG_FWD() after
> parse_action_control_dflt(). This is redundant because
> parse_action_control_dflt() modifies argc and argv itself. Moreover, such
> implementation parses out any following actions option. For example, adding
> action ct with cookie errors:
> 
> $ sudo tc actions add action ct cookie 111111111111
> Bad action type 111111111111
> Usage: ... gact <ACTION> [RAND] [INDEX]
> Where:  ACTION := reclassify | drop | continue | pass | pipe |
>                   goto chain <CHAIN_INDEX> | jump <JUMP_COUNT>
>         RAND := random <RANDTYPE> <ACTION> <VAL>
>         RANDTYPE := netrand | determ
>         VAL : = value not exceeding 10000
>         JUMP_COUNT := Absolute jump from start of action list
>         INDEX := index value used
> 
> With fix:
> 
> $ sudo tc actions add action ct cookie 111111111111
> $ sudo tc actions list action ct
> total acts 1
> 
>         action order 0: ct zone 0 pipe
>          index 1 ref 1 bind 0
>         cookie 111111111111
> 
> Fixes: c8a494314c40 ("tc: Introduce tc ct action")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

Thanks Vlad.

> ---
>  tc/m_ct.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tc/m_ct.c b/tc/m_ct.c
> index 8589cb9a3c51..d79eb5e361ac 100644
> --- a/tc/m_ct.c
> +++ b/tc/m_ct.c
> @@ -316,7 +316,6 @@ parse_ct(struct action_util *a, int *argc_p, char ***argv_p, int tca_id,
>  
>  	parse_action_control_dflt(&argc, &argv, &sel.action, false,
>  				  TC_ACT_PIPE);
> -	NEXT_ARG_FWD();
>  
>  	addattr16(n, MAX_MSG, TCA_CT_ACTION, ct_action);
>  	addattr_l(n, MAX_MSG, TCA_CT_PARMS, &sel, sizeof(sel));
> -- 
> 2.21.0
> 

Re: [PATCH iproute2 net] tc: remove duplicated NEXT_ARG_FWD() in parse_ct()

[Stephen Hemminger]

On Tue, 29 Oct 2019 19:53:46 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:

> Function parse_ct() manually calls NEXT_ARG_FWD() after
> parse_action_control_dflt(). This is redundant because
> parse_action_control_dflt() modifies argc and argv itself. Moreover, such
> implementation parses out any following actions option. For example, adding
> action ct with cookie errors:
> 
> $ sudo tc actions add action ct cookie 111111111111
> Bad action type 111111111111
> Usage: ... gact <ACTION> [RAND] [INDEX]
> Where:  ACTION := reclassify | drop | continue | pass | pipe |
>                   goto chain <CHAIN_INDEX> | jump <JUMP_COUNT>
>         RAND := random <RANDTYPE> <ACTION> <VAL>
>         RANDTYPE := netrand | determ
>         VAL : = value not exceeding 10000
>         JUMP_COUNT := Absolute jump from start of action list
>         INDEX := index value used
> 
> With fix:
> 
> $ sudo tc actions add action ct cookie 111111111111
> $ sudo tc actions list action ct
> total acts 1
> 
>         action order 0: ct zone 0 pipe
>          index 1 ref 1 bind 0
>         cookie 111111111111
> 
> Fixes: c8a494314c40 ("tc: Introduce tc ct action")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Applied