Skip to content

Fix: Mark phantom links used to create multihop EBGP sessions#2502

Merged
ipspace merged 3 commits intodevfrom
ebgp-phantom
Jul 9, 2025
Merged

Fix: Mark phantom links used to create multihop EBGP sessions#2502
ipspace merged 3 commits intodevfrom
ebgp-phantom

Conversation

@ipspace
Copy link
Owner

@ipspace ipspace commented Jul 8, 2025

The ebgp.multihop plugin creates bogus ('phantom' sounds so much better) links to persuade the BGP module to establish BGP sessions between endpoints of multihop EBGP sessions.

Unfortunately, other modules (in particular 'routing.static') find those links and want to use them.

With this change, ebgp.multihop plugin marks the interfaces attached to those links with the '_phantom_link' flag, and the nexthop resolution for static routes avoids those interfaces.

At the moment, the bogus static route next hops seem to be the only undesired side effect of those phantom links, but of course something else is bound to pop up eventually :(

Fixes #2414

@ipspace ipspace requested a review from jbemmel July 8, 2025 14:26

node_found = False
for intf in node.interfaces:
if intf.get('_phantom_link',False):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not simply check for _bgp_session here? It implies "phantom_link"

Copy link
Owner Author

Choose a reason for hiding this comment

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

... because I'd like to have a generic mechanism we might need in the future (one of these days we'll tackle TE, and the TE tunnels might be another case of phantom links).

Copy link
Collaborator

@jbemmel jbemmel left a comment

Choose a reason for hiding this comment

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

I can see you like the phantom thing, but the flag is somewhat superfluous

@ipspace
Copy link
Owner Author

ipspace commented Jul 8, 2025

I can see you like the phantom thing, but the flag is somewhat superfluous

You are correct that the two flags serve exactly the same purpose, so I'll remove the "_bgp_session" one (as it's only used in the cleanup phase). Also, I will add a generic IGP phantom-avoidance code; the current implementation was obviously written before we had anything but OSPF in the VRFs.

@ipspace ipspace marked this pull request as draft July 8, 2025 15:27
The ebgp.multihop plugin creates bogus ('phantom' sounds so much better)
links to persuade the BGP module to establish BGP sessions between
endpoints of multihop EBGP sessions.

Unfortunately, other modules (in particular 'routing.static') find
those links and want to use them.

With this change, ebgp.multihop plugin marks the interfaces attached to
those links with the '_phantom_link' flag, and the nexthop resolution
for static routes avoids those interfaces.

At the moment, the bogus static route next hops seem to be the only
undesired side effect of those phantom links, but of course something
else is bound to pop up eventually :(

Fixes #2414
@ipspace
Copy link
Owner Author

ipspace commented Jul 9, 2025

You are correct that the two flags serve exactly the same purpose, so I'll remove the "_bgp_session" one

And I was wrong. It turns out I need (A) a flag that the interface is used for MH EBGP session (to do stuff like adding config request) and (B) a flag that the link/interface is not real.

I could probably do (A) by scanning the BGP neighbor list, but this approach is simpler (and I don't have to scan all the VRFs). So, we're staying with the two flags; modified code coming in a few minutes.

ipspace added a commit that referenced this pull request Jul 9, 2025
@ipspace ipspace marked this pull request as ready for review July 9, 2025 09:34
@ipspace ipspace merged commit 322c3b6 into dev Jul 9, 2025
10 checks passed
@ipspace ipspace deleted the ebgp-phantom branch July 9, 2025 09:55
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.

[BUG] Static routes are created for fake ebgp.multihop "interface"

2 participants