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

nexthop group extensions #15038

Closed
wants to merge 25 commits into from
Closed

nexthop group extensions #15038

wants to merge 25 commits into from

Conversation

pguibert6WIND
Copy link
Member

@pguibert6WIND pguibert6WIND commented Dec 18, 2023 •
edited
Loading

This is the continuation from #14973.
More options than allow_recursion will have to be handled at nexthop group level:

  • SRTE COLOR
  • IBGP flag
    For the later command, the cases where a NHG will be sent to ZEBRA, but will not be installed, may happen.
    Some sharp extensions have been done for that in that branch.

To leverage ECMP and nexthop group aggregation at protocol level, nexthop group has to be modified to either host a dependent nexthopgroup or a nexthop. For that, two new commands will be added:

  • protocol-controlled is here to prevent ZEBRA from duplicating nexthop-groups
  • group command creates nexthop-group dependencies at protocol level.

The last work will be reused to implement BGP PIC for ECMP (with addpath).

@pguibert6WIND pguibert6WIND marked this pull request as draft December 18, 2023 09:10
@pguibert6WIND
Copy link
Member Author

ci:rerun

1 similar comment
@pguibert6WIND
Copy link
Member Author

ci:rerun

@github-actions github-actions bot added size/XXL and removed size/XL labels Jan 4, 2024
@pguibert6WIND pguibert6WIND force-pushed the nhg_extensions branch 3 times, most recently from aed67da to f2fd112 Compare January 4, 2024 17:11
@pguibert6WIND
Copy link
Member Author

ci:rerun

1 similar comment
@pguibert6WIND
Copy link
Member Author

ci:rerun

@pguibert6WIND pguibert6WIND force-pushed the nhg_extensions branch 4 times, most recently from 5c6dc63 to 12793cf Compare January 8, 2024 20:33
@pguibert6WIND pguibert6WIND marked this pull request as ready for review January 9, 2024 07:43
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot removed the conflicts label Jan 9, 2024
@pguibert6WIND
sharpd, zebra, lib, doc: add recursive support for protocol nhid
738de3f 
In sharpd, configuring a nexthop-group with an IP nexthop that is not
directly connected does not create a 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 192.168.0.0/24 192.0.2.100
> ubuntu2204(config)# nexthop-group ABCD
> ubuntu2204(config-nh-group)# nexthop 192.168.0.100
> 2024/01/17 16:36:23 SHARP: [Y2G2F-ZTW6M] nhg_add: nhg 181818169 not sent: no valid nexthops
> ubuntu2204(config-nh-group)# do show nexthop-group rib 181818169
> Nexthop Group ID: 181818169 does not exist

Nexthops with an undefined interface index are neither handled
in ZEBRA, nor in SHARPD.

On the other hand, if we had created a route pointing to the same
nexthop (by using ZEBRA_ROUTE_ADD zapi), the next-hop would have been
installed, thanks to the ALLOW_RECURSION flag embedded in the zapi_route
structure.

Add the support for recursivity in nexthop groups by introducing
a flags attribute in the nexthop group structure. Define the
NEXTHOP_GROUP_ALLOW_RECURSION value, which will be used by ZEBRA to
check for the next-hop group recursivity. This flag will be set when
the 'allow-recursion' command will be used:

> ubuntu2204(config)# nexthop-group ABCD
> ubuntu2204(config-nh-group)# allow-recursion
> ubuntu2204(config-nh-group)# nexthop 192.168.0.100
> 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:10
>      VRF: default
>      Valid, Installed
>      Depends: (73)
>         via 192.168.0.100 (vrf default) (recursive), weight 1
>            via 192.0.2.100, loop1 (vrf default), weight 1

This flag control is mandatory, as the recursivity may not be allowed
by protocols like eBGP single-hop.

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

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
sharpd, lib: add force-nexthop-config option to sharp in NHG
dc0463e 
In BGP, it is possible to push an eBGP route without using the
ALLOW_RECURSION flag in the route flags option.

