diff mbox series

[iproute2] tc: pedit: add decrement operation

Message ID 20210614203324.236756-1-asbjorn@asbjorn.st
State New
Headers show
Series [iproute2] tc: pedit: add decrement operation | expand

Commit Message

Asbjørn Sloth Tønnesen June 14, 2021, 8:33 p.m. UTC 
Implement a decrement operation for ttl and hoplimit.

Since this is just syntactic sugar, it goes that:

  tc filter add ... action pedit ex munge ip ttl dec ...
  tc filter add ... action pedit ex munge ip6 hoplimit dec ...

is just a more readable version of this:

  tc filter add ... action pedit ex munge ip ttl add 0xff ...
  tc filter add ... action pedit ex munge ip6 hoplimit add 0xff ...

This feature was suggested by some pseudo tc examples in Mellanox's
documentation[1], but wasn't present in neither their mlnx-iproute2
nor iproute2.

In order to avoid adding an extra parameter to parse_cmd(),
I have re-used the `int type` parameter to also carry flags.

Tested with skip_sw on Mellanox ConnectX-6 Dx.

[1] https://docs.mellanox.com/pages/viewpage.action?pageId=47033989

Signed-off-by: Asbjørn Sloth Tønnesen <asbjorn@asbjorn.st>
---
 man/man8/tc-pedit.8 |  8 +++++++-
 tc/m_pedit.c        | 27 ++++++++++++++++++++++-----
 tc/m_pedit.h        |  2 ++
 tc/p_ip.c           |  2 +-
 tc/p_ip6.c          |  2 +-
 5 files changed, 33 insertions(+), 8 deletions(-)

Comments

Stephen Hemminger June 14, 2021, 9:25 p.m. UTC | #1 
On Mon, 14 Jun 2021 20:33:24 +0000
Asbjørn Sloth Tønnesen         <asbjorn@asbjorn.st> wrote:

> @@ -422,16 +434,21 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
>  		goto done;
>  	}
>  
>  	return -1;
> +


Please no unnecessary whitespace changes.

Also you are missing to print_pedit() to print the resulting change.
Asbjørn Sloth Tønnesen June 14, 2021, 10:47 p.m. UTC | #2 
Hi Stephen,

On 6/14/21 9:25 PM, Stephen Hemminger wrote:
> On Mon, 14 Jun 2021 20:33:24 +0000
> Asbjørn Sloth Tønnesen         <asbjorn@asbjorn.st> wrote:
> 
>> @@ -422,16 +434,21 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
>>   		goto done;
>>   	}
>>   
>>   	return -1;
>> +
> 
> 
> Please no unnecessary whitespace changes.

No problem, I tried to give context by using format-patch -U4,
so it didn't became it's own chunk. I included it to be
consistent within the function, having a blank line above
both label.


> Also you are missing to print_pedit() to print the resulting change.

I didn't plan to since it is stored as TCA_PEDIT_KEY_EX_CMD_ADD,
hence it is printed as:

   key #0  at ipv4+8: add ff000000 mask 00ffffff
   or
   key #0  at ipv6+4: add 000000ff mask ffffff00

If really needed I could detect this, and print something else,
but that would break FP and/or JSON compatibility, which I
don't think is worth it.

print_pedit() is generally not a direct representation of it's
input parameters.

   tc filter add [...] action pedit ex \
     munge eth dst set aa:00:42:00:00:33 \
     munge eth src set aa:00:42:00:00:22 \
     munge ip6 hoplimit dec action [..]

becomes:

   key #0  at eth+0: val aa004200 mask 00000000
   key #1  at eth+4: val 00330000 mask 0000ffff
   key #2  at eth+4: val 0000aa00 mask ffff0000
   key #3  at eth+8: val 42000022 mask 00000000
   key #4  at ipv6+4: add 000000ff mask ffffff00

Note: key #1 and #2 could be consolidated.


Shortly after posting, I noticed that I forgot to amend explain(),
that will be in v2.
diff mbox series

Patch

diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index 376ad4a8..2e2662cd 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -76,8 +76,9 @@  pedit - generic packet editor action
 .BR clear " | " invert " | " set
 .IR VAL " | "
 .BR add
 .IR VAL " | "
+.BR decrement " | "
 .BR preserve " } [ " retain
 .IR RVAL " ]"
 
 .ti -8
@@ -95,9 +96,9 @@  chosen automatically based on the header field size.
 .TP
 .B ex
 Use extended pedit.
 .I EXTENDED_LAYERED_OP
