[PATCH iproute2 v2 0/3] tc: parse index argument correctly

[PATCH iproute2 v2 0/3] tc: parse index argument correctly

[Pedro Tammela]

Following the kernel side series, we fix the iproute2 side to parse the
index argument correctly.
It's valid in the TC architecture to pass to create a filter that
references an action object:
"tc filter ... action csum index 1"

v1->v2:
- Don't use matches()

Pedro Tammela (3):
  tc: m_csum: parse index argument correctly
  tc: m_mpls: parse index argument correctly
  tc: m_nat: parse index argument correctly

 tc/m_csum.c | 5 ++++-
 tc/m_mpls.c | 4 ++++
 tc/m_nat.c  | 5 ++++-
 3 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH iproute2 v2 1/3] tc: m_csum: parse index argument correctly

[Pedro Tammela]

'action csum index 1' is a valid cli according to TC's
architecture. Fix the grammar parsing to accept it.

tdc tests:
1..24
ok 1 6d84 - Add csum iph action
ok 2 1862 - Add csum ip4h action
ok 3 15c6 - Add csum ipv4h action
ok 4 bf47 - Add csum icmp action
ok 5 cc1d - Add csum igmp action
ok 6 bccc - Add csum foobar action
ok 7 3bb4 - Add csum tcp action
ok 8 759c - Add csum udp action
ok 9 bdb6 - Add csum udp xor iph action
ok 10 c220 - Add csum udplite action
ok 11 8993 - Add csum sctp action
ok 12 b138 - Add csum ip & icmp action
ok 13 eeda - Add csum ip & sctp action
ok 14 0017 - Add csum udp or tcp action
ok 15 b10b - Add all 7 csum actions
ok 16 ce92 - Add csum udp action with cookie
ok 17 912f - Add csum icmp action with large cookie
ok 18 879b - Add batch of 32 csum tcp actions
ok 19 b4e9 - Delete batch of 32 csum actions
ok 20 0015 - Add batch of 32 csum tcp actions with large cookies
ok 21 989e - Delete batch of 32 csum actions with large cookies
ok 22 d128 - Replace csum action with invalid goto chain control
ok 23 eaf0 - Add csum iph action with no_percpu flag
ok 24 c619 - Reference csum action object in filter