In SHARP, it is possible to do the same with the 'sharp install route'
command, but it is not possible to push a NHG that has not
allow-recursion flag set.

> sharp install routes 9.9.9.20 ... no-recurse
> => pushed
> nexthop-group A
>  nexthop 172.31.0.200
> => not pushed

This is a design issue related to the SHARP daemon only.
Change the design:
- Extend the nexthop group hooks with a running-config call back
- Add a vty command in SHARP only, under the nexthop-group node,
that will permit to push any NHG.

> nexthop-group A
>  force-nexthop-config
>  nexthop 172.31.0.200
>

The nhg will be sent to ZEBRA, and will be attempted to be
resolved.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
lib, zebra, doc: extend ibgp support to nhg in zebra
d5a61e0 
When routes from eBGP peers are pushed in ZEBRA, and are not
resolved over connected paths, an error message is displayed.
This is because neither the ALLOW_RECURSION, nor the IBGP flag
are set on the route. When attempting to do the same in SHARP,
it appears that the NHG do not have this IBGP flag.

> cli# debug zebra rib detail
> [..]
> 2023/12/17 21:09:43 ZEBRA: [NWWQT-S584X]         nexthop_active: Route Type bgp has not turned on recursion 172.31.0.102/32 failed to match
> 2023/12/17 21:09:43 ZEBRA: [PKF9S-7G8XM]         EBGP: see "disable-ebgp-connected-route-check" or "disable-connected-check"

Attempting to push NHGs to ZEBRA does not lead to the same
error messages, for two reasons:
- NHGs are not sent by SHARPD if allow-recursion flag is not set
- the IBGP flag is not implemented in NHGs

This commit includes two changes:
- introduce the ZEBRA_NHG_FLAG in the nexthop_group structure
The flag will be used by ZEBRA when attempting to check if
the nexthops of the NHG are active or not.
- add a vty command in nexthop group that maps over the IBGP
flag.

> ubuntu2204(config)# nexthop-group H
> ubuntu2204(config-nh-group)# ibgp

The below test shows that when turning on ibgp, the EBGP trace
disappears:

> ubuntu2204(config)# debu zebra nexthop de
> ubuntu2204(config)# debu zebra rib de
> ubuntu2204(config)# ip route 172.31.0.102/32 192.168.0.77
> ubuntu2204(config)# nexthop-group H
> ubuntu2204(config-nh-group)# force-nexthop-config
> ubuntu2204(config-nh-group)# nexthop 172.31.0.102
> [..]
> 2023/12/17 22:09:43 ZEBRA: [NWWQT-S584X]         nexthop_active: Route Type sharp has not turned on recursion 172.31.0.102/32 failed to match
> 2023/12/17 22:09:43 ZEBRA: [PKF9S-7G8XM]         EBGP: see "disable-ebgp-connected-route-check" or "disable-connected-check"
> ubuntu2204(config-nh-group)# ibgp
> 2023/12/17 22:14:57 ZEBRA: [NWWQT-S584X]         nexthop_active: Route Type sharp has not turned on recursion 172.31.0.102/32 failed to match

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
lib: fix copy srte_color from zapi_nexthop structure
f1b312f 
When passing from nexthop to zapi_nexthop, the srte color
is copied. Do the same reversely.

Fixes: 31f937f ("lib, zebra: Add SR-TE policy infrastructure to zebra")

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
*: add srte color support in nhg
b5f7f06 
The nexthop group CLI can not configure color support.
Add a new option under nexthop command to configure an
srte color:

> nexthop-group 1
> [..]
>  nexthop 192.0.2.100 color 100

Add srte color support for nhg:
- Add NEXTHOP_GROUP_MESSAGE_COLOR support in NHG.
- Add ZAPI_NEXTHOP_MESSAGE_SRTE support in ZAPI.
- When checking for nexthop active, an SRTE policy with
the (color, nexthop ip) tuple will be looked ip.

