-
Notifications
You must be signed in to change notification settings - Fork 23
Frr nexthop #251
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.
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
Merged
Merged
Frr nexthop #251
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
origin field will be added into nexthop object. There is no common prefix for rt and nexthop, we must choose one nh_ or rt_ for origin type. As origin is defined in gr_nexthop.h, let's choose nh_ instead of rt_. Signed-off-by: Maxime Leroy <[email protected]>
GR_IP4/6_ROUTE evts has not been added. Let's fix it. Signed-off-by: Maxime Leroy <[email protected]>
rjarry
requested changes
Jul 2, 2025
67794e7 to
de565c1
Compare
rjarry
requested changes
Jul 3, 2025
Grout’s Unix-socket control API now exposes a suppress_self_events flag
that drops the asynchronous notifications generated by the caller
itself. FRR enables this flag when registering for IFACE and ROUTE
events, providing the same echo-suppression already used on Netlink
sockets with a BPF program. [1]
Behaviour is effectively unchanged:
* IFACE — FRR never asks Grout to create or delete interfaces.
* ROUTE — Routes installed by FRR were already tagged as self-originated,
so suppressing the echo removes redundant churn without any loop risk.
Address-family events are **not** suppressed so that
GR_EVENT_IP/IP6_ADDR_{ADD,DEL} continue to fire.
[1] https://github.com/FRRouting/frr/blob/011539fcd0cc42dd41a1a66316f62c0451c55950/zebra/kernel_netlink.c#L551
Signed-off-by: Maxime Leroy <[email protected]>
This type is going to be used in gr_nexthop in the next commit. Signed-off-by: Maxime Leroy <[email protected]>
From:
struct gr_nexthop {
gr_nh_type_t type; /* 0 1 */
addr_family_t af; /* 1 1 */
gr_nh_state_t state; /* 2 1 */
gr_nh_flags_t flags; /* 3 1 */
uint32_t nh_id; /* 4 4 */
uint16_t vrf_id; /* 8 2 */
uint16_t iface_id; /* 10 2 */
struct rte_ether_addr mac __attribute__((__aligned__(2))); /* 12 6 */
/* XXX 2 bytes hole, try to pack */
union {
struct {
} addr; /* 20 0 */
ip4_addr_t ipv4; /* 20 4 */
struct rte_ipv6_addr ipv6; /* 20 16 */
}; /* 20 16 */
uint8_t prefixlen; /* 36 1 */
/* size: 40, cachelines: 1, members: 10 */
/* sum members: 35, holes: 1, sum holes: 2 */
/* padding: 3 */
/* forced alignments: 1 */
/* last cacheline: 40 bytes */
} __attribute__((__aligned__(4)))
to:
struct gr_nexthop {
gr_nh_type_t type; /* 0 1 */
addr_family_t af; /* 1 1 */
gr_nh_state_t state; /* 2 1 */
gr_nh_flags_t flags; /* 3 1 */
uint32_t nh_id; /* 4 4 */
uint16_t vrf_id; /* 8 2 */
uint16_t iface_id; /* 10 2 */
struct rte_ether_addr mac __attribute__((__aligned__(2))); /* 12 6 */
uint8_t prefixlen; /* 18 1 */
/* XXX 1 byte hole, try to pack */
union {
struct {
} addr; /* 20 0 */
ip4_addr_t ipv4; /* 20 4 */
struct rte_ipv6_addr ipv6; /* 20 16 */
}; /* 20 16 */
/* size: 36, cachelines: 1, members: 10 */
/* sum members: 35, holes: 1, sum holes: 1 */
/* forced alignments: 1 */
/* last cacheline: 36 bytes */
} __attribute__((__aligned__(4)));
The 1 byte hole will be used by next commit.
Signed-off-by: Maxime Leroy <[email protected]>
Move code converting a grout nexthop into a frr one in a new function. The function will be reused by a later commit. Signed-off-by: Maxime Leroy <[email protected]>
Include a new origin field in nexthops objects exchanged in the API. Allow users to set an arbitrary value. This is mostly intended for routing daemons to identify if a nexthop was learned from BGP, OSPF, etc. Or if it was configured manually by the user. Display the origin of nexthop in the CLI. For now, all nexthop explicitly added by the CLI are GR_NH_ORIGIN_USER. Other API clients can specify any suitable value. GR_NH_ORIGIN_INTERNAL(255) is reserved for dynamically added nexthop when learning new next hops after receiving ARP requests or replies. Hide these nexthops on purpose when replying to a GR_NEXTHOP_LIST request to avoid confusion. Signed-off-by: Maxime Leroy <[email protected]>
In the next commit, the FRR plugin will listen for grout’s nexthop-update events to keep its state in sync. When a nexthop created by FRR completes ARP/NEIGH resolution, grout publishes an update. Zebra then re-adds the nexthop to “grout” to make sure nothing has changed, which clears the resolved MAC address and restarts ARP/NEIGH. The cycle repeats indefinitely. To solve the issue, this commit removes the nexthop-update event when arp/neigh resolution is done; the notification provides no useful information and eliminates the update-loop. Signed-off-by: Maxime Leroy <[email protected]>
Handle DPLANE_OP_NH_{INSTALL,UPDATE,DELETE} so Grout can receive standalone
nexthop objects from Zebra/FRR. Routes must now reference the nexthop
ID instead of embedding a full nexthop in every route update.
Signed-off-by: Maxime Leroy <[email protected]>
If buildtype is configured in debug, let's compile frr in debug mode too. Signed-off-by: Maxime Leroy <[email protected]>
Add support to synchronise grout nexthop into zebra by listening GR_EVENT_NEXTHOP_NEW/UPDATE/DELETE events. Signed-off-by: Maxime Leroy <[email protected]>
grout_gr_nexthop_to_frr_nexthop() always chose gateway-only types,
ignoring when iface_id is set.
Use NEXTHOP_TYPE_IPV{4,6}_IFINDEX when iface_id != 0 and set nh_index to
iface_id + GROUT_INDEX_OFFSET.
Signed-off-by: Maxime Leroy <[email protected]>
For route using a nexthop index, we should not recreate a new nexthop, reuse the one already when we receive GR_EVENT_NEXTHOP_NEW. Note: It doesn't work as expected like for kernel, because zebra code is bugged. See: FRRouting/frr#19119 Signed-off-by: Maxime Leroy <[email protected]>
Grout doesn't support ecmp, let's disable the support in FRR by compiling with --enable-multipath=1. Note: We should support ecmp, because nobody compiles and test with no ecmp support in FRR. Signed-off-by: Maxime Leroy <[email protected]>
Unicast nexthop have a weight of 1 in zebra/kernel. Let's do the same for grout. Signed-off-by: Maxime Leroy <[email protected]>
Upstream FRR builds with `-Wimplicit-fallthrough`, but the zebra plugin in Grout accidentally masked the check via `-Wno-implicit-fallthrough` in `subprojects/frr/meson.build` (`frr_dep`). FRR itself tolerates the resulting warnings because it does **not** use `-Werror`, whereas Grout promotes all warnings to errors. This patch removes the stray `-Wno-*` flag and adds `// fallthrough` comments to every deliberate `switch` drop-through. Signed-off-by: Maxime Leroy <[email protected]>
de565c1 to
e4d8f2f
Compare
| // struct gr_nh_del_resp { }; | ||
|
|
||
| #define GR_NH_LIST REQUEST_TYPE(GR_INFRA_MODULE, 0x0073) | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
struct gr_nh_list_req has padding after the bool field that isn't explicitly initialized; consider zero-initializing the entire struct to avoid leaking uninitialized bytes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Support nexthop id sync with frr plugin