From: Davide Caratti Date: Fri, 2 Mar 2018 18:36:16 +0000 (+0100) Subject: tc: fix parsing of the control action X-Git-Tag: v4.16.0~34 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=75ef7b18d2a13657056706895bf8d8dd3ac93e46;p=thirdparty%2Fiproute2.git tc: fix parsing of the control action If the user didn't specify any control action, don't pop the command line arguments: otherwise, parsing of the next argument (tipically the 'index' keyword) results in an error, causing the following 'tc-testing' failures: Test a6d6: Add skbedit action with index Test 38f3: Delete skbedit action Test a568: Add action with ife type Test b983: Add action without ife type Test 7d50: Add skbmod action to set destination mac Test 9b29: Add skbmod action to set source mac Test e93a: Delete an skbmod action Also, add missing parse for 'ok' control action to m_police, to fix the following 'tc-testing' failure: Test 8dd5: Add police action with control ok tested with: # ./tdc.py test results: all tests ok using kernel 4.16-rc2, except 9aa8 "Get a single skbmod action from a list" (which is failing also before this commit) Fixes: 3572e01a090a ("tc: util: Don't call NEXT_ARG_FWD() in __parse_action_control()") Cc: Michal Privoznik Cc: Wolfgang Bumiller Signed-off-by: Davide Caratti Signed-off-by: Stephen Hemminger --- diff --git a/tc/m_bpf.c b/tc/m_bpf.c index 576f69ccd..1c1f71cdb 100644 --- a/tc/m_bpf.c +++ b/tc/m_bpf.c @@ -129,7 +129,6 @@ opt_bpf: parse_action_control_dflt(&argc, &argv, &parm.action, false, TC_ACT_PIPE); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { diff --git a/tc/m_connmark.c b/tc/m_connmark.c index 47c7a8c2b..37d718541 100644 --- a/tc/m_connmark.c +++ b/tc/m_connmark.c @@ -82,7 +82,6 @@ parse_connmark(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(); if (argc) { if (matches(*argv, "index") == 0) { diff --git a/tc/m_csum.c b/tc/m_csum.c index e1352c082..7b156734f 100644 --- a/tc/m_csum.c +++ b/tc/m_csum.c @@ -124,7 +124,6 @@ parse_csum(struct action_util *a, int *argc_p, } parse_action_control_dflt(&argc, &argv, &sel.action, false, TC_ACT_OK); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { diff --git a/tc/m_gact.c b/tc/m_gact.c index b30b0420b..16c4413f4 100644 --- a/tc/m_gact.c +++ b/tc/m_gact.c @@ -87,12 +87,10 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p, if (argc < 0) return -1; - if (matches(*argv, "gact") != 0 && - parse_action_control(&argc, &argv, &p.action, false) == -1) { + if (!matches(*argv, "gact")) + NEXT_ARG_FWD(); + if (parse_action_control(&argc, &argv, &p.action, false)) usage(); /* does not return */ - } - - NEXT_ARG_FWD(); #ifdef CONFIG_GACT_PROB if (argc > 0) { @@ -113,7 +111,6 @@ parse_gact(struct action_util *a, int *argc_p, char ***argv_p, if (parse_action_control(&argc, &argv, &pp.paction, false) == -1) usage(); - NEXT_ARG_FWD(); if (get_u16(&pp.pval, *argv, 10)) { fprintf(stderr, "Illegal probability val 0x%x\n", diff --git a/tc/m_ife.c b/tc/m_ife.c index 4647f6a6e..205efc9f1 100644 --- a/tc/m_ife.c +++ b/tc/m_ife.c @@ -159,7 +159,6 @@ static int parse_ife(struct action_util *a, int *argc_p, char ***argv_p, parse_action_control_dflt(&argc, &argv, &p.action, false, TC_ACT_PIPE); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_mirred.c b/tc/m_mirred.c index aa7ce6d92..14e5c88d4 100644 --- a/tc/m_mirred.c +++ b/tc/m_mirred.c @@ -103,6 +103,7 @@ parse_direction(struct action_util *a, int *argc_p, char ***argv_p, while (argc > 0) { if (matches(*argv, "action") == 0) { + NEXT_ARG(); break; } else if (!egress && matches(*argv, "egress") == 0) { egress = 1; @@ -202,10 +203,8 @@ parse_direction(struct action_util *a, int *argc_p, char ***argv_p, } - if (p.eaction == TCA_EGRESS_MIRROR || p.eaction == TCA_INGRESS_MIRROR) { + if (p.eaction == TCA_EGRESS_MIRROR || p.eaction == TCA_INGRESS_MIRROR) parse_action_control(&argc, &argv, &p.action, false); - NEXT_ARG_FWD(); - } if (argc) { if (iok && matches(*argv, "index") == 0) { diff --git a/tc/m_nat.c b/tc/m_nat.c index f5de4d4ca..1e4ff51fe 100644 --- a/tc/m_nat.c +++ b/tc/m_nat.c @@ -116,7 +116,6 @@ parse_nat(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct parse_action_control_dflt(&argc, &argv, &sel.action, false, TC_ACT_OK); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_pedit.c b/tc/m_pedit.c index dc57f14ae..26549eeea 100644 --- a/tc/m_pedit.c +++ b/tc/m_pedit.c @@ -672,7 +672,6 @@ int parse_pedit(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, parse_action_control_dflt(&argc, &argv, &sel.sel.action, false, TC_ACT_OK); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_police.c b/tc/m_police.c index ff1dcb7d0..055b50ee5 100644 --- a/tc/m_police.c +++ b/tc/m_police.c @@ -150,15 +150,18 @@ int act_parse_police(struct action_util *a, int *argc_p, char ***argv_p, matches(*argv, "shot") == 0 || matches(*argv, "continue") == 0 || matches(*argv, "pass") == 0 || + matches(*argv, "ok") == 0 || matches(*argv, "pipe") == 0 || matches(*argv, "goto") == 0) { - if (parse_action_control(&argc, &argv, &p.action, false)) - return -1; + if (!parse_action_control(&argc, &argv, &p.action, false)) + goto action_ctrl_ok; + return -1; } else if (strcmp(*argv, "conform-exceed") == 0) { NEXT_ARG(); - if (parse_action_control_slash(&argc, &argv, &p.action, - &presult, true)) - return -1; + if (!parse_action_control_slash(&argc, &argv, &p.action, + &presult, true)) + goto action_ctrl_ok; + return -1; } else if (matches(*argv, "overhead") == 0) { NEXT_ARG(); if (get_u16(&overhead, *argv, 10)) { @@ -174,8 +177,9 @@ int act_parse_police(struct action_util *a, int *argc_p, char ***argv_p, } else { break; } + NEXT_ARG_FWD(); +action_ctrl_ok: ok++; - argc--; argv++; } if (!ok) diff --git a/tc/m_sample.c b/tc/m_sample.c index 31774c0e8..ff5ee6bd1 100644 --- a/tc/m_sample.c +++ b/tc/m_sample.c @@ -100,7 +100,6 @@ static int parse_sample(struct action_util *a, int *argc_p, char ***argv_p, parse_action_control_dflt(&argc, &argv, &p.action, false, TC_ACT_PIPE); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_skbedit.c b/tc/m_skbedit.c index c41a7bb08..aa374fcb3 100644 --- a/tc/m_skbedit.c +++ b/tc/m_skbedit.c @@ -123,7 +123,6 @@ parse_skbedit(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(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_skbmod.c b/tc/m_skbmod.c index bc268dfd5..561b73fb8 100644 --- a/tc/m_skbmod.c +++ b/tc/m_skbmod.c @@ -124,7 +124,6 @@ static int parse_skbmod(struct action_util *a, int *argc_p, char ***argv_p, parse_action_control_dflt(&argc, &argv, &p.action, false, TC_ACT_PIPE); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_tunnel_key.c b/tc/m_tunnel_key.c index 2dc91879c..1cdd03560 100644 --- a/tc/m_tunnel_key.c +++ b/tc/m_tunnel_key.c @@ -175,7 +175,6 @@ static int parse_tunnel_key(struct action_util *a, int *argc_p, char ***argv_p, parse_action_control_dflt(&argc, &argv, &parm.action, false, TC_ACT_PIPE); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/m_vlan.c b/tc/m_vlan.c index edae0d1e6..161759fd4 100644 --- a/tc/m_vlan.c +++ b/tc/m_vlan.c @@ -131,7 +131,6 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p, parse_action_control_dflt(&argc, &argv, &parm.action, false, TC_ACT_PIPE); - NEXT_ARG_FWD(); if (argc) { if (matches(*argv, "index") == 0) { NEXT_ARG(); diff --git a/tc/tc_util.c b/tc/tc_util.c index aceb0d944..8eadbbcf2 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -588,6 +588,7 @@ static int __parse_action_control(int *argc_p, char ***argv_p, int *result_p, } result |= jump_cnt; } + NEXT_ARG_FWD(); *argc_p = argc; *argv_p = argv; *result_p = result; @@ -684,8 +685,8 @@ out: int parse_action_control_slash(int *argc_p, char ***argv_p, int *result1_p, int *result2_p, bool allow_num) { + int result1, result2, argc = *argc_p; char **argv = *argv_p; - int result1, result2; char *p = strchr(*argv, '/'); if (!p) @@ -704,6 +705,9 @@ int parse_action_control_slash(int *argc_p, char ***argv_p, *result1_p = result1; *result2_p = result2; + NEXT_ARG_FWD(); + *argc_p = argc; + *argv_p = argv; return 0; }