All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2 1/2] tc: pedit: parse_cmd: add flags argument
@ 2021-06-18 16:06 Asbjørn Sloth Tønnesen
  2021-06-18 16:06 ` [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation Asbjørn Sloth Tønnesen
  0 siblings, 1 reply; 6+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2021-06-18 16:06 UTC (permalink / raw)
  To: netdev; +Cc: Asbjørn Sloth Tønnesen, David Ahern, Stephen Hemminger

This patch just prepares the flags argument, so it's
available to the next patch.

Signed-off-by: Asbjørn Sloth Tønnesen <asbjorn@asbjorn.st>
---
 tc/m_pedit.c |  4 ++--
 tc/m_pedit.h |  2 +-
 tc/p_eth.c   |  6 +++---
 tc/p_ip.c    | 32 ++++++++++++++++----------------
 tc/p_ip6.c   | 14 +++++++-------
 tc/p_tcp.c   |  6 +++---
 tc/p_udp.c   |  4 ++--
 7 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index 74c91e8d..b745c379 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -330,7 +330,7 @@ static int parse_val(int *argc_p, char ***argv_p, __u32 *val, int type)
 }
 
 int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
-	      struct m_pedit_sel *sel, struct m_pedit_key *tkey)
+	      struct m_pedit_sel *sel, struct m_pedit_key *tkey, int flags)
 {
 	__u32 mask[4] = { 0 };
 	__u32 val[4] = { 0 };
@@ -502,7 +502,7 @@ done:
 		NEXT_ARG();
 	}
 
-	res = parse_cmd(&argc, &argv, len, TU32, retain, sel, tkey);
+	res = parse_cmd(&argc, &argv, len, TU32, retain, sel, tkey, 0);
 
 	*argc_p = argc;
 	*argv_p = argv;
diff --git a/tc/m_pedit.h b/tc/m_pedit.h
index 5d3628a7..7398f66d 100644
--- a/tc/m_pedit.h
+++ b/tc/m_pedit.h
@@ -73,5 +73,5 @@ struct m_pedit_util {
 
 int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type,
 	      __u32 retain,
-	      struct m_pedit_sel *sel, struct m_pedit_key *tkey);
+	      struct m_pedit_sel *sel, struct m_pedit_key *tkey, int flags);
 #endif
diff --git a/tc/p_eth.c b/tc/p_eth.c
index 674f9c11..7b6b61f8 100644
--- a/tc/p_eth.c
+++ b/tc/p_eth.c
@@ -41,21 +41,21 @@ parse_eth(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "type") == 0) {
 		NEXT_ARG();
 		tkey->off = 12;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 
 	if (strcmp(*argv, "dst") == 0) {
 		NEXT_ARG();
 		tkey->off = 0;
-		res = parse_cmd(&argc, &argv, 6, TMAC, RU32, sel, tkey);
+		res = parse_cmd(&argc, &argv, 6, TMAC, RU32, sel, tkey, 0);
 		goto done;
 	}
 
 	if (strcmp(*argv, "src") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 6, TMAC, RU32, sel, tkey);
+		res = parse_cmd(&argc, &argv, 6, TMAC, RU32, sel, tkey, 0);
 		goto done;
 	}
 
diff --git a/tc/p_ip.c b/tc/p_ip.c
index c385ac6d..2d1643d0 100644
--- a/tc/p_ip.c
+++ b/tc/p_ip.c
@@ -40,13 +40,13 @@ parse_ip(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "src") == 0) {
 		NEXT_ARG();
 		tkey->off = 12;
-		res = parse_cmd(&argc, &argv, 4, TIPV4, RU32, sel, tkey);
+		res = parse_cmd(&argc, &argv, 4, TIPV4, RU32, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "dst") == 0) {
 		NEXT_ARG();
 		tkey->off = 16;
-		res = parse_cmd(&argc, &argv, 4, TIPV4, RU32, sel, tkey);
+		res = parse_cmd(&argc, &argv, 4, TIPV4, RU32, sel, tkey, 0);
 		goto done;
 	}
 	/* jamal - look at these and make them either old or new
@@ -56,64 +56,64 @@ parse_ip(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "tos") == 0 || matches(*argv, "dsfield") == 0) {
 		NEXT_ARG();
 		tkey->off = 1;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "ihl") == 0) {
 		NEXT_ARG();
 		tkey->off = 0;
-		res = parse_cmd(&argc, &argv, 1, TU32, 0x0f, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, 0x0f, sel, tkey, 0);
 		goto done;
 	}
 	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, RU8, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "protocol") == 0) {
 		NEXT_ARG();
 		tkey->off = 9;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 	/* jamal - fix this */
 	if (matches(*argv, "precedence") == 0) {
 		NEXT_ARG();
 		tkey->off = 1;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 	/* jamal - validate this at some point */
 	if (strcmp(*argv, "nofrag") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 1, TU32, 0x3F, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, 0x3F, sel, tkey, 0);
 		goto done;
 	}
 	/* jamal - validate this at some point */
 	if (strcmp(*argv, "firstfrag") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 1, TU32, 0x1F, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, 0x1F, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "ce") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 1, TU32, 0x80, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, 0x80, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "df") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 1, TU32, 0x40, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, 0x40, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "mf") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 1, TU32, 0x20, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, 0x20, sel, tkey, 0);
 		goto done;
 	}
 
@@ -126,25 +126,25 @@ parse_ip(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "dport") == 0) {
 		NEXT_ARG();
 		tkey->off = 22;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "sport") == 0) {
 		NEXT_ARG();
 		tkey->off = 20;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "icmp_type") == 0) {
 		NEXT_ARG();
 		tkey->off = 20;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "icmp_code") == 0) {
 		NEXT_ARG();
 		tkey->off = 20;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 	return -1;
diff --git a/tc/p_ip6.c b/tc/p_ip6.c
index 83a6ae81..f9d5d3b0 100644
--- a/tc/p_ip6.c
+++ b/tc/p_ip6.c
@@ -41,43 +41,43 @@ parse_ip6(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "src") == 0) {
 		NEXT_ARG();
 		tkey->off = 8;
-		res = parse_cmd(&argc, &argv, 16, TIPV6, RU32, sel, tkey);
+		res = parse_cmd(&argc, &argv, 16, TIPV6, RU32, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "dst") == 0) {
 		NEXT_ARG();
 		tkey->off = 24;
-		res = parse_cmd(&argc, &argv, 16, TIPV6, RU32, sel, tkey);
+		res = parse_cmd(&argc, &argv, 16, TIPV6, RU32, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "flow_lbl") == 0) {
 		NEXT_ARG();
 		tkey->off = 0;
-		res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey);
+		res = parse_cmd(&argc, &argv, 4, TU32, 0x0007ffff, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "payload_len") == 0) {
 		NEXT_ARG();
 		tkey->off = 4;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "nexthdr") == 0) {
 		NEXT_ARG();
 		tkey->off = 6;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 	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, RU8, sel, tkey, 0);
 		goto done;
 	}
 	if (strcmp(*argv, "traffic_class") == 0) {
 		NEXT_ARG();
 		tkey->off = 1;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 
 		/* Shift the field by 4 bits on success. */
 		if (!res) {
diff --git a/tc/p_tcp.c b/tc/p_tcp.c
index d2dbfd71..ec7b08a2 100644
--- a/tc/p_tcp.c
+++ b/tc/p_tcp.c
@@ -41,21 +41,21 @@ parse_tcp(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "sport") == 0) {
 		NEXT_ARG();
 		tkey->off = 0;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 
 	if (strcmp(*argv, "dport") == 0) {
 		NEXT_ARG();
 		tkey->off = 2;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 
 	if (strcmp(*argv, "flags") == 0) {
 		NEXT_ARG();
 		tkey->off = 13;
-		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, 0);
 		goto done;
 	}
 
diff --git a/tc/p_udp.c b/tc/p_udp.c
index bab456de..742955e6 100644
--- a/tc/p_udp.c
+++ b/tc/p_udp.c
@@ -41,14 +41,14 @@ parse_udp(int *argc_p, char ***argv_p,
 	if (strcmp(*argv, "sport") == 0) {
 		NEXT_ARG();
 		tkey->off = 0;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 
 	if (strcmp(*argv, "dport") == 0) {
 		NEXT_ARG();
 		tkey->off = 2;
-		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey);
+		res = parse_cmd(&argc, &argv, 2, TU32, RU16, sel, tkey, 0);
 		goto done;
 	}
 
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation
  2021-06-18 16:06 [PATCH iproute2 1/2] tc: pedit: parse_cmd: add flags argument Asbjørn Sloth Tønnesen
@ 2021-06-18 16:06 ` Asbjørn Sloth Tønnesen
  2021-06-22 15:39   ` David Ahern
  2021-06-26  4:45   ` David Ahern
  0 siblings, 2 replies; 6+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2021-06-18 16:06 UTC (permalink / raw)
  To: netdev
  Cc: Asbjørn Sloth Tønnesen, David Ahern, Stephen Hemminger,
	Jiri Pirko

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.

