matthew/sonic-buildimage
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

[bgpcfgd]: Use peer commands for BBR, not peer-group (#6048)

Browse Source 
* templates: Move 'allowas-in' command from peer-group to instance configuration

* Use peer itself, don't rely on peer-groups
This commit is contained in:
pavel-shirshov 2020-11-26 09:55:24 -08:00 committed by GitHub
parent 37eb088b74
commit 9e0ea83cd9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 108 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
dockers/docker-fpm-frr/frr/bgpd/templates/general/instance.conf.j2
View File

@ -16,10 +16,20 @@
{% if neighbor_addr | ipv4 %}
address-family ipv4
neighbor {{ neighbor_addr }} peer-group PEER_V4
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'LeafRouter' %}
{% if CONFIG_DB__BGP_BBR['status'] == 'enabled' %}
neighbor {{ neighbor_addr }} allowas-in 1
{% endif %}
{% endif %}
!
{% elif neighbor_addr | ipv6 %}
address-family ipv6
neighbor {{ neighbor_addr }} peer-group PEER_V6
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'LeafRouter' %}
{% if CONFIG_DB__BGP_BBR['status'] == 'enabled' %}
neighbor {{ neighbor_addr }} allowas-in 1
{% endif %}
{% endif %}
!
{% endif %}
!

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

@ -6,10 +6,6 @@
address-family ipv4
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
neighbor PEER_V4 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 %}
neighbor PEER_V4 soft-reconfiguration inbound
neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in
@ -18,10 +14,6 @@
address-family ipv6
{% if CONFIG_DB__DEVICE_METADATA['localhost']['type'] == 'ToRRouter' %}
neighbor PEER_V6 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 %}
neighbor PEER_V6 soft-reconfiguration inbound
neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in

24
src/sonic-bgpcfgd/bgpcfgd/managers_bbr.py
View File

@ -1,4 +1,5 @@
import re
from collections import defaultdict
from swsscommon import swsscommon
@ -106,7 +107,9 @@ class BBRMgr(Manager):
:return: list of commands prepared for FRR
"""
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]
self.cfg_mgr.update()
available_peer_groups = self.__get_available_peer_groups()
available_peers_per_pg = self.__get_available_peers_per_peer_group(available_peer_groups)
cmds = ["router bgp %s" % bgp_asn]
prefix_of_commands = "" if status == "enabled" else "no "
peer_groups_to_restart = set()
@ -114,7 +117,8 @@ class BBRMgr(Manager):
cmds.append(" address-family %s" % af)
for pg_name in sorted(self.bbr_enabled_pgs.keys()):
if pg_name in available_peer_groups and af in self.bbr_enabled_pgs[pg_name]:
cmds.append(" %sneighbor %s allowas-in 1" % (prefix_of_commands, pg_name))
for peer in available_peers_per_pg[pg_name]:
cmds.append(" %sneighbor %s allowas-in 1" % (prefix_of_commands, peer))
peer_groups_to_restart.add(pg_name)
return cmds, list(peer_groups_to_restart)
@ -125,9 +129,25 @@ class BBRMgr(Manager):
"""
re_pg = re.compile(r'^\s*neighbor\s+(\S+)\s+peer-group\s*$')
res = set()
self.cfg_mgr.update()
for line in self.cfg_mgr.get_text():
m = re_pg.match(line)
if m:
res.add(m.group(1))
return res
def __get_available_peers_per_peer_group(self, available_peer_groups):
"""
Extract mapping peer_group->[peers]
:param available_peer_groups: list of peer groups to check
:return: dictionary peer_group->[peers]
"""
re_pg = re.compile(r'^\s*neighbor\s+(\S+)\s+peer-group\s+(\S+)\s*$')
res = defaultdict(list)
for line in self.cfg_mgr.get_text():
m = re_pg.match(line)
if m:
peer = m.group(1)
pg = m.group(2)
if pg in available_peer_groups:
res[pg].append(peer)
return res

8
src/sonic-bgpcfgd/tests/data/general/instance.conf/param_all_v4.json
View File

