zebra: remove kernel route on last address deletion - #19564
Conversation
516000d to
28e0579
Compare
42eee42 to
6bad366
Compare
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: FRRouting/frr#19564
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: FRRouting/frr#19564
6bad366 to
5c1700b
Compare
| return NULL; | ||
| } | ||
|
|
||
| /* Return true if there is at least one connected address in the given family */ |
There was a problem hiding this comment.
V6 has the ability via a sysctl to keep addresses on a interface on down. This code MUST respect this in some manner.
There was a problem hiding this comment.
Hm... connected_count_by_family does nothing to respect this so probably needs fixing?..
In this PR I need this function only for IPv4, can I instead make function if_has_connected_ipv4 not to investigate how to do that?..
Thank you.
There was a problem hiding this comment.
To see current behavior I've added code:
void if_connected_dump_vty(struct vty* vty, struct interface *ifp)
{
vty_out(vty, "== %s\n", ifp->name);
vty_out(vty, "\tIPv4 has_connected: %d\n", if_has_connected_with_family(ifp, AF_INET));
vty_out(vty, "\tIPv4 connected_count: %d\n", connected_count_by_family(ifp, AF_INET));
vty_out(vty, "\tIPv6 has_connected: %d\n", if_has_connected_with_family(ifp, AF_INET6));
vty_out(vty, "\tIPv6 connected_count: %d\n", connected_count_by_family(ifp, AF_INET6));
vty_out(vty, "\tConnected:\n");
struct connected *connected;
frr_each (if_connected, ifp->connected, connected)
vty_out(vty, "\t * %dPF %pFX\n", connected->address->family, connected->address);
}I've added IPv6 address:
sudo ip -6 addr add 2001:db8::1/32 dev eth0Output when interface is up:
== eth0
IPv4 has_connected: 0
IPv4 connected_count: 0
IPv6 has_connected: 1
IPv6 connected_count: 2
Connected:
* AF_INET6 fe80::e2e:fdff:feb9:0/64
* AF_INET6 2001:db8::1/32
Updating configuration and setting interface down:
sysctl -w net.ipv6.conf.eth0.keep_addr_on_down=1
sudo ip -6 link set down eth0Checking addresses:
$ ip -6 addr show dev eth0
2: eth0: <BROADCAST,MULTICAST> mtu 1500 qdisc fq_codel state DOWN group default qlen 1000
altname enp0s4
altname ens4
inet6 2001:db8::1/32 scope global tentative
valid_lft forever preferred_lft forever
After this output is:
== eth0
IPv4 has_connected: 0
IPv4 connected_count: 0
IPv6 has_connected: 1
IPv6 connected_count: 1
Connected:
* AF_INET6 2001:db8::1/32
Seems correct?.. Could you describe what is correct behavior if this is wrong?..
Thank you.
5c1700b to
7082ada
Compare
| 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); |
There was a problem hiding this comment.
RIB_UPDATE_KERNEL is not sent anymore when addr is ipv6, could you clarify why ?
"Linux kernel deletes IPv4 routes when last interface IPv4 address
is deleted, but intentionally doesn't send RTM_DELROUTE in this case.
"
What is the behavior of the kernel for IPv6 ?
There was a problem hiding this comment.
IPV4 test : default dev iface
sudo ip link add dom0 type dummy
sudo ip netns add test
sudo ip link set dom0 netns test
sudo ip netns exec test ip link set dom0 up
sudo ip netns exec test ip link set lo up
sudo ip netns exec test ip monitor route &
sudo ip netns exec test ip addr add 1.1.1.2/24 dev dom0
local 1.1.1.2 dev dom0 table local proto kernel scope host src 1.1.1.2
1.1.1.0/24 dev dom0 proto kernel scope link src 1.1.1.2
broadcast 1.1.1.255 dev dom0 table local proto kernel scope link src 1.1.1.2
sudo ip netns exec test ip route add default dev dom0
default dev dom0 scope link
sudo ip netns exec test ip addr del 1.1.1.2/24 dev dom0
Deleted 1.1.1.0/24 dev dom0 proto kernel scope link src 1.1.1.2
Deleted broadcast 1.1.1.255 dev dom0 table local proto kernel scope link src 1.1.1.2
Deleted local 1.1.1.2 dev dom0 table local proto kernel scope host src 1.1.1.2
IPV6 test: default dev iface
sudo ip netns exec test ip addr add aa::1/64 dev dom0
aa::/64 dev dom0 proto kernel metric 256 pref medium
local aa::1 dev dom0 table local proto kernel metric 0 pref medium
sudo ip netns exec test ip -6 route add default dev dom0
default dev dom0 metric 1024 pref medium
sudo ip netns exec test ip addr del aa::1/64 dev dom0
Deleted broadcast 1.1.1.255 dev dom0 table local proto kernel scope link src 1.1.1.2
Deleted local 1.1.1.2 dev dom0 table local proto kernel scope host src 1.1.1.2
sudo ip netns exec test ip addr del aa::1/64 dev dom0
Deleted local aa::1 dev dom0 table local proto kernel metric 0 pref medium
Deleted aa::/64 dev dom0 proto kernel metric 256 pref medium
sudo ip netns exec test ip -6 route
fe80::/64 dev dom0 proto kernel metric 256 pref medium
default dev dom0 metric 1024 pref medium
Route is kept because the link-local IPv6 address is here.
Remove default ipv6 address
sudo ip netns exec test ip addr del fe80::c86f:4cff:febe:f4f9/64 dev dom0
Deleted local fe80::c86f:4cff:febe:f4f9 dev dom0 table local proto kernel metric 0 pref medium
Deleted fe80::/64 dev dom0 proto kernel metric 256 pref medium
sudo ip netns exec test ip -6 route
default dev dom0 metric 1024 pref medium
Default route is kept in ipv6 -> no netlink message.
sudo ip netns exec test ip link del dom0
Deleted default dev dom0 metric 1024 pref medium
Deleted multicast ff00::/8 dev dom0 table local proto kernel metric 256 pref medium
The default route is deleted when the interface is deleted. And a netlink message is sent.
There was a problem hiding this comment.
Yes, there is no such behaviour for IPv6, so event RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED is sent only for IPv4 intentionally, thanks for asking and detailed testing.
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: FRRouting/frr#19564 (more changes were added for backport to 9.1) (cherry picked from commit 4fbffe2)
7082ada to
4467d09
Compare
068359b to
712d315
Compare
|
There was a tentative of fix for this issue. #11528 |
|
This PR fixes a real Linux kernel issue, but the fix is in common dplane code (zebra_if_addr_update_ctx()), not Linux-specific code. FreeBSD currently bypasses this code path, but FRR is moving toward dplane integration for FreeBSD (see 3ac5756). When FreeBSD address handling migrates to dplane, |
|
@maxime-leroy, thanks, I'll look into it. I've never worked with FreeBSD, so it may take time. If you have any commands to reproduce/show the problem, it will be very helpful. Do you know - doest it send RTM_DELETE for each route also when interface goes down? |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
712d315 to
6b7ec06
Compare
|
@maxime-leroy I've checked it on FreeBSD at last. For me there is currently absolutely same defect on FreeBSD, so maybe when FreeBSD address handling migrates to dplane this fix will fix it there too :) I had I have no experience with FreeBSD, but it seems clear that behaviour/bug is very similar to the one I've fixed here. If you've meant something else, please share more details. Commands with outputs: |
|
@greptile review |
Greptile SummaryThis PR fixes a long-standing FRR/kernel de-sync by detecting when the last IPv4 address is removed from an interface and triggering a targeted RIB walk ( Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Kernel
participant ZebraDplane as Zebra Dplane
participant InterfaceC as interface.c
participant ZebraRib as zebra_rib.c
participant RIB
Kernel->>ZebraDplane: RTM_DELADDR (last IPv4 addr)
ZebraDplane->>InterfaceC: zebra_if_addr_update_ctx()
InterfaceC->>InterfaceC: connected_delete_ipv4()
InterfaceC->>InterfaceC: if_has_connected_with_family(AF_INET) == false?
InterfaceC->>ZebraRib: rib_update(RIB_UPDATE_KERNEL_LAST_IPV4_ADDRESS_DELETED)
ZebraRib->>ZebraRib: rib_update_table() — walk all tables
loop Each kernel route node
ZebraRib->>ZebraRib: rib_update_handle_kernel_route_down_possibility(rn, re)
alt rn->p.family != AF_INET
ZebraRib->>RIB: alive=true (keep IPv6 routes)
else nexthop ifp has no IPv4 addrs
ZebraRib->>RIB: rib_delete() — remove stale IPv4 kernel route
else nexthop ifp has IPv4 addrs
ZebraRib->>RIB: alive=true (keep route)
end
end
Prompt To Fix All With AIThis is a comment left during a code review.
Path: tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py
Line: 459-481
Comment:
**IPv6 gateway nexthop may be unreachable after first address deletion**
`2001:db8:c::/64 via 2001:db8:a::1` has its gateway inside the `2001:db8:a::/64` subnet, which is exclusively covered by `2001:db8:a::2/64`. When `ip -6 addr del 2001:db8:a::2/64 dev dummy0` is executed, the connected prefix `2001:db8:a::/64` disappears, making `2001:db8:a::1` unreachable. Linux may (via `rt6_sync_down_addr`) silently remove this route, causing `_assert_kernel_routes_up` to fail on the check right after the first deletion. This is the same nexthop-in-deleted-subnet problem as the IPv4 gateway, but in the IPv6 test. The surviving `2001:db8:b::7/64` address is in a different subnet and does not rescue reachability to `2001:db8:a::1`.
Consider choosing a gateway that stays reachable throughout, for instance one inside the `2001:db8:b::/64` subnet (covered by the surviving address):
```
ip -6 route add 2001:db8:c::/64 via 2001:db8:b::1
```
That way `2001:db8:c::/64` survives the first deletion and is only removed together with `2001:db8:d::/64` when the last address (`2001:db8:b::7/64`) is gone.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: zebra/zebra_rib.c
Line: 4567-4570
Comment:
**Stale pointer syntax in comment**
The comment uses `rn->p->family` (arrow dereference, implying a pointer), but `rn->p` is a `struct prefix` value — the correct member-access syntax is `rn->p.family`.
```suggestion
/* If not IPv4, set alive
* Check rn->p.family: for nexthop->type=NEXTHOP_TYPE_IFINDEX it
* depends on destination only
*/
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (2): Last reviewed commit: "zebra: topotest for last address deletio..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| router.run(f"ip link set {ifname} up") | ||
| router.run(f"ip -4 addr add 192.168.0.2/24 dev {ifname}") | ||
| router.run(f"ip -4 addr add 192.168.100.7/24 dev {ifname}") | ||
| router.run(f"ip -4 route add 203.0.113.0/24 via 192.168.0.1") |
There was a problem hiding this comment.
The route 203.0.113.0/24 via 192.168.0.1 uses 192.168.0.1 as its nexthop, which is inside the 192.168.0.0/24 subnet connected through 192.168.0.2/24. When 192.168.0.2/24 is deleted, Linux's fib_sync_down_addr removes routes whose nexthops reside in that subnet, so 203.0.113.0/24 may already be gone after the first deletion, causing _assert_kernel_routes_up to fail.
Consider using a gateway in the surviving subnet instead:
| router.run(f"ip -4 route add 203.0.113.0/24 via 192.168.0.1") | |
| router.run(f"ip -4 route add 203.0.113.0/24 via 192.168.100.1") |
That way the gateway route survives the first deletion and is only removed when the last address (192.168.100.7/24) goes away.
Prompt To Fix With AI
This is a comment left during a code review.
Path: tests/topotests/zebra_multiple_connected/test_zebra_multiple_connected.py
Line: 388
Comment:
**Gateway route may be deleted by kernel on first address removal**
The route `203.0.113.0/24 via 192.168.0.1` uses `192.168.0.1` as its nexthop, which is inside the `192.168.0.0/24` subnet connected through `192.168.0.2/24`. When `192.168.0.2/24` is deleted, Linux's `fib_sync_down_addr` removes routes whose nexthops reside in that subnet, so `203.0.113.0/24` may already be gone after the first deletion, causing `_assert_kernel_routes_up` to fail.
Consider using a gateway in the surviving subnet instead:
```suggestion
router.run(f"ip -4 route add 203.0.113.0/24 via 192.168.100.1")
```
That way the gateway route survives the first deletion and is only removed when the last address (`192.168.100.7/24`) goes away.
How can I resolve this? If you propose a fix, please make it concise.
6b7ec06 to
d18731a
Compare
|
@greptile review |
d18731a to
2213276
Compare
Fixes: FRRouting#13561 Linux kernel deletes IPv4 routes when last interface IPv4 address is deleted, but intentionally doesn't send RTM_DELROUTE in this case. IPv6 has no such behaviour: all routes remain intact on last address deletion. 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> fixup code
Add test that checks kernel routes behaviour on last address deletion. For IPv4 last address deletion must delete all routes for kernel and FRR. For IPv6 checks that as expected last address deletion doesn't effect kernel or FRR routes. See FRRouting#13561 Signed-off-by: Kyrylo Yatsenko <hedrok@gmail.com>
2213276 to
7186f84
Compare
|
Added test to show IPv6 behaviour: no route deletion is happening. So I check in the test that this expectation is true + FRR routes are same as kernel's. Added comments to make it clear that it is intentional to process only IPv4 last address deletion (+ renamed RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED -> RIB_UPDATE_KERNEL_LAST_IPV4_ADDRESS_DELETED for clarity) P1 issue from Greptile seems false to me:
I've specifically added test like that because (at least for me) the behaviour seems counterintuitive, and Greptile seems to got into the trap. Route to gateway |
|
@donaldsharp can I request a backport of this to stable/10.6 and stable/10.5 branch please? |
5 participants
Fixes #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:
RIB_UPDATE_KERNEL_LAST_ADDRESS_DELETEDto enum rib_update_eventzebra_if_addr_update_ctxmake more specific check that last address is deleted and triggerRIB_UPDATE_KERNEL_LAST_ADDRESS_DELETEDinstead ofRIB_UPDATE_KERNELin this case. If it was not last address, don't emit any RIB_UPDATE.rib_update_handle_kernel_route_down_possibilitynot only for eventRIB_UPDATE_INTERFACE_DOWN, but also forRIB_UPDATE_KERNEL_LAST_ADDRESS_DELETED.rib_update_handle_kernel_route_down_possibilitynot to consider IPv4 route alive when interface is up, but there are no IPv4 addresses left.Relevant kernel code:
Comment from kernel networking author that RTM_DELROUTE isn't sent intentionally:
https://bugzilla.kernel.org/show_bug.cgi?id=207089
test
Also added test_zebra_kernel_last_ipv4_address_deleted to zebra_multiple_connected/test_zebra_multiple_connected.py with basic flow:
ip -4 addr flush dev {ifname} ip -4 addr add 192.168.0.2/24 dev {ifname} ip -4 route add default via 192.168.0.1 ip -4 addr add 192.168.100.7/24 dev {ifname} ip -4 route add 10.0.170.3 dev {ifname}Check there are
defaultand10.0.170.3routes.ip -4 addr del 192.168.0.2/24 dev {ifname}Check there are still
defaultand10.0.170.3routes.ip -4 addr del 192.168.100.7/24 dev {ifname}Check there are no
defaultand10.0.170.3routes.In each check make sure kernel routes are as expected (using topotest.ip4_route) and zebra routes are as expected (using
show ip route json).