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 <mprivozn@redhat.com>
Cc: Wolfgang Bumiller <w.bumiller@proxmox.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

diff --git a/tc/m_bpf.c b/tc/m_bpf.c
index 576f69ccdab098668ea92e48c6bb4c57959bad70..1c1f71cdb83f9ac6522658315b6cf1671fb1f97b 100644 (file)
--- 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 47c7a8c2b17e7495fd25471e067741bdd6038c19..37d7185415490cb4ecdf8e88eba866066d5e3f6f 100644 (file)
--- 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 e1352c0820f69fdebebf71fd0c1630a8030bd69f..7b156734f64c5581fe60fb7fdf012114b15971fd 100644 (file)
--- 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 b30b0420bb1e14ff202c2ca0539afbfd28a1103e..16c4413f4217d3a6d6af1a7f07edfcad121fbb9d 100644 (file)
--- 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 4647f6a6e779c4befef631e1e166ee2b37ec80fb..205efc9f1d9a73a4a3bedff91d49ff6799cd9992 100644 (file)
--- 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 aa7ce6d923cce233801933827bc911e7c17eb879..14e5c88d44d539fecae0ab0ea5e23f92c8b3e1bc 100644 (file)
--- 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 f5de4d4cac9f376b68bccefce3bc524a24b3f443..1e4ff51fe75aea6bd815928ac8311a619bfb76d1 100644 (file)
--- 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 dc57f14ae1ce5fae75052d04115eb6e7cef7e07a..26549eeea8995a001d51a02826dcbf63a0068f9e 100644 (file)
--- 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 ff1dcb7d0323c7fa7b5952fab411e8b08be92a0f..055b50ee5c2d122a1a8914d851269cb06d6fa299 100644 (file)
--- 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 31774c0e806b4a3f9fa56163d67c07124bed39a5..ff5ee6bd1ef63b6f3390811bc5eb7a5f4ce20c0e 100644 (file)
--- 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 c41a7bb082dad1b1982dcba689c03f27023c7ac3..aa374fcb33ed9b46da3c46cf48eff7a59b56cb5d 100644 (file)
--- 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 bc268dfd5827a2a7809552f88e68921090a64b91..561b73fb8264783b6188b3e0eecbe3c9c4aa519e 100644 (file)
--- 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 2dc91879c237566a376bd28b8b3221fabc3de5ad..1cdd03560c3573bf1142713e4310cdea4477edc7 100644 (file)
--- 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 edae0d1e6f70755c480ab9c9f830ba3516175237..161759fd40630734dd775f8911654ef949175137 100644 (file)
--- 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 aceb0d9449337a64b71596072b5df370f49ea97b..8eadbbcf29b6b51e9da97b02e6940891d989182a 100644 (file)
--- 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;
 }