Skip to content

Commit

Permalink
[bgpcfgd]: Dynamic BBR support (sonic-net#5626)
Browse files Browse the repository at this point in the history
**- 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
  • Loading branch information
pavel-shirshov authored and santhosh-kt committed Feb 25, 2021
1 parent 0704fee commit 9e226d3
Show file tree
Hide file tree
Showing 9 changed files with 471 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
neighbor PEER_V4 allowas-in 1
neighbor PEER_V4_INT allowas-in 1
{% elif CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'LeafRouter' %}
{% if CONFIG_DB__BGP_BBR['status'] == 'enabled' %}
neighbor PEER_V4 allowas-in 1
{% endif %}
{% endif %}
{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
neighbor PEER_V4_INT route-reflector-client
Expand All @@ -24,6 +28,10 @@
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
neighbor PEER_V6 allowas-in 1
neighbor PEER_V6_INT allowas-in 1
{% elif CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'LeafRouter' %}
{% if CONFIG_DB__BGP_BBR['status'] == 'enabled' %}
neighbor PEER_V6 allowas-in 1
{% endif %}
{% endif %}
{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
neighbor PEER_V6_INT route-reflector-client
Expand Down
7 changes: 7 additions & 0 deletions files/image_config/constants/constants.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,17 @@ constants:
v6:
- "deny 0::/0 le 59"
- "deny 0::/0 ge 65"
bbr:
enabled: true
peers:
general: # peer_type
db_table: "BGP_NEIGHBOR"
template_dir: "general"
bbr:
PEER_V4:
- ipv4
PEER_V6:
- ipv6
monitors: # peer_type
enabled: true
db_table: "BGP_MONITORS"
Expand Down
3 changes: 3 additions & 0 deletions src/sonic-bgpcfgd/bgpcfgd/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
from .directory import Directory
from .log import log_notice, log_crit
from .managers_allow_list import BGPAllowListMgr
from .managers_bbr import BBRMgr
from .managers_bgp import BGPPeerMgrBase
from .managers_db import BGPDataBaseMgr
from .managers_intf import InterfaceMgr
Expand Down Expand Up @@ -47,6 +48,8 @@ def do_work():
BGPPeerMgrBase(common_objs, "CONFIG_DB", "BGP_PEER_RANGE", "dynamic", False),
# AllowList Managers
BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES"),
# BBR Manager
BBRMgr(common_objs, "CONFIG_DB", "BGP_BBR"),
]
runner = Runner()
for mgr in managers:
Expand Down
120 changes: 120 additions & 0 deletions src/sonic-bgpcfgd/bgpcfgd/managers_bbr.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
from swsscommon import swsscommon

from .log import log_err, log_info, log_crit
from .manager import Manager
from .utils import run_command


class BBRMgr(Manager):
""" This class initialize "BBR" feature for """
def __init__(self, common_objs, db, table):
"""
Initialize the object
:param common_objs: common object dictionary
:param db: name of the db
:param table: name of the table in the db
"""
super(BBRMgr, self).__init__(
common_objs,
[("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),],
db,
table,
)
self.enabled = False
self.bbr_enabled_pgs = {}
self.directory.put(self.db_name, self.table_name, 'status', "disabled")
self.__init()

def set_handler(self, key, data):
""" Implementation of 'SET' command for this class """
if not self.enabled:
log_info("BBRMgr::BBR is disabled. Drop the request")
return True
if not self.__set_validation(key, data):
return True
cmds = self.__set_prepare_config(data['status'])
rv = self.cfg_mgr.push_list(cmds)
if not rv:
log_crit("BBRMgr::can't apply configuration")
return True
self.__restart_peers()
return True

def del_handler(self, key):
""" Implementation of 'DEL' command for this class """
log_err("The '%s' table shouldn't be removed from the db" % self.table_name)

def __init(self):
""" Initialize BBRMgr. Extracted from constructor """
if not 'bgp' in self.constants:
log_err("BBRMgr::Disabled: 'bgp' key is not found in constants")
return
if 'bbr' in self.constants['bgp'] and \
'enabled' in self.constants['bgp']['bbr'] and \
self.constants['bgp']['bbr']['enabled']:
self.bbr_enabled_pgs = self.__read_pgs()
if self.bbr_enabled_pgs:
self.enabled = True
self.directory.put(self.db_name, self.table_name, 'status', "enabled")
log_info("BBRMgr::Initialized and enabled")
else:
log_info("BBRMgr::Disabled: no BBR enabled peers")
else:
log_info("BBRMgr::Disabled: not enabled in the constants")

def __read_pgs(self):
"""
Read peer-group bbr settings from constants file
:return: return bbr information from constant peer-group settings
"""
if 'peers' not in self.constants['bgp']:
log_info("BBRMgr::no 'peers' was found in constants")
return {}
res = {}
for peer_name, value in self.constants['bgp']['peers'].items():
if 'bbr' not in value:
continue
for pg_name, pg_afs in value['bbr'].items():
res[pg_name] = pg_afs
return res

def __set_validation(self, key, data):
""" Validate set-command arguments
:param key: key of 'set' command
:param data: data of 'set' command
:return: True is the parameters are valid, False otherwise
"""
if key != 'all':
log_err("Invalid key '%s' for table '%s'. Only key value 'all' is supported" % (key, self.table_name))
return False
if 'status' not in data:
log_err("Invalid value '%s' for table '%s', key '%s'. Key 'status' in data is expected" % (data, self.table_name, key))
return False
if data['status'] != "enabled" and data['status'] != "disabled":
log_err("Invalid value '%s' for table '%s', key '%s'. Only 'enabled' and 'disabled' are supported" % (data, self.table_name, key))
return False
return True

def __set_prepare_config(self, status):
"""
Generate FFR configuration to apply changes
:param status: either "enabled" or "disabled"
:return: list of commands prepared for FRR
"""
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]
cmds = ["router bgp %s" % bgp_asn]
prefix_of_commands = "" if status == "enabled" else "no "
for af in ["ipv4", "ipv6"]:
cmds.append(" address-family %s" % af)
for pg_name in sorted(self.bbr_enabled_pgs.keys()):
if af in self.bbr_enabled_pgs[pg_name]:
cmds.append(" %sneighbor %s allowas-in 1" % (prefix_of_commands, pg_name))
return cmds

def __restart_peers(self):
""" Restart peer-groups which support BBR """
for peer_group in sorted(self.bbr_enabled_pgs.keys()):
rc, out, err = run_command(["vtysh", "-c", "clear bgp peer-group %s soft in" % peer_group])
if rc != 0:
log_value = peer_group, rc, out, err
log_crit("BBRMgr::Can't restart bgp peer-group '%s'. rc='%d', out='%s', err='%s'" % log_value)
1 change: 1 addition & 0 deletions src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def add_peer(self, vrf, nbr, data):

kwargs = {
'CONFIG_DB__DEVICE_METADATA': self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME),
'CONFIG_DB__BGP_BBR': self.directory.get_slot('CONFIG_DB', 'BGP_BBR'),
'constants': self.constants,
'bgp_asn': bgp_asn,
'vrf': vrf,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,8 @@
"localhost": {
"type": "ToRRouter"
}
}
}
},
"CONFIG_DB__BGP_BBR": {
"status": "enabled"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@
"type": "LeafRouter",
"sub_role": "BackEnd"
}
},
"CONFIG_DB__BGP_BBR": {
"status": "disabled"
}
}
}
4 changes: 4 additions & 0 deletions src/sonic-bgpcfgd/tests/swsscommon_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
from mock import MagicMock


swsscommon = MagicMock(CFG_DEVICE_METADATA_TABLE_NAME = "DEVICE_METADATA")
Loading

0 comments on commit 9e226d3

Please sign in to comment.