Skip to content

Commit c94f93f

Browse files
[bgpcfgd]: Dynamic BBR support (#5626)
**- Why I did it** To introduce dynamic support of BBR functionality into bgpcfgd. BBR is adding `neighbor PEER_GROUP allowas-in 1' for all BGP peer-groups which points to T0 Now we can add and remove this configuration based on CONFIG_DB entry **- How I did it** I introduced a new CONFIG_DB entry: - table name: "BGP_BBR" - key value: "all". Currently only "all" is supported, which means that all peer-groups which points to T0s will be updated - data value: a dictionary: {"status": "status_value"}, where status_value could be either "enabled" or "disabled" Initially, when bgpcfgd starts, it reads initial BBR status values from the [constants.yml](https://github.com/Azure/sonic-buildimage/pull/5626/files#diff-e6f2fe13a6c276dc2f3b27a5bef79886f9c103194be4fcb28ce57375edf2c23cR34). Then you can control BBR status by changing "BGP_BBR" table in the CONFIG_DB (see examples below). bgpcfgd knows what peer-groups to change fron [constants.yml](https://github.com/Azure/sonic-buildimage/pull/5626/files#diff-e6f2fe13a6c276dc2f3b27a5bef79886f9c103194be4fcb28ce57375edf2c23cR39). The dictionary contains peer-group names as keys, and a list of address-families as values. So when bgpcfgd got a request to change the BBR state, it changes the state only for peer-groups listed in the constants.yml dictionary (and only for address families from the peer-group value). **- How to verify it** Initially, when we start SONiC FRR has BBR enabled for PEER_V4 and PEER_V6: ``` admin@str-s6100-acs-1:~$ vtysh -c 'show run' | egrep 'PEER_V.? allowas' neighbor PEER_V4 allowas-in 1 neighbor PEER_V6 allowas-in 1 ``` Then we apply following configuration to the db: ``` admin@str-s6100-acs-1:~$ cat disable.json { "BGP_BBR": { "all": { "status": "disabled" } } } admin@str-s6100-acs-1:~$ sonic-cfggen -j disable.json -w ``` The log output are: ``` Oct 14 18:40:22.450322 str-s6100-acs-1 DEBUG bgp#bgpcfgd: Received message : '('all', 'SET', (('status', 'disabled'),))' Oct 14 18:40:22.450620 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-f', '/tmp/tmpmWTiuq']'. Oct 14 18:40:22.681084 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V4 soft in']'. Oct 14 18:40:22.904626 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V6 soft in']'. ``` Check FRR configuraiton and see that no allowas parameters are there: ``` admin@str-s6100-acs-1:~$ vtysh -c 'show run' | egrep 'PEER_V.? allowas' admin@str-s6100-acs-1:~$ ``` Then we apply enabling configuration back: ``` admin@str-s6100-acs-1:~$ cat enable.json { "BGP_BBR": { "all": { "status": "enabled" } } } admin@str-s6100-acs-1:~$ sonic-cfggen -j enable.json -w ``` The log output: ``` Oct 14 18:40:41.074720 str-s6100-acs-1 DEBUG bgp#bgpcfgd: Received message : '('all', 'SET', (('status', 'enabled'),))' Oct 14 18:40:41.074720 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-f', '/tmp/tmpDD6SKv']'. Oct 14 18:40:41.587257 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V4 soft in']'. Oct 14 18:40:42.042967 str-s6100-acs-1 DEBUG bgp#bgpcfgd: execute command '['vtysh', '-c', 'clear bgp peer-group PEER_V6 soft in']'. ``` Check FRR configuraiton and see that the BBR configuration is back: ``` admin@str-s6100-acs-1:~$ vtysh -c 'show run' | egrep 'PEER_V.? allowas' neighbor PEER_V4 allowas-in 1 neighbor PEER_V6 allowas-in 1 ``` *** The test coverage *** Below is the test coverage ``` ---------- coverage: platform linux2, python 2.7.12-final-0 ---------- Name Stmts Miss Cover ---------------------------------------------------- bgpcfgd/__init__.py 0 0 100% bgpcfgd/__main__.py 3 3 0% bgpcfgd/config.py 78 41 47% bgpcfgd/directory.py 63 34 46% bgpcfgd/log.py 15 3 80% bgpcfgd/main.py 51 51 0% bgpcfgd/manager.py 41 23 44% bgpcfgd/managers_allow_list.py 385 21 95% bgpcfgd/managers_bbr.py 76 0 100% bgpcfgd/managers_bgp.py 193 193 0% bgpcfgd/managers_db.py 9 9 0% bgpcfgd/managers_intf.py 33 33 0% bgpcfgd/managers_setsrc.py 45 45 0% bgpcfgd/runner.py 39 39 0% bgpcfgd/template.py 64 11 83% bgpcfgd/utils.py 32 24 25% bgpcfgd/vars.py 1 0 100% ---------------------------------------------------- TOTAL 1128 530 53% ``` **- Which release branch to backport (provide reason below if selected)** - [ ] 201811 - [x] 201911 - [x] 202006
1 parent f18bcbf commit c94f93f

File tree

9 files changed

+471
-3
lines changed

9 files changed

+471
-3
lines changed

dockers/docker-fpm-frr/frr/bgpd/templates/general/peer-group.conf.j2

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@
99
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
1010
neighbor PEER_V4 allowas-in 1
1111
neighbor PEER_V4_INT allowas-in 1
12+
{% elif CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'LeafRouter' %}
13+
{% if CONFIG_DB__BGP_BBR['status'] == 'enabled' %}
14+
neighbor PEER_V4 allowas-in 1
15+
{% endif %}
1216
{% endif %}
1317
{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
1418
neighbor PEER_V4_INT route-reflector-client
@@ -24,6 +28,10 @@
2428
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
2529
neighbor PEER_V6 allowas-in 1
2630
neighbor PEER_V6_INT allowas-in 1
31+
{% elif CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'LeafRouter' %}
32+
{% if CONFIG_DB__BGP_BBR['status'] == 'enabled' %}
33+
neighbor PEER_V6 allowas-in 1
34+
{% endif %}
2735
{% endif %}
2836
{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
2937
neighbor PEER_V6_INT route-reflector-client

files/image_config/constants/constants.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,17 @@ constants:
2929
v6:
3030
- "deny 0::/0 le 59"
3131
- "deny 0::/0 ge 65"
32+
bbr:
33+
enabled: true
3234
peers:
3335
general: # peer_type
3436
db_table: "BGP_NEIGHBOR"
3537
template_dir: "general"
38+
bbr:
39+
PEER_V4:
40+
- ipv4
41+
PEER_V6:
42+
- ipv6
3643
monitors: # peer_type
3744
enabled: true
3845
db_table: "BGP_MONITORS"

src/sonic-bgpcfgd/bgpcfgd/main.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from .directory import Directory
1111
from .log import log_notice, log_crit
1212
from .managers_allow_list import BGPAllowListMgr
13+
from .managers_bbr import BBRMgr
1314
from .managers_bgp import BGPPeerMgrBase
1415
from .managers_db import BGPDataBaseMgr
1516
from .managers_intf import InterfaceMgr
@@ -47,6 +48,8 @@ def do_work():
4748
BGPPeerMgrBase(common_objs, "CONFIG_DB", "BGP_PEER_RANGE", "dynamic", False),
4849
# AllowList Managers
4950
BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES"),
51+
# BBR Manager
52+
BBRMgr(common_objs, "CONFIG_DB", "BGP_BBR"),
5053
]
5154
runner = Runner()
5255
for mgr in managers:
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
from swsscommon import swsscommon
2+
3+
from .log import log_err, log_info, log_crit
4+
from .manager import Manager
5+
from .utils import run_command
6+
7+
8+
class BBRMgr(Manager):
9+
""" This class initialize "BBR" feature for """
10+
def __init__(self, common_objs, db, table):
11+
"""
12+
Initialize the object
13+
:param common_objs: common object dictionary
14+
:param db: name of the db
15+
:param table: name of the table in the db
16+
"""
17+
super(BBRMgr, self).__init__(
18+
common_objs,
19+
[("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),],
20+
db,
21+
table,
22+
)
23+
self.enabled = False
24+
self.bbr_enabled_pgs = {}
25+
self.directory.put(self.db_name, self.table_name, 'status', "disabled")
26+
self.__init()
27+
28+
def set_handler(self, key, data):
29+
""" Implementation of 'SET' command for this class """
30+
if not self.enabled:
31+
log_info("BBRMgr::BBR is disabled. Drop the request")
32+
return True
33+
if not self.__set_validation(key, data):
34+
return True
35+
cmds = self.__set_prepare_config(data['status'])
36+
rv = self.cfg_mgr.push_list(cmds)
37+
if not rv:
38+
log_crit("BBRMgr::can't apply configuration")
39+
return True
40+
self.__restart_peers()
41+
return True
42+
43+
def del_handler(self, key):
44+
""" Implementation of 'DEL' command for this class """
45+
log_err("The '%s' table shouldn't be removed from the db" % self.table_name)
46+
47+
def __init(self):
48+
""" Initialize BBRMgr. Extracted from constructor """
49+
if not 'bgp' in self.constants:
50+
log_err("BBRMgr::Disabled: 'bgp' key is not found in constants")
51+
return
52+
if 'bbr' in self.constants['bgp'] and \
53+
'enabled' in self.constants['bgp']['bbr'] and \
54+
self.constants['bgp']['bbr']['enabled']:
55+
self.bbr_enabled_pgs = self.__read_pgs()
56+
if self.bbr_enabled_pgs:
57+
self.enabled = True
58+
self.directory.put(self.db_name, self.table_name, 'status', "enabled")
59+
log_info("BBRMgr::Initialized and enabled")
60+
else:
61+
log_info("BBRMgr::Disabled: no BBR enabled peers")
62+
else:
63+
log_info("BBRMgr::Disabled: not enabled in the constants")
64+
65+
def __read_pgs(self):
66+
"""
67+
Read peer-group bbr settings from constants file
68+
:return: return bbr information from constant peer-group settings
69+
"""
70+
if 'peers' not in self.constants['bgp']:
71+
log_info("BBRMgr::no 'peers' was found in constants")
72+
return {}
73+
res = {}
74+
for peer_name, value in self.constants['bgp']['peers'].items():
75+
if 'bbr' not in value:
76+
continue
77+
for pg_name, pg_afs in value['bbr'].items():
78+
res[pg_name] = pg_afs
79+
return res
80+
81+
def __set_validation(self, key, data):
82+
""" Validate set-command arguments
83+
:param key: key of 'set' command
84+
:param data: data of 'set' command
85+
:return: True is the parameters are valid, False otherwise
86+
"""
87+
if key != 'all':
88+
log_err("Invalid key '%s' for table '%s'. Only key value 'all' is supported" % (key, self.table_name))
89+
return False
90+
if 'status' not in data:
91+
log_err("Invalid value '%s' for table '%s', key '%s'. Key 'status' in data is expected" % (data, self.table_name, key))
92+
return False
93+
if data['status'] != "enabled" and data['status'] != "disabled":
94+
log_err("Invalid value '%s' for table '%s', key '%s'. Only 'enabled' and 'disabled' are supported" % (data, self.table_name, key))
95+
return False
96+
return True
97+
98+
def __set_prepare_config(self, status):
99+
"""
100+
Generate FFR configuration to apply changes
101+
:param status: either "enabled" or "disabled"
102+
:return: list of commands prepared for FRR
103+
"""
104+
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]
105+
cmds = ["router bgp %s" % bgp_asn]
106+
prefix_of_commands = "" if status == "enabled" else "no "
107+
for af in ["ipv4", "ipv6"]:
108+
cmds.append(" address-family %s" % af)
109+
for pg_name in sorted(self.bbr_enabled_pgs.keys()):
110+
if af in self.bbr_enabled_pgs[pg_name]:
111+
cmds.append(" %sneighbor %s allowas-in 1" % (prefix_of_commands, pg_name))
112+
return cmds
113+
114+
def __restart_peers(self):
115+
""" Restart peer-groups which support BBR """
116+
for peer_group in sorted(self.bbr_enabled_pgs.keys()):
117+
rc, out, err = run_command(["vtysh", "-c", "clear bgp peer-group %s soft in" % peer_group])
118+
if rc != 0:
119+
log_value = peer_group, rc, out, err
120+
log_crit("BBRMgr::Can't restart bgp peer-group '%s'. rc='%d', out='%s', err='%s'" % log_value)

src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ def add_peer(self, vrf, nbr, data):
187187

188188
kwargs = {
189189
'CONFIG_DB__DEVICE_METADATA': self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME),
190+
'CONFIG_DB__BGP_BBR': self.directory.get_slot('CONFIG_DB', 'BGP_BBR'),
190191
'constants': self.constants,
191192
'bgp_asn': bgp_asn,
192193
'vrf': vrf,

src/sonic-bgpcfgd/tests/data/general/peer-group.conf/param_all.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,8 @@
33
"localhost": {
44
"type": "ToRRouter"
55
}
6-
}
7-
}
6+
},
7+
"CONFIG_DB__BGP_BBR": {
8+
"status": "enabled"
9+
}
10+
}

src/sonic-bgpcfgd/tests/data/general/peer-group.conf/param_base.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,8 @@
44
"type": "LeafRouter",
55
"sub_role": "BackEnd"
66
}
7+
},
8+
"CONFIG_DB__BGP_BBR": {
9+
"status": "disabled"
710
}
8-
}
11+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
from mock import MagicMock
2+
3+
4+
swsscommon = MagicMock(CFG_DEVICE_METADATA_TABLE_NAME = "DEVICE_METADATA")

0 commit comments

Comments
 (0)