Fixes: 3822cc98 ("tc: add ACT_CSUM action support (csum)")
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 tc/m_csum.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tc/m_csum.c b/tc/m_csum.c
index ba1e3e33..f5fe8f55 100644
--- a/tc/m_csum.c
+++ b/tc/m_csum.c
@@ -94,7 +94,9 @@ parse_csum(struct action_util *a, int *argc_p,
 	while (argc > 0) {
 		if (matches(*argv, "csum") == 0) {
 			NEXT_ARG();
-			if (parse_csum_args(&argc, &argv, &sel)) {
+			if (strcmp(*argv, "index") == 0) {
+				goto skip_args;
+			} else if (parse_csum_args(&argc, &argv, &sel)) {
 				fprintf(stderr, "Illegal csum construct (%s)\n",
 					*argv);
 				explain();
@@ -123,6 +125,7 @@ parse_csum(struct action_util *a, int *argc_p,
 
 	if (argc) {
 		if (matches(*argv, "index") == 0) {
+skip_args:
 			NEXT_ARG();
 			if (get_u32(&sel.index, *argv, 10)) {
 				fprintf(stderr, "Illegal \"index\" (%s) <csum>\n",
-- 
2.34.1

[PATCH iproute2 v2 2/3] tc: m_mpls: parse index argument correctly

[Pedro Tammela]

'action mpls index 1' is a valid cli according to TC's
architecture. Fix the grammar parsing to accept it.

tdc tests:
1..54
ok 1 a933 - Add MPLS dec_ttl action with pipe opcode
ok 2 08d1 - Add mpls dec_ttl action with pass opcode
ok 3 d786 - Add mpls dec_ttl action with drop opcode
ok 4 f334 - Add mpls dec_ttl action with reclassify opcode
ok 5 29bd - Add mpls dec_ttl action with continue opcode
ok 6 48df - Add mpls dec_ttl action with jump opcode
ok 7 62eb - Add mpls dec_ttl action with trap opcode
ok 8 09d2 - Add mpls dec_ttl action with opcode and cookie
ok 9 c170 - Add mpls dec_ttl action with opcode and cookie of max length
ok 10 9118 - Add mpls dec_ttl action with invalid opcode
ok 11 6ce1 - Add mpls dec_ttl action with label (invalid)
ok 12 352f - Add mpls dec_ttl action with tc (invalid)
ok 13 fa1c - Add mpls dec_ttl action with ttl (invalid)
ok 14 6b79 - Add mpls dec_ttl action with bos (invalid)
ok 15 d4c4 - Add mpls pop action with ip proto
ok 16 91fb - Add mpls pop action with ip proto and cookie
ok 17 92fe - Add mpls pop action with mpls proto
ok 18 7e23 - Add mpls pop action with no protocol (invalid)
ok 19 6182 - Add mpls pop action with label (invalid)
ok 20 6475 - Add mpls pop action with tc (invalid)
ok 21 067b - Add mpls pop action with ttl (invalid)
ok 22 7316 - Add mpls pop action with bos (invalid)
ok 23 38cc - Add mpls push action with label
ok 24 c281 - Add mpls push action with mpls_mc protocol
ok 25 5db4 - Add mpls push action with label, tc and ttl
ok 26 7c34 - Add mpls push action with label, tc ttl and cookie of max length
ok 27 16eb - Add mpls push action with label and bos
ok 28 d69d - Add mpls push action with no label (invalid)
ok 29 e8e4 - Add mpls push action with ipv4 protocol (invalid)
ok 30 ecd0 - Add mpls push action with out of range label (invalid)
ok 31 d303 - Add mpls push action with out of range tc (invalid)
ok 32 fd6e - Add mpls push action with ttl of 0 (invalid)
ok 33 19e9 - Add mpls mod action with mpls label
ok 34 1fde - Add mpls mod action with max mpls label
ok 35 0c50 - Add mpls mod action with mpls label exceeding max (invalid)
ok 36 10b6 - Add mpls mod action with mpls label of MPLS_LABEL_IMPLNULL (invalid)
ok 37 57c9 - Add mpls mod action with mpls min tc
ok 38 6872 - Add mpls mod action with mpls max tc
ok 39 a70a - Add mpls mod action with mpls tc exceeding max (invalid)
ok 40 6ed5 - Add mpls mod action with mpls ttl
ok 41 77c1 - Add mpls mod action with mpls ttl and cookie
ok 42 b80f - Add mpls mod action with mpls max ttl
ok 43 8864 - Add mpls mod action with mpls min ttl
ok 44 6c06 - Add mpls mod action with mpls ttl of 0 (invalid)
ok 45 b5d8 - Add mpls mod action with mpls ttl exceeding max (invalid)
ok 46 451f - Add mpls mod action with mpls max bos
ok 47 a1ed - Add mpls mod action with mpls min bos
ok 48 3dcf - Add mpls mod action with mpls bos exceeding max (invalid)
ok 49 db7c - Add mpls mod action with protocol (invalid)
ok 50 b070 - Replace existing mpls push action with new ID
ok 51 95a9 - Replace existing mpls push action with new label, tc, ttl and cookie
ok 52 6cce - Delete mpls pop action
ok 53 d138 - Flush mpls actions
ok 54 7a70 - Reference mpls action object in filter

Fixes: fb57b092 ("tc: add mpls actions")
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 tc/m_mpls.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tc/m_mpls.c b/tc/m_mpls.c
index 9b39d853..dda46805 100644
--- a/tc/m_mpls.c
+++ b/tc/m_mpls.c
@@ -91,6 +91,9 @@ static int parse_mpls(struct action_util *a, int *argc_p, char ***argv_p,
 
 	NEXT_ARG();
 
+	if (strcmp(*argv, "index") == 0)
+		goto skip_args;
+
 	while (argc > 0) {
 		if (matches(*argv, "pop") == 0) {
 			if (check_double_action(action, *argv))
@@ -164,6 +167,7 @@ static int parse_mpls(struct action_util *a, int *argc_p, char ***argv_p,
 
 	if (argc) {
 		if (matches(*argv, "index") == 0) {
+skip_args:
 			NEXT_ARG();
 			if (get_u32(&parm.index, *argv, 10))
 				invarg("illegal index", *argv);
-- 
2.34.1

[PATCH iproute2 v2 3/3] tc: m_nat: parse index argument correctly

[Pedro Tammela]

'action nat index 1' is a valid cli according to TC's
architecture. Fix the grammar parsing to accept it.

tdc tests:
1..28
ok 1 7565 - Add nat action on ingress with default control action
ok 2 fd79 - Add nat action on ingress with pipe control action
ok 3 eab9 - Add nat action on ingress with continue control action
ok 4 c53a - Add nat action on ingress with reclassify control action
ok 5 76c9 - Add nat action on ingress with jump control action
ok 6 24c6 - Add nat action on ingress with drop control action
ok 7 2120 - Add nat action on ingress with maximum index value
ok 8 3e9d - Add nat action on ingress with invalid index value
ok 9 f6c9 - Add nat action on ingress with invalid IP address
ok 10 be25 - Add nat action on ingress with invalid argument
ok 11 a7bd - Add nat action on ingress with DEFAULT IP address
ok 12 ee1e - Add nat action on ingress with ANY IP address
ok 13 1de8 - Add nat action on ingress with ALL IP address
ok 14 8dba - Add nat action on egress with default control action
ok 15 19a7 - Add nat action on egress with pipe control action
ok 16 f1d9 - Add nat action on egress with continue control action
ok 17 6d4a - Add nat action on egress with reclassify control action
ok 18 b313 - Add nat action on egress with jump control action
ok 19 d9fc - Add nat action on egress with drop control action
ok 20 a895 - Add nat action on egress with DEFAULT IP address
ok 21 2572 - Add nat action on egress with ANY IP address
ok 22 37f3 - Add nat action on egress with ALL IP address
ok 23 6054 - Add nat action on egress with cookie
ok 24 79d6 - Add nat action on ingress with cookie
ok 25 4b12 - Replace nat action with invalid goto chain control
ok 26 b811 - Delete nat action with valid index
ok 27 a521 - Delete nat action with invalid index
ok 28 2c81 - Reference nat action object in filter

Fixes: fc2d0206 ("Add NAT action")
Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 tc/m_nat.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tc/m_nat.c b/tc/m_nat.c
index 58315125..95b35584 100644
--- a/tc/m_nat.c
+++ b/tc/m_nat.c
@@ -88,7 +88,9 @@ parse_nat(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct
 	while (argc > 0) {
 		if (matches(*argv, "nat") == 0) {
 			NEXT_ARG();
-			if (parse_nat_args(&argc, &argv, &sel)) {
+			if (strcmp(*argv, "index") == 0) {
+				goto skip_args;
+			} else if (parse_nat_args(&argc, &argv, &sel)) {
 				fprintf(stderr, "Illegal nat construct (%s)\n",
 					*argv);
 				explain();
@@ -113,6 +115,7 @@ parse_nat(struct action_util *a, int *argc_p, char ***argv_p, int tca_id, struct
 
 	if (argc) {
 		if (matches(*argv, "index") == 0) {
+skip_args:
 			NEXT_ARG();
 			if (get_u32(&sel.index, *argv, 10)) {
 				fprintf(stderr, "Nat: Illegal \"index\"\n");
-- 
2.34.1

Re: [PATCH iproute2 v2 3/3] tc: m_nat: parse index argument correctly

[Stephen Hemminger]

On Mon, 27 Feb 2023 15:45:10 -0300
Pedro Tammela <pctammela@mojatatu.com> wrote:

> Fixes: fc2d0206 ("Add NAT action")
In future use more characters for fixes line. Current checkpatch
complains:

WARNING: Please use correct Fixes: style 'Fixes: <12 chars of sha1> ("<title line>")' - ie: 'Fixes: fc2d02069b52 ("Add NAT action")'
#426: 
Fixes: fc2d0206 ("Add NAT action")

Re: [PATCH iproute2 v2 0/3] tc: parse index argument correctly

[patchwork-bot+netdevbpf]

Hello:

This series was applied to iproute2/iproute2.git (main)
by Stephen Hemminger <stephen@networkplumber.org>:

On Mon, 27 Feb 2023 15:45:07 -0300 you wrote:
> Following the kernel side series, we fix the iproute2 side to parse the
> index argument correctly.
> It's valid in the TC architecture to pass to create a filter that
> references an action object:
> "tc filter ... action csum index 1"
> 
> v1->v2:
> - Don't use matches()
> 
> [...]

Here is the summary with links:
  - [iproute2,v2,1/3] tc: m_csum: parse index argument correctly
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=89d7346aa908
  - [iproute2,v2,2/3] tc: m_mpls: parse index argument correctly
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=af6fd6b84554
  - [iproute2,v2,3/3] tc: m_nat: parse index argument correctly
    https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=7375ab684228

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html