[PATCH iproute2-next 0/5] iprule: Add mask support for L4 ports and DSCP

[PATCH iproute2-next 0/5] iprule: Add mask support for L4 ports and DSCP

[Ido Schimmel]

Add mask support for L4 ports and DSCP in ip-rule following kernel
commit a60a27c7849f ("Merge branch 'net-fib_rules-add-port-mask-support'")
and commit 27422c373897 ("Merge branch 'net-fib_rules-add-dscp-mask-support'").

Patches #1-#3 are preparations.

Patches #4 and #5 add mask support for L4 ports and DSCP, respectively.
See the commit messages for example usage and output.

Ido Schimmel (5):
  Sync uAPI headers
  iprule: Move port parsing to a function
  iprule: Allow specifying ports in hexadecimal notation
  iprule: Add port mask support
  iprule: Add DSCP mask support

 include/uapi/linux/fib_rules.h |   3 +
 ip/iprule.c                    | 223 +++++++++++++++++++++++++--------
 man/man8/ip-rule.8.in          |  23 ++--
 3 files changed, 185 insertions(+), 64 deletions(-)

-- 
2.48.1

[PATCH iproute2-next 1/5] Sync uAPI headers

[Ido Schimmel]

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 include/uapi/linux/fib_rules.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
index 00e9890ca3c0..2df6e4035d50 100644
--- a/include/uapi/linux/fib_rules.h
+++ b/include/uapi/linux/fib_rules.h
@@ -70,6 +70,9 @@ enum {
 	FRA_DSCP,	/* dscp */
 	FRA_FLOWLABEL,	/* flowlabel */
 	FRA_FLOWLABEL_MASK,	/* flowlabel mask */
+	FRA_SPORT_MASK,	/* sport mask */
+	FRA_DPORT_MASK,	/* dport mask */
+	FRA_DSCP_MASK,	/* dscp mask */
 	__FRA_MAX
 };
 
-- 
2.48.1

[PATCH iproute2-next 2/5] iprule: Move port parsing to a function

[Ido Schimmel]

In preparation for adding port mask support, move port parsing to a
function.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/iprule.c | 57 +++++++++++++++++++++++++----------------------------
 1 file changed, 27 insertions(+), 30 deletions(-)