Tested with skip_sw on Mellanox ConnectX-6 Dx.

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

v3:
   - Use dedicated flags argument in parse_cmd() (David Ahern)
   - Minor rewording of the man page

v2:
   - Fix whitespace issue (Stephen Hemminger)
   - Add to usage info in explain()

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

diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
index 376ad4a8..15159ddd 100644
--- a/man/man8/tc-pedit.8
+++ b/man/man8/tc-pedit.8
@@ -77,6 +77,7 @@ pedit - generic packet editor action
 .IR VAL " | "
 .BR add
 .IR VAL " | "
+.BR decrement " | "
 .BR preserve " } [ " retain
 .IR RVAL " ]"
 
@@ -96,7 +97,7 @@ chosen automatically based on the header field size.
 .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
@@ -288,6 +289,11 @@ 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
+Decrease 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
diff --git a/tc/m_pedit.c b/tc/m_pedit.c
index b745c379..54949e43 100644
--- a/tc/m_pedit.c
+++ b/tc/m_pedit.c
@@ -41,7 +41,7 @@ static void explain(void)
 		"\t\tATC:= at <atval> offmask <maskval> shift <shiftval>\n"
 		"\t\tNOTE: offval is byte offset, must be multiple of 4\n"
 		"\t\tNOTE: maskval is a 32 bit hex number\n \t\tNOTE: shiftval is a shift value\n"
