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

bgpd,configure: vrf leak setting nexthop as vrf interface #18320

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

chiragshah6
Copy link
Member

@chiragshah6 chiragshah6 commented Mar 5, 2025

Certain switch AISC do not have capability for vrf leaked routes to point to another VRF and perform second route lookup via chaining multiple tables.
The routes need proper l3 interface in order to do forwarding in ASIC in target VRF table.
Hence, moving the setting the nexthop as source vrf to a specific flag.

The following is a connected route with nexthop as VRF tenant1 breaks the HW forwarding it needs proper l3 interface (captured in output2).

Output-1:

mlx-4:mgmt:# ip -d route show vrf tenant2 21.1.0.0/16 unicast 21.1.0.0/16 nhid 331 proto bgp scope global metric 20
        nh_info id 331 dev tenant1 scope host proto zebra

Output-2:

mlx-3:mgmt:# ip -d route show vrf tenant2 21.1.0.0/16 unicast 21.1.0.0/16 nhid 183 proto bgp scope global metric 20
        nh_info id 183 dev vlan101 scope host proto zebra  <<<< L3 interface vlan101

Following four commits introduced the vrf nexthop change:

31fc89b230 bgpd, tests: fix route leaking from the default l3vrf 
db7cf73a33 bgpd: fix interface on leaks from redistribute connected 
067fbab4e4 bgpd: fix interface on leaks from network statement 
8a02d9fe1e bgpd: Set nh ifindex to VRF's interface, not the real

Signed-off-by: Chirag Shah [email protected]

Copy link
Contributor

@mjstapp mjstapp left a comment

Choose a reason for hiding this comment

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

is it necessary to make this a build-time decision? could it be ... a run-time option instead?

@donaldsharp
Copy link
Member

I wouldn't mind a --.... change to the bgp cli. I'm also fine with it being compile time only too. It's not likely that someone is going to make a run time decision one way or another for their products.

@chiragshah6 chiragshah6 force-pushed the bgp_dev4 branch 2 times, most recently from 90d3bd6 to e62f297 Compare March 6, 2025 01:12
@chiragshah6 chiragshah6 marked this pull request as ready for review March 6, 2025 06:04
@louis-6wind louis-6wind self-requested a review March 6, 2025 09:04
@mjstapp
Copy link
Contributor

mjstapp commented Mar 6, 2025

yes, that's what I was thinking. I think it's better if everything is compiled together, etc. And if there's a command-line control, this could be tested more easily too... even with a topotest.

I wouldn't mind a --.... change to the bgp cli. I'm also fine with it being compile time only too. It's not likely that someone is going to make a run time decision one way or another for their products.

Certain switch AISCs do not have capability for vrf leaked routes
to point to another VRF and perform second route lookup via chaining
multiple tables.
The routes need proper l3 interface in order to do forwarding in
ASIC in target VRF table.
Hence, moving the setting the nexthop as source vrf to a specific flag.

The following is a connected route with nexthop as VRF tenant1 breaks the HW forwarding
it needs proper l3 interface (captured in output2).

Output-1:
mlx-4:mgmt:# ip -d route show vrf tenant2 21.1.0.0/16
unicast 21.1.0.0/16 nhid 331 proto bgp scope global metric 20
        nh_info id 331 dev tenant1 scope host proto zebra

Output-2:
mlx-3:mgmt:# ip -d route show vrf tenant2 21.1.0.0/16
unicast 21.1.0.0/16 nhid 183 proto bgp scope global metric 20
        nh_info id 183 dev vlan101 scope host proto zebra  <<<< L3 interface vlan101

Following four commits introduced the vrf nexthop change:
31fc89b bgpd, tests: fix route leaking from the default l3vrf
db7cf73 bgpd: fix interface on leaks from redistribute connected
067fbab bgpd: fix interface on leaks from network statement
8a02d9f bgpd: Set nh ifindex to VRF's interface, not the real

Signed-off-by: Chirag Shah <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants