[PATCH] iproute2 -- add fwmarkmask

[PATCH] iproute2 -- add fwmarkmask

[Michael Richardson]

[-- Attachment #1: Type: text/plain, Size: 3676 bytes --]


{see discussion about RTA_FWMARK in other posting}

diff-tree 573e55d29ef2f014218508d01eee678d82c7c9b7 (from cb420780b79a1d6f5db2082bdac0b310dbc5ebcc)
Author: Michael Richardson <mcr@xelerance.com>
Date:   Mon Feb 20 10:21:05 2006 -0500

    this patch adds a mask operation to the fwmark rule case, permitting
    one to test for only certain bits being set in the fwmark.
    (cherry picked from 5fb85fa9da78cc3822f929f181c651391e35de73 commit)

diff --git a/ChangeLog b/ChangeLog
index 68d2342..b67b81d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,9 @@
+2006-02-20  Michael Richardson <mcr@xelerance.com>
+
+	* Added fwmarkmask option to ip rule to go with addition to kernel.
+	
 2006-01-12  Patrick McHardy <kaber@trash.net>
 
 	* Handle DCCP in ipxfrm.c to allow using port numbers in the selector.
 
 2006-01-10  Masahide NAKAMURA <nakam@linux-ipv6.org>
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index ced29f2..7f4a258 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -263,10 +263,13 @@ enum rtattr_type_t
 	RTA_SESSION,
 	RTA_MP_ALGO,
 	__RTA_MAX
 };
 
+#define RTA_FWMARK      RTA_PROTOINFO
+#define RTA_FWMARK_MASK RTA_CACHEINFO
+
 #define RTA_MAX (__RTA_MAX - 1)
 
 #define RTM_RTA(r)  ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct rtmsg))))
 #define RTM_PAYLOAD(n) NLMSG_PAYLOAD(n,sizeof(struct rtmsg))
 
diff --git a/ip/iprule.c b/ip/iprule.c
index ccf699f..90ffcce 100644
--- a/ip/iprule.c
+++ b/ip/iprule.c
@@ -33,11 +33,11 @@ extern struct rtnl_handle rth;
 static void usage(void) __attribute__((noreturn));
 
 static void usage(void)
 {
 	fprintf(stderr, "Usage: ip rule [ list | add | del | flush ] SELECTOR ACTION\n");
-	fprintf(stderr, "SELECTOR := [ from PREFIX ] [ to PREFIX ] [ tos TOS ] [ fwmark FWMARK ]\n");
+	fprintf(stderr, "SELECTOR := [ from PREFIX ] [ to PREFIX ] [ tos TOS ] [ fwmark FWMARK ] [ fwmarkmask MASK ]\n");
 	fprintf(stderr, "            [ dev STRING ] [ pref NUMBER ]\n");
 	fprintf(stderr, "ACTION := [ table TABLE_ID ]\n");
 	fprintf(stderr, "          [ prohibit | reject | unreachable ]\n");
 	fprintf(stderr, "          [ realms [SRCREALM/]DSTREALM ]\n");
 	fprintf(stderr, "TABLE_ID := [ local | main | default | NUMBER ]\n");
@@ -119,14 +119,22 @@ static int print_rule(const struct socka
 
 	if (r->rtm_tos) {
 		SPRINT_BUF(b1);
 		fprintf(fp, "tos %s ", rtnl_dsfield_n2a(r->rtm_tos, b1, sizeof(b1)));
 	}
-	if (tb[RTA_PROTOINFO]) {
-		fprintf(fp, "fwmark %#x ", *(__u32*)RTA_DATA(tb[RTA_PROTOINFO]));
+	if (tb[RTA_FWMARK]) {
+		__u32 value = *(__u32*)RTA_DATA(tb[RTA_PROTOINFO]);
+		if (tb[RTA_FWMARK_MASK]) {
+			__u32 mask;
+			mask=*(__u32*)RTA_DATA(tb[RTA_FWMARK_MASK]);
+			fprintf(fp, "fwmark %#x&%#x ", value, mask);
+		} else {
+			fprintf(fp, "fwmark %#x&0xffffffff ", value);
+		}
 	}
 
+
 	if (tb[RTA_IIF]) {
 		fprintf(fp, "iif %s ", (char*)RTA_DATA(tb[RTA_IIF]));
 	}
 
 	if (r->rtm_table)
@@ -242,11 +250,17 @@ static int iprule_modify(int cmd, int ar
 		} else if (strcmp(*argv, "fwmark") == 0) {
 			__u32 fwmark;
 			NEXT_ARG();
 			if (get_u32(&fwmark, *argv, 0))
 				invarg("fwmark value is invalid\n", *argv);
-			addattr32(&req.n, sizeof(req), RTA_PROTOINFO, fwmark);
+			addattr32(&req.n, sizeof(req), RTA_FWMARK, fwmark);
+		} else if (strcmp(*argv, "fwmarkmask") == 0) {
+			__u32 fwmarkmask;
+			NEXT_ARG();
+			if (get_u32(&fwmarkmask, *argv, 0))
+				invarg("fwmarkmask value is invalid\n", *argv);
+			addattr32(&req.n, sizeof(req), RTA_FWMARK_MASK, fwmarkmask);
 		} else if (matches(*argv, "realms") == 0) {
 			__u32 realm;
 			NEXT_ARG();
 			if (get_rt_realms(&realm, *argv))
 				invarg("invalid realms\n", *argv);




[-- Attachment #2: Type: application/pgp-signature, Size: 480 bytes --]

Re: [PATCH] iproute2 -- add fwmarkmask

[Patrick McHardy]

Michael Richardson wrote:
> -	if (tb[RTA_PROTOINFO]) {
> -		fprintf(fp, "fwmark %#x ", *(__u32*)RTA_DATA(tb[RTA_PROTOINFO]));
> +	if (tb[RTA_FWMARK]) {
> +		__u32 value = *(__u32*)RTA_DATA(tb[RTA_PROTOINFO]);
> +		if (tb[RTA_FWMARK_MASK]) {
> +			__u32 mask;
> +			mask=*(__u32*)RTA_DATA(tb[RTA_FWMARK_MASK]);
> +			fprintf(fp, "fwmark %#x&%#x ", value, mask);

The normal way to display masks is with a "/". Also I think it shouldn't
display the default mask to avoid breaking scripts that parse the
output.
> +		} else if (strcmp(*argv, "fwmarkmask") == 0) {
> +			__u32 fwmarkmask;
> +			NEXT_ARG();
> +			if (get_u32(&fwmarkmask, *argv, 0))
> +				invarg("fwmarkmask value is invalid\n", *argv);
> +			addattr32(&req.n, sizeof(req), RTA_FWMARK_MASK, fwmarkmask);

ip should be able to parse its own output, and it would also look nicer
if I could just say "fwmark 0x1/32". fwmarkmask is really an incredible
ugly expression :)

Re: [PATCH] iproute2 -- add fwmarkmask

[Michael Richardson]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


>>>>> "Patrick" == Patrick McHardy <kaber@trash.net> writes:
    Patrick> The normal way to display masks is with a "/". Also I think
    Patrick> it shouldn't display the default mask to avoid breaking
    Patrick> scripts that parse the output.

  I generally dislike the /VALUE, since I expect /PREFIX-LEN.
  I agree that it shouldn't show if it is default.

    Patrick> ip should be able to parse its own output, and it would
    Patrick> also look nicer if I could just say "fwmark
    Patrick> 0x1/32". fwmarkmask is really an incredible ugly expression
    Patrick> :)

  Sure. Is that a 32-bit long mask (0xfffffff), or is it a 0x00000020?
  fwmark is not an address.

  Or would you like /32 to be a prefix-based mask, and &value and/or
fwmarkmask to be a value? 

- -- 
]       ON HUMILITY: to err is human. To moo, bovine.           |  firewalls  [
]   Michael Richardson,    Xelerance Corporation, Ottawa, ON    |net architect[
] mcr@xelerance.com      http://www.sandelman.ottawa.on.ca/mcr/ |device driver[
] panic("Just another Debian GNU/Linux using, kernel hacking, security guy"); [
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)
Comment: Finger me for keys

iQEUAwUBQ/4PcoCLcPvd0N1lAQIHhQf3XzPLA91QEx2+XpmYIm8RyB1oKmUUXDP+
s2UrhOKbQwipcq8/hk1t4FKx8J5j/dFHzVXbgPK+ZUwX4+IjHmM3r0sCIcK08xwU
/ZZjf0wqwUI+RcPRFw3zC0+hnwRUIAUxhl3p7h3PigDpPu7AY5tQ1dXc6WNwRjTi
fS7Yozbo225dzvVLKHhSIqOQ4eJFJcPPQdTKQLxnc3gtVoSe41DKMM+x6uix6fG8
se9dngJRbhye1Xgws9AGnBQT9f7JVmCSv7V4SHnNynmnRw3cra8++QEnLZ/vhm5C
JdeVSeDGxAPuKEj6HA2RZu/UOG6RkYNZGPovGKzuPn403x0HNBuf
=BzfV
-----END PGP SIGNATURE-----

Re: [PATCH] iproute2 -- add fwmarkmask

[Patrick McHardy]

Michael Richardson wrote:
> 
> 
>>>>>>>"Patrick" == Patrick McHardy <kaber@trash.net> writes:
> 
>     Patrick> The normal way to display masks is with a "/". Also I think
>     Patrick> it shouldn't display the default mask to avoid breaking
>     Patrick> scripts that parse the output.
> 
>   I generally dislike the /VALUE, since I expect /PREFIX-LEN.
>   I agree that it shouldn't show if it is default.
> 
>     Patrick> ip should be able to parse its own output, and it would
>     Patrick> also look nicer if I could just say "fwmark
>     Patrick> 0x1/32". fwmarkmask is really an incredible ugly expression
>     Patrick> :)
> 
>   Sure. Is that a 32-bit long mask (0xfffffff), or is it a 0x00000020?
>   fwmark is not an address.
> 
>   Or would you like /32 to be a prefix-based mask, and &value and/or
> fwmarkmask to be a value? 

That was not the greatest example :) I think it should be a bitmask.