mirror of
https://github.com/vyos/vyos-build.git
synced 2025-10-01 20:28:40 +02:00
T6962: frr: fix wrong kernel routes updates
The patch 0007-zebra-remove-kernel-route-on-last-address-deletion.patch fixes root cause of the issue: Zebra didn't do anything on last IPv4 address deletion though kernel in such case deletes all IPv4 routes. FRR PR: https://github.com/FRRouting/frr/pull/19564
This commit is contained in:
Kyrylo Yatsenko
parent
1a6747b038
commit
4fbffe2efd
@ -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
|
||||||
|
|
||||||
Loading…
x
Reference in New Issue
Block a user