From cb73324026eb3f9c315735b9020890f43eeaac43 Mon Sep 17 00:00:00 2001 From: Andrea Claudi Date: Wed, 5 Jun 2019 13:12:06 +0200 Subject: [PATCH] tc: fix parsing of the control action Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1714660 Upstream Status: iproute2.git commit 75ef7b18d2a13 Conflicts: context change due to missing commit 35f2a7639dca4 ("tc/actions: introduce support for jump action") commit 75ef7b18d2a13657056706895bf8d8dd3ac93e46 Author: Davide Caratti Date: Fri Mar 2 19:36:16 2018 +0100 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 --- tc/m_bpf.c | 1 - tc/m_connmark.c | 1 - tc/m_csum.c | 1 - tc/m_gact.c | 9 +++------ tc/m_ife.c | 1 - tc/m_mirred.c | 5 ++--- tc/m_nat.c | 1 - tc/m_pedit.c | 1 - tc/m_police.c | 16 ++++++++++------ tc/m_sample.c | 1 - tc/m_skbedit.c | 1 - tc/m_skbmod.c | 1 - tc/m_tunnel_key.c | 1 - tc/m_vlan.c | 1 - tc/tc_util.c | 6 +++++- 15 files changed, 20 insertions(+), 27 deletions(-) diff --git a/tc/m_bpf.c b/tc/m_bpf.c index c2bad5640707c..57283030a35f5 100644 --- a/tc/m_bpf.c +++ b/tc/m_bpf.c @@ -125,7 +125,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 47c7a8c2b17e7..37d7185415490 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 e1352c0820f69..7b156734f64c5 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 dd9542a5cc644..45eecf7ea1647 100644 --- a/tc/m_gact.c +++ b/tc/m_gact.c @@ -86,12 +86,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) { @@ -112,7 +110,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", pp.pval); return -1; diff --git a/tc/m_ife.c b/tc/m_ife.c index 54fad8f70e73a..e3521e62c178c 100644 --- a/tc/m_ife.c +++ b/tc/m_ife.c @@ -158,7 +158,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 b09b016c2ca39..b1f45f1e6ecb0 100644 --- a/tc/m_mirred.c +++ b/tc/m_mirred.c @@ -76,6 +76,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; @@ -170,10 +171,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 bb455f080b3a4..31b68fb6bd784 100644 --- a/tc/m_nat.c +++ b/tc/m_nat.c @@ -117,7 +117,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 3391be95da38c..5d89ab1d832ab 100644 --- a/tc/m_pedit.c +++ b/tc/m_pedit.c @@ -673,7 +673,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 86117db0482ec..b79545961f4d7 100644 --- a/tc/m_police.c +++ b/tc/m_police.c @@ -151,15 +151,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)) { @@ -175,8 +178,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 31774c0e806b4..ff5ee6bd1ef63 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 c41a7bb082dad..aa374fcb33ed9 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 00318d42642a5..ba79308ba8354 100644 --- a/tc/m_skbmod.c +++ b/tc/m_skbmod.c @@ -125,7 +125,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 0ff3f1a2b9876..cdde64a15b929 100644 --- a/tc/m_tunnel_key.c +++ b/tc/m_tunnel_key.c @@ -197,7 +197,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 0b2966ce82e53..cccb4996b05f3 100644 --- a/tc/m_vlan.c +++ b/tc/m_vlan.c @@ -137,7 +137,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 4584d4a448fb4..65695ea592ed8 100644 --- a/tc/tc_util.c +++ b/tc/tc_util.c @@ -507,6 +507,7 @@ static int __parse_action_control(int *argc_p, char ***argv_p, int *result_p, } result |= chain_index; } + NEXT_ARG_FWD(); *argc_p = argc; *argv_p = argv; *result_p = result; @@ -603,8 +604,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) @@ -624,6 +625,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; } -- 2.21.0