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

From: Hangbin Liu <liuhangbin@gmail.com>
To: Ido Schimmel <idosch@idosch.org>
Cc: David Ahern <dsahern@kernel.org>,
	Stephen Hemminger <stephen@networkplumber.org>,
	netdev@vger.kernel.org, "David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Thomas Haller <thaller@redhat.com>
Subject: Re: [PATCH net-next] ipv4/fib: send RTM_DELROUTE notify when flush fib
Date: Mon, 24 Jul 2023 16:56:37 +0800	[thread overview]
Message-ID: <ZL48xbowL8QQRr9s@Laptop-X1> (raw)
In-Reply-To: <ZLzY42I/GjWCJ5Do@shredder>

On Sun, Jul 23, 2023 at 10:38:11AM +0300, Ido Schimmel wrote:
> > > >>> @@ -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?
> > > > 
> > > > No. Do you think it is expected with this patch or not?
> > > 
> > > The intent is that notifications are sent for link events but not route
> > > events which are easily deduced from the link events.
> > 
> > Sorry, I didn't get what you mean. The link events should be notified to user
> > via rtmsg_ifinfo_event()? So I think here we ignore the route events caused by
> > link down looks reasonable.
> 
> The route in the scenario I mentioned wasn't deleted because of a link
> event, but because the source address was deleted yet no notification
> was emitted. IMO, this is wrong given the description of the patch.

OK. I got what you mean now. Is there a way to get the device the route
bond in fib_table_flush() so we can also check the dev flag?

> 
> I assume NetworkManager already knows how to delete routes given
> RTM_DELLINK events. Can't it be taught to react to RTM_DELADDR events as
> well? Then this functionality will always work, regardless of the kernel
> version being used.

The NetworkManager keeps a cache of the routes. Missing/Wrong events mean that
the cache becomes inconsistent. The IPv4 will not send src route delete info
if it's bond to other device. While IPv6 only modify the src route instead of
delete it, and also no notify. So NetworkManager developers complained and
hope to have a consistent and clear notification about route modify/delete.

Thanks
Hangbin