Add a NHG sharp test for SRTE color:
- a NHG matching an SRTE policy is configured
- NHG sharp is extended to watch a given NHT color
> sharp watch nexthop 172.31.0.200 color 1
- Upon NHT call back, the NHG is re-added, and updates
the nexthop.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra, lib, doc: add protocol-controlled command to nhg
9ea4a6c 
Today, when a protocol creates a NHG, then ZEBRA automatically creates
a NHG dependency with this NHG. Creating dynamic NHGs at ZEBRA layer
goes against the concept of a protocol daemon that wants to entirely
control its NHG.
The below example considers the sharpd created nexthop as a NHG
with id 179687502, while the nexthop information is stored in a zebra
created nexthop 6462.

> ubuntu2204(config)# nexthop-group 10
> 2023/11/23 10:27:34 SHARP: [Q5NBA-GN1BG] NHG ID assigned: 179687502
> ubuntu2204(config-nh-group)# nexthop 192.0.2.200 loop10
> [..]
> ID: 179687502 (sharp)
>    RefCnt: 1
>    Uptime: 00:00:17
>    VRF: default
>    Valid, Installed
>    Depends: (6462)
>          via 192.0.2.200, loop1 (vrf default), weight 1
> ID: 6462 (sharp)
>     RefCnt: 2
>     Uptime: 00:00:17
>     VRF: default
>     Valid, Installed
>     Interface Index: 2964
>           via 192.0.2.200, loop1 (vrf default), weight 1
>     Dependents: (179687502)
>
> # ip nexthop ls
> [..]
> id 179687502 group 6462 proto 194
> id 6462 via 192.0.2.200 dev loop1 scope link proto 194

Add a CLI option to NHG that request zebra to not move the nexthop
information in a separate NHG.

> ubuntu2204(config-nh-group)# protocol-controlled
> ubuntu2204(config-nh-group)# nexthop 192.0.2.200 loop10
> ubuntu2204(config-nh-group)# do show nexthop-group rib sharpd
> ID: 179687504 (sharp)
>     RefCnt: 1
>     Uptime: 00:00:04
>     VRF: default
>     Valid, Installed
>           via 192.0.2.200, loop1 (vrf default), weight 1
>
> # ip nexthop ls
> [..]
> id 179687504 via 192.0.2.200 dev loop1 scope link proto 194

Add associate tests for that in topotests.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
*: move [backup_]nexthop from zapi_nhg into nhg_nexthop sub-structure
2e4736a 
The zapi nexthop group structure is unable to reference other
nexthop groups. Actually, only nexthop IP addresses can be
associated to a nexthop group via the ZAPI interface.

This first commit is a preliminary commit that prepares the
ZAPI interface in order to host a second type of nexthop group,
by moving the current zapi_nexthop definition into a specific
sub-structure. This commit does not change the functionality.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
*: add nexthop_group group ids support, part 1
129a217 
Today, protocol daemons can not create ECMP next-hops by using
multiple NHGs. The below configuration shows an ECMP NHG.

> nexthop-group ABCD
> nexthop 192.168.0.100 loop1
> nexthop 192.168.0.105 loop1
>

In the 'multiple NHG' case, 2 NHGs would be used for each
nexthop, and the 3rd ony would group the two first, similar
to what is done with iproute2:

> ip nexthop add id 210 via 192.168.0.100 dev loop1
> ip nexthop add id 211 via 192.168.0.105 dev loop1
> ip nexthop add id 212 group 210/211

Extend the nexthop group structure to support a list of nexthop
groups. Expose a vty API inside the nexthop group node, to
host a new command to configure a group name. That group
name points to another nexthop group.

> nexthop-group ABCD
> group ECMP1
> group ECMP2
> exit
> nexthop-group ECMP1
> nexthop 192.168.0.100 loop1
> exit
> nexthop-group ECMP2
> nexthop 192.168.0.105 loop1
> exit

