Re: [PATCH net-next] ipv4/fib: send RTM_DELROUTE notify when flush fib

Ido Schimmel
2023-07-20T14:30:07Z

On Thu, Jul 20, 2023 at 03:51:13PM +0800, Hangbin Liu wrote:
> On Tue, Jul 18, 2023 at 08:58:14AM -0700, Stephen Hemminger wrote:
> > On Tue, 18 Jul 2023 13:19:06 +0300
> > Ido Schimmel <idosch@idosch.org> wrote:
> >
> > > fib_table_flush() isn't only called when an address is deleted, but also
> > > when an interface is deleted or put down. The lack of notification in
> > > these cases is deliberate. Commit 7c6bb7d2faaf ("net/ipv6: Add knob to
> > > skip DELROUTE message on device down") introduced a sysctl to make IPv6
> > > behave like IPv4 in this regard, but this patch breaks it.
> > >
> > > IMO, the number of routes being flushed because a preferred source
> > > address is deleted is significantly lower compared to interface down /
> > > deletion, so generating notifications in this case is probably OK. It
>
> How about ignore route deletion for link down? e.g.
>
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 74d403dbd2b4..11c0f325e887 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -2026,6 +2026,7 @@ void fib_table_flush_external(struct fib_table *tb)
> int fib_table_flush(struct net *net, struct fib_table *tb, bool flush_all)
> {
> struct trie *t = (struct trie *)tb->tb_data;
> + struct nl_info info = { .nl_net = net };
> struct key_vector *pn = t->kv;
> unsigned long cindex = 1;
> struct hlist_node *tmp;
> @@ -2088,6 +2089,11 @@ int fib_table_flush(struct net *net, struct fib_table *tb, bool flush_all)
>
> fib_notify_alias_delete(net, n->key, &n->leaf, fa,
> NULL);
> + if (!(fi->fib_flags & RTNH_F_LINKDOWN)) {
> + rtmsg_fib(RTM_DELROUTE, htonl(n->key), fa,
> + KEYLENGTH - fa->fa_slen, tb->tb_id, &info, 0);
> + }

Will you get a notification in this case for 198.51.100.0/24?

# ip link add name dummy1 up type dummy
# ip link add name dummy2 up type dummy
# ip address add 192.0.2.1/24 dev dummy1
# ip route add 198.51.100.0/24 dev dummy2 src 192.0.2.1
# ip link set dev dummy2 carrier off
# ip -4 r s
192.0.2.0/24 dev dummy1 proto kernel scope link src 192.0.2.1
198.51.100.0/24 dev dummy2 scope link src 192.0.2.1 linkdown
# ip address del 192.0.2.1/24 dev dummy1
# ip -4 r s

> hlist_del_rcu(&fa->fa_list);
> fib_release_info(fa->fa_info);
> alias_free_mem_rcu(fa);
>
> > > also seems to be consistent with IPv6 given that rt6_remove_prefsrc()
> > > calls fib6_clean_all() and not fib6_clean_all_skip_notify().
> >
> > Agree. Imagine the case of 3 million routes and device goes down.
> > There is a reason IPv4 behaves the way it does.