diff --git a/ip/iprule.c b/ip/iprule.c
index ea30d418712c..61e092bc5693 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -600,6 +600,29 @@ static int flush_rule(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
+static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
+{
+	char *sep;
+
+	sep = strchr(arg, '-');
+	if (sep) {
+		*sep = '\0';
+
+		if (get_u16(&r->start, arg, 10))
+			invarg("invalid port range start", arg);
+
+		if (get_u16(&r->end, sep + 1, 10))
+			invarg("invalid port range end", sep + 1);
+
+		return;
+	}
+
+	if (get_u16(&r->start, arg, 10))
+		invarg("invalid port", arg);
+
+	r->end = r->start;
+}
+
 static void iprule_flowlabel_parse(char *arg, __u32 *flowlabel,
 				   __u32 *flowlabel_mask)
 {
@@ -746,27 +769,11 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
 				invarg("Invalid \"ipproto\" value\n", *argv);
 			filter.ipproto = ipproto;
 		} else if (strcmp(*argv, "sport") == 0) {
-			struct fib_rule_port_range r;
-			int ret;
-
 			NEXT_ARG();
-			ret = sscanf(*argv, "%hu-%hu", &r.start, &r.end);
-			if (ret == 1)
-				r.end = r.start;
-			else if (ret != 2)
-				invarg("invalid port range\n", *argv);
-			filter.sport = r;
+			iprule_port_parse(*argv, &filter.sport);
 		} else if (strcmp(*argv, "dport") == 0) {
-			struct fib_rule_port_range r;
-			int ret;
-
 			NEXT_ARG();
-			ret = sscanf(*argv, "%hu-%hu", &r.start, &r.end);
-			if (ret == 1)
-				r.end = r.start;
-			else if (ret != 2)
-				invarg("invalid dport range\n", *argv);
-			filter.dport = r;
+			iprule_port_parse(*argv, &filter.dport);
 		} else if (strcmp(*argv, "dscp") == 0) {
 			__u32 dscp;
 
@@ -1036,26 +1043,16 @@ static int iprule_modify(int cmd, int argc, char **argv)
 			addattr8(&req.n, sizeof(req), FRA_IP_PROTO, ipproto);
 		} else if (strcmp(*argv, "sport") == 0) {
 			struct fib_rule_port_range r;
-			int ret = 0;
 
 			NEXT_ARG();
-			ret = sscanf(*argv, "%hu-%hu", &r.start, &r.end);
-			if (ret == 1)
-				r.end = r.start;
-			else if (ret != 2)
-				invarg("invalid port range\n", *argv);
+			iprule_port_parse(*argv, &r);
 			addattr_l(&req.n, sizeof(req), FRA_SPORT_RANGE, &r,
 				  sizeof(r));
 		} else if (strcmp(*argv, "dport") == 0) {
 			struct fib_rule_port_range r;
-			int ret = 0;
 
 			NEXT_ARG();
-			ret = sscanf(*argv, "%hu-%hu", &r.start, &r.end);
-			if (ret == 1)
-				r.end = r.start;
-			else if (ret != 2)
-				invarg("invalid dport range\n", *argv);
+			iprule_port_parse(*argv, &r);
 			addattr_l(&req.n, sizeof(req), FRA_DPORT_RANGE, &r,
 				  sizeof(r));
 		} else if (strcmp(*argv, "dscp") == 0) {
-- 
2.48.1

Re: [PATCH iproute2-next 2/5] iprule: Move port parsing to a function

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> In preparation for adding port mask support, move port parsing to a
> function.
>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

[PATCH iproute2-next 3/5] iprule: Allow specifying ports in hexadecimal notation

[Ido Schimmel]

This will be useful when enabling port masks in the next patch.

Before:

 # ip rule add sport 0x1 table 100
 Invalid "sport"

After:

 # ip rule add sport 0x1 table 100
 $ ip rule show sport 0x1
 32765:  from all sport 1 lookup 100

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/iprule.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ip/iprule.c b/ip/iprule.c
index 61e092bc5693..64d389bebb76 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -608,16 +608,16 @@ static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
 	if (sep) {
 		*sep = '\0';
 
-		if (get_u16(&r->start, arg, 10))
+		if (get_u16(&r->start, arg, 0))
 			invarg("invalid port range start", arg);
 
-		if (get_u16(&r->end, sep + 1, 10))
+		if (get_u16(&r->end, sep + 1, 0))
 			invarg("invalid port range end", sep + 1);
 
 		return;
 	}
 
-	if (get_u16(&r->start, arg, 10))
+	if (get_u16(&r->start, arg, 0))
 		invarg("invalid port", arg);
 
 	r->end = r->start;
-- 
2.48.1

Re: [PATCH iproute2-next 3/5] iprule: Allow specifying ports in hexadecimal notation

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> This will be useful when enabling port masks in the next patch.
>
> Before:
>
>  # ip rule add sport 0x1 table 100
>  Invalid "sport"
>
> After:
>
>  # ip rule add sport 0x1 table 100
>  $ ip rule show sport 0x1
>  32765:  from all sport 1 lookup 100
>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

[PATCH iproute2-next 4/5] iprule: Add port mask support

[Ido Schimmel]

Add port mask support, allowing users to specify a source or destination
port with an optional mask. Example:

 # ip rule add sport 80 table 100
 # ip rule add sport 90/0xffff table 200
 # ip rule add dport 1000-2000 table 300
 # ip rule add sport 0x123/0xfff table 400
 # ip rule add dport 0x4/0xff table 500
 # ip rule add dport 0x8/0xf table 600
 # ip rule del dport 0x8/0xf table 600

In non-JSON output, the mask is not printed in case of exact match:

 $ ip rule show
 0:      from all lookup local
 32761:  from all dport 0x4/0xff lookup 500
 32762:  from all sport 0x123/0xfff lookup 400
 32763:  from all dport 1000-2000 lookup 300
 32764:  from all sport 90 lookup 200
 32765:  from all sport 80 lookup 100
 32766:  from all lookup main
 32767:  from all lookup default

Dump can be filtered by port value and mask:

 $ ip rule show sport 80
 32765:  from all sport 80 lookup 100
 $ ip rule show sport 90
 32764:  from all sport 90 lookup 200
 $ ip rule show sport 0x123/0x0fff
 32762:  from all sport 0x123/0xfff lookup 400
 $ ip rule show dport 4/0xff
 32761:  from all dport 0x4/0xff lookup 500

In JSON output, the port mask is printed as an hexadecimal string to be
consistent with other masks. The port value is printed as an integer in
order not to break existing scripts:

 $ ip -j -p rule show sport 0x123/0xfff table 400
 [ {
         "priority": 32762,
         "src": "all",
         "sport": 291,
         "sport_mask": "0xfff",
         "table": "400"
     } ]

The mask attribute is only sent to the kernel in case of inexact match
so that iproute2 will continue working with kernels that do not support
the attribute.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/iprule.c           | 103 +++++++++++++++++++++++++++++++++++++-----
 man/man8/ip-rule.8.in |  14 +++---
 2 files changed, 100 insertions(+), 17 deletions(-)

diff --git a/ip/iprule.c b/ip/iprule.c
index 64d389bebb76..fbe69a3b6293 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -23,6 +23,8 @@
 #include "ip_common.h"
 #include "json_print.h"
 
+#define PORT_MAX_MASK 0xFFFF
+
 enum list_action {
 	IPRULE_LIST,
 	IPRULE_FLUSH,
@@ -44,8 +46,8 @@ static void usage(void)
 		"            [ iif STRING ] [ oif STRING ] [ pref NUMBER ] [ l3mdev ]\n"
 		"            [ uidrange NUMBER-NUMBER ]\n"
 		"            [ ipproto PROTOCOL ]\n"
-		"            [ sport [ NUMBER | NUMBER-NUMBER ]\n"
-		"            [ dport [ NUMBER | NUMBER-NUMBER ] ]\n"
+		"            [ sport [ NUMBER[/MASK] | NUMBER-NUMBER ]\n"
+		"            [ dport [ NUMBER[/MASK] | NUMBER-NUMBER ] ]\n"
 		"            [ dscp DSCP ] [ flowlabel FLOWLABEL[/MASK] ]\n"
 		"ACTION := [ table TABLE_ID ]\n"
 		"          [ protocol PROTO ]\n"
@@ -80,6 +82,7 @@ static struct
 	int protocolmask;
 	struct fib_rule_port_range sport;
 	struct fib_rule_port_range dport;
+	__u16 sport_mask, dport_mask;
 	__u8 ipproto;
 } filter;
 
@@ -186,8 +189,9 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 			return false;
 	}
 
-	if (filter.sport.start) {
+	if (filter.sport_mask) {
 		const struct fib_rule_port_range *r;
+		__u16 sport_mask = PORT_MAX_MASK;
 
 		if (!tb[FRA_SPORT_RANGE])
 			return false;
@@ -196,10 +200,16 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 		if (r->start != filter.sport.start ||
 		    r->end != filter.sport.end)
 			return false;
+
+		if (tb[FRA_SPORT_MASK])
+			sport_mask = rta_getattr_u16(tb[FRA_SPORT_MASK]);
+		if (filter.sport_mask != sport_mask)
+			return false;
 	}
 
-	if (filter.dport.start) {
+	if (filter.dport_mask) {
 		const struct fib_rule_port_range *r;
+		__u16 dport_mask = PORT_MAX_MASK;
 
 		if (!tb[FRA_DPORT_RANGE])
 			return false;
@@ -208,6 +218,11 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 		if (r->start != filter.dport.start ||
 		    r->end != filter.dport.end)
 			return false;
+
+		if (tb[FRA_DPORT_MASK])
+			dport_mask = rta_getattr_u16(tb[FRA_DPORT_MASK]);
+		if (filter.dport_mask != dport_mask)
+			return false;
 	}
 
 	if (filter.tun_id) {
@@ -390,7 +405,26 @@ int print_rule(struct nlmsghdr *n, void *arg)
 		struct fib_rule_port_range *r = RTA_DATA(tb[FRA_SPORT_RANGE]);
 
 		if (r->start == r->end) {
-			print_uint(PRINT_ANY, "sport", " sport %u", r->start);
+			if (tb[FRA_SPORT_MASK]) {
+				__u16 mask;
+
+				mask = rta_getattr_u16(tb[FRA_SPORT_MASK]);
+				print_uint(PRINT_JSON, "sport", NULL, r->start);
+				print_0xhex(PRINT_JSON, "sport_mask", NULL,
+					    mask);
+				if (mask == PORT_MAX_MASK) {
+					print_uint(PRINT_FP, NULL, " sport %u",
+						   r->start);
+				} else {
+					print_0xhex(PRINT_FP, NULL,
+						    " sport %#x", r->start);
+					print_0xhex(PRINT_FP, NULL, "/%#x",
+						    mask);
+				}
+			} else {
+				print_uint(PRINT_ANY, "sport", " sport %u",
+					   r->start);
+			}
 		} else {
 			print_uint(PRINT_ANY, "sport_start", " sport %u",
 				   r->start);
@@ -402,7 +436,26 @@ int print_rule(struct nlmsghdr *n, void *arg)
 		struct fib_rule_port_range *r = RTA_DATA(tb[FRA_DPORT_RANGE]);
 
 		if (r->start == r->end) {
-			print_uint(PRINT_ANY, "dport", " dport %u", r->start);
+			if (tb[FRA_DPORT_MASK]) {
+				__u16 mask;
+
+				mask = rta_getattr_u16(tb[FRA_DPORT_MASK]);
+				print_uint(PRINT_JSON, "dport", NULL, r->start);
+				print_0xhex(PRINT_JSON, "dport_mask", NULL,
+					    mask);
+				if (mask == 0xFFFF) {
+					print_uint(PRINT_FP, NULL, " dport %u",
+						   r->start);
+				} else {
+					print_0xhex(PRINT_FP, NULL,
+						    " dport %#x", r->start);
+					print_0xhex(PRINT_FP, NULL, "/%#x",
+						    mask);
+				}
+			} else {
+				print_uint(PRINT_ANY, "dport", " dport %u",
+					   r->start);
+			}
 		} else {
 			print_uint(PRINT_ANY, "dport_start", " dport %u",
 				   r->start);
@@ -600,10 +653,13 @@ static int flush_rule(struct nlmsghdr *n, void *arg)
 	return 0;
 }
 
-static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
+static void iprule_port_parse(char *arg, struct fib_rule_port_range *r,
+			      __u16 *mask)
 {
 	char *sep;
 
+	*mask = PORT_MAX_MASK;
+
 	sep = strchr(arg, '-');
 	if (sep) {
 		*sep = '\0';
@@ -617,6 +673,21 @@ static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
 		return;
 	}
 
+	sep = strchr(arg, '/');
+	if (sep) {
+		*sep = '\0';
+
+		if (get_u16(&r->start, arg, 0))
+			invarg("invalid port", arg);
+
+		r->end = r->start;
+
+		if (get_u16(mask, sep + 1, 0))
+			invarg("invalid mask", sep + 1);
+
+		return;
+	}
+
 	if (get_u16(&r->start, arg, 0))
 		invarg("invalid port", arg);
 
@@ -770,10 +841,12 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
 			filter.ipproto = ipproto;
 		} else if (strcmp(*argv, "sport") == 0) {
 			NEXT_ARG();
-			iprule_port_parse(*argv, &filter.sport);
+			iprule_port_parse(*argv, &filter.sport,
+					  &filter.sport_mask);
 		} else if (strcmp(*argv, "dport") == 0) {
 			NEXT_ARG();
-			iprule_port_parse(*argv, &filter.dport);
+			iprule_port_parse(*argv, &filter.dport,
+					  &filter.dport_mask);
 		} else if (strcmp(*argv, "dscp") == 0) {
 			__u32 dscp;
 
@@ -1043,18 +1116,26 @@ static int iprule_modify(int cmd, int argc, char **argv)
 			addattr8(&req.n, sizeof(req), FRA_IP_PROTO, ipproto);
 		} else if (strcmp(*argv, "sport") == 0) {
 			struct fib_rule_port_range r;
+			__u16 sport_mask;
 
 			NEXT_ARG();
-			iprule_port_parse(*argv, &r);
+			iprule_port_parse(*argv, &r, &sport_mask);
 			addattr_l(&req.n, sizeof(req), FRA_SPORT_RANGE, &r,
 				  sizeof(r));
+			if (sport_mask != PORT_MAX_MASK)
+				addattr16(&req.n, sizeof(req), FRA_SPORT_MASK,
+					  sport_mask);
 		} else if (strcmp(*argv, "dport") == 0) {
 			struct fib_rule_port_range r;
+			__u16 dport_mask;
 
 			NEXT_ARG();
-			iprule_port_parse(*argv, &r);
+			iprule_port_parse(*argv, &r, &dport_mask);
 			addattr_l(&req.n, sizeof(req), FRA_DPORT_RANGE, &r,
 				  sizeof(r));
+			if (dport_mask != PORT_MAX_MASK)
+				addattr16(&req.n, sizeof(req), FRA_DPORT_MASK,
+					  dport_mask);
 		} else if (strcmp(*argv, "dscp") == 0) {
 			__u32 dscp;
 
diff --git a/man/man8/ip-rule.8.in b/man/man8/ip-rule.8.in
index 6fc741d4f470..4945ccd55076 100644
--- a/man/man8/ip-rule.8.in
+++ b/man/man8/ip-rule.8.in
@@ -52,10 +52,10 @@ ip-rule \- routing policy database management
 .B ipproto
 .IR PROTOCOL " ] [ "
 .BR sport " [ "
-.IR NUMBER " | "
+.IR NUMBER\fR[\fB/\fIMASK "] | "
 .IR NUMBER "-" NUMBER " ] ] [ "
 .BR dport " [ "
-.IR NUMBER " | "
+.IR NUMBER\fR[\fB/\fIMASK "] | "
 .IR NUMBER "-" NUMBER " ] ] [ "
 .B  tun_id
 .IR TUN_ID " ] [ "
@@ -270,12 +270,14 @@ value to match.
 select the ip protocol value to match.
 
 .TP
-.BI sport " NUMBER | NUMBER-NUMBER"
-select the source port value to match. supports port range.
+.BI sport " NUMBER\fR[\fB/\fIMASK\fR] | NUMBER-NUMBER"
+select the source port value to match with an optional mask. supports port
+range.
 
 .TP
-.BI dport " NUMBER | NUMBER-NUMBER"
-select the destination port value to match. supports port range.
+.BI dport " NUMBER\fR[\fB/\fIMASK\fR] | NUMBER-NUMBER"
+select the destination port value to match with an optional mask. supports port
+range.
 
 .TP
 .BI priority " PREFERENCE"
-- 
2.48.1

Re: [PATCH iproute2-next 4/5] iprule: Add port mask support

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> Add port mask support, allowing users to specify a source or destination
> port with an optional mask. Example:
>
>  # ip rule add sport 80 table 100
>  # ip rule add sport 90/0xffff table 200
>  # ip rule add dport 1000-2000 table 300
>  # ip rule add sport 0x123/0xfff table 400
>  # ip rule add dport 0x4/0xff table 500
>  # ip rule add dport 0x8/0xf table 600
>  # ip rule del dport 0x8/0xf table 600
>
> In non-JSON output, the mask is not printed in case of exact match:
>
>  $ ip rule show
>  0:      from all lookup local
>  32761:  from all dport 0x4/0xff lookup 500
>  32762:  from all sport 0x123/0xfff lookup 400
>  32763:  from all dport 1000-2000 lookup 300
>  32764:  from all sport 90 lookup 200
>  32765:  from all sport 80 lookup 100
>  32766:  from all lookup main
>  32767:  from all lookup default
>
> Dump can be filtered by port value and mask:
>
>  $ ip rule show sport 80
>  32765:  from all sport 80 lookup 100
>  $ ip rule show sport 90
>  32764:  from all sport 90 lookup 200
>  $ ip rule show sport 0x123/0x0fff
>  32762:  from all sport 0x123/0xfff lookup 400
>  $ ip rule show dport 4/0xff
>  32761:  from all dport 0x4/0xff lookup 500
>
> In JSON output, the port mask is printed as an hexadecimal string to be
> consistent with other masks. The port value is printed as an integer in
> order not to break existing scripts:
>
>  $ ip -j -p rule show sport 0x123/0xfff table 400
>  [ {
>          "priority": 32762,
>          "src": "all",
>          "sport": 291,
>          "sport_mask": "0xfff",
>          "table": "400"
>      } ]
>
> The mask attribute is only sent to the kernel in case of inexact match
> so that iproute2 will continue working with kernels that do not support
> the attribute.
>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>

Two minor suggestions and a couple notes to self below. Looks OK overall.

> ---
>  ip/iprule.c           | 103 +++++++++++++++++++++++++++++++++++++-----
>  man/man8/ip-rule.8.in |  14 +++---
>  2 files changed, 100 insertions(+), 17 deletions(-)
>
> diff --git a/ip/iprule.c b/ip/iprule.c
> index 64d389bebb76..fbe69a3b6293 100644
> --- a/ip/iprule.c
> +++ b/ip/iprule.c
> @@ -23,6 +23,8 @@
>  #include "ip_common.h"
>  #include "json_print.h"
>  
> +#define PORT_MAX_MASK 0xFFFF
> +
>  enum list_action {
>  	IPRULE_LIST,
>  	IPRULE_FLUSH,
> @@ -44,8 +46,8 @@ static void usage(void)
>  		"            [ iif STRING ] [ oif STRING ] [ pref NUMBER ] [ l3mdev ]\n"
>  		"            [ uidrange NUMBER-NUMBER ]\n"
>  		"            [ ipproto PROTOCOL ]\n"
> -		"            [ sport [ NUMBER | NUMBER-NUMBER ]\n"
> -		"            [ dport [ NUMBER | NUMBER-NUMBER ] ]\n"
> +		"            [ sport [ NUMBER[/MASK] | NUMBER-NUMBER ]\n"
> +		"            [ dport [ NUMBER[/MASK] | NUMBER-NUMBER ] ]\n"
>  		"            [ dscp DSCP ] [ flowlabel FLOWLABEL[/MASK] ]\n"
>  		"ACTION := [ table TABLE_ID ]\n"
>  		"          [ protocol PROTO ]\n"
> @@ -80,6 +82,7 @@ static struct
>  	int protocolmask;
>  	struct fib_rule_port_range sport;
>  	struct fib_rule_port_range dport;
> +	__u16 sport_mask, dport_mask;
>  	__u8 ipproto;
>  } filter;
>  
> @@ -186,8 +189,9 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
>  			return false;
>  	}
>  
> -	if (filter.sport.start) {
> +	if (filter.sport_mask) {

OK, sport_mask now implies sport.start because of the changes in
iprule_port_parse().

>  		const struct fib_rule_port_range *r;
> +		__u16 sport_mask = PORT_MAX_MASK;
>  
>  		if (!tb[FRA_SPORT_RANGE])
>  			return false;
> @@ -196,10 +200,16 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
>  		if (r->start != filter.sport.start ||
>  		    r->end != filter.sport.end)
>  			return false;
> +
> +		if (tb[FRA_SPORT_MASK])
> +			sport_mask = rta_getattr_u16(tb[FRA_SPORT_MASK]);
> +		if (filter.sport_mask != sport_mask)
> +			return false;
>  	}
>  
> -	if (filter.dport.start) {
> +	if (filter.dport_mask) {
>  		const struct fib_rule_port_range *r;
> +		__u16 dport_mask = PORT_MAX_MASK;
>  
>  		if (!tb[FRA_DPORT_RANGE])
>  			return false;
> @@ -208,6 +218,11 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
>  		if (r->start != filter.dport.start ||
>  		    r->end != filter.dport.end)
>  			return false;
> +
> +		if (tb[FRA_DPORT_MASK])
> +			dport_mask = rta_getattr_u16(tb[FRA_DPORT_MASK]);
> +		if (filter.dport_mask != dport_mask)
> +			return false;
>  	}
>  
>  	if (filter.tun_id) {
> @@ -390,7 +405,26 @@ int print_rule(struct nlmsghdr *n, void *arg)
>  		struct fib_rule_port_range *r = RTA_DATA(tb[FRA_SPORT_RANGE]);
>  
>  		if (r->start == r->end) {
> -			print_uint(PRINT_ANY, "sport", " sport %u", r->start);
> +			if (tb[FRA_SPORT_MASK]) {
> +				__u16 mask;
> +
> +				mask = rta_getattr_u16(tb[FRA_SPORT_MASK]);
> +				print_uint(PRINT_JSON, "sport", NULL, r->start);
> +				print_0xhex(PRINT_JSON, "sport_mask", NULL,
> +					    mask);
> +				if (mask == PORT_MAX_MASK) {
> +					print_uint(PRINT_FP, NULL, " sport %u",
> +						   r->start);
> +				} else {
> +					print_0xhex(PRINT_FP, NULL,
> +						    " sport %#x", r->start);

Looks good, for JSON we always emit as uint, for FP we emit uint in
backward-compatible scenarios.

> +					print_0xhex(PRINT_FP, NULL, "/%#x",
> +						    mask);
> +				}
> +			} else {
> +				print_uint(PRINT_ANY, "sport", " sport %u",
> +					   r->start);

Hm, yeah, and on an old kernel we don't even get the mask in JSON.
Makes sense.

> +			}
>  		} else {
>  			print_uint(PRINT_ANY, "sport_start", " sport %u",
>  				   r->start);
> @@ -402,7 +436,26 @@ int print_rule(struct nlmsghdr *n, void *arg)
>  		struct fib_rule_port_range *r = RTA_DATA(tb[FRA_DPORT_RANGE]);
>  
>  		if (r->start == r->end) {
> -			print_uint(PRINT_ANY, "dport", " dport %u", r->start);
> +			if (tb[FRA_DPORT_MASK]) {
> +				__u16 mask;
> +
> +				mask = rta_getattr_u16(tb[FRA_DPORT_MASK]);
> +				print_uint(PRINT_JSON, "dport", NULL, r->start);
> +				print_0xhex(PRINT_JSON, "dport_mask", NULL,
> +					    mask);
> +				if (mask == 0xFFFF) {
> +					print_uint(PRINT_FP, NULL, " dport %u",
> +						   r->start);
> +				} else {
> +					print_0xhex(PRINT_FP, NULL,
> +						    " dport %#x", r->start);
> +					print_0xhex(PRINT_FP, NULL, "/%#x",
> +						    mask);
> +				}
> +			} else {
> +				print_uint(PRINT_ANY, "dport", " dport %u",
> +					   r->start);
> +			}
>  		} else {
>  			print_uint(PRINT_ANY, "dport_start", " dport %u",
>  				   r->start);
> @@ -600,10 +653,13 @@ static int flush_rule(struct nlmsghdr *n, void *arg)
>  	return 0;
>  }
>  
> -static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
> +static void iprule_port_parse(char *arg, struct fib_rule_port_range *r,
> +			      __u16 *mask)
>  {
>  	char *sep;
>  
> +	*mask = PORT_MAX_MASK;
> +
>  	sep = strchr(arg, '-');
>  	if (sep) {
>  		*sep = '\0';
> @@ -617,6 +673,21 @@ static void iprule_port_parse(char *arg, struct fib_rule_port_range *r)
>  		return;
>  	}
>  
> +	sep = strchr(arg, '/');
> +	if (sep) {
> +		*sep = '\0';
> +
> +		if (get_u16(&r->start, arg, 0))
> +			invarg("invalid port", arg);
> +
> +		r->end = r->start;
> +
> +		if (get_u16(mask, sep + 1, 0))
> +			invarg("invalid mask", sep + 1);
> +
> +		return;
> +	}
> +
>  	if (get_u16(&r->start, arg, 0))
>  		invarg("invalid port", arg);
>  

I think this duplicates the port number parsing unnecessarily. How
about:

+	sep = strchr(arg, '/');
+	if (sep) {
+		*sep = '\0';
+		if (get_u16(mask, sep + 1, 0))
+			invarg("invalid mask", sep + 1);
+	}
+
 	if (get_u16(&r->start, arg, 0))
 		invarg("invalid port", arg);

> @@ -770,10 +841,12 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
>  			filter.ipproto = ipproto;
>  		} else if (strcmp(*argv, "sport") == 0) {
>  			NEXT_ARG();
> -			iprule_port_parse(*argv, &filter.sport);
> +			iprule_port_parse(*argv, &filter.sport,
> +					  &filter.sport_mask);
>  		} else if (strcmp(*argv, "dport") == 0) {
>  			NEXT_ARG();
> -			iprule_port_parse(*argv, &filter.dport);
> +			iprule_port_parse(*argv, &filter.dport,
> +					  &filter.dport_mask);
>  		} else if (strcmp(*argv, "dscp") == 0) {
>  			__u32 dscp;
>  
> @@ -1043,18 +1116,26 @@ static int iprule_modify(int cmd, int argc, char **argv)
>  			addattr8(&req.n, sizeof(req), FRA_IP_PROTO, ipproto);
>  		} else if (strcmp(*argv, "sport") == 0) {
>  			struct fib_rule_port_range r;
> +			__u16 sport_mask;
>  
>  			NEXT_ARG();
> -			iprule_port_parse(*argv, &r);
> +			iprule_port_parse(*argv, &r, &sport_mask);
>  			addattr_l(&req.n, sizeof(req), FRA_SPORT_RANGE, &r,
>  				  sizeof(r));
> +			if (sport_mask != PORT_MAX_MASK)
> +				addattr16(&req.n, sizeof(req), FRA_SPORT_MASK,
> +					  sport_mask);
>  		} else if (strcmp(*argv, "dport") == 0) {
>  			struct fib_rule_port_range r;
> +			__u16 dport_mask;
>  
>  			NEXT_ARG();
> -			iprule_port_parse(*argv, &r);
> +			iprule_port_parse(*argv, &r, &dport_mask);
>  			addattr_l(&req.n, sizeof(req), FRA_DPORT_RANGE, &r,
>  				  sizeof(r));
> +			if (dport_mask != PORT_MAX_MASK)
> +				addattr16(&req.n, sizeof(req), FRA_DPORT_MASK,
> +					  dport_mask);
>  		} else if (strcmp(*argv, "dscp") == 0) {
>  			__u32 dscp;
>  
> diff --git a/man/man8/ip-rule.8.in b/man/man8/ip-rule.8.in
> index 6fc741d4f470..4945ccd55076 100644
> --- a/man/man8/ip-rule.8.in
> +++ b/man/man8/ip-rule.8.in
> @@ -52,10 +52,10 @@ ip-rule \- routing policy database management
>  .B ipproto
>  .IR PROTOCOL " ] [ "
>  .BR sport " [ "
> -.IR NUMBER " | "
> +.IR NUMBER\fR[\fB/\fIMASK "] | "
>  .IR NUMBER "-" NUMBER " ] ] [ "
>  .BR dport " [ "
> -.IR NUMBER " | "
> +.IR NUMBER\fR[\fB/\fIMASK "] | "
>  .IR NUMBER "-" NUMBER " ] ] [ "
>  .B  tun_id
>  .IR TUN_ID " ] [ "
> @@ -270,12 +270,14 @@ value to match.
>  select the ip protocol value to match.
>  
>  .TP
> -.BI sport " NUMBER | NUMBER-NUMBER"
> -select the source port value to match. supports port range.
> +.BI sport " NUMBER\fR[\fB/\fIMASK\fR] | NUMBER-NUMBER"
> +select the source port value to match with an optional mask. supports port
> +range.

s/supports/Supports/. And below.

>  
>  .TP
> -.BI dport " NUMBER | NUMBER-NUMBER"
> -select the destination port value to match. supports port range.
> +.BI dport " NUMBER\fR[\fB/\fIMASK\fR] | NUMBER-NUMBER"
> +select the destination port value to match with an optional mask. supports port
> +range.
>  
>  .TP
>  .BI priority " PREFERENCE"

Re: [PATCH iproute2-next 4/5] iprule: Add port mask support

[Ido Schimmel]

On Mon, Feb 24, 2025 at 03:03:25PM +0100, Petr Machata wrote:
> Two minor suggestions and a couple notes to self below. Looks OK overall.

Fixed both. Will post v2 tomorrow. Thanks!

[PATCH iproute2-next 5/5] iprule: Add DSCP mask support

[Ido Schimmel]

Add DSCP mask support, allowing users to specify a DSCP value with an
optional mask. Example:

 # ip rule add dscp 1 table 100
 # ip rule add dscp 0x02/0x3f table 200
 # ip rule add dscp AF42/0x3f table 300
 # ip rule add dscp 0x10/0x30 table 400

In non-JSON output, the DSCP mask is not printed in case of exact match
and the DSCP value is printed in hexadecimal format in case of inexact
match:

 $ ip rule show
 0:      from all lookup local
 32762:  from all lookup 400 dscp 0x10/0x30
 32763:  from all lookup 300 dscp AF42
 32764:  from all lookup 200 dscp 2
 32765:  from all lookup 100 dscp 1
 32766:  from all lookup main
 32767:  from all lookup default

Dump can be filtered by DSCP value and mask:

 $ ip rule show dscp 1
 32765:  from all lookup 100 dscp 1
 $ ip rule show dscp AF42
 32763:  from all lookup 300 dscp AF42
 $ ip rule show dscp 0x10/0x30
 32762:  from all lookup 400 dscp 0x10/0x30

In JSON output, the DSCP mask is printed as an hexadecimal string to be
consistent with other masks. The DSCP value is printed as an integer in
order not to break existing scripts:

 $ ip -j -p -N rule show dscp 0x10/0x30
 [ {
         "priority": 32762,
         "src": "all",
         "table": "400",
         "dscp": "16",
         "dscp_mask": "0x30"
     } ]

The mask attribute is only sent to the kernel in case of inexact match
so that iproute2 will continue working with kernels that do not support
the attribute.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 ip/iprule.c           | 73 ++++++++++++++++++++++++++++++++-----------
 man/man8/ip-rule.8.in |  9 +++---
 2 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/ip/iprule.c b/ip/iprule.c
index fbe69a3b6293..d4d57b8c96df 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -24,6 +24,7 @@
 #include "json_print.h"
 
 #define PORT_MAX_MASK 0xFFFF
+#define DSCP_MAX_MASK 0x3F
 
 enum list_action {
 	IPRULE_LIST,
@@ -48,7 +49,7 @@ static void usage(void)
 		"            [ ipproto PROTOCOL ]\n"
 		"            [ sport [ NUMBER[/MASK] | NUMBER-NUMBER ]\n"
 		"            [ dport [ NUMBER[/MASK] | NUMBER-NUMBER ] ]\n"
-		"            [ dscp DSCP ] [ flowlabel FLOWLABEL[/MASK] ]\n"
+		"            [ dscp DSCP[/MASK] ] [ flowlabel FLOWLABEL[/MASK] ]\n"
 		"ACTION := [ table TABLE_ID ]\n"
 		"          [ protocol PROTO ]\n"
 		"          [ nat ADDRESS ]\n"
@@ -238,14 +239,21 @@ static bool filter_nlmsg(struct nlmsghdr *n, struct rtattr **tb, int host_len)
 	}
 
 	if (filter.dscpmask) {
-		if (tb[FRA_DSCP]) {
-			__u8 dscp = rta_getattr_u8(tb[FRA_DSCP]);
+		__u8 dscp_mask = DSCP_MAX_MASK;
+		__u8 dscp;
 
-			if (filter.dscp != dscp)
-				return false;
-		} else {
+		if (!tb[FRA_DSCP])
+			return false;
+
+		dscp = rta_getattr_u8(tb[FRA_DSCP]);
+		if (filter.dscp != dscp)
+			return false;
+
+		if (tb[FRA_DSCP_MASK])
+			dscp_mask = rta_getattr_u8(tb[FRA_DSCP_MASK]);
+
+		if (filter.dscpmask != dscp_mask)
 			return false;
-		}
 	}
 
 	if (filter.flowlabel_mask) {
@@ -552,8 +560,24 @@ int print_rule(struct nlmsghdr *n, void *arg)
 	if (tb[FRA_DSCP]) {
 		__u8 dscp = rta_getattr_u8(tb[FRA_DSCP]);
 
-		print_string(PRINT_ANY, "dscp", " dscp %s",
-			     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
+		if (tb[FRA_DSCP_MASK]) {
+			__u8 mask = rta_getattr_u8(tb[FRA_DSCP_MASK]);
+
+			print_string(PRINT_JSON, "dscp", NULL,
+				     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
+			print_0xhex(PRINT_JSON, "dscp_mask", NULL, mask);
+			if (mask == DSCP_MAX_MASK) {
+				print_string(PRINT_FP, NULL, " dscp %s",
+					     rtnl_dscp_n2a(dscp, b1,
+							   sizeof(b1)));
+			} else {
+				print_0xhex(PRINT_FP, NULL, " dscp %#x", dscp);
+				print_0xhex(PRINT_FP, NULL, "/%#x", mask);
+			}
+		} else {
+			print_string(PRINT_ANY, "dscp", " dscp %s",
+				     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
+		}
 	}
 
 	/* The kernel will either provide both attributes, or none */
@@ -694,6 +718,21 @@ static void iprule_port_parse(char *arg, struct fib_rule_port_range *r,
 	r->end = r->start;
 }
 
+static void iprule_dscp_parse(char *arg, __u32 *dscp, __u32 *mask)
+{
+	char *slash;
+
+	*mask = DSCP_MAX_MASK;
+
+	slash = strchr(arg, '/');
+	if (slash != NULL)
+		*slash = '\0';
+	if (rtnl_dscp_a2n(dscp, arg))
+		invarg("invalid dscp", arg);
+	if (slash && get_u32(mask, slash + 1, 0))
+		invarg("invalid dscp mask", slash + 1);
+}
+
 static void iprule_flowlabel_parse(char *arg, __u32 *flowlabel,
 				   __u32 *flowlabel_mask)
 {
@@ -848,13 +887,9 @@ static int iprule_list_flush_or_save(int argc, char **argv, int action)
 			iprule_port_parse(*argv, &filter.dport,
 					  &filter.dport_mask);
 		} else if (strcmp(*argv, "dscp") == 0) {
-			__u32 dscp;
-
 			NEXT_ARG();
-			if (rtnl_dscp_a2n(&dscp, *argv))
-				invarg("invalid dscp\n", *argv);
-			filter.dscp = dscp;
-			filter.dscpmask = 1;
+			iprule_dscp_parse(*argv, &filter.dscp,
+					  &filter.dscpmask);
 		} else if (strcmp(*argv, "flowlabel") == 0) {
 			NEXT_ARG();
 
@@ -1137,12 +1172,14 @@ static int iprule_modify(int cmd, int argc, char **argv)
 				addattr16(&req.n, sizeof(req), FRA_DPORT_MASK,
 					  dport_mask);
 		} else if (strcmp(*argv, "dscp") == 0) {
-			__u32 dscp;
+			__u32 dscp, dscp_mask;
 
 			NEXT_ARG();
-			if (rtnl_dscp_a2n(&dscp, *argv))
-				invarg("invalid dscp\n", *argv);
+			iprule_dscp_parse(*argv, &dscp, &dscp_mask);
 			addattr8(&req.n, sizeof(req), FRA_DSCP, dscp);
+			if (dscp_mask != DSCP_MAX_MASK)
+				addattr8(&req.n, sizeof(req), FRA_DSCP_MASK,
+					 dscp_mask);
 		} else if (strcmp(*argv, "flowlabel") == 0) {
 			__u32 flowlabel, flowlabel_mask;
 
diff --git a/man/man8/ip-rule.8.in b/man/man8/ip-rule.8.in
index 4945ccd55076..7d6a82b2c492 100644
--- a/man/man8/ip-rule.8.in
+++ b/man/man8/ip-rule.8.in
@@ -37,7 +37,7 @@ ip-rule \- routing policy database management
 .B  tos
 .IR TOS " ] [ "
 .B  dscp
-.IR DSCP " ] [ "
+.IR DSCP\fR[\fB/\fIMASK "] ] [ "
 .B  fwmark
 .IR FWMARK\fR[\fB/\fIMASK "] ] [ "
 .B  iif
@@ -239,9 +239,10 @@ a device.
 select the TOS value to match.
 
 .TP
-.BI dscp " DSCP"
-select the DSCP value to match. DSCP values can be written either directly as
-numeric values (valid values are 0-63), or using symbolic names specified in
+.BI dscp " DSCP\fR[\fB/\fIMASK\fR]"
+select the DSCP value to match with an optional mask. DSCP values can be
+written either directly as numeric values (valid values are 0-63), or using
+symbolic names specified in
 .BR @SYSCONF_USR_DIR@/rt_dsfield " or " @SYSCONF_ETC_DIR@/rt_dsfield
 (has precedence if exists).
 However, note that the file specifies full 8-bit dsfield values, whereas
-- 
2.48.1

Re: [PATCH iproute2-next 5/5] iprule: Add DSCP mask support

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> Add DSCP mask support, allowing users to specify a DSCP value with an
> optional mask. Example:
>
>  # ip rule add dscp 1 table 100
>  # ip rule add dscp 0x02/0x3f table 200
>  # ip rule add dscp AF42/0x3f table 300
>  # ip rule add dscp 0x10/0x30 table 400
>
> In non-JSON output, the DSCP mask is not printed in case of exact match
> and the DSCP value is printed in hexadecimal format in case of inexact
> match:
>
>  $ ip rule show
>  0:      from all lookup local
>  32762:  from all lookup 400 dscp 0x10/0x30
>  32763:  from all lookup 300 dscp AF42
>  32764:  from all lookup 200 dscp 2
>  32765:  from all lookup 100 dscp 1
>  32766:  from all lookup main
>  32767:  from all lookup default
>
> Dump can be filtered by DSCP value and mask:
>
>  $ ip rule show dscp 1
>  32765:  from all lookup 100 dscp 1
>  $ ip rule show dscp AF42
>  32763:  from all lookup 300 dscp AF42
>  $ ip rule show dscp 0x10/0x30
>  32762:  from all lookup 400 dscp 0x10/0x30
>
> In JSON output, the DSCP mask is printed as an hexadecimal string to be
> consistent with other masks. The DSCP value is printed as an integer in
> order not to break existing scripts:
>
>  $ ip -j -p -N rule show dscp 0x10/0x30
>  [ {
>          "priority": 32762,
>          "src": "all",
>          "table": "400",
>          "dscp": "16",
>          "dscp_mask": "0x30"
>      } ]
>
> The mask attribute is only sent to the kernel in case of inexact match
> so that iproute2 will continue working with kernels that do not support
> the attribute.
>
> Signed-off-by: Ido Schimmel <idosch@nvidia.com>

Reviewed-by: Petr Machata <petrm@nvidia.com>

> @@ -552,8 +560,24 @@ int print_rule(struct nlmsghdr *n, void *arg)
>  	if (tb[FRA_DSCP]) {
>  		__u8 dscp = rta_getattr_u8(tb[FRA_DSCP]);
>  
> -		print_string(PRINT_ANY, "dscp", " dscp %s",
> -			     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));

Hm, this should have been an integer under -N. Too late for that :-/

> +		if (tb[FRA_DSCP_MASK]) {
> +			__u8 mask = rta_getattr_u8(tb[FRA_DSCP_MASK]);
> +
> +			print_string(PRINT_JSON, "dscp", NULL,
> +				     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
> +			print_0xhex(PRINT_JSON, "dscp_mask", NULL, mask);
> +			if (mask == DSCP_MAX_MASK) {
> +				print_string(PRINT_FP, NULL, " dscp %s",
> +					     rtnl_dscp_n2a(dscp, b1,
> +							   sizeof(b1)));
> +			} else {
> +				print_0xhex(PRINT_FP, NULL, " dscp %#x", dscp);
> +				print_0xhex(PRINT_FP, NULL, "/%#x", mask);
> +			}
> +		} else {
> +			print_string(PRINT_ANY, "dscp", " dscp %s",
> +				     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
> +		}
>  	}
>  
>  	/* The kernel will either provide both attributes, or none */

Re: [PATCH iproute2-next 5/5] iprule: Add DSCP mask support

[Ido Schimmel]

On Mon, Feb 24, 2025 at 03:35:51PM +0100, Petr Machata wrote:
> 
> Ido Schimmel <idosch@nvidia.com> writes:
> 
> > Add DSCP mask support, allowing users to specify a DSCP value with an
> > optional mask. Example:
> >
> >  # ip rule add dscp 1 table 100
> >  # ip rule add dscp 0x02/0x3f table 200
> >  # ip rule add dscp AF42/0x3f table 300
> >  # ip rule add dscp 0x10/0x30 table 400
> >
> > In non-JSON output, the DSCP mask is not printed in case of exact match
> > and the DSCP value is printed in hexadecimal format in case of inexact
> > match:
> >
> >  $ ip rule show
> >  0:      from all lookup local
> >  32762:  from all lookup 400 dscp 0x10/0x30
> >  32763:  from all lookup 300 dscp AF42
> >  32764:  from all lookup 200 dscp 2
> >  32765:  from all lookup 100 dscp 1
> >  32766:  from all lookup main
> >  32767:  from all lookup default
> >
> > Dump can be filtered by DSCP value and mask:
> >
> >  $ ip rule show dscp 1
> >  32765:  from all lookup 100 dscp 1
> >  $ ip rule show dscp AF42
> >  32763:  from all lookup 300 dscp AF42
> >  $ ip rule show dscp 0x10/0x30
> >  32762:  from all lookup 400 dscp 0x10/0x30
> >
> > In JSON output, the DSCP mask is printed as an hexadecimal string to be
> > consistent with other masks. The DSCP value is printed as an integer in
> > order not to break existing scripts:
> >
> >  $ ip -j -p -N rule show dscp 0x10/0x30
> >  [ {
> >          "priority": 32762,
> >          "src": "all",
> >          "table": "400",
> >          "dscp": "16",
> >          "dscp_mask": "0x30"
> >      } ]
> >
> > The mask attribute is only sent to the kernel in case of inexact match
> > so that iproute2 will continue working with kernels that do not support
> > the attribute.
> >
> > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
> 
> Reviewed-by: Petr Machata <petrm@nvidia.com>
> 
> > @@ -552,8 +560,24 @@ int print_rule(struct nlmsghdr *n, void *arg)
> >  	if (tb[FRA_DSCP]) {
> >  		__u8 dscp = rta_getattr_u8(tb[FRA_DSCP]);
> >  
> > -		print_string(PRINT_ANY, "dscp", " dscp %s",
> > -			     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
> 
> Hm, this should have been an integer under -N. Too late for that :-/

I assume you mean 16 vs "16" in the last example? It is a deliberate
decision:

https://lore.kernel.org/netdev/d3cd276a-b3b0-4ccc-9b51-dbedd841d7af@kernel.org/

Re: [PATCH iproute2-next 5/5] iprule: Add DSCP mask support

[Petr Machata]

Ido Schimmel <idosch@nvidia.com> writes:

> On Mon, Feb 24, 2025 at 03:35:51PM +0100, Petr Machata wrote:
>> 
>> Ido Schimmel <idosch@nvidia.com> writes:
>> 
>> > Add DSCP mask support, allowing users to specify a DSCP value with an
>> > optional mask. Example:
>> >
>> >  # ip rule add dscp 1 table 100
>> >  # ip rule add dscp 0x02/0x3f table 200
>> >  # ip rule add dscp AF42/0x3f table 300
>> >  # ip rule add dscp 0x10/0x30 table 400
>> >
>> > In non-JSON output, the DSCP mask is not printed in case of exact match
>> > and the DSCP value is printed in hexadecimal format in case of inexact
>> > match:
>> >
>> >  $ ip rule show
>> >  0:      from all lookup local
>> >  32762:  from all lookup 400 dscp 0x10/0x30
>> >  32763:  from all lookup 300 dscp AF42
>> >  32764:  from all lookup 200 dscp 2
>> >  32765:  from all lookup 100 dscp 1
>> >  32766:  from all lookup main
>> >  32767:  from all lookup default
>> >
>> > Dump can be filtered by DSCP value and mask:
>> >
>> >  $ ip rule show dscp 1
>> >  32765:  from all lookup 100 dscp 1
>> >  $ ip rule show dscp AF42
>> >  32763:  from all lookup 300 dscp AF42
>> >  $ ip rule show dscp 0x10/0x30
>> >  32762:  from all lookup 400 dscp 0x10/0x30
>> >
>> > In JSON output, the DSCP mask is printed as an hexadecimal string to be
>> > consistent with other masks. The DSCP value is printed as an integer in
>> > order not to break existing scripts:
>> >
>> >  $ ip -j -p -N rule show dscp 0x10/0x30
>> >  [ {
>> >          "priority": 32762,
>> >          "src": "all",
>> >          "table": "400",
>> >          "dscp": "16",
>> >          "dscp_mask": "0x30"
>> >      } ]
>> >
>> > The mask attribute is only sent to the kernel in case of inexact match
>> > so that iproute2 will continue working with kernels that do not support
>> > the attribute.
>> >
>> > Signed-off-by: Ido Schimmel <idosch@nvidia.com>
>> 
>> Reviewed-by: Petr Machata <petrm@nvidia.com>
>> 
>> > @@ -552,8 +560,24 @@ int print_rule(struct nlmsghdr *n, void *arg)
>> >  	if (tb[FRA_DSCP]) {
>> >  		__u8 dscp = rta_getattr_u8(tb[FRA_DSCP]);
>> >  
>> > -		print_string(PRINT_ANY, "dscp", " dscp %s",
>> > -			     rtnl_dscp_n2a(dscp, b1, sizeof(b1)));
>> 
>> Hm, this should have been an integer under -N. Too late for that :-/
>
> I assume you mean 16 vs "16" in the last example? It is a deliberate
> decision:
>
> https://lore.kernel.org/netdev/d3cd276a-b3b0-4ccc-9b51-dbedd841d7af@kernel.org/

Yeah, that's what I meant. I agree that if `ip` has it like that
throughout, it's better to respect it.