Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

API for using recursive nexthop-groups at protocol layer #16947

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

API for using recursive nexthop-groups at protocol layer #16947

wants to merge 6 commits into from

Conversation

pguibert6WIND
Copy link
Member

draft

@pguibert6WIND pguibert6WIND marked this pull request as draft September 27, 2024 16:13
@pguibert6WIND pguibert6WIND force-pushed the zebra_nhg_recursive_only branch from 2aeb51a to 29b50f0 Compare October 14, 2024 12:06
@pguibert6WIND pguibert6WIND mentioned this pull request Oct 14, 2024
Draft
@pguibert6WIND
Copy link
Member Author

next to this, I plan to add more commits coming from #16028.
In the order, I want to bring in a new sharp test to really test the protocol recursive nexthop groups usage.
=> see c6bbba2

@pguibert6WIND pguibert6WIND force-pushed the zebra_nhg_recursive_only branch 2 times, most recently from b2c6039 to baee66d Compare October 15, 2024 08:03
@pguibert6WIND
Copy link
Member Author

ci:rerun isis_srv6_topo1 fails

@FRRouting FRRouting deleted a comment from github-actions bot Oct 16, 2024
@pguibert6WIND pguibert6WIND changed the title Zebra nhg recursive only API for using recursive nexthop-groups at protocol layer Oct 16, 2024
@pguibert6WIND pguibert6WIND marked this pull request as ready for review October 16, 2024 14:01
@pguibert6WIND pguibert6WIND requested a review from mjstapp October 17, 2024 04:22
@pguibert6WIND pguibert6WIND force-pushed the zebra_nhg_recursive_only branch from baee66d to ad50397 Compare October 28, 2024 15:49
@pguibert6WIND
zebra: rework zebra_nhg_proto_add() parameters
502251b 
This small rework prepares next commit.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra, lib: add allow-recursion flag to nexthop-group
cf246cd 
Nexthop-groups do not support route recursion. It means that if its
nexthop is not directly connected, there is no resolution of the nexhop
against other types of route.
The nexthop-group is always marked inactive regardless of whether the
nexthop can be resolved.

ROUTE_ADD ZAPI messages from daemons can convey a
ZEBRA_FLAG_ALLOW_RECURSION flag to tell zebra whether it should resolve
nexthop that are not directly connected. The flag is unset by default.
In a similar way, add a NEXTHOP_GROUP_ALLOW_RECURSION for the NHG_ADD
ZAPI message, which is unset by default.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
lib, doc: add recursive support configuration for nexthop-group
97d4cb0 
In sharpd, configuring a nexthop-group with an IP nexthop that is not
directly connected does create an inactive NHG context in zebra:

> ubuntu2204(config)# interface loop1
> ubuntu2204(config-if)# ip address 192.0.2.1/24
> ubuntu2204(config-if)# exi
> ubuntu2204(config)# ip route 10.200.0.0/24 192.0.2.100
> ubuntu2204(config)# nexthop-group ABCD
> ubuntu2204(config-nh-group)# nexthop 10.200.0.62
> 2024/01/17 16:52:44 SHARP: [JWRCN-N9K90] Installed nhg 181818168
> ubuntu2204(config-nh-group)# do show nexthop-group rib 181818168
> ID: 181818168 (sharp)
>      RefCnt: 1
>      Uptime: 00:00:04
>      VRF: default
>      Depends: (841)
>            via 10.200.0.62 (vrf default) inactive, weight 1

Add the 'allow-recursion' vty command under nexthop-group configuration.
When set, the nexthop-group ABCD is added or updated, and will update
the nexthop resolution as expected.

> ubuntu2204(config)# interface loop1
> ubuntu2204(config-if)# ip address 192.0.2.1/24
> ubuntu2204(config-if)# exi
> ubuntu2204(config)# ip route 10.200.0.0/24 192.0.2.100
> ubuntu2204(config)# nexthop-group ABCD
> ubuntu2204(config-nh-group)# allow-recursion
> ubuntu2204(config-nh-group)# nexthop 10.200.0.62
> 2024/01/17 16:57:44 SHARP: [JWRCN-N9K90] Installed nhg 181818168
> ubuntu2204(config-nh-group)# do show nexthop-group rib 181818168
> ID: 181818168 (sharp)
>      RefCnt: 1
>      Uptime: 00:00:04
>      VRF: default
>      Valid, Installed
>      Depends: (842)
>         via 10.200.0.62 (vrf default) (recursive), weight 1
>            via 192.0.2.100, loop1 (vrf default), weight 1

The allow-recursion flag is disabled by default, as it is today with
other control plane daemons.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: fix submit protocol NHG add/update to nexthop_active() function
267fa00 
All protocol daemons that configure a non directly connected
nexthop-group will create an active nexthop-group. This is wrong,
as the nexthop may not be reachable, and the nexthop-group should
be considered inactive.

Actually, protocol nexthop-groups are always considered as active at
ZEBRA level, whereas they should be submitted to the nexthop
reachability mechanism which is done when a route is configured.

For instance, when BGP sends a ROUTE_ADD message with the nexthop
of the BGP update to ZEBRA. ZEBRA will resolve the prefix reachability
by looking at the nexthop reachability: The nexthop_active(prefix,
nexthop) function is called.

Fix this by calling the nexthop_active() function, when the nexthop is
not a nexthop interface.

Note that without the prefix to apply nexthop to, it will not be
possible to do some specific checks like recursivity against ourselves.

For example, the below 192.168.3.0/24 BGP prefix is resolved over himself.

> # show bgp ipv4
> [..]
>  *>i 192.168.3.0/24   192.168.3.3              0    100      0 i
> # show ip route 192.168.3.3
> Routing entry for 192.168.3.0/24
>   Known via "bgp", distance 200, metric 0
>   Last update 00:00:34 ago
>     192.168.3.3 inactive, weight 1
> Routing entry for 192.168.3.0/24
>  Known via "static", distance 1, metric 0, best
>   Last update 00:00:36 ago
>   * 192.168.5.10, via r1-eth2, weight 1
> # output
> 2024/06/12 16:15:47.656466 ZEBRA: [ZJVZ4-XEGPF] default(0:254):192.168.3.0/24: Examine re 0x55d446afd0c0 (bgp) status: Changed flags: Recursion iBGP dist 200 metric 0
> 2024/06/12 16:15:47.656470 ZEBRA: [YPVDZ-KQPM9]         nexthop_active: Matched against ourself and prefix length is not max bit length

For those corner cases, using protocol nexthop groups will not be
possible: each protocol will have to check those cases.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
sharpd: unlock to create unresolved NHGs
30e910f 
Add the ability for the sharpd daemon to program nexthop-groups
that are unresolved: this includes blackhole routes, but also
nexthops that have no interface information.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
topotests: add nexthop-group allow-recursion test
6a73cf2 
Add a nexthop group test that ensures that a recursive
next-hop is resolved in zebra.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND pguibert6WIND force-pushed the zebra_nhg_recursive_only branch from ad50397 to 6a73cf2 Compare November 6, 2024 17:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mjstapp mjstapp Awaiting requested review from mjstapp

Assignees
No one assigned
Labels
bugfix documentation libfrr master sharp size/L zebra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@pguibert6WIND