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

[bgpcfgd]: Fixes for BBR #5956

Merged
merged 3 commits into from
Nov 19, 2020
Merged

[bgpcfgd]: Fixes for BBR #5956

merged 3 commits into from
Nov 19, 2020

Conversation

pavel-shirshov
Copy link
Contributor

@pavel-shirshov pavel-shirshov commented Nov 18, 2020

- Why I did it
The current implementation of BBR has two issues:

  1. constants.yml uses key "bgp.bbr.enabled" as a key to enable disable BBR feature and as a default state for the BBR feature.
  2. If the current configuration doesn't have some peer-groups, which are listed in constants.yml bgp.peers.*.bbr bgpcfgd will report an error of updating such peer-groups.

- How I did it

  1. I added a key bgp.bbr.default_state with values "enabled" or "disabled" into constants.yml. Now bgp.bbr.enabled controls will bgpcfgd support BBR feature or not. And bgp.bbr.default_state control initial state of the BBR.
  2. I added an extra-code to check what peer-groups in the current config. After that bgpcfgd skips all peer-groups from constants,yml which are not presented in bgp config.

- How to verify it
Build an image and check the behaviour

admin@str-s6100-acs-1:~$ sonic-cfggen -j disable.txt -w    
admin@str-s6100-acs-1:~$ vtysh -c 'show run' | grep allow      
admin@str-s6100-acs-1:~$ sonic-cfggen -j enable.txt -w     
admin@str-s6100-acs-1:~$ vtysh -c 'show run' | grep allow
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1

Nov 18 22:26:29.815909 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'show running-config']'.
Nov 18 22:26:30.038863 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-f', '/tmp/tmp5o11zi0e']'.
Nov 18 22:26:30.258212 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V4 soft in']'.
Nov 18 22:26:30.476447 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V6 soft in']'.
Nov 18 22:27:00.247345 str-s6100-acs-1 DEBUG bgp#bgpcfgd: Received message : '('all', 'SET', (('status', 'enabled'),))'
Nov 18 22:27:00.247853 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'show running-config']'.
Nov 18 22:27:00.476119 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-f', '/tmp/tmp_pptzt_j']'.
Nov 18 22:27:00.700638 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V4 soft in']'.
Nov 18 22:27:00.920070 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V6 soft in']'.

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@pavel-shirshov pavel-shirshov self-assigned this Nov 18, 2020
@pavel-shirshov pavel-shirshov changed the title [bgpcfgd]: Fixed for BBR [bgpcfgd]: Fixes for BBR Nov 18, 2020
lguohan
lguohan previously approved these changes Nov 18, 2020
@pavel-shirshov pavel-shirshov marked this pull request as ready for review November 18, 2020 22:56
@pavel-shirshov pavel-shirshov merged commit a92732f into master Nov 19, 2020
@pavel-shirshov pavel-shirshov deleted the pavelsh/bbr_fix branch November 19, 2020 08:08
abdosi pushed a commit that referenced this pull request Nov 19, 2020
* Add explicit default state into the constants.yml
* Enable/disable only peer-groups, available in the config
* Retrieve updates from frr before using configuration

Co-authored-by: Pavel Shirshov <pavel.contrib@gmail.com>
@pavel-shirshov pavel-shirshov linked an issue Nov 23, 2020 that may be closed by this pull request
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
* Add explicit default state into the constants.yml
* Enable/disable only peer-groups, available in the config
* Retrieve updates from frr before using configuration

Co-authored-by: Pavel Shirshov <pavel.contrib@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamic BGP_BBR is not working as expected
3 participants