Ifindex nexthop #271
Ifindex nexthop #271
Conversation
Reviewer's GuideThis PR extends nexthop support to allow interface-only (ifindex) nexthops by introducing AF_UNSPEC handling and a dedicated ID allocator, refactors configuration to unify pool and hash reallocation via an ID pool, updates the CLI/API syntax to accept explicit IDs and gateway omission, and synchronizes FRR integrations by removing the GROUT_INDEX_OFFSET and treating unspec nexthops using direct iface IDs. ER diagram for nexthop and gr_id_pool relationshiperDiagram
NEXTHOP ||..|| GR_ID_POOL : allocates
NEXTHOP {
uint32_t nh_id
uint16_t iface_id
addr_family_t af
}
GR_ID_POOL {
uint32_t max_ids
uint32_t used
}
Class diagram for gr_id_pool ID allocator (new)classDiagram
class gr_id_pool {
+uint32_t max_ids
+uint32_t used
+struct rte_bitmap bmp
+gr_id_pool_create(name, max_ids)
+gr_id_pool_destroy(p)
+gr_id_pool_used(p)
+gr_id_pool_avail(p)
+gr_id_pool_get(p)
+gr_id_pool_book(p, id)
+gr_id_pool_put(p, id)
}
Class diagram for nexthop ID management changesclassDiagram
class nexthop {
+uint32_t nh_id
+addr_family_t af
+uint16_t vrf_id
+uint16_t iface_id
+struct gr_nexthop base
+int ref_count
+... (other attributes)
+nexthop_id_get()
+nexthop_id_put()
}
class gr_id_pool {
+gr_id_pool_get()
+gr_id_pool_put()
+gr_id_pool_book()
}
nexthop --> gr_id_pool : uses for ID allocation
Class diagram for addr_family_t enum and gr_af_name changesclassDiagram
class addr_family_t {
<<enum>>
+GR_AF_UNSPEC
+GR_AF_IP4
+GR_AF_IP6
}
class gr_af_name {
+gr_af_name(af)
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThese changes introduce support for the unspecified address family ( Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (15)
🚧 Files skipped from review as they are similar to previous changes (14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @maxime-leroy - I've reviewed your changes - here's some feedback:
- There’s a typo in the abort message in set_nexthop_key (“AF_UNSPECT”); please correct it to “AF_UNSPEC”.
- This PR spans control-plane, CLI, FRR integration, and tests all at once—consider splitting it into smaller, focused PRs per feature for easier review.
- Error handling around gr_id_pool creation and resizing mixes errno_set_null, errno_log_null, and raw -errno returns; standardize on one approach for clarity.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a typo in the abort message in set_nexthop_key (“AF_UNSPECT”); please correct it to “AF_UNSPEC”.
- This PR spans control-plane, CLI, FRR integration, and tests all at once—consider splitting it into smaller, focused PRs per feature for easier review.
- Error handling around gr_id_pool creation and resizing mixes errno_set_null, errno_log_null, and raw -errno returns; standardize on one approach for clarity.
## Individual Comments
### Comment 1
<location> `modules/infra/control/nexthop.c:102` </location>
<code_context>
+ return;
+}
+
+static int nexthop_id_get(struct nexthop *nh) {
+ int ret;
+
</code_context>
<issue_to_address>
Possible race condition if pool_id is modified concurrently.
Consider adding synchronization to protect 'pool_id' and the ID pool, or clearly document that only a single writer is allowed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
maxime-leroy
force-pushed
the
ifindex_nexthop
branch
from
2040a07 to
35abb0a
Compare
modules/infra/control/nexthop.c
Outdated
| } | ||
| break; | ||
| case GR_AF_UNSPEC: | ||
| ABORT("AF_UNSPECT has no nexthop key with gw"); |
There was a problem hiding this comment.
Typo in abort message: "AF_UNSPECT" should be "AF_UNSPEC".
modules/infra/cli/nexthop.c
Outdated
| scols_line_set_data(line, 1, ""); | ||
| scols_line_sprintf(line, 2, "%s", gr_nh_type_name(nh)); | ||
| scols_line_sprintf(line, 3, "%s", gr_af_name(nh->af)); | ||
| scols_line_sprintf(line, 4, ADDR_F, ADDR_W(nh->af), &nh->addr); | ||
| if (nh->af == AF_UNSPEC) |
There was a problem hiding this comment.
Inconsistent use of AF_UNSPEC vs GR_AF_UNSPEC; prefer GR_AF_UNSPEC when testing nh->af.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
main/gr_id_pool.h (1)
14-14: Remove unnecessary<stdio.h>inclusionThe
<stdio.h>header is not used in this file and should be removed to reduce dependencies.frr/zebra_dplane_grout.c (1)
489-489: Use zebra shutdown mechanism instead of exit(1)Using
exit(1)in event handlers bypasses cleanup routines. Consider using the proper zebra abort/shutdown mechanism to ensure graceful termination.Also applies to: 494-494
modules/infra/control/nexthop.c (2)
81-82: Ignoring return value ofsnprintf; potential truncation of the pool name is not handled.
102-128: Possible race condition if pool_id is modified concurrently.Consider adding synchronization to protect 'pool_id' and the ID pool, or clearly document that only a single writer is allowed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
api/gr_net_types.h(1 hunks)frr/if_grout.c(1 hunks)frr/rt_grout.c(4 hunks)frr/zebra_dplane_grout.c(1 hunks)main/gr_id_pool.h(1 hunks)modules/infra/api/nexthop.c(3 hunks)modules/infra/cli/nexthop.c(6 hunks)modules/infra/control/nexthop.c(15 hunks)modules/ip/cli/route.c(1 hunks)modules/ip6/cli/route.c(1 hunks)modules/srv6/control/route.c(1 hunks)smoke/config_test.sh(1 hunks)smoke/ip6_forward_frr_test.sh(1 hunks)smoke/ip6_forward_test.sh(2 hunks)smoke/ip_forward_frr_test.sh(1 hunks)smoke/ip_forward_test.sh(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
smoke/config_test.sh (1)
modules/infra/control/gr_nh_control.h (1)
nexthop(18-34)
modules/ip/cli/route.c (3)
cli/table.c (1)
scols_line_sprintf(10-23)modules/infra/cli/iface.c (1)
iface_from_id(130-143)modules/infra/control/iface.c (1)
iface_from_id(170-177)
smoke/ip6_forward_test.sh (1)
modules/ip6/control/route.c (1)
nexthop(202-202)
modules/infra/api/nexthop.c (2)
main/gr_module.h (1)
api_out(16-19)modules/infra/control/nexthop.c (1)
nexthop_lookup_by_id(474-481)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04)
- GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled)
- GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04)
- GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04)
- GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04)
🔇 Additional comments (37)
modules/srv6/control/route.c (1)
81-81: LGTM! Flag change aligns with interface-based nexthop supportThe removal of
GR_NH_F_GATEWAYflag is correct for supporting nexthops that represent interfaces rather than IP gateways, which aligns with the PR's objective of supporting link-only nexthops.smoke/ip6_forward_frr_test.sh (1)
21-25: Test changes correctly validate new interface-based nexthop functionalityThe conditional assignment of addresses to different interfaces and the route specification via interface name properly test the new nexthop ID and GR_AF_UNSPEC support introduced in this PR.
Also applies to: 33-33
smoke/ip_forward_frr_test.sh (1)
21-25: IPv4 test changes mirror IPv6 and correctly validate interface-based routingThe conditional interface assignment and route configuration via interface name properly test the new nexthop functionality for IPv4, maintaining consistency with the IPv6 test changes.
Also applies to: 33-33
frr/if_grout.c (1)
177-180: LGTM! Good defensive programming.The explicit handling of
GR_AF_UNSPECwith proper error logging and context cleanup prevents undefined behavior and provides clear feedback about unsupported functionality. This aligns well with the broader introduction of unspecified address family support across the codebase.api/gr_net_types.h (2)
26-26: LGTM! Proper enum extension.Adding
GR_AF_UNSPEC = AF_UNSPECcorrectly maps to the standard unspecified address family constant, providing a consistent foundation for the new nexthop functionality.
33-34: LGTM! Consistent string representation.The string representation "Unspec" follows the existing naming pattern and provides clear identification of the unspecified address family in logging and display functions.
modules/ip6/cli/route.c (1)
78-87: LGTM! Enhanced nexthop display logic.The conditional display logic correctly handles both interface-based (
GR_AF_UNSPEC) and IP-based nexthops. The fallback from interface name to interface ID wheniface_from_id()fails provides robust error handling. This implementation is consistent with the similar changes in the IPv4 route display module.smoke/ip6_forward_test.sh (2)
15-16: LGTM! Testing nexthop ID functionality.The script correctly validates the new nexthop ID-based routing by creating a nexthop with ID 45 on interface
$p2and then referencing it in the route addition. This tests the integration of the newGR_AF_UNSPECnexthop support introduced in this PR.
26-30: LGTM! Enhanced test topology.The conditional interface assignment creates different network configurations for each namespace, providing more comprehensive test coverage of the routing functionality. This change enhances the test's ability to validate various routing scenarios.
modules/ip/cli/route.c (1)
78-88: LGTM! Consistent nexthop display implementation.The nexthop display logic mirrors the IPv6 implementation exactly, providing consistent user experience across both IP versions. The conditional handling of
GR_AF_UNSPECwith proper fallback behavior ensures robust display of interface-based nexthops.smoke/config_test.sh (2)
9-16: LGTM! New nexthop syntax properly implements ID-based management.The updated syntax with explicit
idandgwkeywords is cleaner and more consistent. Line 10 correctly demonstrates the new link-only nexthop feature (no gateway specified).
34-34: Deletion command correctly updated to use nexthop ID.The change from IP-based to ID-based deletion is consistent with the new nexthop management system.
smoke/ip_forward_test.sh (2)
15-16: Correct usage of link-only nexthop with route referencing by ID.The nexthop creation without a gateway and the route referencing it by ID properly demonstrates the new functionality.
26-30: Verify the intent of asymmetric interface assignments.The conditional assignment of IP addresses to different interfaces (lo for namespace 0, physical interface for namespace 1) creates asymmetric network configurations. Please confirm this is intentional for testing specific routing scenarios.
frr/rt_grout.c (4)
230-233: Correct handling of GR_AF_UNSPEC nexthops.The mapping to
NEXTHOP_TYPE_IFINDEXandAF_UNSPECfamily is appropriate for interface-only nexthops.
301-301: Family mismatch check correctly relaxed for AF_UNSPEC nexthops.Allowing AF_UNSPEC nexthops to work with any route family is appropriate since interface-only nexthops can forward traffic regardless of address family.
857-861: Proper handling of interface-only nexthops from FRR.The code correctly identifies NEXTHOP_TYPE_IFINDEX as interface-only nexthops and sets the appropriate GR_AF_UNSPEC family, working around FRR's default AFI_IP assignment.
900-902: Maintains kernel compatibility for AF_UNSPEC nexthops.Converting AF_UNSPEC to AF_INET aligns with kernel behavior for interface-only nexthops, ensuring consistent handling.
modules/infra/api/nexthop.c (4)
50-50: Good practice: Initialize flags before the switch statement.Moving flag initialization outside the switch ensures a clean state regardless of the address family path taken.
64-68: Thorough validation for GR_AF_UNSPEC nexthops.The code correctly ensures no IP address is set for UNSPEC nexthops and appropriately sets the LINK and STATIC flags.
82-83: Logical restriction: No MAC address for interface-only nexthops.Since UNSPEC nexthops have no IP address, there's no need for MAC address association. The validation correctly prevents this invalid configuration.
148-153: Deletion logic correctly simplified to ID-only lookup.The removal of fallback lookups aligns with the new ID-based nexthop management system, making the code cleaner and more predictable.
modules/infra/cli/nexthop.c (4)
65-66: ID parsing correctly prioritized with proper error handling.Moving ID parsing first with immediate error checking improves user experience by catching ID errors early.
78-79: Correct handling of nexthops without gateway addresses.Setting AF to UNSPEC when no IP is provided enables the creation of interface-only nexthops, aligning with the new functionality.
248-248: Improved command syntax with explicit keywords.The new syntax with
[id ID]and[gw IP]keywords is more intuitive and self-documenting. The simplified delete command aligns with ID-based management.Also applies to: 260-260
158-161: Clear visual indication for interface-only nexthops.Displaying "?" for UNSPEC nexthops provides immediate visual feedback that these are interface-only nexthops without gateway addresses.
modules/infra/control/nexthop.c (11)
90-100: LGTM! Clean ID release implementation.The function correctly handles both pool-managed and user-specified IDs, ensuring only IDs within the configured range are returned to the pool.
160-162: LGTM! Appropriate handling of AF_UNSPEC.The ABORT is correct here as AF_UNSPEC nexthops are interface-only and should never have gateway addresses requiring key generation.
241-243: LGTM! Well-integrated ID pool management.The ID pool creation and cleanup follow the same pattern as other resources, with proper error handling and rollback logic.
Also applies to: 251-252, 264-265
289-289: LGTM! Consistent AF_UNSPEC support.
341-341: LGTM! Allows creation of AF_UNSPEC nexthops.
373-373: LGTM! Correct ID lifecycle management during updates.The implementation properly releases the old ID and allocates/books the new one with appropriate error handling.
Also applies to: 382-382
413-414: LGTM! Correct equality check for AF_UNSPEC.AF_UNSPEC nexthops have no addresses to compare, so skipping address comparison is appropriate.
463-464: LGTM! Appropriate handling of AF_UNSPEC lookup.AF_UNSPEC nexthops cannot be looked up by address, so returning NULL is correct.
498-498: LGTM! Ensures ID cleanup on nexthop destruction.
606-608: LGTM! ID pool initialization follows established patterns.
643-665: LGTM! Minimal but correct AF_UNSPEC operations.The implementation correctly handles reference counting for AF_UNSPEC nexthops and appropriately returns EINVAL for solicitation (which doesn't apply to interface-only nexthops).
frr/zebra_dplane_grout.c
Outdated
| default: | ||
| break; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Log unknown address families instead of silently ignoring
The empty default case silently ignores unknown address family values. Consider adding debug logging to help identify potential issues:
default:
+ gr_log_debug("Unknown address family %d in notification", gr_nh->af);
break;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| default: | |
| break; | |
| default: | |
| gr_log_debug("Unknown address family %d in notification", gr_nh->af); | |
| break; |
🤖 Prompt for AI Agents
In frr/zebra_dplane_grout.c around lines 318 to 319, the default case in the
switch statement silently ignores unknown address family values. Modify the
default case to add debug logging that outputs the unknown address family value,
helping to identify potential issues during runtime.
35abb0a to
b968ab5
Compare
| struct gr_id_pool *p; | ||
|
|
||
| b_size = rte_bitmap_get_memory_footprint(max_ids); | ||
| p_size = sizeof(struct gr_id_pool) - sizeof(struct rte_bitmap) + b_size; |
There was a problem hiding this comment.
Potential integer overflow computing p_size = sizeof(...) - sizeof(...) + b_size. Validate that b_size isn’t large enough to wrap p_size.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
modules/infra/control/nexthop.c (2)
90-100: ID release logic is correct, but address the race condition concern.The logic correctly handles ID release and range checking for managed vs user-specified IDs.
However, the previously identified race condition concern about concurrent
pool_idmodification remains unaddressed. Consider adding synchronization or clearly documenting the single-writer assumption.
77-88: Address the snprintf truncation issue identified in previous reviews.The return value of
snprintfis still being ignored, and potential truncation of the pool name is not handled.Apply this diff to handle potential truncation:
static struct gr_id_pool *create_idpool(const struct gr_nexthop_config *c) { if (pool_id != NULL && gr_id_pool_used(pool_id)) return errno_set_null(EBUSY); char name[128]; - snprintf(name, sizeof(name), "nexthop-ids-%u", c->max_count); + int ret = snprintf(name, sizeof(name), "nexthop-ids-%u", c->max_count); + if (ret >= sizeof(name)) + return errno_set_null(ENAMETOOLONG); + struct gr_id_pool *pid = gr_id_pool_create(name, c->max_count); if (pid == NULL) return errno_log_null(rte_errno, "gr_id_pool_create"); return pid; }
🧹 Nitpick comments (1)
modules/infra/control/nexthop.c (1)
463-464: LGTM! Correct handling of AF_UNSPEC lookups.Appropriately returns NULL for AF_UNSPEC since these nexthops are interface-based and can't be looked up by address.
Consider using
GR_AF_UNSPECconsistently instead ofAF_UNSPECfor alignment with other parts of the code.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
api/gr_net_types.h(1 hunks)frr/if_grout.c(1 hunks)frr/rt_grout.c(4 hunks)frr/zebra_dplane_grout.c(1 hunks)main/gr_id_pool.h(1 hunks)modules/infra/api/nexthop.c(3 hunks)modules/infra/cli/nexthop.c(6 hunks)modules/infra/control/nexthop.c(15 hunks)modules/ip/cli/route.c(1 hunks)modules/ip6/cli/route.c(1 hunks)modules/srv6/control/route.c(1 hunks)smoke/config_test.sh(1 hunks)smoke/ip6_forward_frr_test.sh(1 hunks)smoke/ip6_forward_test.sh(2 hunks)smoke/ip_forward_frr_test.sh(1 hunks)smoke/ip_forward_test.sh(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- modules/srv6/control/route.c
🚧 Files skipped from review as they are similar to previous changes (14)
- frr/zebra_dplane_grout.c
- smoke/config_test.sh
- modules/ip6/cli/route.c
- smoke/ip6_forward_frr_test.sh
- api/gr_net_types.h
- smoke/ip6_forward_test.sh
- frr/if_grout.c
- smoke/ip_forward_frr_test.sh
- modules/ip/cli/route.c
- smoke/ip_forward_test.sh
- frr/rt_grout.c
- modules/infra/cli/nexthop.c
- modules/infra/api/nexthop.c
- main/gr_id_pool.h
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-and-tests (gcc-14, address, debug, ubuntu-24.04)
- GitHub Check: build-and-tests (clang-18, none, debugoptimized, ubuntu-24.04)
- GitHub Check: build-and-tests (gcc-13, none, debugoptimized, ubuntu-24.04, -Dfrr=enabled)
- GitHub Check: build-and-tests (clang-16, none, debugoptimized, ubuntu-24.04)
- GitHub Check: build-and-tests (clang-15, none, debugoptimized, ubuntu-22.04)
🔇 Additional comments (9)
modules/infra/control/nexthop.c (9)
6-6: LGTM! Clean addition of ID pool infrastructure.The include and static variable declaration are appropriate for the new ID pool functionality.
Also applies to: 27-27
102-128: LGTM! Comprehensive ID allocation logic with proper error handling.The function correctly handles all nexthop ID allocation scenarios:
- Internal nexthops (no ID needed)
- Auto-allocation from the managed pool
- User-specified IDs both within and outside the managed range
- Proper cleanup on failure
The error handling ensures no resource leaks.
160-162: LGTM! Correct handling of AF_UNSPEC nexthops.The ABORT is appropriate since AF_UNSPEC nexthops are interface-based and don't have gateway addresses. The typo from previous reviews has been fixed.
289-289: LGTM! Necessary addition to support AF_UNSPEC operations registration.Correctly extends the valid address families to include GR_AF_UNSPEC.
341-341: LGTM! Enables creation of AF_UNSPEC nexthops.Correctly extends nexthop creation to support the new GR_AF_UNSPEC address family.
373-373: LGTM! Proper ID lifecycle management during updates.Correctly releases the old ID before update and allocates/gets the new ID after update, ensuring proper resource management.
Also applies to: 382-382
413-414: LGTM! Correct equality comparison for AF_UNSPEC nexthops.Appropriately skips address comparison for interface-based AF_UNSPEC nexthops while still comparing other relevant fields.
498-498: LGTM! Comprehensive integration of ID pool management.The ID pool is properly integrated throughout the nexthop lifecycle:
- ID release during nexthop destruction
- Pool creation, replacement, and cleanup in configuration management
- Initialization during module startup
The implementation follows established patterns for resource management in this codebase.
Also applies to: 224-224, 241-243, 251-252, 264-265, 606-608
643-665: LGTM! Clean and appropriate AF_UNSPEC operations implementation.The operations are correctly implemented for interface-based nexthops:
- Simple reference counting for add/del operations
- Appropriate EINVAL return for solicit (can't solicit an interface)
- Proper registration during module initialization
The minimal implementation reflects the nature of interface-based nexthops that don't require address resolution.
Gateway IP will no longer reliably identify a nexthop once gateway-less entries are supported, so numeric IDs will become the primary key. This patch adds an ID pool based on DPDK `rte_bitmap`, sized to the configured max-nexthops. Bitmap bit 0 represents nexthop ID 1, and the allocator returns (bit_index + 1); ID 0 stays invalid. IDs above max-nexthops are still allowed manually and are not tracked by the bitmap. Internal nexthops have still no ID set. Signed-off-by: Maxime Leroy <[email protected]>
Gateway-less nexthops break the “lookup by gateway IP” model, so every API and CLI path now manipulates nexthops by their numeric ID. The CLI is updated in consequence: `del nexthop` requires an ID, while `add nexthop` keeps the optional `id` token (now placed before the address) so existing scripts remain valid. Smoke tests are updated to match. Signed-off-by: Maxime Leroy <[email protected]>
A nexthop can now be defined with only its output interface, no gateway IP. The patch introduces `GR_AF_UNSPEC`, extends API/CLI parsing, and registers a dedicated AF handler that skips neighbour resolution and flags; data-plane paths treat the entry as directly connected. Route listing shows the interface name when `af == GR_AF_UNSPEC`, keeping output readable while existing gateway-based flows continue to work. Signed-off-by: Maxime Leroy <[email protected]>
The dplane plugin now maps Zebra’s NEXTHOP_TYPE_IFINDEX to GR_AF_UNSPEC and pushes it to Grout, letting routes specify just the ifindex with no gateway. Signed-off-by: Maxime Leroy <[email protected]>
maxime-leroy
force-pushed
the
ifindex_nexthop
branch
from
b968ab5 to
f5088d1
Compare
| return; | ||
| } | ||
|
|
||
| if (nh_family != family) { | ||
| if (nh_family != AF_UNSPEC && nh_family != family) { |
There was a problem hiding this comment.
Logic bug: comparing the pointer nh_family to AF_UNSPEC/family rather than its value. You need to dereference (*nh_family) in these comparisons.
|
|
||
| struct gr_id_pool { | ||
| uint32_t max_ids; | ||
| uint32_t used; |
There was a problem hiding this comment.
Member used is non-atomic and is read concurrently by multiple threads (single writer, multiple readers). This is a data-race/UB; consider making it an atomic or protecting access with a lock.
2 participants
include #269 && #270
Summary by Sourcery
Introduce a global ID allocator for nexthops and allow users to specify or dynamically assign nexthop IDs, support link-only nexthops via unspecified address family, remove legacy index offset logic, refactor configuration allocation, and update CLI, API, FRR integration, and tests to leverage ID-based nexthops.
New Features:
Enhancements:
Tests:
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes