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.

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

nl_l3: fix nnhs handling a bit #441

Merged
merged 3 commits into from
Jun 6, 2024
Merged

nl_l3: fix nnhs handling a bit #441

merged 3 commits into from
Jun 6, 2024

Conversation

KanjiMonster
Copy link
Contributor

Fix a few issues around number of nexthops handling:

  • we rejected/ignored routes with 0 nexthops (nnhs = 0) on add, but don't do so in removal (though I'm not convinced this is an actual thing that can happen)
  • later on we assume nnhs = 0 means no reachable(?) nexthop yet, but we already rejected routes with nnhs = 0 earlier, so this can't happen! Instead, check if l3_interfaces is empty
  • when removing a route, don't use the amount of l3_interfaces to determine if this is a ECMP route, but use nnhs instead

Like we do when adding a route, ignore routes with no nexthops.

Signed-off-by: Jonas Gorski <[email protected]>
We already rejected routes with nnhs == 0 earlier, so nnhs cannot be 0
at this stage. Instead, we have only two options, nnhs == 1 (single
nexthop or on-link route), or nnhs > 1 (ecmp route).

So convert the switch to a if-else and make the 0 for on-link and routes
with unresolved neighbor explicit.

Signed-off-by: Jonas Gorski <[email protected]>
We used the number of total nexthops for determining ecmp routes on
creation, so we need to do the same on deletion.

Signed-off-by: Jonas Gorski <[email protected]>
@KanjiMonster KanjiMonster requested a review from rubensfig June 4, 2024 09:25
@KanjiMonster
Copy link
Contributor Author

Note that nnhs just means number of nexthop objects, but a nexthop object may just reference a device for directly connected routes, and not necessarily gateways with IPs.

Technically it would be possible to have ECMP routes with multiple on-link device nexthops, but 0 gateways.

Copy link
Contributor

@rubensfig rubensfig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rubensfig rubensfig merged commit dd3c2ff into main Jun 6, 2024
4 checks passed
@rubensfig rubensfig deleted the jogo_route_nnhs_fixes branch June 6, 2024 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants