[PATCH 02/02] add mask options to fwmark masking code

[PATCH 02/02] add mask options to fwmark masking code

[Michael Richardson]

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


[PATCH] This patch introduces a mask to the fwmark test cases in the advanced
routing. This let's one test individual bits of the fwmark to determine
how things should be routed (pick a routing table). This patch retains
compatibility with tests that do not set the mask by assuming a mask
of 0 is equivalent to a mask of 0xffffffff.

Sign-off-by: Michael Richardson <mcr@xelerance.com>


---

 include/linux/rtnetlink.h |    1 +
 net/ipv4/fib_rules.c      |   19 +++++++++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

bcdda64a16d4dfda6d95452bbf8541999121831a
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index 27fd17e..a5b55c2 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -266,6 +266,7 @@ enum rtattr_type_t
 };
 
 #define RTA_FWMARK      RTA_PROTOINFO
+#define RTA_FWMARK_MASK RTA_CACHEINFO
 
 #define RTA_MAX (__RTA_MAX - 1)
 
diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
index de327b3..69eed89 100644
--- a/net/ipv4/fib_rules.c
+++ b/net/ipv4/fib_rules.c
@@ -68,6 +68,7 @@ struct fib_rule
 	u8		r_tos;
 #ifdef CONFIG_IP_ROUTE_FWMARK
 	u32		r_fwmark;
+        u32             r_fwmark_mask;
 #endif
 	int		r_ifindex;
 #ifdef CONFIG_NET_CLS_ROUTE
