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

IBGP local-as sessions (if supported) on Dell OS10, VyOS #368

Closed
jbemmel opened this issue Aug 31, 2022 · 16 comments
Closed

IBGP local-as sessions (if supported) on Dell OS10, VyOS #368

jbemmel opened this issue Aug 31, 2022 · 16 comments

Comments

@jbemmel
Copy link
Collaborator

jbemmel commented Aug 31, 2022

A quick glance at the recent code changes shows a new 'localas_ibgp' neighbor type. It is currently only referenced within the bgp module, but it can be present in the input for templates.

This breaks logic like https://github.com/ipspace/netlab/blob/dev/netsim/ansible/templates/bgp/eos.macro.j2#L11 which assumes there are only 'ibgp' or 'ebgp' values.

This may be a work in progress - just documenting what I noticed

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

You're absolutely right. Unfortunately, there are vendors (at the very minimum Cisco IOS XE) who found it expedient to implement an abomination where the use of local-as can turn what should be an EBGP session into an IBGP session.

We can't handle that as an IBGP session because we shall not set the neighbor update-source. We can't handle it as an EBGP session as we might have to set neighbor route-reflector-client.

In any case, I will add a "device feature" check to make it disabled by default, expecting that whoever will set that flag in the topology-defaults will also do the right thing in the device configuration templates. Any takers (cc @ssasso)?

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

BTW, there might be another session type coming in near future: "vrf_ibgp"

@ssasso has mentioned it a long while ago, and now that I'm doing all this crazy stuff, I could implement that one as well.

In any case, I would suggest restructuring templates that don't use peer groups to follow this logic:

  • if session_type == 'ibgp' ==> set update source to loopback
  • if session_type contains 'ibgp' ==> do the other IBGP stuff like route reflector clients, IBGP community setup...
  • otherwise we're dealing with ebgp session.

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

I'm disgusted. Arista EOS also supports this monstrosity. Implemented in 9574892

@ipspace ipspace changed the title Missing logic to handle new "localas_ibgp" bgp neighbor type IBGP local-as sessions (if supported) on Nokia, Dell OS10, VyOS, FRR, Cumulus VX Sep 1, 2022
@jbemmel
Copy link
Collaborator Author

jbemmel commented Sep 1, 2022

I can see why we’d need (subtly) different types, agree that a device feature flag (bgp.local_as?) could be added to signal support in templates

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

We need some way to figure out what needs to be configured (in particular, 'update source') in the templates. We could either check the session type or session type and additional flags, which could be local_as or vrf or who knows what else the vendors will throw at us.

Session type won't be set to 'localas_ibgp' unless the device feature flag indicates that's supported, so no worries -- it won't appear if your device doesn't support that (no device should, but that's a different story).

So, what's the problem?

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

Here's another reason why it might be better to use localas_ibgp instead of ibgp and local_as:

  • You will have to handle localas_ibgp sessions differently from the ibgp sessions -- the source address of the TCP session is different (directly connected versus loopback), and the next-hop-self handling might be different (I had to use next-hop-peer on Arista EOS).
  • If you use BGP templates (or neighbor policy or whatever it's called), you will have to put IBGP peers created through local-as hacks in a different group anyway because they are really a mix of IBGP and EBGP parameters.
  • Use localas_ibgp type directly translates into yet-another group name, otherwise you'd have to calculate group names on the fly.

I know that's a minor detail, but it might make some templates a tiny bit cleaner (one of these days I have to rewrite Arista and Cisco templates to use BGP policies)

@jbemmel
Copy link
Collaborator Author

jbemmel commented Sep 1, 2022

For BGP unnumbered on SR Linux I sometimes use 'bgp-unnumbered-{local_as}' as the peer group name, since the 'local_as' parameter can only be set in the group, not per interface neighbor

I have no problem here

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

For BGP unnumbered on SR Linux I sometimes use 'bgp-unnumbered-{local_as}' as the peer group name, since the 'local_as' parameter can only be set in the group, not per interface neighbor

😱🤦‍♂️

Would it help to have the groups precomputed, for example based on a configurable set of grouping attributes (type/unnumbered/local-as)?

@jbemmel
Copy link
Collaborator Author

jbemmel commented Sep 1, 2022

For this platform specific issue, I think handling it in the templates is more appropriate.

However, I would like to see a generic BGP peer group mechanism for consistency across platforms - I think it's common enough, and would lead to more readable / comparable configs

@ipspace
Copy link
Owner

ipspace commented Sep 1, 2022

However, I would like to see a generic BGP peer group mechanism for consistency across platforms - I think it's common enough, and would lead to more readable / comparable configs

Sounds like a fun idea. Will do ;)

@jbemmel
Copy link
Collaborator Author

jbemmel commented Sep 1, 2022

bgp.local_as for srlinux is in #372

I've completely refactored the bgp templates to only create peer groups that are actually referenced by neighbors.
The logic includes several groups:

  • ibgp-ipv4
  • ibgp-ipv6
  • ebgp-unnumbered
  • ebgp-unnumbered-[local_as]
  • ebgp

The 2 IBGP groups are needed because there is only a single transport address, and the group sets it to the loopback address for all ibgp neighbors. Alternative would be to set the transport address for each neighbor separately, then a single ibgp peer group would suffice

localas_ibgp logic is/will be handled by overriding both peer-as and local-as at the neighbor level (tbd)

@jbemmel jbemmel changed the title IBGP local-as sessions (if supported) on Nokia, Dell OS10, VyOS, FRR, Cumulus VX IBGP local-as sessions (if supported) on Dell OS10, VyOS, FRR, Cumulus VX Sep 6, 2022
@ipspace
Copy link
Owner

ipspace commented Sep 24, 2022

@jbemmel: Just FYI: in the commit 79c2d56 you changed the FRR feature flags to indicate FRR supports IBGP local-as. The FRR documentation has a pretty clear opinion about that:

This command is only allowed for eBGP peers.

You also indicated FRR supports vrf_local_as without implementing it.

I fixed both in 53725e4

@ipspace ipspace changed the title IBGP local-as sessions (if supported) on Dell OS10, VyOS, FRR, Cumulus VX IBGP local-as sessions (if supported) on Dell OS10, VyOS Sep 24, 2022
@ipspace
Copy link
Owner

ipspace commented Sep 24, 2022

@ssasso Could you please check whether Dell OS10 or VyOS support IBGP local-as sessions (local-as equal to remote-as) so we can close this one if the answer is no, which is suspect is the case for VyOS as it seems to be riding on top of FRR.

@jbemmel
Copy link
Collaborator Author

jbemmel commented Sep 24, 2022

I was feeling generous ;)

For SR Linux I structured the templates in a way that local-as gets configured in 1 place for both cases, hence the mix-up

Note the comment about ipv6: True in the vrf case

@ipspace
Copy link
Owner

ipspace commented Sep 25, 2022

I was feeling generous ;)

I'm sure the first user stumbling upon your generosity would be as delighted as I was 🤨

For SR Linux I structured the templates in a way that local-as gets configured in 1 place for both cases, hence the mix-up

It's not the question of whether the configuration is correct (it is), but whether it works (it does not), and whether you checked it before saying it works.

@ssasso
Copy link
Collaborator

ssasso commented Sep 26, 2022

@ssasso Could you please check whether Dell OS10 or VyOS support IBGP local-as sessions (local-as equal to remote-as) so we can close this one if the answer is no, which is suspect is the case for VyOS as it seems to be riding on top of FRR.

On VyOS is, no,

as well on Dell OS10:

image

Closing this.

@ssasso ssasso closed this as completed Sep 26, 2022
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

No branches or pull requests

3 participants