-		"\t\tCMD:= clear | invert | set <setval>| add <addval> | retain\n"
+		"\t\tCMD:= clear | invert | set <setval> | add <addval> | decrement | retain\n"
 		"\t<LAYERED>:= ip <ipdata> | ip6 <ip6data>\n"
 		" \t\t| udp <udpdata> | tcp <tcpdata> | icmp <icmpdata>\n"
 		"\tCONTROL:= reclassify | pipe | drop | continue | pass |\n"
@@ -360,15 +360,24 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
 		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 & PEDIT_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 {
@@ -431,6 +440,10 @@ done:
 	*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,
diff --git a/tc/m_pedit.h b/tc/m_pedit.h
index 7398f66d..549bcf86 100644
--- a/tc/m_pedit.h
+++ b/tc/m_pedit.h
@@ -39,6 +39,10 @@
 
 #define PEDITKINDSIZ 16
 
+enum m_pedit_flags {
+	PEDIT_ALLOW_DEC = 1<<0,
+};
+
 struct m_pedit_key {
 	__u32           mask;  /* AND */
 	__u32           val;   /*XOR */
diff --git a/tc/p_ip.c b/tc/p_ip.c
index 2d1643d0..8eed9e8d 100644
--- a/tc/p_ip.c
+++ b/tc/p_ip.c
@@ -68,7 +68,7 @@ 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, 0);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, PEDIT_ALLOW_DEC);
 		goto done;
 	}
 	if (strcmp(*argv, "protocol") == 0) {
diff --git a/tc/p_ip6.c b/tc/p_ip6.c
index f9d5d3b0..f855c59e 100644
--- a/tc/p_ip6.c
+++ b/tc/p_ip6.c
@@ -71,7 +71,7 @@ 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, 0);
+		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, PEDIT_ALLOW_DEC);
 		goto done;
 	}
 	if (strcmp(*argv, "traffic_class") == 0) {
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation
  2021-06-18 16:06 ` [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation Asbjørn Sloth Tønnesen
@ 2021-06-22 15:39   ` David Ahern
  2021-06-24 20:24     ` Jamal Hadi Salim
  2021-06-26  4:45   ` David Ahern
  1 sibling, 1 reply; 6+ messages in thread
From: David Ahern @ 2021-06-22 15:39 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, netdev, Jamal Hadi Salim, Cong Wang
  Cc: Stephen Hemminger, Jiri Pirko

[ looks fine to me; adding tc folks to make sure they see it ]

On 6/18/21 10:06 AM, Asbjørn Sloth Tønnesen wrote:
> 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.
> 
> Tested with skip_sw on Mellanox ConnectX-6 Dx.
> 
> [1] https://docs.mellanox.com/pages/viewpage.action?pageId=47033989
> 
> v3:
>    - Use dedicated flags argument in parse_cmd() (David Ahern)
>    - Minor rewording of the man page
> 
> v2:
>    - Fix whitespace issue (Stephen Hemminger)
>    - Add to usage info in explain()
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <asbjorn@asbjorn.st>
> ---
>  man/man8/tc-pedit.8 |  8 +++++++-
>  tc/m_pedit.c        | 25 +++++++++++++++++++------
>  tc/m_pedit.h        |  4 ++++
>  tc/p_ip.c           |  2 +-
>  tc/p_ip6.c          |  2 +-
>  5 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/man/man8/tc-pedit.8 b/man/man8/tc-pedit.8
> index 376ad4a8..15159ddd 100644
> --- a/man/man8/tc-pedit.8
> +++ b/man/man8/tc-pedit.8
> @@ -77,6 +77,7 @@ pedit - generic packet editor action
>  .IR VAL " | "
>  .BR add
>  .IR VAL " | "
> +.BR decrement " | "
>  .BR preserve " } [ " retain
>  .IR RVAL " ]"
>  
> @@ -96,7 +97,7 @@ chosen automatically based on the header field size.
>  .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
> @@ -288,6 +289,11 @@ 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
> +Decrease 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
> diff --git a/tc/m_pedit.c b/tc/m_pedit.c
> index b745c379..54949e43 100644
> --- a/tc/m_pedit.c
> +++ b/tc/m_pedit.c
> @@ -41,7 +41,7 @@ static void explain(void)
>  		"\t\tATC:= at <atval> offmask <maskval> shift <shiftval>\n"
>  		"\t\tNOTE: offval is byte offset, must be multiple of 4\n"
>  		"\t\tNOTE: maskval is a 32 bit hex number\n \t\tNOTE: shiftval is a shift value\n"
> -		"\t\tCMD:= clear | invert | set <setval>| add <addval> | retain\n"
> +		"\t\tCMD:= clear | invert | set <setval> | add <addval> | decrement | retain\n"
>  		"\t<LAYERED>:= ip <ipdata> | ip6 <ip6data>\n"
>  		" \t\t| udp <udpdata> | tcp <tcpdata> | icmp <icmpdata>\n"
>  		"\tCONTROL:= reclassify | pipe | drop | continue | pass |\n"
> @@ -360,15 +360,24 @@ int parse_cmd(int *argc_p, char ***argv_p, __u32 len, int type, __u32 retain,
>  		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 & PEDIT_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 {
> @@ -431,6 +440,10 @@ done:
>  	*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,
> diff --git a/tc/m_pedit.h b/tc/m_pedit.h
> index 7398f66d..549bcf86 100644
> --- a/tc/m_pedit.h
> +++ b/tc/m_pedit.h
> @@ -39,6 +39,10 @@
>  
>  #define PEDITKINDSIZ 16
>  
> +enum m_pedit_flags {
> +	PEDIT_ALLOW_DEC = 1<<0,
> +};
> +
>  struct m_pedit_key {
>  	__u32           mask;  /* AND */
>  	__u32           val;   /*XOR */
> diff --git a/tc/p_ip.c b/tc/p_ip.c
> index 2d1643d0..8eed9e8d 100644
> --- a/tc/p_ip.c
> +++ b/tc/p_ip.c
> @@ -68,7 +68,7 @@ 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, 0);
> +		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, PEDIT_ALLOW_DEC);
>  		goto done;
>  	}
>  	if (strcmp(*argv, "protocol") == 0) {
> diff --git a/tc/p_ip6.c b/tc/p_ip6.c
> index f9d5d3b0..f855c59e 100644
> --- a/tc/p_ip6.c
> +++ b/tc/p_ip6.c
> @@ -71,7 +71,7 @@ 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, 0);
> +		res = parse_cmd(&argc, &argv, 1, TU32, RU8, sel, tkey, PEDIT_ALLOW_DEC);
>  		goto done;
>  	}
>  	if (strcmp(*argv, "traffic_class") == 0) {
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation
  2021-06-22 15:39   ` David Ahern
@ 2021-06-24 20:24     ` Jamal Hadi Salim
  2021-06-25 19:21       ` Asbjørn Sloth Tønnesen
  0 siblings, 1 reply; 6+ messages in thread
From: Jamal Hadi Salim @ 2021-06-24 20:24 UTC (permalink / raw)
  To: David Ahern, Asbjørn Sloth Tønnesen, netdev, Cong Wang
  Cc: Stephen Hemminger, Jiri Pirko

Hi Asbjørn,


On 2021-06-22 11:39 a.m., David Ahern wrote:
> [ looks fine to me; adding tc folks to make sure they see it ]
> 
> On 6/18/21 10:06 AM, Asbjørn Sloth Tønnesen wrote:
>> 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 ...
>>

So you "add" essentially one's complement of the value you are trying to
decrement with?

>> 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.
>>
>> Tested with skip_sw on Mellanox ConnectX-6 Dx.
>>
>> [1] https://docs.mellanox.com/pages/viewpage.action?pageId=47033989


I didnt see an example which showed using "dec" but what you described
above makes sense. And the changes below look sane. So:

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation
  2021-06-24 20:24     ` Jamal Hadi Salim
@ 2021-06-25 19:21       ` Asbjørn Sloth Tønnesen
  0 siblings, 0 replies; 6+ messages in thread
From: Asbjørn Sloth Tønnesen @ 2021-06-25 19:21 UTC (permalink / raw)
  To: Jamal Hadi Salim, David Ahern, netdev, Cong Wang
  Cc: Stephen Hemminger, Jiri Pirko, Amir Vadai

Hi Jamal,

Thank you for your review.

On 6/24/21 8:24 PM, Jamal Hadi Salim wrote:
> So you "add" essentially one's complement of the value you are trying to
> decrement with?

Almost (off by one), it's basically decrement by overflowing it, which
is safe since the operation is masked. One should however have another
rule, that matches on TTL == 1. so it can get a proper ICMP error.

Decrementing TTL by one was actually the prime example presented
when Amir Vadai introduced TCA_PEDIT_KEY_EX_CMD_ADD.

kernel   853a14ba net/act_pedit: Introduce 'add' operation     [1]
iproute2 8d193d96 tc/pedit: p_ip: introduce editing ttl header [2]

[1] https://git.kernel.org/torvalds/c/853a14ba
[2] https://git.kernel.org/pub/scm/network/iproute2/iproute2.git/commit/?id=8d193d96


>>> 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.
>>>
>>> Tested with skip_sw on Mellanox ConnectX-6 Dx.
>>>
>>> [1] https://docs.mellanox.com/pages/viewpage.action?pageId=47033989
> 
> 
> I didnt see an example which showed using "dec" but what you described
> above makes sense.

It is indeed a large document, so to be more specific:

https://docs.mellanox.com/pages/viewpage.action?pageId=47033989#highlighter_159101
 > Using TC rules:
 > IPv4:
 > tc filter add [..] munge ip ttl dec [..]
 > IPv6:
 > tc filter add [..] munge ipv6 hlimit dec [..]

"ipv6 hlimit" should obviously be "ip6 hoplimit".


-- 
Best regards
Asbjørn Sloth Tønnesen

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation
  2021-06-18 16:06 ` [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation Asbjørn Sloth Tønnesen
  2021-06-22 15:39   ` David Ahern
@ 2021-06-26  4:45   ` David Ahern
  1 sibling, 0 replies; 6+ messages in thread
From: David Ahern @ 2021-06-26  4:45 UTC (permalink / raw)
  To: Asbjørn Sloth Tønnesen, netdev; +Cc: Stephen Hemminger, Jiri Pirko

On 6/18/21 10:06 AM, Asbjørn Sloth Tønnesen wrote:
> 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.
> 
> Tested with skip_sw on Mellanox ConnectX-6 Dx.
> 
> [1] https://docs.mellanox.com/pages/viewpage.action?pageId=47033989
> 
> v3:
>    - Use dedicated flags argument in parse_cmd() (David Ahern)
>    - Minor rewording of the man page
> 
> v2:
>    - Fix whitespace issue (Stephen Hemminger)
>    - Add to usage info in explain()
> 
> Signed-off-by: Asbjørn Sloth Tønnesen <asbjorn@asbjorn.st>
> ---
>  man/man8/tc-pedit.8 |  8 +++++++-
>  tc/m_pedit.c        | 25 +++++++++++++++++++------
>  tc/m_pedit.h        |  4 ++++
>  tc/p_ip.c           |  2 +-
>  tc/p_ip6.c          |  2 +-
>  5 files changed, 32 insertions(+), 9 deletions(-)
> 

applied both to iproute2-next. Thanks,


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-26  4:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18 16:06 [PATCH iproute2 1/2] tc: pedit: parse_cmd: add flags argument Asbjørn Sloth Tønnesen
2021-06-18 16:06 ` [PATCH iproute2 v3 2/2] tc: pedit: add decrement operation Asbjørn Sloth Tønnesen
2021-06-22 15:39   ` David Ahern
2021-06-24 20:24     ` Jamal Hadi Salim
2021-06-25 19:21       ` Asbjørn Sloth Tønnesen
2021-06-26  4:45   ` David Ahern

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.