-and the add
+and the add/decrement
 .I CMD_SPEC
 are allowed only in this mode.
 .TP
 .BI offset " OFFSET " "\fR{ \fBu32 \fR| \fBu16 \fR| \fBu8 \fR}"
@@ -287,8 +288,13 @@  Add the addressed data by a specific value. The size of
 is defined by the size of the addressed header field in
 .IR EXTENDED_LAYERED_OP .
 This operation is supported only for extended layered op.
 .TP
+.BI decrement
+Decrement the addressed data by one.
+This operation is supported only for
+.BR ip " " ttl " and " ip6 " " hoplimit "."
+.TP
 .B preserve
 Keep the addressed data as is.
 .TP
 .BI retain " RVAL"
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 74c91e8d..9ac52336 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -339,8 +339,11 @@  int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 	__u32 o = 0xFF;
 	int res = -1;
 	int argc = *argc_p;
 	char **argv = *argv_p;
+	int flags = type;
+
+	type &= 0xff; /* strip flags */
 
 	if (argc <= 0)
 		return -1;
 
@@ -359,17 +362,26 @@  int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		   matches(*argv, "add") == 0) {
 		if (matches(*argv, "add") == 0)
 			tkey->cmd = TCA_PEDIT_KEY_EX_CMD_ADD;
 
-		if (!sel->extended && tkey->cmd) {
-			fprintf(stderr,
-				"Non extended mode. only 'set' command is supported\n");
-			return -1;
-		}
+		if (!sel->extended && tkey->cmd)
+			goto non_ext_only_set_cmd;
 
 		NEXT_ARG();
 		if (parse_val(&argc, &argv, val, type))
 			return -1;
+	} else if (matches(*argv, "decrement") == 0) {
+		if ((flags & TFLAG_ALLOW_DEC) == 0) {
+			fprintf(stderr,
+				"decrement command is not supported for this field\n");
+			return -1;
+		}
+
+		if (!sel->extended)
+			goto non_ext_only_set_cmd;
+
+		tkey->cmd = TCA_PEDIT_KEY_EX_CMD_ADD;
+		*v = retain; /* decrement by overflow */
 	} else if (matches(*argv, "preserve") == 0) {
 		retain = 0;
 	} else {
 		if (matches(*argv, "clear") != 0)
@@ -422,16 +434,21 @@  int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		goto done;
 	}
 
 	return -1;
+
 done:
 	if (pedit_debug)
 		printf("parse_cmd done argc %d %s offset %d length %d\n",
 		       argc, *argv, tkey->off, len);
 	*argc_p = argc;
 	*argv_p = argv;
 	return res;
 
+non_ext_only_set_cmd:
+	fprintf(stderr,
+		"Non extended mode. only 'set' command is supported\n");
+	return -1;
 }
 
 static int parse_offset(int *argc_p, char ***argv_p, struct m_pedit_sel *sel,
 			struct m_pedit_key *tkey)
diff --git a/tc/m_pedit.h b/tc/m_pedit.h
index 5d3628a7..ed6bb8da 100644
--- a/tc/m_pedit.h
+++ b/tc/m_pedit.h
@@ -32,8 +32,10 @@ 
 #define TINT 3
 #define TU32 4
 #define TMAC 5
 
+#define TFLAG_ALLOW_DEC (1<<8)
+
 #define RU32 0xFFFFFFFF
 #define RU16 0xFFFF
 #define RU8 0xFF
 
diff --git a/tc/p_ip.c b/tc/p_ip.c
index c385ac6d..5c5a94bf 100644
--- a/tc/p_ip.c
+++ b/tc/p_ip.c
@@ -67,9 +67,9 @@  parse_ip(int *argc_p, char ***argv_p,
 	}
 	if (strcmp(*argv, "ttl") == 0) {
 		NEXT_ARG();
 		tkey->off = 8;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32 | TFLAG_ALLOW_DEC, RU8, sel, tkey);
 		goto done;
 	}
 	if (strcmp(*argv, "protocol") == 0) {
 		NEXT_ARG();
diff --git a/tc/p_ip6.c b/tc/p_ip6.c
index 83a6ae81..c82b1244 100644
--- a/tc/p_ip6.c
+++ b/tc/p_ip6.c
@@ -70,9 +70,9 @@  parse_ip6(int *argc_p, char ***argv_p,
 	}
 	if (strcmp(*argv, "hoplimit") == 0) {
 		NEXT_ARG();
 		tkey->off = 7;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32 | TFLAG_ALLOW_DEC, RU8, sel, tkey);
 		goto done;
 	}
 	if (strcmp(*argv, "traffic_class") == 0) {
 		NEXT_ARG();