* [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
@ 2013-10-17 11:13 Kristian Evensen
2013-10-17 13:43 ` Sergei Shtylyov
0 siblings, 1 reply; 12+ messages in thread
From: Kristian Evensen @ 2013-10-17 11:13 UTC (permalink / raw)
To: netdev; +Cc: Kristian Evensen
From: Kristian Evensen <kristian.evensen@gmail.com>
When a link is set as down using for example "ip link set dev X down", routes
are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
removed, and makes rtnelink route deletion behavior consistent across commands.
The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
include required information otherwise unavailable in trie_flush_list().
One use case that is simplified by this patch is IPv4 WAN connection monitoring.
Instead of listening for and handling both RTM_*ROUTE and RTM_*LINK-messages, it
is sufficient to handle RTM_*ROUTE.
Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
---
net/ipv4/fib_trie.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index ec9a9ef..acd5b3b 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1698,15 +1698,23 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg)
return 0;
}
-static int trie_flush_list(struct list_head *head)
+static int trie_flush_list(struct list_head *head, u32 tb_id, t_key key,
+ int plen)
{
struct fib_alias *fa, *fa_node;
int found = 0;
+ struct nl_info cfg;
+
+ memset(&cfg, 0, sizeof(cfg));
list_for_each_entry_safe(fa, fa_node, head, fa_list) {
struct fib_info *fi = fa->fa_info;
if (fi && (fi->fib_flags & RTNH_F_DEAD)) {
+ cfg.nl_net = fi->fib_net;
+ rtmsg_fib(RTM_DELROUTE, htonl(key), fa, plen, tb_id,
+ &cfg, 0);
+
list_del_rcu(&fa->fa_list);
fib_release_info(fa->fa_info);
alias_free_mem_rcu(fa);
@@ -1716,7 +1724,7 @@ static int trie_flush_list(struct list_head *head)
return found;
}
-static int trie_flush_leaf(struct leaf *l)
+static int trie_flush_leaf(struct leaf *l, u32 tb_id)
{
int found = 0;
struct hlist_head *lih = &l->list;
@@ -1724,7 +1732,7 @@ static int trie_flush_leaf(struct leaf *l)
struct leaf_info *li = NULL;
hlist_for_each_entry_safe(li, tmp, lih, hlist) {
- found += trie_flush_list(&li->falh);
+ found += trie_flush_list(&li->falh, tb_id, l->key, li->plen);
if (list_empty(&li->falh)) {
hlist_del_rcu(&li->hlist);
@@ -1813,7 +1821,7 @@ int fib_table_flush(struct fib_table *tb)
int found = 0;
for (l = trie_firstleaf(t); l; l = trie_nextleaf(l)) {
- found += trie_flush_leaf(l);
+ found += trie_flush_leaf(l, tb->tb_id);
if (ll && hlist_empty(&ll->list))
trie_leaf_remove(t, ll);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 11:13 [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Kristian Evensen
@ 2013-10-17 13:43 ` Sergei Shtylyov
2013-10-17 16:40 ` Kristian Evensen
0 siblings, 1 reply; 12+ messages in thread
From: Sergei Shtylyov @ 2013-10-17 13:43 UTC (permalink / raw)
To: Kristian Evensen; +Cc: netdev
Hello.
On 17-10-2013 15:13, Kristian Evensen wrote:
> From: Kristian Evensen <kristian.evensen@gmail.com>
> When a link is set as down using for example "ip link set dev X down", routes
> are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
> patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
> removed, and makes rtnelink route deletion behavior consistent across commands.
> The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
> include required information otherwise unavailable in trie_flush_list().
> One use case that is simplified by this patch is IPv4 WAN connection monitoring.
> Instead of listening for and handling both RTM_*ROUTE and RTM_*LINK-messages, it
> is sufficient to handle RTM_*ROUTE.
> Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
> ---
> net/ipv4/fib_trie.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index ec9a9ef..acd5b3b 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -1698,15 +1698,23 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg)
> return 0;
> }
>
> -static int trie_flush_list(struct list_head *head)
> +static int trie_flush_list(struct list_head *head, u32 tb_id, t_key key,
> + int plen)
The continuation line should start right under *struct* on the first line,
according to the networking coding style.
> {
> struct fib_alias *fa, *fa_node;
> int found = 0;
> + struct nl_info cfg;
> +
> + memset(&cfg, 0, sizeof(cfg));
>
> list_for_each_entry_safe(fa, fa_node, head, fa_list) {
> struct fib_info *fi = fa->fa_info;
>
> if (fi && (fi->fib_flags & RTNH_F_DEAD)) {
> + cfg.nl_net = fi->fib_net;
> + rtmsg_fib(RTM_DELROUTE, htonl(key), fa, plen, tb_id,
> + &cfg, 0);
Here the line should start right under RTM_DELROUTE.
WBR, Sergei
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 13:43 ` Sergei Shtylyov
@ 2013-10-17 16:40 ` Kristian Evensen
2013-10-17 16:54 ` Kristian Evensen
0 siblings, 1 reply; 12+ messages in thread
From: Kristian Evensen @ 2013-10-17 16:40 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: Network Development
Hi,
> The continuation line should start right under *struct* on the first
> line, according to the networking coding style.
Thank you very much for letting me know. Will resubmit the patch.
-Kristian
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 16:40 ` Kristian Evensen
@ 2013-10-17 16:54 ` Kristian Evensen
2013-10-17 17:23 ` Joe Perches
2013-10-17 19:52 ` [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Julian Anastasov
0 siblings, 2 replies; 12+ messages in thread
From: Kristian Evensen @ 2013-10-17 16:54 UTC (permalink / raw)
To: netdev; +Cc: Kristian Evensen
From: Kristian Evensen <kristian.evensen@gmail.com>
When a link is set as down using for example "ip link set dev X down", routes
are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
removed, and makes rtnelink route deletion behavior consistent across commands.
The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
include required information otherwise unavailable in trie_flush_list().
One use case that is simplified by this patch is IPv4 WAN connection monitoring.
Instead of listening for and handling both RTM_*ROUTE and RTM_*LINK-messages, it
is sufficient to handle RTM_*ROUTE.
Signed-off-by: Kristian Evensen <kristian.evensen@gmail.com>
---
net/ipv4/fib_trie.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index ec9a9ef..629747a 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1698,15 +1698,23 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg)
return 0;
}
-static int trie_flush_list(struct list_head *head)
+static int trie_flush_list(struct list_head *head, u32 tb_id, t_key key,
+ int plen)
{
struct fib_alias *fa, *fa_node;
int found = 0;
+ struct nl_info cfg;
+
+ memset(&cfg, 0, sizeof(cfg));
list_for_each_entry_safe(fa, fa_node, head, fa_list) {
struct fib_info *fi = fa->fa_info;
if (fi && (fi->fib_flags & RTNH_F_DEAD)) {
+ cfg.nl_net = fi->fib_net;
+ rtmsg_fib(RTM_DELROUTE, htonl(key), fa, plen, tb_id,
+ &cfg, 0);
+
list_del_rcu(&fa->fa_list);
fib_release_info(fa->fa_info);
alias_free_mem_rcu(fa);
@@ -1716,7 +1724,7 @@ static int trie_flush_list(struct list_head *head)
return found;
}
-static int trie_flush_leaf(struct leaf *l)
+static int trie_flush_leaf(struct leaf *l, u32 tb_id)
{
int found = 0;
struct hlist_head *lih = &l->list;
@@ -1724,7 +1732,7 @@ static int trie_flush_leaf(struct leaf *l)
struct leaf_info *li = NULL;
hlist_for_each_entry_safe(li, tmp, lih, hlist) {
- found += trie_flush_list(&li->falh);
+ found += trie_flush_list(&li->falh, tb_id, l->key, li->plen);
if (list_empty(&li->falh)) {
hlist_del_rcu(&li->hlist);
@@ -1813,7 +1821,7 @@ int fib_table_flush(struct fib_table *tb)
int found = 0;
for (l = trie_firstleaf(t); l; l = trie_nextleaf(l)) {
- found += trie_flush_leaf(l);
+ found += trie_flush_leaf(l, tb->tb_id);
if (ll && hlist_empty(&ll->list))
trie_leaf_remove(t, ll);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 16:54 ` Kristian Evensen
@ 2013-10-17 17:23 ` Joe Perches
2013-10-17 20:14 ` Kristian Evensen
2013-10-17 19:52 ` [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Julian Anastasov
1 sibling, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-10-17 17:23 UTC (permalink / raw)
To: Kristian Evensen; +Cc: netdev
On Thu, 2013-10-17 at 18:54 +0200, Kristian Evensen wrote:
> From: Kristian Evensen <kristian.evensen@gmail.com>
>
> When a link is set as down using for example "ip link set dev X down", routes
> are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
> patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
> removed, and makes rtnelink route deletion behavior consistent across commands.
> The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
> include required information otherwise unavailable in trie_flush_list().
[]
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
[]
> @@ -1698,15 +1698,23 @@ int fib_table_delete(struct fib_table *tb, struct fib_config *cfg)
> return 0;
> }
>
> -static int trie_flush_list(struct list_head *head)
> +static int trie_flush_list(struct list_head *head, u32 tb_id, t_key key,
> + int plen)
> {
> struct fib_alias *fa, *fa_node;
> int found = 0;
> + struct nl_info cfg;
> +
> + memset(&cfg, 0, sizeof(cfg));
Perhaps this memset should be moved into the
list_for_each_entry_safe loop where cfg is used.
> list_for_each_entry_safe(fa, fa_node, head, fa_list) {
> struct fib_info *fi = fa->fa_info;
>
> if (fi && (fi->fib_flags & RTNH_F_DEAD)) {
memset(&cfg, 0, sizeof(cfg));
?
> + cfg.nl_net = fi->fib_net;
> + rtmsg_fib(RTM_DELROUTE, htonl(key), fa, plen, tb_id,
> + &cfg, 0);
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 17:23 ` Joe Perches
@ 2013-10-17 20:14 ` Kristian Evensen
2013-10-17 20:34 ` [PATCH net-next] fib: Use const struct nl_info * in rtmsg_fib Joe Perches
0 siblings, 1 reply; 12+ messages in thread
From: Kristian Evensen @ 2013-10-17 20:14 UTC (permalink / raw)
To: Joe Perches; +Cc: Network Development
Hi,
On Thu, Oct 17, 2013 at 7:23 PM, Joe Perches <joe@perches.com> wrote:
>
> Perhaps this memset should be moved into the
> list_for_each_entry_safe loop where cfg is used.
Thanks for your comment. I decided to put it there to avoid calling it
for each route that will be deleted. Since the struct is never
modified (only read in fib_rtmsg()) and we have the rtnl_lock, I
assumed it to be safe. Are there cases where this assumption is wrong?
-Kristian
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH net-next] fib: Use const struct nl_info * in rtmsg_fib
2013-10-17 20:14 ` Kristian Evensen
@ 2013-10-17 20:34 ` Joe Perches
2013-10-18 18:42 ` David Miller
0 siblings, 1 reply; 12+ messages in thread
From: Joe Perches @ 2013-10-17 20:34 UTC (permalink / raw)
To: Kristian Evensen; +Cc: Network Development
The rtmsg_fib function doesn't modify this argument so mark
it const.
Signed-off-by: Joe Perches <joe@perches.com>
---
On Thu, 2013-10-17 at 22:14 +0200, Kristian Evensen wrote:
> Hi,
Hi Kristian.
> On Thu, Oct 17, 2013 at 7:23 PM, Joe Perches <joe@perches.com> wrote:
> >
> > Perhaps this memset should be moved into the
> > list_for_each_entry_safe loop where cfg is used.
>
> Thanks for your comment. I decided to put it there to avoid calling it
> for each route that will be deleted. Since the struct is never
> modified (only read in fib_rtmsg()) and we have the rtnl_lock, I
> assumed it to be safe. Are there cases where this assumption is wrong?
I didn't look too hard.
I just glanced at the prototype and saw it wasn't const.
If rtmsg_fib() doesn't modify cfg (and it doesn't seem to)
then the prototype and function should be marked const.
And you're right, it doesn't need to be inside the loop.
cheers, Joe
net/ipv4/fib_lookup.h | 2 +-
net/ipv4/fib_semantics.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/fib_lookup.h b/net/ipv4/fib_lookup.h
index af0f14a..50cfb3e 100644
--- a/net/ipv4/fib_lookup.h
+++ b/net/ipv4/fib_lookup.h
@@ -32,7 +32,7 @@ extern int fib_dump_info(struct sk_buff *skb, u32 pid, u32 seq, int event,
int dst_len, u8 tos, struct fib_info *fi,
unsigned int);
extern void rtmsg_fib(int event, __be32 key, struct fib_alias *fa,
- int dst_len, u32 tb_id, struct nl_info *info,
+ int dst_len, u32 tb_id, const struct nl_info *info,
unsigned int nlm_flags);
extern struct fib_alias *fib_find_alias(struct list_head *fah,
u8 tos, u32 prio);
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index d5dbca5..e63f47a 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -380,7 +380,7 @@ static inline size_t fib_nlmsg_size(struct fib_info *fi)
}
void rtmsg_fib(int event, __be32 key, struct fib_alias *fa,
- int dst_len, u32 tb_id, struct nl_info *info,
+ int dst_len, u32 tb_id, const struct nl_info *info,
unsigned int nlm_flags)
{
struct sk_buff *skb;
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 16:54 ` Kristian Evensen
2013-10-17 17:23 ` Joe Perches
@ 2013-10-17 19:52 ` Julian Anastasov
2013-10-17 20:11 ` Kristian Evensen
2013-10-17 20:19 ` David Miller
1 sibling, 2 replies; 12+ messages in thread
From: Julian Anastasov @ 2013-10-17 19:52 UTC (permalink / raw)
To: Kristian Evensen; +Cc: netdev
Hello,
On Thu, 17 Oct 2013, Kristian Evensen wrote:
> From: Kristian Evensen <kristian.evensen@gmail.com>
>
> When a link is set as down using for example "ip link set dev X down", routes
> are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
> patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
> removed, and makes rtnelink route deletion behavior consistent across commands.
> The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
> include required information otherwise unavailable in trie_flush_list().
>
> One use case that is simplified by this patch is IPv4 WAN connection monitoring.
> Instead of listening for and handling both RTM_*ROUTE and RTM_*LINK-messages, it
> is sufficient to handle RTM_*ROUTE.
IIRC, such notifications were not implemented
to avoid storms to routing daemons. Not sure if this is
still true.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 19:52 ` [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Julian Anastasov
@ 2013-10-17 20:11 ` Kristian Evensen
2013-10-17 20:19 ` David Miller
2013-10-17 20:19 ` David Miller
1 sibling, 1 reply; 12+ messages in thread
From: Kristian Evensen @ 2013-10-17 20:11 UTC (permalink / raw)
To: Julian Anastasov; +Cc: Network Development
Hi,
On Thu, Oct 17, 2013 at 9:52 PM, Julian Anastasov <ja@ssi.bg> wrote:
> IIRC, such notifications were not implemented
> to avoid storms to routing daemons. Not sure if this is
> still true.
Thanks for letting me know. I was wondering if this was the case, but
then I saw that the deletion of IPv6 routes are announced (when
running "ip link set dev X down") and assumed that message storms was
(is?) not considered a problem.
-Kristian
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 20:11 ` Kristian Evensen
@ 2013-10-17 20:19 ` David Miller
0 siblings, 0 replies; 12+ messages in thread
From: David Miller @ 2013-10-17 20:19 UTC (permalink / raw)
To: kristian.evensen; +Cc: ja, netdev
From: Kristian Evensen <kristian.evensen@gmail.com>
Date: Thu, 17 Oct 2013 22:11:39 +0200
> On Thu, Oct 17, 2013 at 9:52 PM, Julian Anastasov <ja@ssi.bg> wrote:
>> IIRC, such notifications were not implemented
>> to avoid storms to routing daemons. Not sure if this is
>> still true.
>
> Thanks for letting me know. I was wondering if this was the case, but
> then I saw that the deletion of IPv6 routes are announced (when
> running "ip link set dev X down") and assumed that message storms was
> (is?) not considered a problem.
I really wish IPV6 wouldn't be inconsistent and behave that way.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down
2013-10-17 19:52 ` [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Julian Anastasov
2013-10-17 20:11 ` Kristian Evensen
@ 2013-10-17 20:19 ` David Miller
1 sibling, 0 replies; 12+ messages in thread
From: David Miller @ 2013-10-17 20:19 UTC (permalink / raw)
To: ja; +Cc: kristian.evensen, netdev
From: Julian Anastasov <ja@ssi.bg>
Date: Thu, 17 Oct 2013 22:52:43 +0300 (EEST)
>
> Hello,
>
> On Thu, 17 Oct 2013, Kristian Evensen wrote:
>
>> From: Kristian Evensen <kristian.evensen@gmail.com>
>>
>> When a link is set as down using for example "ip link set dev X down", routes
>> are deleted, but RTM_DELROUTE messages are not sent to RTNLGRP_IPV4_ROUTE. This
>> patch makes trie_flush_list() send a RTM_DELROUTE for each route that is
>> removed, and makes rtnelink route deletion behavior consistent across commands.
>> The parameter lists for trie_flush_list() and trie_flush_leaf() are expanded to
>> include required information otherwise unavailable in trie_flush_list().
>>
>> One use case that is simplified by this patch is IPv4 WAN connection monitoring.
>> Instead of listening for and handling both RTM_*ROUTE and RTM_*LINK-messages, it
>> is sufficient to handle RTM_*ROUTE.
>
> IIRC, such notifications were not implemented
> to avoid storms to routing daemons. Not sure if this is
> still true.
I'm definitely not applying this patch.
Routing daemons maintain the routing table internally, and they
therefore can purge their internal data structures when the link
down event occurs.
This is how it has worked forever and for real BGP tables the
RTM_DELROUTE traffic would be enormous and at nearly DoS levels
for anyone listening for them.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2013-10-18 18:42 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17 11:13 [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Kristian Evensen
2013-10-17 13:43 ` Sergei Shtylyov
2013-10-17 16:40 ` Kristian Evensen
2013-10-17 16:54 ` Kristian Evensen
2013-10-17 17:23 ` Joe Perches
2013-10-17 20:14 ` Kristian Evensen
2013-10-17 20:34 ` [PATCH net-next] fib: Use const struct nl_info * in rtmsg_fib Joe Perches
2013-10-18 18:42 ` David Miller
2013-10-17 19:52 ` [PATCH net-next] fib_trie: Send RTM_DELROUTE when link goes down Julian Anastasov
2013-10-17 20:11 ` Kristian Evensen
2013-10-17 20:19 ` David Miller
2013-10-17 20:19 ` David Miller
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).