Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
On Wed, Mar 28, 2007 at 09:34:36PM +0200, Thomas Graf wrote: * David Miller [EMAIL PROTECTED] 2007-03-28 11:24 Another idea Thomas and I tossed around was to have some kind of way for the rule insertion to indicate that the flush should be deferred and I kind of prefer that explicitness. Right, although I believe the flag should not only defer it but not flush at all. This would be the optimal solution for scripts which can do a ip ro flush cache as they know what they're doing. By default it's better the flush immediately, because the old behavior is totally unexpected. I insert a rule and it dosn't show up?, nobody expects that. It's a tough call, I'd favour immediate flush as well but I can see the point in delaying by ip_rt_min_delay which can be configured by the user. So people can choose to immediately flush by setting it to 0. It would also be consistent to the flush after route changes, the same delay is used there. Of course you both are right - but (...I've some doubts): - there is a difference between tools: route or ip route (as a successor) and ip rule; the latter is intended for advanced things, so users have to expect... (or RTFM!). - of course immediate flush seems to be more natural, but it isn't like that and rules (other rules) are changed, so maybe some transitory way is needed; these 2s look like a good compromise, but after looking into rt_cache_flush - it's not for all (I know - we don't like multipath - but untill it's here...) and these locks and timers aren't for free, too; so, IMHO, something like -n[oflush] option is a mustbe. - for consistency probably all ip objects should be verified: to flush or not to flush by default. Cheers, Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
On Thu, Mar 29, 2007 at 12:03:26PM +0200, Jarek Poplawski wrote: ... rt_cache_flush - it's not for all (I know - we don't like multipath - but untill it's here...)[...] Sorry, I forgot it's already not there... Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
On 27-03-2007 14:38, Thomas Graf wrote: The results of FIB rules lookups are cached in the routing cache except for IPv6 as no such cache exists. So far, it was the responsibility of the user to flush the cache after modifying any rules. This lead to many false bug reports due to misunderstanding of this concept. This patch automatically flushes the route cache after inserting or deleting a rule. I hope I'm wrong, but isn't this at the cost of admins working with long rules' sets, which (probably) take extra time now? Regards, Jarek P. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
* Jarek Poplawski [EMAIL PROTECTED] 2007-03-28 13:19
I hope I'm wrong, but isn't this at the cost of admins
working with long rules' sets, which (probably) take extra
time now?
That's right, it makes the insert and delete operation more
expensive.
A compromise would be to delay the flushing and wait for
some time (default 2 seconds) whether more rules or routes
are being added before flushing.
[NET] fib_rules: delay route cache flush by ip_rt_min_delay
Signed-off-by: Thomas Graf [EMAIL PROTECTED]
Index: linux/net-2.6.22/net/decnet/dn_rules.c
===
--- linux.orig/net-2.6.22/net/decnet/dn_rules.c 2007-03-28 17:41:22.0
+0200
+++ linux/net-2.6.22/net/decnet/dn_rules.c 2007-03-28 17:41:39.0
+0200
@@ -242,7 +242,7 @@ static u32 dn_fib_rule_default_pref(void
static void dn_fib_rule_flush_cache(void)
{
- dn_rt_cache_flush(0);
+ dn_rt_cache_flush(-1);
}
static struct fib_rules_ops dn_fib_rules_ops = {
Index: linux/net-2.6.22/net/ipv4/fib_rules.c
===
--- linux.orig/net-2.6.22/net/ipv4/fib_rules.c 2007-03-28 17:41:18.0
+0200
+++ linux/net-2.6.22/net/ipv4/fib_rules.c 2007-03-28 17:41:30.0
+0200
@@ -300,7 +300,7 @@ static size_t fib4_rule_nlmsg_payload(st
static void fib4_rule_flush_cache(void)
{
- rt_cache_flush(0);
+ rt_cache_flush(-1);
}
static struct fib_rules_ops fib4_rules_ops = {
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
From: Thomas Graf [EMAIL PROTECTED] Date: Wed, 28 Mar 2007 17:49:03 +0200 * Jarek Poplawski [EMAIL PROTECTED] 2007-03-28 13:19 I hope I'm wrong, but isn't this at the cost of admins working with long rules' sets, which (probably) take extra time now? That's right, it makes the insert and delete operation more expensive. A compromise would be to delay the flushing and wait for some time (default 2 seconds) whether more rules or routes are being added before flushing. Another idea Thomas and I tossed around was to have some kind of way for the rule insertion to indicate that the flush should be deferred and I kind of prefer that explicitness. By default it's better the flush immediately, because the old behavior is totally unexpected. I insert a rule and it dosn't show up?, nobody expects that. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
* David Miller [EMAIL PROTECTED] 2007-03-28 11:24 Another idea Thomas and I tossed around was to have some kind of way for the rule insertion to indicate that the flush should be deferred and I kind of prefer that explicitness. Right, although I believe the flag should not only defer it but not flush at all. This would be the optimal solution for scripts which can do a ip ro flush cache as they know what they're doing. By default it's better the flush immediately, because the old behavior is totally unexpected. I insert a rule and it dosn't show up?, nobody expects that. It's a tough call, I'd favour immediate flush as well but I can see the point in delaying by ip_rt_min_delay which can be configured by the user. So people can choose to immediately flush by setting it to 0. It would also be consistent to the flush after route changes, the same delay is used there. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
From: Thomas Graf [EMAIL PROTECTED] Date: Wed, 28 Mar 2007 21:34:36 +0200 So people can choose to immediately flush by setting it to 0. It would also be consistent to the flush after route changes, the same delay is used there. That's a good point I hadn't considered. Therefore, I think I'll apply your patch, considering that. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND] [NET] fib_rules: Flush route cache after rule modifications
The results of FIB rules lookups are cached in the routing cache
except for IPv6 as no such cache exists. So far, it was the
responsibility of the user to flush the cache after modifying any
rules. This lead to many false bug reports due to misunderstanding
of this concept.
This patch automatically flushes the route cache after inserting
or deleting a rule.
Thanks to Muli Ben-Yehuda [EMAIL PROTECTED] for catching a bug
in the previous patch.
Signed-off-by: Thomas Graf [EMAIL PROTECTED]
Index: net-2.6.22/include/net/fib_rules.h
===
--- net-2.6.22.orig/include/net/fib_rules.h 2007-03-27 13:54:52.0
+0200
+++ net-2.6.22/include/net/fib_rules.h 2007-03-27 14:16:24.0 +0200
@@ -59,6 +59,10 @@ struct fib_rules_ops
u32 (*default_pref)(void);
size_t (*nlmsg_payload)(struct fib_rule *);
+ /* Called after modifications to the rules set, must flush
+* the route cache if one exists. */
+ void(*flush_cache)(void);
+
int nlgroup;
struct nla_policy *policy;
struct list_head*rules_list;
Index: net-2.6.22/net/core/fib_rules.c
===
--- net-2.6.22.orig/net/core/fib_rules.c2007-03-27 13:53:29.0
+0200
+++ net-2.6.22/net/core/fib_rules.c 2007-03-27 15:36:05.0 +0200
@@ -44,6 +44,12 @@ static void rules_ops_put(struct fib_rul
module_put(ops-owner);
}
+static void flush_route_cache(struct fib_rules_ops *ops)
+{
+ if (ops-flush_cache)
+ ops-flush_cache();
+}
+
int fib_rules_register(struct fib_rules_ops *ops)
{
int err = -EEXIST;
@@ -314,6 +320,7 @@ static int fib_nl_newrule(struct sk_buff
list_add_rcu(rule-list, ops-rules_list);
notify_rule_change(RTM_NEWRULE, rule, ops, nlh, NETLINK_CB(skb).pid);
+ flush_route_cache(ops);
rules_ops_put(ops);
return 0;
@@ -404,6 +411,7 @@ static int fib_nl_delrule(struct sk_buff
notify_rule_change(RTM_DELRULE, rule, ops, nlh,
NETLINK_CB(skb).pid);
fib_rule_put(rule);
+ flush_route_cache(ops);
rules_ops_put(ops);
return 0;
}
Index: net-2.6.22/net/ipv4/fib_rules.c
===
--- net-2.6.22.orig/net/ipv4/fib_rules.c2007-03-27 13:54:57.0
+0200
+++ net-2.6.22/net/ipv4/fib_rules.c 2007-03-27 15:35:43.0 +0200
@@ -298,6 +298,11 @@ static size_t fib4_rule_nlmsg_payload(st
+ nla_total_size(4); /* flow */
}
+static void fib4_rule_flush_cache(void)
+{
+ rt_cache_flush(0);
+}
+
static struct fib_rules_ops fib4_rules_ops = {
.family = AF_INET,
.rule_size = sizeof(struct fib4_rule),
@@ -309,6 +314,7 @@ static struct fib_rules_ops fib4_rules_o
.fill = fib4_rule_fill,
.default_pref = fib4_rule_default_pref,
.nlmsg_payload = fib4_rule_nlmsg_payload,
+ .flush_cache= fib4_rule_flush_cache,
.nlgroup= RTNLGRP_IPV4_RULE,
.policy = fib4_rule_policy,
.rules_list = fib4_rules,
Index: net-2.6.22/net/decnet/dn_rules.c
===
--- net-2.6.22.orig/net/decnet/dn_rules.c 2007-03-27 14:00:56.0
+0200
+++ net-2.6.22/net/decnet/dn_rules.c2007-03-27 15:35:43.0 +0200
@@ -31,6 +31,7 @@
#include net/dn_fib.h
#include net/dn_neigh.h
#include net/dn_dev.h
+#include net/dn_route.h
static struct fib_rules_ops dn_fib_rules_ops;
@@ -239,6 +240,11 @@ static u32 dn_fib_rule_default_pref(void
return 0;
}
+static void dn_fib_rule_flush_cache(void)
+{
+ dn_rt_cache_flush(0);
+}
+
static struct fib_rules_ops dn_fib_rules_ops = {
.family = AF_DECnet,
.rule_size = sizeof(struct dn_fib_rule),
@@ -249,6 +255,7 @@ static struct fib_rules_ops dn_fib_rules
.compare= dn_fib_rule_compare,
.fill = dn_fib_rule_fill,
.default_pref = dn_fib_rule_default_pref,
+ .flush_cache= dn_fib_rule_flush_cache,
.nlgroup= RTNLGRP_DECnet_RULE,
.policy = dn_fib_rule_policy,
.rules_list = dn_fib_rules,
-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND] [NET] fib_rules: Flush route cache after rule modifications
From: Thomas Graf [EMAIL PROTECTED] Date: Tue, 27 Mar 2007 15:38:45 +0200 The results of FIB rules lookups are cached in the routing cache except for IPv6 as no such cache exists. So far, it was the responsibility of the user to flush the cache after modifying any rules. This lead to many false bug reports due to misunderstanding of this concept. This patch automatically flushes the route cache after inserting or deleting a rule. Thanks to Muli Ben-Yehuda [EMAIL PROTECTED] for catching a bug in the previous patch. Signed-off-by: Thomas Graf [EMAIL PROTECTED] Applied, thanks. - To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
