From cb73324026eb3f9c315735b9020890f43eeaac43 Mon Sep 17 00:00:00 2001

From: Andrea Claudi <aclaudi@redhat.com>

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 <dcaratti@redhat.com>

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 <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>

---

 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.20.1