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

Prioritize configuration from config_db over constants.yml during BBR status initialization #19590

Merged
merged 12 commits into from
Aug 16, 2024

Conversation

Gfrom2016
Copy link
Contributor

@Gfrom2016 Gfrom2016 commented Jul 16, 2024

Why I did it

There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
  • Microsoft ADO (number only): 28302790

How I did it

Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it

Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'

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

  • 202205
  • 202211
  • 202305
  • 202311
  • 202405

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
@Gfrom2016 Gfrom2016 changed the title Read bbr status from config_db first Initialize BBR status from config_db first Jul 17, 2024
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
@Gfrom2016 Gfrom2016 changed the title Initialize BBR status from config_db first Prioritize configuration from config_db over constants.yml during BBR status initialization Jul 23, 2024
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
@Gfrom2016 Gfrom2016 marked this pull request as ready for review July 29, 2024 01:29
@StormLiangMS
Copy link
Contributor

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
@StormLiangMS
Copy link
Contributor

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StormLiangMS
Copy link
Contributor

hi @Gfrom2016 could we communicate with @prsunny to make sure the understanding is fully aligned that the BBR status would be effected by peers config in constants.yaml file?

And we may need to add a nightly test to cover this?

Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
Signed-off-by: zitingguo-ms <zitingguo@microsoft.com>
@StormLiangMS
Copy link
Contributor

/azp run Azure.sonic-buildimage

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Gfrom2016
Copy link
Contributor Author

@siqbal1986 Addressed your comment, please help review it again, thanks.

Copy link
Contributor

@prsunny prsunny left a comment

Choose a reason for hiding this comment

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

lgtm

@StormLiangMS StormLiangMS merged commit 084ba1a into sonic-net:master Aug 16, 2024
20 of 21 checks passed
matiAlfaro pushed a commit to Marvell-switching/sonic-buildimage that referenced this pull request Aug 21, 2024
… status initialization (sonic-net#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 21, 2024
… status initialization (sonic-net#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202305: #19974

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 23, 2024
… status initialization (sonic-net#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20005

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Aug 27, 2024
… status initialization (sonic-net#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #20025

mssonicbld pushed a commit that referenced this pull request Aug 28, 2024
… status initialization (#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'
@Gfrom2016 Gfrom2016 deleted the change_bbr_init_hierarchy branch October 14, 2024 02:34
mssonicbld pushed a commit that referenced this pull request Oct 18, 2024
… status initialization (#19590)

Why I did it
There are two places which we can configure the BBR disabled/enabled, one is constants.yml, the other is by config_db. During run time, config_db win. But at the init stage, constants.yml win. This is a wrong logic, constants.yml only win when there is no such config in the config_db.

Work item tracking
Microsoft ADO (number only): 28302790
How I did it
Initialize BBR status from constants.yml only when config_db doesn't have BBR entry.

How to verify it
Build image and run the following:

# bbr status in constants.yml is set to disabled
# change the bbr status in config_db to enabled and reload config
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hset "BGP_BBR|all" "status" "enabled"
(integer) 0
admin@str2-7050cx3-acs-01:~$ redis-cli -n 4 hget "BGP_BBR|all" "status"
"enabled"
admin@str2-7050cx3-acs-01:~$ vtysh -c 'show run' | grep 'allowas'
  neighbor PEER_V4 allowas-in 1
  neighbor PEER_V6 allowas-in 1
admin@str2-7050cx3-acs-01:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
admin@str2-7050cx3-acs-01:~$ sudo config reload -y

# check bgpcfgd log, bbr default status is enabled
2024 Aug 14 05:42:47.653216 str2-7050cx3-acs-01 INFO bgp#bgpcfgd: BBRMgr::Initialized and enabled from config_db. Default state: 'enabled'
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.

8 participants