This commit only focuses to changes in the ZAPI interface, and on
the sharpd daemon. The handling of group ids on zebra is stopped
in 'zebra_nhg_proto_add()' function.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: factorise 'show nexthop-group rib' nexthops
efd1001 
Reduce the code size by reusing the same code to display
nexthops and backup nexthops.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: move early rib_evpn_route_[add/delete]() function in early_ fu…
755460b 
…nction

Move the rib_evpn_route_add() and rib_evpn_route_delete() functions
in a separate place.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: move route_entry_dump_nh() into route_entry_dump_nhg()
757aaa1 
Factorise the nexthop display handling in a separate function.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
lib, zebra: add nhg group ids support, part 2, add nhg ref
48fbc9c 
When a nexthop group with dependencies is attached to a route entry,
the real nexthop flags are not easily reachable for read or write
operations.

In order to do so, at the route install, or at the nexthop group
update, for each nexthop dependency, use an internal nhg pointer to
remember the used nexthop group. The pointer is refreshed at nexthop
group replacement.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: add nexthop group ids support, part 3
9fa19cc 
The zebra nexthop group support is not adapted to create
dependencies between group when a request comes from the
routing protocols.

Add those extra changes in the zebra nexthop group library
for that, a stub is temporarily added in dataplane_ctx_init.

The changes ignore for now the zebra dataplane handling.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: add nexthop group ids support, part 4, netlink
9e88ee0 
Add support to install routes attached to a nexthop group
which is dependent of other nexthop groups:
- Disable the src option.
- The dataplane stub is removed

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: rework show_nexthop_group_out_nexthop() separate function
05c235b 
Create a separate function to handle separately the nexthop structure.
On the next commit, when a nexthop group is used, the nexthop of
each nexthop group dependency is used.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: rework 'vty_show_ip_route()' function
d8d4c0f 
The show_route_nexthop_group_helper() and the
show_nexthop_group_json_helper() helper functions are introduced
to separate nexthop handling from the remaining 'show ip route'
command.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: add nexthop group ids support, part 5, show
ad826b7 
The following commands can not be used when using nhg groups,
forged by the protocol:
- 'show nexthop-group rib'
- 'show ip route'

Add the support for the 2 commands

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: rework no nexthop available test
1393cfd 
Some tests may check if there is a valid nexthop in a
nexthop-group. Generalize the check by accounting for the
number of nexthops in a nexthop group.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: rework mpls uninstall based on nexthop group
32eea72 
Create a mpls_ftn_uninstall_nhg() function that handles the
nexthop parsing for each route entry. This parsing is common
to primary and backup nexthops.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: add support for 'mpls uninstall' of nexthop group dependencies
9d8b551 
The nexthop group dependencies may also have an NHLFE entry.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: rework zsend_redistribute_route() function with nhg
aab3273 
Create a second function zsend_redistribute_route_nhg() to
handle nexthop group.

Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
zebra: add support for redistributed routes with nexthop groups
f3c50c0 
Signed-off-by: Philippe Guibert <[email protected]>
@pguibert6WIND
topotests: all_protocol_startup, add nexthop-group test with nhg group
27d4fdc 
An ECMP configuration is tested with nexthop group.

Signed-off-by: Philippe Guibert <[email protected]>
@github-actions GitHub Actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@pguibert6WIND pguibert6WIND marked this pull request as draft January 30, 2024 16:31
@pguibert6WIND
Copy link
Member Author

replaced with #15488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@donaldsharp donaldsharp donaldsharp requested changes

@pbrisset pbrisset Awaiting requested review from pbrisset

Assignees
No one assigned
Labels
bgp bugfix conflicts documentation libfrr master sharp size/XXL tests Topotests, make check, etc zebra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@pguibert6WIND @donaldsharp