Skip to content

[201911] Updated BBR to use peer group name as prefix.#6497

Closed
abdosi wants to merge 2 commits intosonic-net:201911from
abdosi:bbr_change
Closed

[201911] Updated BBR to use peer group name as prefix.#6497
abdosi wants to merge 2 commits intosonic-net:201911from
abdosi:bbr_change

Conversation

@abdosi
Copy link
Contributor

@abdosi abdosi commented Jan 20, 2021

What/Why I did:

Same change as in master (PR #6515) for 201911.
In 201911 BBR is configured for each peer instead of peer-group.

How I verify:

  • Manually Verified BBR is enabled/disabled correctly for the peer in the peer-group.

neighbor 10.0.0.33 peer-group TIER0_V4_DEPLOYMENT_ID_0
neighbor 10.0.0.63 peer-group TIER0_V4_DEPLOYMENT_ID_2
neighbor 10.0.0.33 allowas-in 1
neighbor 10.0.0.63 allowas-in 1
neighbor fc00::7a peer-group TIER0_V6_DEPLOYMENT_ID_0
neighbor fc00::7e peer-group TIER0_V6_DEPLOYMENT_ID_2
neighbor fc00::7a allowas-in 1
neighbor fc00::7e allowas-in 1

  • Added Unit test
adosi@68cd7f721b7f:/sonic/src/sonic-bgpcfgd$ pytest tests/test_bbr.py
============================================================================================== test session starts ==============================================================================================
platform linux2 -- Python 2.7.13, pytest-3.0.6, py-1.4.32, pluggy-0.4.0
rootdir: /sonic/src/sonic-bgpcfgd, inifile: pytest.ini
plugins: cov-2.4.0
collected 28 items

tests/test_bbr.py ............................

---------- coverage: platform linux2, python 2.7.13-final-0 ----------
Name                             Stmts   Miss  Cover
----------------------------------------------------
bgpcfgd/__init__.py                  0      0   100%
bgpcfgd/__main__.py                  3      3     0%
bgpcfgd/config.py                   69     69     0%
bgpcfgd/directory.py                63     34    46%
bgpcfgd/frr.py                      49     49     0%
bgpcfgd/log.py                      15      5    67%
bgpcfgd/main.py                     53     53     0%
bgpcfgd/manager.py                  41     23    44%
bgpcfgd/managers_allow_list.py     425    425     0%
bgpcfgd/managers_bbr.py             98      0   100%
bgpcfgd/managers_bgp.py            192    192     0%
bgpcfgd/managers_db.py               9      9     0%
bgpcfgd/managers_intf.py            33     33     0%
bgpcfgd/managers_setsrc.py          44     44     0%
bgpcfgd/runner.py                   44     44     0%
bgpcfgd/template.py                 64     42    34%
bgpcfgd/utils.py                    19     19     0%
bgpcfgd/vars.py                      1      0   100%
----------------------------------------------------
TOTAL                             1222   1044    15%


=========================================================================================== 28 passed in 0.23 seconds ===========================================================================================

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi abdosi marked this pull request as ready for review January 20, 2021 02:39
@abdosi abdosi requested review from lguohan and shi-su January 20, 2021 02:39
@lguohan
Copy link
Collaborator

lguohan commented Jan 20, 2021

do we need this for master branch?

@lguohan
Copy link
Collaborator

lguohan commented Jan 20, 2021

also, can you check how to add the unit test for this feature?

@lguohan Added unit test cases.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi
Copy link
Contributor Author

abdosi commented Jan 21, 2021

do we need this for master branch?

@lguohan Yes will be creating separate PR for master

@abdosi abdosi changed the title BBR Changes to handle Peer group name [201911] BBR Changes to handle Peer group name Jan 21, 2021
@abdosi abdosi changed the title [201911] BBR Changes to handle Peer group name [201911] Updated BBR to use peer group name as prefix. Jan 21, 2021
@lguohan
Copy link
Collaborator

lguohan commented Jan 21, 2021

let's merge the master image pr and then cherry-pick.

@abdosi
Copy link
Contributor Author

abdosi commented Jan 22, 2021

cherry-pick from master PR. Closing this.

@abdosi abdosi closed this Jan 22, 2021
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

Successfully merging this pull request may close these issues.

2 participants