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

Fix bug SS and Overlay C support bgp max-path/ecmp #581

Merged
merged 1 commit into from
Jan 6, 2021
Merged

Fix bug SS and Overlay C support bgp max-path/ecmp #581

merged 1 commit into from
Jan 6, 2021

Conversation

onurgashi
Copy link
Contributor

Change Summary

This fixes the bug so we factor in the Super Spine and Overlay Controller to configure bgp max-paths and ecmp

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Documentation content changes
  • Other (please describe):

Related Issue(s)

Fixes #537

Component(s) name

eos_l3ls_evpn

Proposed changes

super-spine-router-bgp-base.j2

{%         if bgp_maximum_paths is defined and bgp_maximum_paths is not none and bgp_ecmp is defined and bgp_ecmp is not none %}
    - maximum-paths {{ bgp_maximum_paths }} ecmp {{ bgp_ecmp }}
{%         elif bgp_maximum_paths is defined and bgp_maximum_paths is not none %}
    - maximum-paths {{ bgp_maximum_paths }}
{%         else %}
    - maximum-paths {{ super_spine.nodes | length }} ecmp {{ super_spine.nodes | length }}
{%         endif %}

overlay-controller-router-bgp-base.j2

{%         if bgp_maximum_paths is defined and bgp_maximum_paths is not none and bgp_ecmp is defined and bgp_ecmp is not none %}
    - maximum-paths {{ bgp_maximum_paths }} ecmp {{ bgp_ecmp }}
{%         elif bgp_maximum_paths is defined and bgp_maximum_paths is not none %}
    - maximum-paths {{ bgp_maximum_paths }}
{%         else %}
    - maximum-paths {{ switch.remote_switches_interfaces | length }} ecmp {{ switch.remote_switches_interfaces | length }}
{%         endif %}

How to test

Default takes into account the math, if we use the knobs, we override the math

bgp_maximum_paths: 4
bgp_ecmp: 4

Super Spine

router bgp 65001
   router-id 192.168.100.1
   no bgp default ipv4-unicast
   distance bgp 20 200 200
   graceful-restart restart-time 300
   graceful-restart
   maximum-paths 4 ecmp 4

Route Server

router bgp 65100
   router-id 172.32.250.1
   no bgp default ipv4-unicast
   distance bgp 20 200 200
   graceful-restart restart-time 300
   graceful-restart
   maximum-paths 4 ecmp 4

Checklist:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed (pre-commit, make linting and make sanity-lint).
  • I have updated molecule CI testing accordingly

@onurgashi onurgashi added type: bug Something isn't working role: eos_l3ls_evpn issue related to eos_l3ls_evpn role labels Jan 4, 2021
@carlbuchmann
Copy link
Member

Copy link
Member

@carlbuchmann carlbuchmann left a comment

Choose a reason for hiding this comment

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

looks good! - ok merge once tested in with https://github.com/ClausHolbechArista/claus-l3ls-dev

@github-actions github-actions bot added role: eos_cli_config_gen issue related to eos_cli_config_gen role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated type: code quality CI and development toolset type: documentation Improvements or additions to documentation labels Jan 6, 2021
Copy link
Contributor

@ClausHolbechArista ClausHolbechArista left a comment

Choose a reason for hiding this comment

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

@ClausHolbechArista ClausHolbechArista merged commit 638e4c5 into aristanetworks:devel Jan 6, 2021
@titom73 titom73 added this to the v2.0.0rc1 milestone Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
role: eos_cli_config_gen issue related to eos_cli_config_gen role role: eos_l3ls_evpn issue related to eos_l3ls_evpn role state: CI Updated CI scenario have been updated in the PR state: Documentation role Updated type: bug Something isn't working type: code quality CI and development toolset type: documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update BGP max-paths and ecmp for Super Spine and OverlayC in l3ls_evpn
4 participants