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

SR Linux: Simplify the IBGP export policy with the new 'next-hop: self' action #1307

Merged
merged 2 commits into from
Sep 8, 2024

Conversation

ipspace
Copy link
Owner

@ipspace ipspace commented Sep 7, 2024

SR Linux needs an export to set next-hop to "self" on EBGP routes but not on reflected IBGP routes.
The original template created two per-AF export policies and set the BGP next hop to the IP address
of the loopback interface.

With the new "action.bgp.next-hop: self" option, we can simplify that into a single route map that does
not depend on IP addresses or address families.

Last but not least, we're removing the internal BGP communities used to mark IBGP routes.

Copy link

cloudflare-workers-and-pages bot commented Sep 7, 2024

Deploying netlab-release with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9b9344b
Status:🚫  Build failed.

View logs

@ipspace ipspace requested a review from hellt September 7, 2024 10:35
- name: ebgp-next-hop-self
match:
protocol: bgp
action:
policy-result: next-policy
bgp:
next-hop:
set: "{{ nh }}"
set: self
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ipspace I am curious, what is the use case here where the nhs is needed on the ebgp session? I believe 4271 rules should be applied by default on eBGP sessions.

Or is it some redistribution use case?

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's not on an EBGP session. The policy is used on IBGP sessions, and we cannot use the per-neighbor/AF setting because that would also impact the reflected IBGP routes.

The statement is called "ebgp-next-hop-self" because we have to implement the next hop change on EBGP routes. Any better ideas to rename the statement?

Copy link
Collaborator

Choose a reason for hiding this comment

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

purely up to you, maybe I'd name it bgp-next-hop-self, as the policy is universally matching on the bgp protocol

Copy link
Collaborator

@jbemmel jbemmel Sep 8, 2024

Choose a reason for hiding this comment

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

I seem to recall having tested this on iBGP EVPN sessions, and I believe I saw that next hop self wasn't being applied even if the policy asked for it (in R-R scenario). Could be different in recent releases, but we may want to test for that

Copy link
Owner Author

Choose a reason for hiding this comment

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

If you're saying "I don't know whether the action.bgp.next-hop.set: self would work", it does (I'm so glad I wasted so much time writing integration tests), or were you thinking about applying next-hop-self to the peer group? That doesn't work either, the next hop is still changed on reflected routes.

@ipspace ipspace merged commit 32489d3 into dev Sep 8, 2024
10 of 11 checks passed
@ipspace ipspace deleted the srl_nhs branch September 8, 2024 15:47
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