Skip to content

Ifindex rt #209

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Ifindex rt #209

wants to merge 3 commits into from

Conversation

maxime-leroy
Copy link
Contributor

Add capacity to configure route with interface and gateway.

For now, it's just a WIP series.

We need that for FRR plugin.

@christophefontaine
Copy link
Collaborator

I'm not sure rejecting unconditionally an existing route with a different next hop is the correct behavior.

From a FIB perspective, I agree that now we should only have 1 next hop for any given route.

Yet, from a RIB perspective, we can have different next hop for the same route.

We would need a differentiator (eg: metric, link speed, ...) to decide the best route to render from the ROB to the FIB.

With dynamic routing, won't we have multiple gateways for a single route ?

@vjardin
Copy link
Contributor

vjardin commented Apr 30, 2025

Let's keep it simple for the current design. Mid term, next hop group shall be considered in order to support multiple gateways and interfaces with their updates. Dynamic routing will benefit of nhg for such Grout datapath.

Rejecting helps to avoid a miss configuration for the time being and I understand that it is enough for the routing/FIB manager.

@maxime-leroy
Copy link
Contributor Author

I'm not sure rejecting unconditionally an existing route with a different next hop is the correct behavior.
It's the current behavior. My commit doesn't change that.

From a FIB perspective, I agree that now we should only have 1 next hop for any given route.

Yet, from a RIB perspective, we can have different next hop for the same route.

We would need a differentiator (eg: metric, link speed, ...) to decide the best route to render from the ROB to the FIB

There is different topic like ecmp, route selection from rib.

But it's not the topic of this commit.

With dynamic routing, won't we have multiple gateways for a single route ?

When you do:
add ip route 10.0.0.0/24 via 1.1.1.1
add ip route 10.0.0.0/24 via 2.2.2.2
-> there is no error
show ip route -> only show the first route

My patch doesn't change anything. Grout don't support multiple next nop in the rib now. It just return an error when we tried to do that.

When multipe nexthop will be support, this commit is not need. Until that i believe it useful to get an error when a route is not added in the rib.

Previously, adding a route that already existed would silently succeed if
exist_ok was true, even when the nexthop was different.

This behavior is incorrect, as the user expects either the same route with
the same nexthop to exist, or an error to be returned.

This patch ensures that an error is returned when a route exists with a
different nexthop, regardless of the exist_ok flag.

Signed-off-by: Maxime Leroy <[email protected]>
Previously, attempting to delete a route whose nexthop is directly
connected (i.e., not a gateway) would fail with -EBUSY. For example:
 add  ip address 6.6.7.7/24 iface p4
 add ip route 83.0.0.0/24 via 6.6.7.7
 del ip route 83.0.0.0/24
 error: command failed: Device or resource busy

This behavior stems from route4/6_del, which checks whether the nexthop is
a gateway before allowing deletion. If not, it assumes the nexthop is still
in use and blocks the operation.

However, there is no functional reason to prevent deletion in this
case. The underlying rib4/6_del logic ensures that a nexthop is only fully
removed when its reference count drops to zero—preserving it as long as
it's still needed by a connected route.

Signed-off-by: Maxime Leroy <[email protected]>
Thanks to this commit, the following type of route can be added:
ip route add 10.0.0.2 via 172.1.1.2 dev p0

WIP -> just to get feedback

Signed-off-by: Maxime Leroy <[email protected]>
@christophefontaine
Copy link
Collaborator

Understood, looks nice !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants