* [PATCH] fib_rules: add minimum prefix length @ 2013-07-23 22:02 Stefan Tomanek 2013-07-23 22:38 ` Stephen Hemminger 2013-07-24 2:14 ` Hannes Frederic Sowa 0 siblings, 2 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-23 22:02 UTC (permalink / raw) To: netdev This change adds a minimum prefix length to the structures of routing rules. If a rule is added with a minimum prefix length >0, only routes meeting this threshold will be considered. Any other (more general) routing table entries will be ignored. When configuring a system with multiple network uplinks and default routes, it is often convinient to reference the main routing table multiple times - but omitting the default route. Using this patch and a modified "ip" utility, this can be achieved by using the following command sequence: $ ip route add table secuplink default via 10.42.23.1 $ ip rule add pref 100 table main prefixlength 1 $ ip rule add pref 150 fwmark 0xA table secuplink With this setup, packets marked 0xA will be processed by the additional routing table "secuplink", but only if no suitable route in the main routing table can be found. By using a minimal prefixlength of 1, the default route (/0) of the table "main" is hidden to packets processed by rule 100; packets traveling to destinations with more specific routing entries are processed as usual. Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- include/net/fib_rules.h | 1 + include/uapi/linux/fib_rules.h | 2 +- net/core/fib_rules.c | 5 +++++ net/ipv4/fib_rules.c | 11 ++++++++++- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h index e361f48..b098a9d 100644 --- a/include/net/fib_rules.h +++ b/include/net/fib_rules.h @@ -18,6 +18,7 @@ struct fib_rule { u32 pref; u32 flags; u32 table; + u8 table_prefixlen_min; u8 action; u32 target; struct fib_rule __rcu *ctarget; diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h index 51da65b..59cd31b 100644 --- a/include/uapi/linux/fib_rules.h +++ b/include/uapi/linux/fib_rules.h @@ -45,7 +45,7 @@ enum { FRA_FLOW, /* flow/class id */ FRA_UNUSED6, FRA_UNUSED7, - FRA_UNUSED8, + FRA_TABLE_PREFIXLEN_MIN, FRA_TABLE, /* Extended table id */ FRA_FWMASK, /* mask for netfilter mark */ FRA_OIFNAME, diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 2173544..8a15621 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -337,6 +337,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) rule->action = frh->action; rule->flags = frh->flags; rule->table = frh_get_table(frh, tb); + if (tb[FRA_TABLE_PREFIXLEN_MIN]) + rule->table_prefixlen_min = nla_get_u8(tb[FRA_TABLE_PREFIXLEN_MIN]); if (!tb[FRA_PRIORITY] && ops->default_pref) rule->pref = ops->default_pref(ops); @@ -523,6 +525,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops, + nla_total_size(IFNAMSIZ) /* FRA_OIFNAME */ + nla_total_size(4) /* FRA_PRIORITY */ + nla_total_size(4) /* FRA_TABLE */ + + nla_total_size(1) /* FRA_TABLE_PREFIXLEN_MIN */ + nla_total_size(4) /* FRA_FWMARK */ + nla_total_size(4); /* FRA_FWMASK */ @@ -548,6 +551,8 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule, frh->table = rule->table; if (nla_put_u32(skb, FRA_TABLE, rule->table)) goto nla_put_failure; + if (nla_put_u8(skb, FRA_TABLE_PREFIXLEN_MIN, rule->table_prefixlen_min)) + goto nla_put_failure; frh->res1 = 0; frh->res2 = 0; frh->action = rule->action; diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index 26aa65d..94e9051 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -95,8 +95,17 @@ static int fib4_rule_action(struct fib_rule *rule, struct flowi *flp, goto errout; err = fib_table_lookup(tbl, &flp->u.ip4, (struct fib_result *) arg->result, arg->flags); - if (err > 0) + if (err > 0) { err = -EAGAIN; + goto errout; + } + /* do not accept result if the route does not meet the required prefix length */ + if (arg->result) { + if (((struct fib_result *)arg->result)->prefixlen < rule->table_prefixlen_min) { + err = -EAGAIN; + goto errout; + } + } errout: return err; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-23 22:02 [PATCH] fib_rules: add minimum prefix length Stefan Tomanek @ 2013-07-23 22:38 ` Stephen Hemminger 2013-07-23 22:52 ` Stefan Tomanek 2013-07-24 2:14 ` Hannes Frederic Sowa 1 sibling, 1 reply; 33+ messages in thread From: Stephen Hemminger @ 2013-07-23 22:38 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev On Wed, 24 Jul 2013 00:02:21 +0200 Stefan Tomanek <stefan.tomanek@wertarbyte.de> wrote: > diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h > index 51da65b..59cd31b 100644 > --- a/include/uapi/linux/fib_rules.h > +++ b/include/uapi/linux/fib_rules.h > @@ -45,7 +45,7 @@ enum { > FRA_FLOW, /* flow/class id */ > FRA_UNUSED6, > FRA_UNUSED7, > - FRA_UNUSED8, > + FRA_TABLE_PREFIXLEN_MIN, > FRA_TABLE, /* Extended table id */ > FRA_FWMASK, /* mask for netfilter mark */ > FRA_OIFNAME, Not sure if reusing an entry or adding a new value is better to retain compatibility. Also don't you have to update FRA_GENERIC_POLICY? ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-23 22:38 ` Stephen Hemminger @ 2013-07-23 22:52 ` Stefan Tomanek 0 siblings, 0 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-23 22:52 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev Dies schrieb Stephen Hemminger (stephen@networkplumber.org): > > --- a/include/uapi/linux/fib_rules.h > > +++ b/include/uapi/linux/fib_rules.h > > @@ -45,7 +45,7 @@ enum { > > FRA_FLOW, /* flow/class id */ > > FRA_UNUSED6, > > FRA_UNUSED7, > > - FRA_UNUSED8, > > + FRA_TABLE_PREFIXLEN_MIN, > > FRA_TABLE, /* Extended table id */ > > FRA_FWMASK, /* mask for netfilter mark */ > > FRA_OIFNAME, > > Not sure if reusing an entry or adding a new value is better > to retain compatibility. Well, neither am I; my first changesets did in fact act a new value, but I then switched to reusing an existing one. I am open to suggestions from people having more experience with the existing code base. > Also don't you have to update FRA_GENERIC_POLICY? Well, I'm not sure - as I said, I was glad that I was able to add this functionality (which I was craving for quite some time now) to the kernel, so please feel free to point out (and possibly iron out) any issues with it. Thank you very much for your response Stefan Tomanek ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-23 22:02 [PATCH] fib_rules: add minimum prefix length Stefan Tomanek 2013-07-23 22:38 ` Stephen Hemminger @ 2013-07-24 2:14 ` Hannes Frederic Sowa 2013-07-24 7:57 ` Stefan Tomanek 2013-07-25 16:29 ` Stefan Tomanek 1 sibling, 2 replies; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-24 2:14 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev Hi! On Wed, Jul 24, 2013 at 12:02:21AM +0200, Stefan Tomanek wrote: > This change adds a minimum prefix length to the structures of routing rules. > If a rule is added with a minimum prefix length >0, only routes meeting this > threshold will be considered. Any other (more general) routing table entries > will be ignored. > > When configuring a system with multiple network uplinks and default routes, it > is often convinient to reference the main routing table multiple times - but > omitting the default route. Using this patch and a modified "ip" utility, this > can be achieved by using the following command sequence: Yeah, it is sometimes pretty hideous to set up, especially if one uses ppp stuff and such. But I am unsure if this change does actually improve that considerable. Static setups should be easily doable right now and for ppp/vpn stuff, I fear, it would still lack a bit of flexibility. Off-topic: I had the idea of supporting per process routing tables to deal with that: ip route exec table foo pppd ... So that every change of pppd and childs would only affect table foo (the details could be hairy but currently not enough time to find out). > diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c > index 26aa65d..94e9051 100644 > --- a/net/ipv4/fib_rules.c > +++ b/net/ipv4/fib_rules.c > @@ -95,8 +95,17 @@ static int fib4_rule_action(struct fib_rule *rule, struct flowi *flp, > goto errout; > > err = fib_table_lookup(tbl, &flp->u.ip4, (struct fib_result *) arg->result, arg->flags); > - if (err > 0) > + if (err > 0) { > err = -EAGAIN; > + goto errout; > + } > + /* do not accept result if the route does not meet the required prefix length */ > + if (arg->result) { > + if (((struct fib_result *)arg->result)->prefixlen < rule->table_prefixlen_min) { > + err = -EAGAIN; > + goto errout; > + } > + } > errout: > return err; > } I would try to factor the prefixlen_min check out into a e.g. fib4_rule_constrain function for which a new field in fib_rules_ops needs to be created as callback. Also it would be nice to have IPv6 support, too. ;) Greetings, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-24 2:14 ` Hannes Frederic Sowa @ 2013-07-24 7:57 ` Stefan Tomanek 2013-07-25 16:29 ` Stefan Tomanek 1 sibling, 0 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-24 7:57 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa Dies schrieb Hannes Frederic Sowa (hannes@stressinduktion.org): > Yeah, it is sometimes pretty hideous to set up, especially if one uses > ppp stuff and such. But I am unsure if this change does actually improve > that considerable. Static setups should be easily doable right now and for > ppp/vpn stuff, I fear, it would still lack a bit of flexibility. Well, it does work for me. I am using a dynamic PPP uplink and an OpenVPN tunnel where some marked traffic is guided through. To achieve this, I always had to configure pppd with "nodefaultroute" just to add the default route it manually in a separate table, complicating the configuration process. With my patches added, I can just reference a "masked" version of the main routing table at first, ignoring the default route placed there by pppd: echo "vpn" >> /etc/iproute2/rt_tables ip route add table vpn default via tun0 ip rule add pref 100 lookup main prefixlength 0 ip rule add pref 200 fwmark 0xA lookup vpn | V [ table main prefixlength >0 ] | V <fwmark 0xA?> -> [ table vpn ] | | | ,--------------´ V V [ table main ] That way, there is no need to reconfigure pppd, dhclient etc. If a specific route of the main table matches, it will we used. If the main table just points to the default route (prefixlengt == 0), it will be ignored and the packet travels to the next rule. In the end, the complete main table might still be consulted, including the previously shunned default route. Works great and requires little to no hacking around distribution specific network scripts. > I would try to factor the prefixlen_min check out into a > e.g. fib4_rule_constrain function for which a new field in fib_rules_ops > needs to be created as callback. Also it would be nice to have IPv6 > support, too. ;) Why not, sure. Working solutions today, better solutions tomorrow :-) ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-24 2:14 ` Hannes Frederic Sowa 2013-07-24 7:57 ` Stefan Tomanek @ 2013-07-25 16:29 ` Stefan Tomanek 2013-07-25 17:43 ` [PATCH] fib_rules/fib6rules: " Stefan Tomanek 2013-07-25 18:17 ` [PATCH] fib_rules: " Hannes Frederic Sowa 1 sibling, 2 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-25 16:29 UTC (permalink / raw) To: netdev Dies schrieb Hannes Frederic Sowa (hannes@stressinduktion.org): > I would try to factor the prefixlen_min check out into a > e.g. fib4_rule_constrain function for which a new field in fib_rules_ops > needs to be created as callback. Also it would be nice to have IPv6 > support, too. ;) I was working on my patchset again and considered your suggestion; however I'm not sure whether factoring out the constraints into a separate function is actually that useful, since they are only called from one specific location for each protocol; can you think of another useful application? I however did like the idea of adding IPv6 support, so I did - I'll post the new patch later on. Thanks for your feedback. ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] fib_rules/fib6rules: add minimum prefix length 2013-07-25 16:29 ` Stefan Tomanek @ 2013-07-25 17:43 ` Stefan Tomanek 2013-07-25 18:17 ` [PATCH] fib_rules: " Hannes Frederic Sowa 1 sibling, 0 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-25 17:43 UTC (permalink / raw) To: netdev This change adds a minimum prefix length to the structures of routing rules. If a rule is added with a minimum prefix length >0, only routes meeting this threshold will be considered. Any other (more general) routing table entries will be ignored. When configuring a system with multiple network uplinks and default routes, it is often convinient to reference the main routing table multiple times - but omitting the default route. Using this patch and a modified "ip" utility, this can be achieved by using the following command sequence: $ ip route add table secuplink default via 10.42.23.1 $ ip rule add pref 100 table main prefixlength 1 $ ip rule add pref 150 fwmark 0xA table secuplink With this setup, packets marked 0xA will be processed by the additional routing table "secuplink", but only if no suitable route in the main routing table can be found. By using a minimal prefixlength of 1, the default route (/0) of the table "main" is hidden to packets processed by rule 100; packets traveling to destinations with more specific routing entries are processed as usual. Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- include/net/fib_rules.h | 2 ++ include/uapi/linux/fib_rules.h | 2 +- net/core/fib_rules.c | 5 +++++ net/ipv4/fib_rules.c | 11 ++++++++++- net/ipv6/fib6_rules.c | 8 ++++++++ 5 files changed, 26 insertions(+), 2 deletions(-) diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h index e361f48..70f020f 100644 --- a/include/net/fib_rules.h +++ b/include/net/fib_rules.h @@ -18,6 +18,7 @@ struct fib_rule { u32 pref; u32 flags; u32 table; + u8 table_prefixlen_min; u8 action; u32 target; struct fib_rule __rcu *ctarget; @@ -80,6 +81,7 @@ struct fib_rules_ops { [FRA_FWMARK] = { .type = NLA_U32 }, \ [FRA_FWMASK] = { .type = NLA_U32 }, \ [FRA_TABLE] = { .type = NLA_U32 }, \ + [FRA_TABLE_PREFIXLEN_MIN] = { .type = NLA_U8 }, \ [FRA_GOTO] = { .type = NLA_U32 } static inline void fib_rule_get(struct fib_rule *rule) diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h index 51da65b..59cd31b 100644 --- a/include/uapi/linux/fib_rules.h +++ b/include/uapi/linux/fib_rules.h @@ -45,7 +45,7 @@ enum { FRA_FLOW, /* flow/class id */ FRA_UNUSED6, FRA_UNUSED7, - FRA_UNUSED8, + FRA_TABLE_PREFIXLEN_MIN, FRA_TABLE, /* Extended table id */ FRA_FWMASK, /* mask for netfilter mark */ FRA_OIFNAME, diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 2173544..8a15621 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -337,6 +337,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) rule->action = frh->action; rule->flags = frh->flags; rule->table = frh_get_table(frh, tb); + if (tb[FRA_TABLE_PREFIXLEN_MIN]) + rule->table_prefixlen_min = nla_get_u8(tb[FRA_TABLE_PREFIXLEN_MIN]); if (!tb[FRA_PRIORITY] && ops->default_pref) rule->pref = ops->default_pref(ops); @@ -523,6 +525,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops, + nla_total_size(IFNAMSIZ) /* FRA_OIFNAME */ + nla_total_size(4) /* FRA_PRIORITY */ + nla_total_size(4) /* FRA_TABLE */ + + nla_total_size(1) /* FRA_TABLE_PREFIXLEN_MIN */ + nla_total_size(4) /* FRA_FWMARK */ + nla_total_size(4); /* FRA_FWMASK */ @@ -548,6 +551,8 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule, frh->table = rule->table; if (nla_put_u32(skb, FRA_TABLE, rule->table)) goto nla_put_failure; + if (nla_put_u8(skb, FRA_TABLE_PREFIXLEN_MIN, rule->table_prefixlen_min)) + goto nla_put_failure; frh->res1 = 0; frh->res2 = 0; frh->action = rule->action; diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index 26aa65d..94e9051 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -95,8 +95,17 @@ static int fib4_rule_action(struct fib_rule *rule, struct flowi *flp, goto errout; err = fib_table_lookup(tbl, &flp->u.ip4, (struct fib_result *) arg->result, arg->flags); - if (err > 0) + if (err > 0) { err = -EAGAIN; + goto errout; + } + /* do not accept result if the route does not meet the required prefix length */ + if (arg->result) { + if (((struct fib_result *)arg->result)->prefixlen < rule->table_prefixlen_min) { + err = -EAGAIN; + goto errout; + } + } errout: return err; } diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 2e1a432..d2fa795 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -79,6 +79,14 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp, struct fib6_rule *r = (struct fib6_rule *)rule; /* + * do not accept result if the route does + * not meet the required prefix length + */ + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { + goto again; + } + + /* * If we need to find a source address for this traffic, * we check the result if it meets requirement of the rule. */ -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-25 16:29 ` Stefan Tomanek 2013-07-25 17:43 ` [PATCH] fib_rules/fib6rules: " Stefan Tomanek @ 2013-07-25 18:17 ` Hannes Frederic Sowa 2013-07-25 18:28 ` Stefan Tomanek 1 sibling, 1 reply; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-25 18:17 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev On Thu, Jul 25, 2013 at 06:29:32PM +0200, Stefan Tomanek wrote: > Dies schrieb Hannes Frederic Sowa (hannes@stressinduktion.org): > > > I would try to factor the prefixlen_min check out into a > > e.g. fib4_rule_constrain function for which a new field in fib_rules_ops > > needs to be created as callback. Also it would be nice to have IPv6 > > support, too. ;) > > I was working on my patchset again and considered your suggestion; however I'm > not sure whether factoring out the constraints into a separate function is > actually that useful, since they are only called from one specific location for > each protocol; can you think of another useful application? I don't have a strong opinion on that but I do find that it does better fit into the design of fib_rules and will be easier to extend in future IMHO. > I however did like the idea of adding IPv6 support, so I did - > I'll post the new patch later on. Cool, thanks. Greetings, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-25 18:17 ` [PATCH] fib_rules: " Hannes Frederic Sowa @ 2013-07-25 18:28 ` Stefan Tomanek 2013-07-25 21:46 ` Andrew Collins 0 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-07-25 18:28 UTC (permalink / raw) To: netdev Dies schrieb Hannes Frederic Sowa (hannes@stressinduktion.org): > > I however did like the idea of adding IPv6 support, so I did - > > I'll post the new patch later on. > > Cool, thanks. I've just completed the patch and sent it to the list, and I also placed the changes in my github account, just in case anyone wants to take a quick peek: https://github.com/wertarbyte/linux/compare/prefixlength ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-25 18:28 ` Stefan Tomanek @ 2013-07-25 21:46 ` Andrew Collins 2013-07-25 22:11 ` Stefan Tomanek 2013-07-26 10:54 ` [PATCH] fib_rules: add minimum prefix length Stefan Tomanek 0 siblings, 2 replies; 33+ messages in thread From: Andrew Collins @ 2013-07-25 21:46 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev On Thu, Jul 25, 2013 at 12:28 PM, Stefan Tomanek <stefan.tomanek@wertarbyte.de> wrote: > Dies schrieb Hannes Frederic Sowa (hannes@stressinduktion.org): > >> > I however did like the idea of adding IPv6 support, so I did - >> > I'll post the new patch later on. >> >> Cool, thanks. > > I've just completed the patch and sent it to the list, and I also placed the > changes in my github account, just in case anyone wants to take a quick peek: > https://github.com/wertarbyte/linux/compare/prefixlength > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > + if (arg->result) { ... This NULL check seems unnecessary (fib_table_lookup doesn't check). I like the idea, but I agree with Hannes that it'd be nice to split this out into a fib constraint callback. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-25 21:46 ` Andrew Collins @ 2013-07-25 22:11 ` Stefan Tomanek 2013-07-26 10:46 ` [PATCH] fib_rules: add .suppress operation Stefan Tomanek 2013-07-26 10:54 ` [PATCH] fib_rules: add minimum prefix length Stefan Tomanek 1 sibling, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-07-25 22:11 UTC (permalink / raw) To: netdev; +Cc: Andrew Collins Dies schrieb Andrew Collins (bsderandrew@gmail.com): > > I've just completed the patch and sent it to the list, and I also placed the > > changes in my github account, just in case anyone wants to take a quick peek: > > https://github.com/wertarbyte/linux/compare/prefixlength > > > + if (arg->result) { ... > > This NULL check seems unnecessary (fib_table_lookup doesn't check). I wasn't sure if it was possible for fib_table_lookup to put a NULL pointer there, so I wanted to make sure following it with ->prefixlen was possible. > I like the idea, but I agree with Hannes that it'd be nice to split > this out into a fib constraint callback. I'm still not convinced that creating such a callback that will never be called from outside fib{4,}_rule_action() is worth the effort, but my insight into the code is still limited; if it's the right thing to do [tm], I won't stand in the way :-) ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH] fib_rules: add .suppress operation 2013-07-25 22:11 ` Stefan Tomanek @ 2013-07-26 10:46 ` Stefan Tomanek 2013-07-26 17:05 ` Hannes Frederic Sowa ` (2 more replies) 0 siblings, 3 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-26 10:46 UTC (permalink / raw) To: netdev; +Cc: Andrew Collins This change adds a new operation to the fib_rules_ops struct; it allows the suppression of routing decisions if certain criteria are not met by its results. The first implemented constraint is a minimum prefix length added to the structures of routing rules. If a rule is added with a minimum prefix length >0, only routes meeting this threshold will be considered. Any other (more general) routing table entries will be ignored. When configuring a system with multiple network uplinks and default routes, it is often convinient to reference the main routing table multiple times - but omitting the default route. Using this patch and a modified "ip" utility, this can be achieved by using the following command sequence: $ ip route add table secuplink default via 10.42.23.1 $ ip rule add pref 100 table main prefixlength 1 $ ip rule add pref 150 fwmark 0xA table secuplink With this setup, packets marked 0xA will be processed by the additional routing table "secuplink", but only if no suitable route in the main routing table can be found. By using a minimal prefixlength of 1, the default route (/0) of the table "main" is hidden to packets processed by rule 100; packets traveling to destinations with more specific routing entries are processed as usual. Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- include/net/fib_rules.h | 4 ++++ include/uapi/linux/fib_rules.h | 2 +- net/core/fib_rules.c | 8 ++++++++ net/ipv4/fib_rules.c | 9 +++++++++ net/ipv6/fib6_rules.c | 12 ++++++++++++ 5 files changed, 34 insertions(+), 1 deletion(-) diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h index e361f48..bb15664 100644 --- a/include/net/fib_rules.h +++ b/include/net/fib_rules.h @@ -18,6 +18,7 @@ struct fib_rule { u32 pref; u32 flags; u32 table; + u8 table_prefixlen_min; u8 action; u32 target; struct fib_rule __rcu *ctarget; @@ -46,6 +47,8 @@ struct fib_rules_ops { int (*action)(struct fib_rule *, struct flowi *, int, struct fib_lookup_arg *); + int (*suppress)(struct fib_rule *, + struct fib_lookup_arg *); int (*match)(struct fib_rule *, struct flowi *, int); int (*configure)(struct fib_rule *, @@ -80,6 +83,7 @@ struct fib_rules_ops { [FRA_FWMARK] = { .type = NLA_U32 }, \ [FRA_FWMASK] = { .type = NLA_U32 }, \ [FRA_TABLE] = { .type = NLA_U32 }, \ + [FRA_TABLE_PREFIXLEN_MIN] = { .type = NLA_U8 }, \ [FRA_GOTO] = { .type = NLA_U32 } static inline void fib_rule_get(struct fib_rule *rule) diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h index 51da65b..59cd31b 100644 --- a/include/uapi/linux/fib_rules.h +++ b/include/uapi/linux/fib_rules.h @@ -45,7 +45,7 @@ enum { FRA_FLOW, /* flow/class id */ FRA_UNUSED6, FRA_UNUSED7, - FRA_UNUSED8, + FRA_TABLE_PREFIXLEN_MIN, FRA_TABLE, /* Extended table id */ FRA_FWMASK, /* mask for netfilter mark */ FRA_OIFNAME, diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 2173544..1e0e1f4 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -227,6 +227,9 @@ jumped: err = ops->action(rule, fl, flags, arg); if (err != -EAGAIN) { + if (ops->suppress && ops->suppress(rule, arg)) { + continue; + } if ((arg->flags & FIB_LOOKUP_NOREF) || likely(atomic_inc_not_zero(&rule->refcnt))) { arg->rule = rule; @@ -337,6 +340,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) rule->action = frh->action; rule->flags = frh->flags; rule->table = frh_get_table(frh, tb); + if (tb[FRA_TABLE_PREFIXLEN_MIN]) + rule->table_prefixlen_min = nla_get_u8(tb[FRA_TABLE_PREFIXLEN_MIN]); if (!tb[FRA_PRIORITY] && ops->default_pref) rule->pref = ops->default_pref(ops); @@ -523,6 +528,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops, + nla_total_size(IFNAMSIZ) /* FRA_OIFNAME */ + nla_total_size(4) /* FRA_PRIORITY */ + nla_total_size(4) /* FRA_TABLE */ + + nla_total_size(1) /* FRA_TABLE_PREFIXLEN_MIN */ + nla_total_size(4) /* FRA_FWMARK */ + nla_total_size(4); /* FRA_FWMASK */ @@ -548,6 +554,8 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule, frh->table = rule->table; if (nla_put_u32(skb, FRA_TABLE, rule->table)) goto nla_put_failure; + if (nla_put_u8(skb, FRA_TABLE_PREFIXLEN_MIN, rule->table_prefixlen_min)) + goto nla_put_failure; frh->res1 = 0; frh->res2 = 0; frh->action = rule->action; diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index 26aa65d..cb5fcc1 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -101,6 +101,14 @@ errout: return err; } +static int fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { + /* do not accept result if the route does not meet the required prefix length */ + struct fib_result *result = (struct fib_result *) arg->result; + if (result->prefixlen < rule->table_prefixlen_min) { + return 1; + } + return 0; +} static int fib4_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -267,6 +275,7 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = { .rule_size = sizeof(struct fib4_rule), .addr_size = sizeof(u32), .action = fib4_rule_action, + .suppress = fib4_rule_suppress, .match = fib4_rule_match, .configure = fib4_rule_configure, .delete = fib4_rule_delete, diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 2e1a432..dcba659 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -111,6 +111,17 @@ out: return rt == NULL ? -EAGAIN : 0; } +static int fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { + struct rt6_info *rt = (struct rt6_info *) arg->result; + /* + * do not accept result if the route does + * not meet the required prefix length + */ + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { + return 1; + } + return 0; +} static int fib6_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -244,6 +255,7 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = { .addr_size = sizeof(struct in6_addr), .action = fib6_rule_action, .match = fib6_rule_match, + .suppress = fib6_rule_suppress, .configure = fib6_rule_configure, .compare = fib6_rule_compare, .fill = fib6_rule_fill, -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-26 10:46 ` [PATCH] fib_rules: add .suppress operation Stefan Tomanek @ 2013-07-26 17:05 ` Hannes Frederic Sowa 2013-07-27 6:08 ` Hannes Frederic Sowa 2013-07-27 6:26 ` Hannes Frederic Sowa 2013-07-27 7:07 ` Hannes Frederic Sowa 2 siblings, 1 reply; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-26 17:05 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev, Andrew Collins On Fri, Jul 26, 2013 at 12:46:57PM +0200, Stefan Tomanek wrote: > if (err != -EAGAIN) { > + if (ops->suppress && ops->suppress(rule, arg)) { > + continue; > + } > if ((arg->flags & FIB_LOOKUP_NOREF) || > likely(atomic_inc_not_zero(&rule->refcnt))) { > arg->rule = rule; > > [...] > > +static int fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > + struct rt6_info *rt = (struct rt6_info *) arg->result; > + /* > + * do not accept result if the route does > + * not meet the required prefix length > + */ > + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { > + return 1; > + } > + return 0; > +} Urks, fib6_rule_action is broken. The switch (rule->action) does update the rt entry but does not signal the correct error code to stop iterating the rules in case it finds a blackhole, prohibit etc. action (it always signals -EAGAIN). A change in this logic could have impact to your patch as I currently don't know how the null handling of arg->result will turn out. IPv6 does not preinitialize arg->result as IPv4 does. I am looking for a solution. Thanks, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-26 17:05 ` Hannes Frederic Sowa @ 2013-07-27 6:08 ` Hannes Frederic Sowa 0 siblings, 0 replies; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-27 6:08 UTC (permalink / raw) To: Stefan Tomanek, netdev, Andrew Collins On Fri, Jul 26, 2013 at 07:05:56PM +0200, Hannes Frederic Sowa wrote: > On Fri, Jul 26, 2013 at 12:46:57PM +0200, Stefan Tomanek wrote: > > if (err != -EAGAIN) { > > + if (ops->suppress && ops->suppress(rule, arg)) { > > + continue; > > + } We need to make sure route lookup succeeded: if (!err && ops->suppress && ops->suppress(rule, arg)) > > +static int fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > > + struct rt6_info *rt = (struct rt6_info *) arg->result; > > + /* > > + * do not accept result if the route does > > + * not meet the required prefix length > > + */ > > + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { > > + return 1; > > + } > > + return 0; > > +} > > Urks, fib6_rule_action is broken. The switch (rule->action) does update the rt > entry but does not signal the correct error code to stop iterating the rules > in case it finds a blackhole, prohibit etc. action (it always signals > -EAGAIN). I have to take back that fib6_rule_action is broken, it just showes some unwanted side effects in this constalation. > A change in this logic could have impact to your patch as I currently > don't know how the null handling of arg->result will turn out. IPv6 does not > preinitialize arg->result as IPv4 does. > > I am looking for a solution. This compile-only-tested patch would make the error reporting consistent with its ipv4 counterpart. arg->result should only be NULL iff the function returns -EAGAIN. So we are clean here. (It seems this patch also fixes a potential nullptr dereference.) diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 2e1a432..4c8bac7 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -55,26 +55,33 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp, struct fib6_table *table; struct net *net = rule->fr_net; pol_lookup_t lookup = arg->lookup_ptr; + int err = 0; switch (rule->action) { case FR_ACT_TO_TBL: break; case FR_ACT_UNREACHABLE: + err = -ENETUNREACH; rt = net->ipv6.ip6_null_entry; goto discard_pkt; default: case FR_ACT_BLACKHOLE: + err = -EINVAL; rt = net->ipv6.ip6_blk_hole_entry; goto discard_pkt; case FR_ACT_PROHIBIT: + err = -EACCES; rt = net->ipv6.ip6_prohibit_entry; goto discard_pkt; } table = fib6_get_table(net, rule->table); - if (table) - rt = lookup(net, table, flp6, flags); + if (!table) { + err = -EAGAIN; + goto out; + } + rt = lookup(net, table, flp6, flags); if (rt != net->ipv6.ip6_null_entry) { struct fib6_rule *r = (struct fib6_rule *)rule; @@ -101,6 +108,7 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp, } again: ip6_rt_put(rt); + err = -EAGAIN; rt = NULL; goto out; @@ -108,7 +116,7 @@ discard_pkt: dst_hold(&rt->dst); out: arg->result = rt; - return rt == NULL ? -EAGAIN : 0; + return err; } ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-26 10:46 ` [PATCH] fib_rules: add .suppress operation Stefan Tomanek 2013-07-26 17:05 ` Hannes Frederic Sowa @ 2013-07-27 6:26 ` Hannes Frederic Sowa 2013-07-27 7:07 ` Hannes Frederic Sowa 2 siblings, 0 replies; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-27 6:26 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev, Andrew Collins Oh, I overlooked something. On Fri, Jul 26, 2013 at 12:46:57PM +0200, Stefan Tomanek wrote: > --- a/net/ipv6/fib6_rules.c > +++ b/net/ipv6/fib6_rules.c > @@ -111,6 +111,17 @@ out: > return rt == NULL ? -EAGAIN : 0; > } > > +static int fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > + struct rt6_info *rt = (struct rt6_info *) arg->result; > + /* > + * do not accept result if the route does > + * not meet the required prefix length > + */ > + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { > + return 1; > + } > + return 0; > +} In case of IPv6 (we don't clone the result but merely return a reference to the rt6_info), we also need to decrement the reference count if we suppress the route. A ip6_rt_put should do the trick. Greetings, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-26 10:46 ` [PATCH] fib_rules: add .suppress operation Stefan Tomanek 2013-07-26 17:05 ` Hannes Frederic Sowa 2013-07-27 6:26 ` Hannes Frederic Sowa @ 2013-07-27 7:07 ` Hannes Frederic Sowa 2013-07-27 10:21 ` Stefan Tomanek ` (4 more replies) 2 siblings, 5 replies; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-27 7:07 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev, Andrew Collins Hm, IPv4 actually seems to have the same problem: On Fri, Jul 26, 2013 at 12:46:57PM +0200, Stefan Tomanek wrote: > +static int fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > + /* do not accept result if the route does not meet the required prefix length */ > + struct fib_result *result = (struct fib_result *) arg->result; > + if (result->prefixlen < rule->table_prefixlen_min) { > + return 1; > + } > + return 0; > +} In case of suppressing the route, we need to decrement the reference counter of fib_info: if (!(fib_flags & FIB_LOOKUP_NOREF)) fib_info_put(result->fi); Also: > index e361f48..bb15664 100644 > --- a/include/net/fib_rules.h > +++ b/include/net/fib_rules.h > @@ -18,6 +18,7 @@ struct fib_rule { > u32 pref; > u32 flags; > u32 table; > + u8 table_prefixlen_min; > u8 action; > u32 target; > struct fib_rule __rcu *ctarget; > @@ -46,6 +47,8 @@ struct fib_rules_ops { > int (*action)(struct fib_rule *, > struct flowi *, int, > struct fib_lookup_arg *); > + int (*suppress)(struct fib_rule *, > + struct fib_lookup_arg *); bool and true/false would be nicer. > int (*match)(struct fib_rule *, > struct flowi *, int); > int (*configure)(struct fib_rule *, > Greetings, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-27 7:07 ` Hannes Frederic Sowa @ 2013-07-27 10:21 ` Stefan Tomanek 2013-07-27 15:10 ` Hannes Frederic Sowa 2013-07-30 6:52 ` [PATCH v2 1/2] fib6_rules: make error handling consistent with IPv4 Stefan Tomanek ` (3 subsequent siblings) 4 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-07-27 10:21 UTC (permalink / raw) To: netdev; +Cc: Andrew Collins, Hannes Frederic Sowa Thanks for your input, I've put together all the proposed patches in my github branch: https://github.com/wertarbyte/linux/compare/fib_rule_suppress Any more ideas, before I squash it all together? ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-27 10:21 ` Stefan Tomanek @ 2013-07-27 15:10 ` Hannes Frederic Sowa 2013-07-28 10:36 ` Stefan Tomanek 0 siblings, 1 reply; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-07-27 15:10 UTC (permalink / raw) To: Stefan Tomanek; +Cc: netdev, Andrew Collins, stephen On Sat, Jul 27, 2013 at 12:21:49PM +0200, Stefan Tomanek wrote: > Thanks for your input, I've put together all the proposed patches in my github > branch: > > https://github.com/wertarbyte/linux/compare/fib_rule_suppress > > Any more ideas, before I squash it all together? This should fix the compile error (was a c&p error from me in the previous mail). - if (!(fib_flags & FIB_LOOKUP_NOREF)) + if (!(arg->flags & FIB_LOOKUP_NOREF)) This should get a considerable amount of stress testing now. ;) Greetings, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add .suppress operation 2013-07-27 15:10 ` Hannes Frederic Sowa @ 2013-07-28 10:36 ` Stefan Tomanek 0 siblings, 0 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-28 10:36 UTC (permalink / raw) To: netdev, Andrew Collins, stephen Dies schrieb Hannes Frederic Sowa (hannes@stressinduktion.org): > This should fix the compile error (was a c&p error from me in the previous > mail). > > - if (!(fib_flags & FIB_LOOKUP_NOREF)) > + if (!(arg->flags & FIB_LOOKUP_NOREF)) > > This should get a considerable amount of stress testing now. ;) I amended that change and made a forced push, so now the branch should be up to date and in working condition: https://github.com/wertarbyte/linux/compare/fib_rule_suppress ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v2 1/2] fib6_rules: make error handling consistent with IPv4 2013-07-27 7:07 ` Hannes Frederic Sowa 2013-07-27 10:21 ` Stefan Tomanek @ 2013-07-30 6:52 ` Stefan Tomanek 2013-07-30 6:53 ` [PATCH v2 2/2] fib_rules: add .suppress operation Stefan Tomanek ` (2 subsequent siblings) 4 siblings, 0 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-30 6:52 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa, Andrew Collins (by Hannes Frederic Sowa <hannes@stressinduktion.org>) This compile-only-tested patch would make the error reporting consistent with its ipv4 counterpart. arg->result should only be NULL iff the function returns -EAGAIN. So we are clean here. (It seems this patch also fixes a potential nullptr dereference.) Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- net/ipv6/fib6_rules.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 2e1a432..4c8bac7 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -55,26 +55,33 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp, struct fib6_table *table; struct net *net = rule->fr_net; pol_lookup_t lookup = arg->lookup_ptr; + int err = 0; switch (rule->action) { case FR_ACT_TO_TBL: break; case FR_ACT_UNREACHABLE: + err = -ENETUNREACH; rt = net->ipv6.ip6_null_entry; goto discard_pkt; default: case FR_ACT_BLACKHOLE: + err = -EINVAL; rt = net->ipv6.ip6_blk_hole_entry; goto discard_pkt; case FR_ACT_PROHIBIT: + err = -EACCES; rt = net->ipv6.ip6_prohibit_entry; goto discard_pkt; } table = fib6_get_table(net, rule->table); - if (table) - rt = lookup(net, table, flp6, flags); + if (!table) { + err = -EAGAIN; + goto out; + } + rt = lookup(net, table, flp6, flags); if (rt != net->ipv6.ip6_null_entry) { struct fib6_rule *r = (struct fib6_rule *)rule; @@ -101,6 +108,7 @@ static int fib6_rule_action(struct fib_rule *rule, struct flowi *flp, } again: ip6_rt_put(rt); + err = -EAGAIN; rt = NULL; goto out; @@ -108,7 +116,7 @@ discard_pkt: dst_hold(&rt->dst); out: arg->result = rt; - return rt == NULL ? -EAGAIN : 0; + return err; } -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* [PATCH v2 2/2] fib_rules: add .suppress operation 2013-07-27 7:07 ` Hannes Frederic Sowa 2013-07-27 10:21 ` Stefan Tomanek 2013-07-30 6:52 ` [PATCH v2 1/2] fib6_rules: make error handling consistent with IPv4 Stefan Tomanek @ 2013-07-30 6:53 ` Stefan Tomanek 2013-07-30 7:03 ` David Miller 2013-07-30 7:46 ` [PATCH v3] " Stefan Tomanek 2013-08-01 0:17 ` [PATCH v4] " Stefan Tomanek 4 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-07-30 6:53 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa, Andrew Collins This change adds a new operation to the fib_rules_ops struct; it allows the suppression of routing decisions if certain criteria are not met by its results. The first implemented constraint is a minimum prefix length added to the structures of routing rules. If a rule is added with a minimum prefix length >0, only routes meeting this threshold will be considered. Any other (more general) routing table entries will be ignored. When configuring a system with multiple network uplinks and default routes, it is often convinient to reference the main routing table multiple times - but omitting the default route. Using this patch and a modified "ip" utility, this can be achieved by using the following command sequence: $ ip route add table secuplink default via 10.42.23.1 $ ip rule add pref 100 table main prefixlength 1 $ ip rule add pref 150 fwmark 0xA table secuplink With this setup, packets marked 0xA will be processed by the additional routing table "secuplink", but only if no suitable route in the main routing table can be found. By using a minimal prefixlength of 1, the default route (/0) of the table "main" is hidden to packets processed by rule 100; packets traveling to destinations with more specific routing entries are processed as usual. Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- include/net/fib_rules.h | 4 ++++ include/uapi/linux/fib_rules.h | 2 +- net/core/fib_rules.c | 9 +++++++++ net/ipv4/fib_rules.c | 12 ++++++++++++ net/ipv6/fib6_rules.c | 13 +++++++++++++ 5 files changed, 39 insertions(+), 1 deletion(-) diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h index e361f48..2f286dc 100644 --- a/include/net/fib_rules.h +++ b/include/net/fib_rules.h @@ -18,6 +18,7 @@ struct fib_rule { u32 pref; u32 flags; u32 table; + u8 table_prefixlen_min; u8 action; u32 target; struct fib_rule __rcu *ctarget; @@ -46,6 +47,8 @@ struct fib_rules_ops { int (*action)(struct fib_rule *, struct flowi *, int, struct fib_lookup_arg *); + bool (*suppress)(struct fib_rule *, + struct fib_lookup_arg *); int (*match)(struct fib_rule *, struct flowi *, int); int (*configure)(struct fib_rule *, @@ -80,6 +83,7 @@ struct fib_rules_ops { [FRA_FWMARK] = { .type = NLA_U32 }, \ [FRA_FWMASK] = { .type = NLA_U32 }, \ [FRA_TABLE] = { .type = NLA_U32 }, \ + [FRA_TABLE_PREFIXLEN_MIN] = { .type = NLA_U8 }, \ [FRA_GOTO] = { .type = NLA_U32 } static inline void fib_rule_get(struct fib_rule *rule) diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h index 51da65b..59cd31b 100644 --- a/include/uapi/linux/fib_rules.h +++ b/include/uapi/linux/fib_rules.h @@ -45,7 +45,7 @@ enum { FRA_FLOW, /* flow/class id */ FRA_UNUSED6, FRA_UNUSED7, - FRA_UNUSED8, + FRA_TABLE_PREFIXLEN_MIN, FRA_TABLE, /* Extended table id */ FRA_FWMASK, /* mask for netfilter mark */ FRA_OIFNAME, diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 2173544..809efd5 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -226,6 +226,10 @@ jumped: else err = ops->action(rule, fl, flags, arg); + if (!err && ops->suppress && ops->suppress(rule, arg)) { + continue; + } + if (err != -EAGAIN) { if ((arg->flags & FIB_LOOKUP_NOREF) || likely(atomic_inc_not_zero(&rule->refcnt))) { @@ -337,6 +341,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) rule->action = frh->action; rule->flags = frh->flags; rule->table = frh_get_table(frh, tb); + if (tb[FRA_TABLE_PREFIXLEN_MIN]) + rule->table_prefixlen_min = nla_get_u8(tb[FRA_TABLE_PREFIXLEN_MIN]); if (!tb[FRA_PRIORITY] && ops->default_pref) rule->pref = ops->default_pref(ops); @@ -523,6 +529,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops, + nla_total_size(IFNAMSIZ) /* FRA_OIFNAME */ + nla_total_size(4) /* FRA_PRIORITY */ + nla_total_size(4) /* FRA_TABLE */ + + nla_total_size(1) /* FRA_TABLE_PREFIXLEN_MIN */ + nla_total_size(4) /* FRA_FWMARK */ + nla_total_size(4); /* FRA_FWMASK */ @@ -548,6 +555,8 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule, frh->table = rule->table; if (nla_put_u32(skb, FRA_TABLE, rule->table)) goto nla_put_failure; + if (nla_put_u8(skb, FRA_TABLE_PREFIXLEN_MIN, rule->table_prefixlen_min)) + goto nla_put_failure; frh->res1 = 0; frh->res2 = 0; frh->action = rule->action; diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index 26aa65d..f93beb6 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -101,6 +101,17 @@ errout: return err; } +static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { + /* do not accept result if the route does not meet the required prefix length */ + struct fib_result *result = (struct fib_result *) arg->result; + if (result->prefixlen < rule->table_prefixlen_min) { + if (!(arg->flags & FIB_LOOKUP_NOREF)) { + fib_info_put(result->fi); + } + return true; + } + return false; +} static int fib4_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -267,6 +278,7 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = { .rule_size = sizeof(struct fib4_rule), .addr_size = sizeof(u32), .action = fib4_rule_action, + .suppress = fib4_rule_suppress, .match = fib4_rule_match, .configure = fib4_rule_configure, .delete = fib4_rule_delete, diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 4c8bac7..63372ab 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -119,6 +119,18 @@ out: return err; } +static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { + struct rt6_info *rt = (struct rt6_info *) arg->result; + /* + * do not accept result if the route does + * not meet the required prefix length + */ + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { + ip6_rt_put(rt); + return true; + } + return false; +} static int fib6_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -252,6 +264,7 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = { .addr_size = sizeof(struct in6_addr), .action = fib6_rule_action, .match = fib6_rule_match, + .suppress = fib6_rule_suppress, .configure = fib6_rule_configure, .compare = fib6_rule_compare, .fill = fib6_rule_fill, -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] fib_rules: add .suppress operation 2013-07-30 6:53 ` [PATCH v2 2/2] fib_rules: add .suppress operation Stefan Tomanek @ 2013-07-30 7:03 ` David Miller 2013-07-30 7:23 ` Stefan Tomanek 0 siblings, 1 reply; 33+ messages in thread From: David Miller @ 2013-07-30 7:03 UTC (permalink / raw) To: stefan.tomanek; +Cc: netdev, hannes, bsderandrew From: Stefan Tomanek <stefan.tomanek@wertarbyte.de> Date: Tue, 30 Jul 2013 08:53:17 +0200 > @@ -101,6 +101,17 @@ errout: > return err; > } > > +static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { ... > @@ -119,6 +119,18 @@ out: > return err; > } > > +static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > + struct rt6_info *rt = (struct rt6_info *) arg->result; > + /* > + * do not accept result if the route does > + * not meet the required prefix length > + */ Please format this stuff correctly. Functions are declared like this: static type NAME(TYPE ARG1, TYPE ARG2) { } You don't put the openning curly brace at the end of the line with the closing parenthesis of the argument list. Next, comments are to be formatted: /* Like * this. */ Thanks. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] fib_rules: add .suppress operation 2013-07-30 7:03 ` David Miller @ 2013-07-30 7:23 ` Stefan Tomanek 2013-07-30 7:34 ` David Miller 0 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-07-30 7:23 UTC (permalink / raw) To: David Miller; +Cc: netdev, hannes, bsderandrew Dies schrieb David Miller (davem@davemloft.net): > > +static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > ... > > @@ -119,6 +119,18 @@ out: > > return err; > > } > > > > +static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) { > > + struct rt6_info *rt = (struct rt6_info *) arg->result; > > + /* > > + * do not accept result if the route does > > + * not meet the required prefix length > > + */ > > Please format this stuff correctly. > > Functions are declared like this: > > static type NAME(TYPE ARG1, TYPE ARG2) > { > } Noted and fixed. > You don't put the openning curly brace at the end of the line with the > closing parenthesis of the argument list. > > Next, comments are to be formatted: > > /* Like > * this. > */ Well, to be fair, most comments in the existing code base actually use the style I employed (in fact, I was copying it), so it seems like a blurry line at least. /* * If we need to find a source address for this traffic, * we check the result if it meets requirement of the rule. */ -----8<---- /* * If FIB_RULE_FIND_SADDR is set and we do not have a * source address for the traffic, we defer check for * source address. */ -----8<---- /* The lock is not required here, the list in unreacheable * at the moment this function is called */ -----8<---- /* compatibility: if the mark value is non-zero all bits * are compared unless a mask is explicitly specified. */ -----8<---- /* * There are unresolved goto rules in the list, check if * any of them are pointing to this new rule. */ -----8<---- /* * Check if this rule is a target to any of them. If so, * disable them. As this operation is eventually very * expensive, it is only performed if goto rules have * actually been added. */ -----8<---- ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v2 2/2] fib_rules: add .suppress operation 2013-07-30 7:23 ` Stefan Tomanek @ 2013-07-30 7:34 ` David Miller 0 siblings, 0 replies; 33+ messages in thread From: David Miller @ 2013-07-30 7:34 UTC (permalink / raw) To: stefan.tomanek; +Cc: netdev, hannes, bsderandrew From: Stefan Tomanek <stefan.tomanek@wertarbyte.de> Date: Tue, 30 Jul 2013 09:23:05 +0200 > Well, to be fair, most comments in the existing code base actually use the style > I employed (in fact, I was copying it), so it seems like a blurry line at least. Existing mistakes are not excuses for duplicating them :-) They are a good excuse to send a patch fixing them, however. ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v3] fib_rules: add .suppress operation 2013-07-27 7:07 ` Hannes Frederic Sowa ` (2 preceding siblings ...) 2013-07-30 6:53 ` [PATCH v2 2/2] fib_rules: add .suppress operation Stefan Tomanek @ 2013-07-30 7:46 ` Stefan Tomanek 2013-07-31 22:13 ` David Miller 2013-08-01 0:17 ` [PATCH v4] " Stefan Tomanek 4 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-07-30 7:46 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa, Andrew Collins This change adds a new operation to the fib_rules_ops struct; it allows the suppression of routing decisions if certain criteria are not met by its results. The first implemented constraint is a minimum prefix length added to the structures of routing rules. If a rule is added with a minimum prefix length >0, only routes meeting this threshold will be considered. Any other (more general) routing table entries will be ignored. When configuring a system with multiple network uplinks and default routes, it is often convinient to reference the main routing table multiple times - but omitting the default route. Using this patch and a modified "ip" utility, this can be achieved by using the following command sequence: $ ip route add table secuplink default via 10.42.23.1 $ ip rule add pref 100 table main prefixlength 1 $ ip rule add pref 150 fwmark 0xA table secuplink With this setup, packets marked 0xA will be processed by the additional routing table "secuplink", but only if no suitable route in the main routing table can be found. By using a minimal prefixlength of 1, the default route (/0) of the table "main" is hidden to packets processed by rule 100; packets traveling to destinations with more specific routing entries are processed as usual. Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- include/net/fib_rules.h | 4 ++++ include/uapi/linux/fib_rules.h | 2 +- net/core/fib_rules.c | 9 +++++++++ net/ipv4/fib_rules.c | 15 +++++++++++++++ net/ipv6/fib6_rules.c | 13 +++++++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h index e361f48..2f286dc 100644 --- a/include/net/fib_rules.h +++ b/include/net/fib_rules.h @@ -18,6 +18,7 @@ struct fib_rule { u32 pref; u32 flags; u32 table; + u8 table_prefixlen_min; u8 action; u32 target; struct fib_rule __rcu *ctarget; @@ -46,6 +47,8 @@ struct fib_rules_ops { int (*action)(struct fib_rule *, struct flowi *, int, struct fib_lookup_arg *); + bool (*suppress)(struct fib_rule *, + struct fib_lookup_arg *); int (*match)(struct fib_rule *, struct flowi *, int); int (*configure)(struct fib_rule *, @@ -80,6 +83,7 @@ struct fib_rules_ops { [FRA_FWMARK] = { .type = NLA_U32 }, \ [FRA_FWMASK] = { .type = NLA_U32 }, \ [FRA_TABLE] = { .type = NLA_U32 }, \ + [FRA_TABLE_PREFIXLEN_MIN] = { .type = NLA_U8 }, \ [FRA_GOTO] = { .type = NLA_U32 } static inline void fib_rule_get(struct fib_rule *rule) diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h index 51da65b..59cd31b 100644 --- a/include/uapi/linux/fib_rules.h +++ b/include/uapi/linux/fib_rules.h @@ -45,7 +45,7 @@ enum { FRA_FLOW, /* flow/class id */ FRA_UNUSED6, FRA_UNUSED7, - FRA_UNUSED8, + FRA_TABLE_PREFIXLEN_MIN, FRA_TABLE, /* Extended table id */ FRA_FWMASK, /* mask for netfilter mark */ FRA_OIFNAME, diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 2173544..809efd5 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -226,6 +226,10 @@ jumped: else err = ops->action(rule, fl, flags, arg); + if (!err && ops->suppress && ops->suppress(rule, arg)) { + continue; + } + if (err != -EAGAIN) { if ((arg->flags & FIB_LOOKUP_NOREF) || likely(atomic_inc_not_zero(&rule->refcnt))) { @@ -337,6 +341,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) rule->action = frh->action; rule->flags = frh->flags; rule->table = frh_get_table(frh, tb); + if (tb[FRA_TABLE_PREFIXLEN_MIN]) + rule->table_prefixlen_min = nla_get_u8(tb[FRA_TABLE_PREFIXLEN_MIN]); if (!tb[FRA_PRIORITY] && ops->default_pref) rule->pref = ops->default_pref(ops); @@ -523,6 +529,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops, + nla_total_size(IFNAMSIZ) /* FRA_OIFNAME */ + nla_total_size(4) /* FRA_PRIORITY */ + nla_total_size(4) /* FRA_TABLE */ + + nla_total_size(1) /* FRA_TABLE_PREFIXLEN_MIN */ + nla_total_size(4) /* FRA_FWMARK */ + nla_total_size(4); /* FRA_FWMASK */ @@ -548,6 +555,8 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule, frh->table = rule->table; if (nla_put_u32(skb, FRA_TABLE, rule->table)) goto nla_put_failure; + if (nla_put_u8(skb, FRA_TABLE_PREFIXLEN_MIN, rule->table_prefixlen_min)) + goto nla_put_failure; frh->res1 = 0; frh->res2 = 0; frh->action = rule->action; diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index 26aa65d..0e3df33 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -101,6 +101,20 @@ errout: return err; } +static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) +{ + /* do not accept result if the route does + * not meet the required prefix length + */ + struct fib_result *result = (struct fib_result *) arg->result; + if (result->prefixlen < rule->table_prefixlen_min) { + if (!(arg->flags & FIB_LOOKUP_NOREF)) { + fib_info_put(result->fi); + } + return true; + } + return false; +} static int fib4_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -267,6 +281,7 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = { .rule_size = sizeof(struct fib4_rule), .addr_size = sizeof(u32), .action = fib4_rule_action, + .suppress = fib4_rule_suppress, .match = fib4_rule_match, .configure = fib4_rule_configure, .delete = fib4_rule_delete, diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 4c8bac7..554a4fb 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -119,6 +119,18 @@ out: return err; } +static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) +{ + struct rt6_info *rt = (struct rt6_info *) arg->result; + /* do not accept result if the route does + * not meet the required prefix length + */ + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { + ip6_rt_put(rt); + return true; + } + return false; +} static int fib6_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -252,6 +264,7 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = { .addr_size = sizeof(struct in6_addr), .action = fib6_rule_action, .match = fib6_rule_match, + .suppress = fib6_rule_suppress, .configure = fib6_rule_configure, .compare = fib6_rule_compare, .fill = fib6_rule_fill, -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v3] fib_rules: add .suppress operation 2013-07-30 7:46 ` [PATCH v3] " Stefan Tomanek @ 2013-07-31 22:13 ` David Miller 2013-08-01 0:24 ` Stefan Tomanek 0 siblings, 1 reply; 33+ messages in thread From: David Miller @ 2013-07-31 22:13 UTC (permalink / raw) To: stefan.tomanek; +Cc: netdev, hannes, bsderandrew From: Stefan Tomanek <stefan.tomanek@wertarbyte.de> Date: Tue, 30 Jul 2013 09:46:36 +0200 > This change adds a new operation to the fib_rules_ops struct; it allows the > suppression of routing decisions if certain criteria are not met by its > results. > > The first implemented constraint is a minimum prefix length added to the > structures of routing rules. If a rule is added with a minimum prefix length >>0, only routes meeting this threshold will be considered. Any other (more > general) routing table entries will be ignored. > > When configuring a system with multiple network uplinks and default routes, it > is often convinient to reference the main routing table multiple times - but > omitting the default route. Using this patch and a modified "ip" utility, this > can be achieved by using the following command sequence: > > $ ip route add table secuplink default via 10.42.23.1 > > $ ip rule add pref 100 table main prefixlength 1 > $ ip rule add pref 150 fwmark 0xA table secuplink > > With this setup, packets marked 0xA will be processed by the additional routing > table "secuplink", but only if no suitable route in the main routing table can > be found. By using a minimal prefixlength of 1, the default route (/0) of the > table "main" is hidden to packets processed by rule 100; packets traveling to > destinations with more specific routing entries are processed as usual. > > Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> I just want to mention that the more quirky crap we put into the FIB rules layer, the harder it will every be to make a scalable data structure for FIB rule handling. Right now it's basically a linear walk of rules, with processing at each level. And every single ipv4 route lookup is going to go through this entire process. Anyways, there are coding style problems in your change which you'll need to address: > + if (!err && ops->suppress && ops->suppress(rule, arg)) { > + continue; > + } Since statement basic blocks should not use curly braces. > + if (!(arg->flags & FIB_LOOKUP_NOREF)) { > + fib_info_put(result->fi); > + } Likewise. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3] fib_rules: add .suppress operation 2013-07-31 22:13 ` David Miller @ 2013-08-01 0:24 ` Stefan Tomanek 2013-08-01 0:26 ` David Miller 0 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-08-01 0:24 UTC (permalink / raw) To: David Miller; +Cc: netdev, hannes, bsderandrew Dies schrieb David Miller (davem@davemloft.net): > I just want to mention that the more quirky crap we put into the FIB > rules layer, the harder it will every be to make a scalable data > structure for FIB rule handling. > > Right now it's basically a linear walk of rules, with processing at > each level. And it still is: but instead of just having pre-conditions whether to consult a table, the patch introduces post-conditions that can reject a routing decision retrieved from it. > Anyways, there are coding style problems in your change which you'll > need to address: Fixed in latest patch, thanks for the hints. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v3] fib_rules: add .suppress operation 2013-08-01 0:24 ` Stefan Tomanek @ 2013-08-01 0:26 ` David Miller 0 siblings, 0 replies; 33+ messages in thread From: David Miller @ 2013-08-01 0:26 UTC (permalink / raw) To: stefan.tomanek; +Cc: netdev, hannes, bsderandrew From: Stefan Tomanek <stefan.tomanek@wertarbyte.de> Date: Thu, 1 Aug 2013 02:24:07 +0200 > Dies schrieb David Miller (davem@davemloft.net): > >> I just want to mention that the more quirky crap we put into the FIB >> rules layer, the harder it will every be to make a scalable data >> structure for FIB rule handling. >> >> Right now it's basically a linear walk of rules, with processing at >> each level. > > And it still is: but instead of just having pre-conditions whether to > consult a table, the patch introduces post-conditions that can reject > a routing decision retrieved from it. This doesn't change my argument at all. The fact remains that the more complex conditions we add to the fib rule lookup, the harder it will be to optimize fib rule lookups into an O(1) or O(log n) operation. ^ permalink raw reply [flat|nested] 33+ messages in thread
* [PATCH v4] fib_rules: add .suppress operation 2013-07-27 7:07 ` Hannes Frederic Sowa ` (3 preceding siblings ...) 2013-07-30 7:46 ` [PATCH v3] " Stefan Tomanek @ 2013-08-01 0:17 ` Stefan Tomanek 2013-08-01 0:27 ` David Miller 4 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-08-01 0:17 UTC (permalink / raw) To: netdev; +Cc: Hannes Frederic Sowa, Andrew Collins, David Miller This change adds a new operation to the fib_rules_ops struct; it allows the suppression of routing decisions if certain criteria are not met by its results. The first implemented constraint is a minimum prefix length added to the structures of routing rules. If a rule is added with a minimum prefix length >0, only routes meeting this threshold will be considered. Any other (more general) routing table entries will be ignored. When configuring a system with multiple network uplinks and default routes, it is often convinient to reference the main routing table multiple times - but omitting the default route. Using this patch and a modified "ip" utility, this can be achieved by using the following command sequence: $ ip route add table secuplink default via 10.42.23.1 $ ip rule add pref 100 table main prefixlength 1 $ ip rule add pref 150 fwmark 0xA table secuplink With this setup, packets marked 0xA will be processed by the additional routing table "secuplink", but only if no suitable route in the main routing table can be found. By using a minimal prefixlength of 1, the default route (/0) of the table "main" is hidden to packets processed by rule 100; packets traveling to destinations with more specific routing entries are processed as usual. Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> --- include/net/fib_rules.h | 4 ++++ include/uapi/linux/fib_rules.h | 2 +- net/core/fib_rules.c | 8 ++++++++ net/ipv4/fib_rules.c | 14 ++++++++++++++ net/ipv6/fib6_rules.c | 13 +++++++++++++ 5 files changed, 40 insertions(+), 1 deletion(-) diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h index e361f48..2f286dc 100644 --- a/include/net/fib_rules.h +++ b/include/net/fib_rules.h @@ -18,6 +18,7 @@ struct fib_rule { u32 pref; u32 flags; u32 table; + u8 table_prefixlen_min; u8 action; u32 target; struct fib_rule __rcu *ctarget; @@ -46,6 +47,8 @@ struct fib_rules_ops { int (*action)(struct fib_rule *, struct flowi *, int, struct fib_lookup_arg *); + bool (*suppress)(struct fib_rule *, + struct fib_lookup_arg *); int (*match)(struct fib_rule *, struct flowi *, int); int (*configure)(struct fib_rule *, @@ -80,6 +83,7 @@ struct fib_rules_ops { [FRA_FWMARK] = { .type = NLA_U32 }, \ [FRA_FWMASK] = { .type = NLA_U32 }, \ [FRA_TABLE] = { .type = NLA_U32 }, \ + [FRA_TABLE_PREFIXLEN_MIN] = { .type = NLA_U8 }, \ [FRA_GOTO] = { .type = NLA_U32 } static inline void fib_rule_get(struct fib_rule *rule) diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h index 51da65b..59cd31b 100644 --- a/include/uapi/linux/fib_rules.h +++ b/include/uapi/linux/fib_rules.h @@ -45,7 +45,7 @@ enum { FRA_FLOW, /* flow/class id */ FRA_UNUSED6, FRA_UNUSED7, - FRA_UNUSED8, + FRA_TABLE_PREFIXLEN_MIN, FRA_TABLE, /* Extended table id */ FRA_FWMASK, /* mask for netfilter mark */ FRA_OIFNAME, diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 2173544..2ef5040 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -226,6 +226,9 @@ jumped: else err = ops->action(rule, fl, flags, arg); + if (!err && ops->suppress && ops->suppress(rule, arg)) + continue; + if (err != -EAGAIN) { if ((arg->flags & FIB_LOOKUP_NOREF) || likely(atomic_inc_not_zero(&rule->refcnt))) { @@ -337,6 +340,8 @@ static int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr* nlh) rule->action = frh->action; rule->flags = frh->flags; rule->table = frh_get_table(frh, tb); + if (tb[FRA_TABLE_PREFIXLEN_MIN]) + rule->table_prefixlen_min = nla_get_u8(tb[FRA_TABLE_PREFIXLEN_MIN]); if (!tb[FRA_PRIORITY] && ops->default_pref) rule->pref = ops->default_pref(ops); @@ -523,6 +528,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops, + nla_total_size(IFNAMSIZ) /* FRA_OIFNAME */ + nla_total_size(4) /* FRA_PRIORITY */ + nla_total_size(4) /* FRA_TABLE */ + + nla_total_size(1) /* FRA_TABLE_PREFIXLEN_MIN */ + nla_total_size(4) /* FRA_FWMARK */ + nla_total_size(4); /* FRA_FWMASK */ @@ -548,6 +554,8 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule, frh->table = rule->table; if (nla_put_u32(skb, FRA_TABLE, rule->table)) goto nla_put_failure; + if (nla_put_u8(skb, FRA_TABLE_PREFIXLEN_MIN, rule->table_prefixlen_min)) + goto nla_put_failure; frh->res1 = 0; frh->res2 = 0; frh->action = rule->action; diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c index 26aa65d..9f29066 100644 --- a/net/ipv4/fib_rules.c +++ b/net/ipv4/fib_rules.c @@ -101,6 +101,19 @@ errout: return err; } +static bool fib4_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) +{ + /* do not accept result if the route does + * not meet the required prefix length + */ + struct fib_result *result = (struct fib_result *) arg->result; + if (result->prefixlen < rule->table_prefixlen_min) { + if (!(arg->flags & FIB_LOOKUP_NOREF)) + fib_info_put(result->fi); + return true; + } + return false; +} static int fib4_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -267,6 +280,7 @@ static const struct fib_rules_ops __net_initconst fib4_rules_ops_template = { .rule_size = sizeof(struct fib4_rule), .addr_size = sizeof(u32), .action = fib4_rule_action, + .suppress = fib4_rule_suppress, .match = fib4_rule_match, .configure = fib4_rule_configure, .delete = fib4_rule_delete, diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index 4c8bac7..554a4fb 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -119,6 +119,18 @@ out: return err; } +static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg) +{ + struct rt6_info *rt = (struct rt6_info *) arg->result; + /* do not accept result if the route does + * not meet the required prefix length + */ + if (rt->rt6i_dst.plen < rule->table_prefixlen_min) { + ip6_rt_put(rt); + return true; + } + return false; +} static int fib6_rule_match(struct fib_rule *rule, struct flowi *fl, int flags) { @@ -252,6 +264,7 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = { .addr_size = sizeof(struct in6_addr), .action = fib6_rule_action, .match = fib6_rule_match, + .suppress = fib6_rule_suppress, .configure = fib6_rule_configure, .compare = fib6_rule_compare, .fill = fib6_rule_fill, -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [PATCH v4] fib_rules: add .suppress operation 2013-08-01 0:17 ` [PATCH v4] " Stefan Tomanek @ 2013-08-01 0:27 ` David Miller 2013-08-01 1:26 ` Stefan Tomanek 0 siblings, 1 reply; 33+ messages in thread From: David Miller @ 2013-08-01 0:27 UTC (permalink / raw) To: stefan.tomanek; +Cc: netdev, hannes, bsderandrew From: Stefan Tomanek <stefan.tomanek@wertarbyte.de> Date: Thu, 1 Aug 2013 02:17:15 +0200 > This change adds a new operation to the fib_rules_ops struct; it allows the > suppression of routing decisions if certain criteria are not met by its > results. > > The first implemented constraint is a minimum prefix length added to the > structures of routing rules. If a rule is added with a minimum prefix length >>0, only routes meeting this threshold will be considered. Any other (more > general) routing table entries will be ignored. > > When configuring a system with multiple network uplinks and default routes, it > is often convinient to reference the main routing table multiple times - but > omitting the default route. Using this patch and a modified "ip" utility, this > can be achieved by using the following command sequence: > > $ ip route add table secuplink default via 10.42.23.1 > > $ ip rule add pref 100 table main prefixlength 1 > $ ip rule add pref 150 fwmark 0xA table secuplink > > With this setup, packets marked 0xA will be processed by the additional routing > table "secuplink", but only if no suitable route in the main routing table can > be found. By using a minimal prefixlength of 1, the default route (/0) of the > table "main" is hidden to packets processed by rule 100; packets traveling to > destinations with more specific routing entries are processed as usual. > > Signed-off-by: Stefan Tomanek <stefan.tomanek@wertarbyte.de> Applied, thanks. ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4] fib_rules: add .suppress operation 2013-08-01 0:27 ` David Miller @ 2013-08-01 1:26 ` Stefan Tomanek 2013-08-01 5:35 ` Hannes Frederic Sowa 0 siblings, 1 reply; 33+ messages in thread From: Stefan Tomanek @ 2013-08-01 1:26 UTC (permalink / raw) To: David Miller; +Cc: netdev, hannes, bsderandrew Dies schrieb David Miller (davem@davemloft.net): > Applied, thanks. Thank you very much; Hannes Frederic Sowa noted some irregularities with the error reporting in fib6_rules (message ID <20130727060802.GD20273@order.stressinduktion.org>) that might interfere with the function of my patch; perhaps his change should be applied as well? I've reposted his changes with git format-patch as message ID <20130730065258.GG13357@zirkel.wertarbyte.de> ("[PATCH v2 1/2] fib6_rules: make error handling consistent with IPv4"). ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH v4] fib_rules: add .suppress operation 2013-08-01 1:26 ` Stefan Tomanek @ 2013-08-01 5:35 ` Hannes Frederic Sowa 0 siblings, 0 replies; 33+ messages in thread From: Hannes Frederic Sowa @ 2013-08-01 5:35 UTC (permalink / raw) To: Stefan Tomanek; +Cc: David Miller, netdev, bsderandrew On Thu, Aug 01, 2013 at 03:26:21AM +0200, Stefan Tomanek wrote: > Dies schrieb David Miller (davem@davemloft.net): > > > Applied, thanks. > > Thank you very much; Hannes Frederic Sowa noted some irregularities with the > error reporting in fib6_rules (message ID > <20130727060802.GD20273@order.stressinduktion.org>) that might interfere with > the function of my patch; perhaps his change should be applied as well? I've > reposted his changes with git format-patch as message ID > <20130730065258.GG13357@zirkel.wertarbyte.de> ("[PATCH v2 1/2] fib6_rules: make > error handling consistent with IPv4"). I can take care of this and will repost the patch today. Thank you, Hannes ^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [PATCH] fib_rules: add minimum prefix length 2013-07-25 21:46 ` Andrew Collins 2013-07-25 22:11 ` Stefan Tomanek @ 2013-07-26 10:54 ` Stefan Tomanek 1 sibling, 0 replies; 33+ messages in thread From: Stefan Tomanek @ 2013-07-26 10:54 UTC (permalink / raw) To: Andrew Collins; +Cc: netdev Dies schrieb Andrew Collins (bsderandrew@gmail.com): > I like the idea, but I agree with Hannes that it'd be nice to split > this out into a fib constraint callback. Here we go again: https://github.com/wertarbyte/linux/compare/fib_rule_suppress ^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2013-08-01 5:35 UTC | newest] Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-23 22:02 [PATCH] fib_rules: add minimum prefix length Stefan Tomanek 2013-07-23 22:38 ` Stephen Hemminger 2013-07-23 22:52 ` Stefan Tomanek 2013-07-24 2:14 ` Hannes Frederic Sowa 2013-07-24 7:57 ` Stefan Tomanek 2013-07-25 16:29 ` Stefan Tomanek 2013-07-25 17:43 ` [PATCH] fib_rules/fib6rules: " Stefan Tomanek 2013-07-25 18:17 ` [PATCH] fib_rules: " Hannes Frederic Sowa 2013-07-25 18:28 ` Stefan Tomanek 2013-07-25 21:46 ` Andrew Collins 2013-07-25 22:11 ` Stefan Tomanek 2013-07-26 10:46 ` [PATCH] fib_rules: add .suppress operation Stefan Tomanek 2013-07-26 17:05 ` Hannes Frederic Sowa 2013-07-27 6:08 ` Hannes Frederic Sowa 2013-07-27 6:26 ` Hannes Frederic Sowa 2013-07-27 7:07 ` Hannes Frederic Sowa 2013-07-27 10:21 ` Stefan Tomanek 2013-07-27 15:10 ` Hannes Frederic Sowa 2013-07-28 10:36 ` Stefan Tomanek 2013-07-30 6:52 ` [PATCH v2 1/2] fib6_rules: make error handling consistent with IPv4 Stefan Tomanek 2013-07-30 6:53 ` [PATCH v2 2/2] fib_rules: add .suppress operation Stefan Tomanek 2013-07-30 7:03 ` David Miller 2013-07-30 7:23 ` Stefan Tomanek 2013-07-30 7:34 ` David Miller 2013-07-30 7:46 ` [PATCH v3] " Stefan Tomanek 2013-07-31 22:13 ` David Miller 2013-08-01 0:24 ` Stefan Tomanek 2013-08-01 0:26 ` David Miller 2013-08-01 0:17 ` [PATCH v4] " Stefan Tomanek 2013-08-01 0:27 ` David Miller 2013-08-01 1:26 ` Stefan Tomanek 2013-08-01 5:35 ` Hannes Frederic Sowa 2013-07-26 10:54 ` [PATCH] fib_rules: add minimum prefix length Stefan Tomanek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).