Merge pull request #1029 from hedrok/T6962-frr-zebra-kernel-routes

T6962: frr: fix wrong kernel routes updates
This commit is contained in:
Daniil Baturin 2025-09-16 15:33:54 +01:00 committed by GitHub
commit d7c3843909
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -0,0 +1,214 @@
From 1ba7319656c548075e466a8ad1d068f7b1d66756 Mon Sep 17 00:00:00 2001
From: Kyrylo Yatsenko <hedrok@gmail.com>
Date: Thu, 11 Sep 2025 17:43:08 +0300
Subject: [PATCH] zebra: remove kernel route on last address deletion
Fixes issue #13561
Linux kernel deletes IPv4 routes when last interface IPv4 address
is deleted, but intentionally doesn't send RTM_DELROUTE in this case.
FRR has function rib_update_handle_kernel_route_down_possibility
that handles setting interface down, but not removal of last address.
To fix the situation:
* Add RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED to enum rib_update_event
* In zebra_if_addr_update_ctx make more specific check that last
address is deleted and trigger RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED
instead of RIB_UPDATE_KERNEL in this case. If it was not last address,
don't emit any RIB_UPDATE.
* Call rib_update_handle_kernel_route_down_possibility not only
for event RIB_UPDATE_INTERFACE_DOWN, but also for
RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED.
* Change rib_update_handle_kernel_route_down_possibility not to
consider IPv4 route alive when interface is up, but there
are no IPv4 addresses left.
Relevant kernel code:
* net/ipv4/fib_frontend.c fib_inetaddr_event
* net/ipv4/fib_semantics.c fib_sync_down_dev
Comment from kernel networking author that RTM_DELROUTE isn't sent
intentionally:
https://bugzilla.kernel.org/show_bug.cgi?id=207089
Signed-off-by: Kyrylo Yatsenko <hedrok@gmail.com>
---
lib/if.c | 12 +++++++++++
lib/if.h | 1 +
zebra/interface.c | 8 ++++---
zebra/rib.h | 1 +
zebra/zebra_rib.c | 55 +++++++++++++++++++++++++++++++++++++++--------
5 files changed, 65 insertions(+), 12 deletions(-)
diff --git a/lib/if.c b/lib/if.c
index 8f15230f23..2402d17992 100644
--- a/lib/if.c
+++ b/lib/if.c
@@ -899,6 +899,18 @@ nbr_connected_log(struct nbr_connected *connected, char *str)
zlog_info("%s", logbuf);
}
+/* Return true if there is at least one connected address in the given family */
+bool if_has_connected_with_family(struct interface *ifp, int family)
+{
+ struct connected *connected;
+
+ frr_each (if_connected, ifp->connected, connected)
+ if (connected->address->family == family)
+ return true;
+
+ return false;
+}
+
/* count the number of connected addresses that are in the given family */
unsigned int connected_count_by_family(struct interface *ifp, int family)
{
diff --git a/lib/if.h b/lib/if.h
index 0dc56bd210..233213ca84 100644
--- a/lib/if.h
+++ b/lib/if.h
@@ -606,6 +606,7 @@ extern struct connected *connected_lookup_prefix(struct interface *ifp,
const struct prefix *p);
extern struct connected *connected_lookup_prefix_exact(struct interface *ifp,
const struct prefix *p);
+extern bool if_has_connected_with_family(struct interface *ifp, int family);
extern unsigned int connected_count_by_family(struct interface *ifp, int family);
extern struct nbr_connected *nbr_connected_new(void);
extern void nbr_connected_free(struct nbr_connected *connected);
diff --git a/zebra/interface.c b/zebra/interface.c
index e2c2b4a80c..48343355ba 100644
--- a/zebra/interface.c
+++ b/zebra/interface.c
@@ -1313,11 +1313,13 @@ static void zebra_if_addr_update_ctx(struct zebra_dplane_ctx *ctx,
}
/*
- * Linux kernel does not send route delete on interface down/addr del
+ * Linux kernel does not send route delete on interface down/last addr del
* so we have to re-process routes it owns (i.e. kernel routes)
+ * See rib_update_handle_kernel_route_down_possibility for more details
*/
- if (op != DPLANE_OP_INTF_ADDR_ADD)
- rib_update(RIB_UPDATE_KERNEL);
+ if (op != DPLANE_OP_INTF_ADDR_ADD && addr->family == AF_INET &&
+ !if_has_connected_with_family(ifp, AF_INET))
+ rib_update(RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED);
}
static void zebra_if_update_ctx(struct zebra_dplane_ctx *ctx,
diff --git a/zebra/rib.h b/zebra/rib.h
index 5fedb07335..2f1954de9d 100644
--- a/zebra/rib.h
+++ b/zebra/rib.h
@@ -330,6 +330,7 @@ enum rib_update_event {
RIB_UPDATE_KERNEL,
RIB_UPDATE_RMAP_CHANGE,
RIB_UPDATE_OTHER,
+ RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED,
RIB_UPDATE_MAX
};
void rib_update_finish(void);
diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c
index 752f8282df..eaba976265 100644
--- a/zebra/zebra_rib.c
+++ b/zebra/zebra_rib.c
@@ -4465,6 +4465,9 @@ static const char *rib_update_event2str(enum rib_update_event event)
case RIB_UPDATE_OTHER:
ret = "RIB_UPDATE_OTHER";
break;
+ case RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED:
+ ret = "RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED";
+ break;
case RIB_UPDATE_MAX:
break;
}
@@ -4487,13 +4490,45 @@ rib_update_handle_kernel_route_down_possibility(struct route_node *rn,
bool alive = false;
for (ALL_NEXTHOPS(re->nhe->nhg, nexthop)) {
+ if (!nexthop->ifindex || nexthop->type == NEXTHOP_TYPE_BLACKHOLE) {
+ /* blackhole nexthops have no interfaces */
+ alive = true;
+ break;
+ }
+
struct interface *ifp = if_lookup_by_index(nexthop->ifindex,
nexthop->vrf_id);
- if ((ifp && if_is_up(ifp)) || nexthop->type == NEXTHOP_TYPE_BLACKHOLE) {
+ if (!ifp || !if_is_up(ifp)) {
+ /* interface is not up, not alive */
+ continue;
+ }
+
+ /*
+ * Kernel deletes IPv4 routes from interface when last IPv4 address is deleted
+ * It is not important whether nexthop is reachable after address
+ * is deleted - route is deleted only after last address deletion.
+ *
+ * See net/ipv4/fib_frontend.c fib_inetaddr_event,
+ * net/ipv4/fib_semantics.c fib_sync_down_dev
+ */
+
+ /* If not IPv4, set alive
+ * Check rn->p->family: for nexthop->type=NEXTHOP_TYPE_IFINDEX it
+ * depends on destination only
+ */
+ if (rn->p.family != AF_INET) {
alive = true;
break;
}
+
+ /* Check if there are any IPv4 addresses connected, if yes - set alive */
+ if (if_has_connected_with_family(ifp, AF_INET)) {
+ alive = true;
+ break;
+ }
+
+ /* Otherwise kernel deletes the route */
}
if (!alive) {
@@ -4518,8 +4553,9 @@ static void rib_update_route_node(struct route_node *rn, int type,
bool re_changed = false;
RNODE_FOREACH_RE_SAFE (rn, re, next) {
- if (event == RIB_UPDATE_INTERFACE_DOWN && type == re->type &&
- type == ZEBRA_ROUTE_KERNEL)
+ if ((event == RIB_UPDATE_INTERFACE_DOWN ||
+ event == RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED) &&
+ type == re->type && type == ZEBRA_ROUTE_KERNEL)
rib_update_handle_kernel_route_down_possibility(rn, re);
else if (type == ZEBRA_ROUTE_ALL || type == re->type) {
SET_FLAG(re->status, ROUTE_ENTRY_CHANGED);
@@ -4562,17 +4598,18 @@ void rib_update_table(struct route_table *table, enum rib_update_event event,
* If we are looking at a route node and the node
* has already been queued we don't
* need to queue it up again, unless it is
- * an interface down event as that we need
- * to process this no matter what.
+ * an interface down event or last address down event
+ * as that we need
+ * to process these no matter what.
*/
- if (rn->info &&
- CHECK_FLAG(rib_dest_from_rnode(rn)->flags,
- RIB_ROUTE_ANY_QUEUED) &&
- event != RIB_UPDATE_INTERFACE_DOWN)
+ if (rn->info && CHECK_FLAG(rib_dest_from_rnode(rn)->flags, RIB_ROUTE_ANY_QUEUED) &&
+ event != RIB_UPDATE_INTERFACE_DOWN &&
+ event != RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED)
continue;
switch (event) {
case RIB_UPDATE_INTERFACE_DOWN:
+ case RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED:
case RIB_UPDATE_KERNEL:
rib_update_route_node(rn, ZEBRA_ROUTE_KERNEL, event);
break;
--
2.50.1