@@ -117,6 +118,7 @@ int inet_rtm_delrule(struct sk_buff *skb
 		    rtm->rtm_tos == r->r_tos &&
 #ifdef CONFIG_IP_ROUTE_FWMARK
 		    (!rta[RTA_FWMARK-1] || memcmp(RTA_DATA(rta[RTA_FWMARK-1]), &r->r_fwmark, 4) == 0) &&
+		    (!rta[RTA_FWMARK_MASK-1] || memcmp(RTA_DATA(rta[RTA_FWMARK_MASK-1]), &r->r_fwmark_mask, 4) == 0) &&
 #endif
 		    (!rtm->rtm_type || rtm->rtm_type == r->r_action) &&
 		    (!rta[RTA_PRIORITY-1] || memcmp(RTA_DATA(rta[RTA_PRIORITY-1]), &r->r_preference, 4) == 0) &&
@@ -202,6 +204,17 @@ int inet_rtm_newrule(struct sk_buff *skb
 #ifdef CONFIG_IP_ROUTE_FWMARK
 	if (rta[RTA_FWMARK-1]) 
 		memcpy(&new_r->r_fwmark,      RTA_DATA(rta[RTA_FWMARK-1]), 4);
+	if (rta[RTA_FWMARK_MASK-1]) 
+		memcpy(&new_r->r_fwmark_mask, RTA_DATA(rta[RTA_FWMARK_MASK-1]), 4);
+	/*
+	 * if the user doesn't set a mask, then set it to care about
+	 * all bits. This retains compatibility. Note it is impossible
+	 * to match SOMETHING & 0xMASK == 0, because fwmark==0 means
+	 * do not match fwmark at all.
+	 */
+	if(new_r->r_fwmark_mask == 0) {
+		new_r->r_fwmark_mask = 0xffffffff;
+	}
 #endif
 	new_r->r_action = rtm->rtm_type;
 	new_r->r_flags = rtm->rtm_flags;
@@ -298,7 +311,7 @@ FRprintk("Lookup: %u.%u.%u.%u <- %u.%u.%
 		    ((daddr^r->r_dst) & r->r_dstmask) ||
 		    (r->r_tos && r->r_tos != flp->fl4_tos) ||
 #ifdef CONFIG_IP_ROUTE_FWMARK
-		    (r->r_fwmark && r->r_fwmark != flp->fl4_fwmark) ||
+		    (r->r_fwmark && r->r_fwmark != (flp->fl4_fwmark & r->r_fwmark_mask)) ||
 #endif
 		    (r->r_ifindex && r->r_ifindex != flp->iif))
 			continue;
@@ -382,8 +395,10 @@ static __inline__ int inet_fill_rule(str
 	rtm->rtm_src_len = r->r_src_len;
 	rtm->rtm_tos = r->r_tos;
 #ifdef CONFIG_IP_ROUTE_FWMARK
-	if (r->r_fwmark) 
+	if (r->r_fwmark) {
 		RTA_PUT(skb, RTA_FWMARK, 4, &r->r_fwmark);
+		RTA_PUT(skb, RTA_FWMARK_MASK, 4, &r->r_fwmark_mask);
+	}
 #endif
 	rtm->rtm_table = r->r_table;
 	rtm->rtm_protocol = 0;
-- 





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

Re: [PATCH 02/02] add mask options to fwmark masking code

[Patrick McHardy]

Michael Richardson wrote:
> [PATCH] This patch introduces a mask to the fwmark test cases in the advanced
> routing. This let's one test individual bits of the fwmark to determine
> how things should be routed (pick a routing table). This patch retains
> compatibility with tests that do not set the mask by assuming a mask
> of 0 is equivalent to a mask of 0xffffffff.


> bcdda64a16d4dfda6d95452bbf8541999121831a
> diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
> index 27fd17e..a5b55c2 100644
> --- a/include/linux/rtnetlink.h
> +++ b/include/linux/rtnetlink.h
> @@ -266,6 +266,7 @@ enum rtattr_type_t
>  };
>  
>  #define RTA_FWMARK      RTA_PROTOINFO
> +#define RTA_FWMARK_MASK RTA_CACHEINFO

Please introduce a new attribute for this instead of overloading
RTA_CACHEINFO.

> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c
> index de327b3..69eed89 100644
> --- a/net/ipv4/fib_rules.c
> +++ b/net/ipv4/fib_rules.c
> @@ -68,6 +68,7 @@ struct fib_rule
>  	u8		r_tos;
>  #ifdef CONFIG_IP_ROUTE_FWMARK
>  	u32		r_fwmark;
> +        u32             r_fwmark_mask;

Both patches have whitespace issues. You should also change decnet,
which also supports routing by fwmark. Other than that the patch
looks fine, in fact its nearly identical to a patch I wanted to
send soon which does the same :)

Re: [PATCH 02/02] add mask options to fwmark masking code

[Patrick McHardy]

Michael Richardson wrote:
> [PATCH] This patch introduces a mask to the fwmark test cases in the advanced
> routing. This let's one test individual bits of the fwmark to determine
> how things should be routed (pick a routing table). This patch retains
> compatibility with tests that do not set the mask by assuming a mask
> of 0 is equivalent to a mask of 0xffffffff.

One last comment: instead of looking at the value you should check if
the mask attribute is present to decide when to use the compat mask.

Re: [PATCH 02/02] add mask options to fwmark masking code

[Carl-Daniel Hailfinger]

Michael Richardson schrieb:
> [PATCH] This patch introduces a mask to the fwmark test cases in the advanced
> routing. This let's one test individual bits of the fwmark to determine
> how things should be routed (pick a routing table). This patch retains
> compatibility with tests that do not set the mask by assuming a mask
> of 0 is equivalent to a mask of 0xffffffff.

Sorry if I misunderstood the intention of your patch, but isn't similar code
already in mainline?

linux-2.6.16-rc3/net/sched/cls_u32.c:146
#ifdef CONFIG_CLS_U32_MARK
                if ((skb->nfmark & n->mark.mask) != n->mark.val) {


Regards,
Carl-Daniel
-- 
http://www.hailfinger.org/

Re: [PATCH 02/02] add mask options to fwmark masking code

[Michael Richardson]

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


>>>>> "Patrick" == Patrick McHardy <kaber@trash.net> writes:
    >> #define RTA_FWMARK RTA_PROTOINFO +#define RTA_FWMARK_MASK
    >> RTA_CACHEINFO

    Patrick> Please introduce a new attribute for this instead of
    Patrick> overloading RTA_CACHEINFO.

  I would be happy to do that.
  Should I also un-overload FWMARK, with backwards compatibility?

    >> diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index
    >> de327b3..69eed89 100644 --- a/net/ipv4/fib_rules.c +++
    >> b/net/ipv4/fib_rules.c @@ -68,6 +68,7 @@ struct fib_rule u8
    >> r_tos; #ifdef CONFIG_IP_ROUTE_FWMARK u32 r_fwmark; + u32
    >> r_fwmark_mask;

    Patrick> Both patches have whitespace issues. You should also change

  uhm. okay.
  I'm surprised, since I produced it with git-format-patch. Maybe there
are tabs that emacs screwed up.

- -- 
]       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

iQEVAwUBQ/4O2ICLcPvd0N1lAQK/egf6A0iQ1hvecR4BeaCrQiu53beGZd6zHldk
o6logfar94kPP/H/D/kMcNeAvL2a3cJ8wyfyP02Cav8gP1C3X+XV+yLtA9jHIrdK
nqQ1gw7F4Cj2+v7du/jS8GxNMWevXhJ7f9hvnzh8+DHMUCjqiksgsuIgcRQYrqOQ
vxYERvR5TojEIaJfg8kH/lJRn3sm/APuMphM6c6SAeqrWpAdijbZb4LSNpGH50ci
nNhUp+FxoP8vVFTMTu7M1MK4fpCIWA/PxBkmy3YDhcQx1+mE2nrEqHdbKfx9uY+t
0mxR8UC5sthhn94/VCjcqWOoHe3S/Gi+WWoPtwN1sFe5BujwU7Vcfw==
=yKIA
-----END PGP SIGNATURE-----

Re: [PATCH 02/02] add mask options to fwmark masking code

[Patrick McHardy]

Michael Richardson wrote:
> 
> 
>>>>>>>"Patrick" == Patrick McHardy <kaber@trash.net> writes:
> 
>     >> #define RTA_FWMARK RTA_PROTOINFO +#define RTA_FWMARK_MASK
>     >> RTA_CACHEINFO
> 
>     Patrick> Please introduce a new attribute for this instead of
>     Patrick> overloading RTA_CACHEINFO.
> 
>   I would be happy to do that.
>   Should I also un-overload FWMARK, with backwards compatibility?

No, that one is fine since it doesn't already have a different meaning.