@ -1,4 +1,12 @@
{
"CONFIG_DB__DEVICE_METADATA": {
"localhost": {
"type": "LeafRouter"
}
},
"CONFIG_DB__BGP_BBR": {
"status": "enabled"
},
"neighbor_addr": "10.10.10.10",
"bgp_session": {
"asn": "555",

8
src/sonic-bgpcfgd/tests/data/general/instance.conf/param_all_v6.json
View File

@ -1,4 +1,12 @@
{
"CONFIG_DB__DEVICE_METADATA": {
"localhost": {
"type": "LeafRouter"
}
},
"CONFIG_DB__BGP_BBR": {
"status": "enabled"
},
"neighbor_addr": "fc::10",
"bgp_session": {
"asn": "555",

7
src/sonic-bgpcfgd/tests/data/general/instance.conf/param_base_v4.json
View File

@ -1,7 +1,12 @@
{
"CONFIG_DB__DEVICE_METADATA": {
"localhost": {}
"localhost": {
"type": "LeafRouter"
}
},
"CONFIG_DB__BGP_BBR": {
"status": "disabled"
},
"neighbor_addr": "10.10.10.10",
"bgp_session": {
"asn": "555",

7
src/sonic-bgpcfgd/tests/data/general/instance.conf/param_base_v6.json
View File

@ -1,7 +1,12 @@
{
"CONFIG_DB__DEVICE_METADATA": {
"localhost": {}
"localhost": {
"type": "LeafRouter"
}
},
"CONFIG_DB__BGP_BBR": {
"status": "disabled"
},
"neighbor_addr": "fc00::2",
"bgp_session": {
"asn": "555",

1
src/sonic-bgpcfgd/tests/data/general/instance.conf/result_all_v4.conf
View File

@ -7,6 +7,7 @@
neighbor 10.10.10.10 shutdown
address-family ipv4
neighbor 10.10.10.10 peer-group PEER_V4
neighbor 10.10.10.10 allowas-in 1
neighbor 10.10.10.10 route-reflector-client
neighbor 10.10.10.10 next-hop-self
neighbor 10.10.10.10 activate

1
src/sonic-bgpcfgd/tests/data/general/instance.conf/result_all_v6.conf
View File

@ -7,6 +7,7 @@
neighbor fc::10 shutdown
address-family ipv6
neighbor fc::10 peer-group PEER_V6
neighbor fc::10 allowas-in 1
neighbor fc::10 route-reflector-client
neighbor fc::10 next-hop-self
neighbor fc::10 activate

64
src/sonic-bgpcfgd/tests/test_bbr.py
View File

@ -250,7 +250,7 @@ def test___set_validation_4():
def test___set_validation_5():
__set_validation_common("all", {"status": "disabled"}, None, True)
def __set_prepare_config_common(status, bbr_enabled_pgs, available_pgs, expected_cmds):
def __set_prepare_config_common(status, bbr_enabled_pgs, available_pgs, mapping_pgs, expected_cmds):
cfg_mgr = MagicMock()
common_objs = {
'directory': Directory(),
@ -268,6 +268,7 @@ def __set_prepare_config_common(status, bbr_enabled_pgs, available_pgs, expected
}
m.bbr_enabled_pgs = bbr_enabled_pgs
m._BBRMgr__get_available_peer_groups = MagicMock(return_value = available_pgs)
m._BBRMgr__get_available_peers_per_peer_group = MagicMock(return_value = mapping_pgs)
cmds, peer_groups = m._BBRMgr__set_prepare_config(status)
assert cmds == expected_cmds
assert set(peer_groups) == available_pgs
@ -276,26 +277,28 @@ def test___set_prepare_config_enabled():
__set_prepare_config_common("enabled", {
"PEER_V4": ["ipv4", "ipv6"],
"PEER_V6": ["ipv6"],
}, {"PEER_V4", "PEER_V6"}, [
}, {"PEER_V4", "PEER_V6"},
{"PEER_V6": ['fc00::1'], "PEER_V4":['10.0.0.1']},[
'router bgp 65500',
' address-family ipv4',
' neighbor PEER_V4 allowas-in 1',
' neighbor 10.0.0.1 allowas-in 1',
' address-family ipv6',
' neighbor PEER_V4 allowas-in 1',
' neighbor PEER_V6 allowas-in 1',
' neighbor 10.0.0.1 allowas-in 1',
' neighbor fc00::1 allowas-in 1',
])
def test___set_prepare_config_disabled():
__set_prepare_config_common("disabled", {
"PEER_V4": ["ipv4", "ipv6"],
"PEER_V6": ["ipv6"],
}, {"PEER_V4", "PEER_V6"}, [
}, {"PEER_V4", "PEER_V6"},
{"PEER_V6": ['fc00::1'], "PEER_V4": ['10.0.0.1']}, [
'router bgp 65500',
' address-family ipv4',
' no neighbor PEER_V4 allowas-in 1',
' no neighbor 10.0.0.1 allowas-in 1',
' address-family ipv6',
' no neighbor PEER_V4 allowas-in 1',
' no neighbor PEER_V6 allowas-in 1',
' no neighbor 10.0.0.1 allowas-in 1',
' no neighbor fc00::1 allowas-in 1',
])
def test___set_prepare_config_enabled_part():
@ -303,13 +306,14 @@ def test___set_prepare_config_enabled_part():
"PEER_V4": ["ipv4", "ipv6"],
"PEER_V6": ["ipv6"],
"PEER_V8": ["ipv4"]
}, {"PEER_V4", "PEER_V6"}, [
}, {"PEER_V4", "PEER_V6"},
{"PEER_V6": ['fc00::1'], "PEER_V4": ['10.0.0.1']}, [
'router bgp 65500',
' address-family ipv4',
' neighbor PEER_V4 allowas-in 1',
' neighbor 10.0.0.1 allowas-in 1',
' address-family ipv6',
' neighbor PEER_V4 allowas-in 1',
' neighbor PEER_V6 allowas-in 1',
' neighbor 10.0.0.1 allowas-in 1',
' neighbor fc00::1 allowas-in 1',
])
def test___set_prepare_config_disabled_part():
@ -317,16 +321,16 @@ def test___set_prepare_config_disabled_part():
"PEER_V4": ["ipv4", "ipv6"],
"PEER_V6": ["ipv6"],
"PEER_v10": ["ipv4"],
}, {"PEER_V4", "PEER_V6"}, [
}, {"PEER_V4", "PEER_V6"},
{"PEER_V6": ['fc00::1'], "PEER_V4": ['10.0.0.1']}, [
'router bgp 65500',
' address-family ipv4',
' no neighbor PEER_V4 allowas-in 1',
' no neighbor 10.0.0.1 allowas-in 1',
' address-family ipv6',
' no neighbor PEER_V4 allowas-in 1',
' no neighbor PEER_V6 allowas-in 1',
' no neighbor 10.0.0.1 allowas-in 1',
' no neighbor fc00::1 allowas-in 1',
])
def test__get_available_peer_groups():
cfg_mgr = MagicMock()
common_objs = {
@ -355,3 +359,27 @@ def test__get_available_peer_groups():
])
res = m._BBRMgr__get_available_peer_groups()
assert res == {"PEER_V4", "PEER_V6"}
def test__get_available_peers_per_peer_group():
cfg_mgr = MagicMock()
common_objs = {
'directory': Directory(),
'cfg_mgr': cfg_mgr,
'tf': TemplateFabric(),
'constants': global_constants,
}
m = BBRMgr(common_objs, "CONFIG_DB", "BGP_BBR")
m.cfg_mgr.get_text = MagicMock(return_value=[
' neighbor PEER_V4 peer-group',
' neighbor PEER_V6 peer-group',
' neighbor 10.0.0.1 peer-group PEER_V4',
' neighbor fc00::1 peer-group PEER_V6',
' neighbor 10.0.0.10 peer-group PEER_V4',
' neighbor fc00::2 peer-group PEER_V6',
' ',
])
res = m._BBRMgr__get_available_peers_per_peer_group(['PEER_V4', "PEER_V6"])
assert dict(res) == {
"PEER_V4": ['10.0.0.1', '10.0.0.10'],
"PEER_V6": ['fc00::1', 'fc00::2'],
}