ipspace / netlab

Making virtual networking labs suck less
https://netlab.tools
Other
431 stars 64 forks source link

Implement bgp bandwidth auto for SR OS #1214

Closed jbemmel closed 3 months ago

jbemmel commented 3 months ago
ipspace commented 3 months ago

The 'add communities to EBGP' code does nothing as you modify a local variable but not store it. Anyway, we need to enable extended communities for EBGP only if the device is able to set Link Bandwidth in outgoing EBGP updates (for the moment, only EOS and FRR/CL).

Also, it looks like the changes to SR Linux / SR OS templates are not included in this pull request.

jbemmel commented 3 months ago

The 'add communities to EBGP' code does nothing as you modify a local variable but not store it.

'get' returns a reference, I verified that the topology output contains the 'extended' community for EBGP. You are correct that the code assumes bgp.communities is already defined; if it is not, it would be modifying a local variable

Anyway, we need to enable extended communities for EBGP only if the device is able to set Link Bandwidth in outgoing EBGP updates (for the moment, only EOS and FRR/CL).

Updated, SR OS does support outgoing updates. There is a missing 3rd type: Locally add a community to incoming routes

Also, it looks like the changes to SR Linux / SR OS templates are not included in this pull request.

Added now

ipspace commented 3 months ago

Updated, SR OS does support outgoing updates. There is a missing 3rd type: Locally add a community to incoming routes

That's what bgp.bandwidth.in: auto (or just bgp.bandwidth: auto) is supposed to be doing, or did you have something else in mind?

Added now

Thank you, merging.

jbemmel commented 3 months ago

Updated, SR OS does support outgoing updates. There is a missing 3rd type: Locally add a community to incoming routes

That's what bgp.bandwidth.in: auto (or just bgp.bandwidth: auto) is supposed to be doing, or did you have something else in mind?

There are 3 knobs:

  1. Add bandwidth extended community to incoming EBGP routes
  2. Accept (trust) extended community on incoming routes
  3. Send extended community on advertised routes

1 is currently being called "in", and #3 is "out". I didn't add #2 yet, because there is no flag for it

ipspace commented 3 months ago

There are 3 knobs:

On SR OS ;)

  1. Accept (trust) extended community on incoming routes

I could add bgp.bandwidth.in: accept keyword, but we're back to adding nerd knobs for one particular platform. Not sure anyone else has that same nerd knob; if you want to scrub communities, you do it with a route-map.

jbemmel commented 3 months ago

There are 3 knobs:

On SR OS ;)

  1. Accept (trust) extended community on incoming routes

I could add bgp.bandwidth.in: accept keyword, but we're back to adding nerd knobs for one particular platform. Not sure anyone else has that same nerd knob; if you want to scrub communities, you do it with a route-map.

I'm perfectly happy with leaving things the way they are for now, merely pointing out that there is some asymmetry in the semantics around "in" versus "out" that could be confusing to some people