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

[bgpcfgd]: Support default action for "Allow prefix" feature (#6370)

Browse Source 
* Use 20 and 30 route-map entries instead of 2 and 3 for TSA

* Added support for dynamic "Allow list" default action.

Co-authored-by: Pavel Shirshov <pavel.contrib@gmail.com>
This commit is contained in:
pavel-shirshov 2021-01-08 14:03:26 -08:00 committed by GitHub
parent 04cd1d61e8
commit 83715cfc49
No account linked to committer's email address
14 changed files with 291 additions and 36 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
dockers/docker-fpm-frr/TSA
View File

@ -6,9 +6,9 @@ function check_not_installed()
config=$(vtysh -c "show run")
for route_map_name in $(echo "$config" | sed -ne 's/ neighbor \S* route-map \(\S*\) out/\1/p');
do
echo "$config" | grep -q "route-map $route_map_name permit 2"
echo "$config" | grep -q "route-map $route_map_name permit 20"
c=$((c+$?))
echo "$config" | grep -q "route-map $route_map_name deny 3"
echo "$config" | grep -q "route-map $route_map_name deny 30"
c=$((c+$?))
done
return $c

4
dockers/docker-fpm-frr/TSB
View File

@ -7,10 +7,10 @@ function check_installed()
config=$(vtysh -c "show run")
for route_map_name in $(echo "$config" | sed -ne 's/ neighbor \S* route-map \(\S*\) out/\1/p');
do
echo "$config" | grep -q "route-map $route_map_name permit 2"
echo "$config" | grep -q "route-map $route_map_name permit 20"
c=$((c+$?))
e=$((e+1))
echo "$config" | grep -q "route-map $route_map_name deny 3"
echo "$config" | grep -q "route-map $route_map_name deny 30"
c=$((c+$?))
e=$((e+1))
done

8
dockers/docker-fpm-frr/TSC
View File

@ -6,9 +6,9 @@ function check_not_installed()
config=$(vtysh -c "show run")
for route_map_name in $(echo "$config" | sed -ne 's/ neighbor \S* route-map \(\S*\) out/\1/p' | egrep 'V4|V6');
do
echo "$config" | grep -q "route-map $route_map_name permit 2"
echo "$config" | grep -q "route-map $route_map_name permit 20"
c=$((c+$?))
echo "$config" | grep -q "route-map $route_map_name deny 3"
echo "$config" | grep -q "route-map $route_map_name deny 30"
c=$((c+$?))
done
return $c
@ -21,10 +21,10 @@ function check_installed()
config=$(vtysh -c "show run")
for route_map_name in $(echo "$config" | sed -ne 's/ neighbor \S* route-map \(\S*\) out/\1/p' | egrep 'V4|V6');
do
echo "$config" | grep -q "route-map $route_map_name permit 2"
echo "$config" | grep -q "route-map $route_map_name permit 20"
c=$((c+$?))
e=$((e+1))
echo "$config" | grep -q "route-map $route_map_name deny 3"
echo "$config" | grep -q "route-map $route_map_name deny 30"
c=$((c+$?))
e=$((e+1))
done

25
dockers/docker-fpm-frr/frr/bgpd/templates/general/policies.conf.j2
View File

@ -3,14 +3,22 @@
!
!
!
{% if constants.bgp.allow_list is defined and constants.bgp.allow_list.enabled is defined and constants.bgp.allow_list.enabled %}
{% if constants.bgp.allow_list.default_action is defined and constants.bgp.allow_list.default_action.strip() == 'deny' %}
{% if constants.bgp.allow_list is defined and constants.bgp.allow_list.enabled is defined and constants.bgp.allow_list.enabled and constants.bgp.allow_list.drop_community is defined %}
!
!
! please don't remove. 65535 entries are default rules
! which works when allow_list is enabled, but new configuration
! is not applied
!
{% if allow_list_default_action == 'deny' %}
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
set community no-export additive
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
set community no-export additive
{% else %}
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
set community {{ constants.bgp.allow_list.drop_community }} additive
!
@ -18,14 +26,23 @@ route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
set community {{ constants.bgp.allow_list.drop_community }} additive
{% endif %}
!
route-map FROM_BGP_PEER_V4 permit 2
bgp community-list standard allow_list_default_community permit no-export
bgp community-list standard allow_list_default_community permit {{ constants.bgp.allow_list.drop_community }}
!
route-map FROM_BGP_PEER_V4 permit 10
call ALLOW_LIST_DEPLOYMENT_ID_0_V4
on-match next
!
route-map FROM_BGP_PEER_V6 permit 2
route-map FROM_BGP_PEER_V4 permit 11
match community allow_list_default_community
!
route-map FROM_BGP_PEER_V6 permit 10
call ALLOW_LIST_DEPLOYMENT_ID_0_V6
on-match next
!
route-map FROM_BGP_PEER_V6 permit 11
match community allow_list_default_community
!
{% endif %}
!
!

4
dockers/docker-fpm-frr/frr/bgpd/tsa/bgpd.tsa.isolate.conf.j2
View File

@ -1,5 +1,5 @@
route-map {{ route_map_name }} permit 2
route-map {{ route_map_name }} permit 20
match {{ ip_protocol }} address prefix-list PL_Loopback{{ ip_version }}
set community {{ constants.bgp.traffic_shift_community }}
route-map {{ route_map_name }} deny 3
route-map {{ route_map_name }} deny 30
!

4
dockers/docker-fpm-frr/frr/bgpd/tsa/bgpd.tsa.unisolate.conf.j2
View File

@ -1,3 +1,3 @@
no route-map {{ route_map_name }} permit 2
no route-map {{ route_map_name }} deny 3
no route-map {{ route_map_name }} permit 20
no route-map {{ route_map_name }} deny 30
!

82
src/sonic-bgpcfgd/bgpcfgd/managers_allow_list.py
View File

@ -36,8 +36,6 @@ class BGPAllowListMgr(Manager):
db,
table,
)
self.cfg_mgr = common_objs["cfg_mgr"]
self.constants = common_objs["constants"]
self.key_re = re.compile(r"^DEPLOYMENT_ID\|\d+\|\S+$|^DEPLOYMENT_ID\|\d+$")
self.enabled = self.__get_enabled()
self.__load_constant_lists()
@ -63,7 +61,8 @@ class BGPAllowListMgr(Manager):
prefixes_v4 = str(data['prefixes_v4']).split(",")
if "prefixes_v6" in data:
prefixes_v6 = str(data['prefixes_v6']).split(",")
self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6)
default_action_community = self.__get_default_action_community(data)
self.__update_policy(deployment_id, community_value, prefixes_v4, prefixes_v6, default_action_community)
return True
def __set_handler_validate(self, key, data):
@ -96,6 +95,9 @@ class BGPAllowListMgr(Manager):
if not prefixes_v4 and not prefixes_v6:
log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with no prefixes specified: %s" % str(data))
return False
if "default_action" in data and data["default_action"] != "permit" and data["default_action"] != "deny":
log_err("BGPAllowListMgr::Received BGP ALLOWED 'SET' message with invalid 'default_action' field: '%s'" % str(data))
return False
return True
def del_handler(self, key):
@ -124,13 +126,14 @@ class BGPAllowListMgr(Manager):
return False
return True
def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6):
def __update_policy(self, deployment_id, community_value, prefixes_v4, prefixes_v6, default_action):
"""
Update "allow list" policy with parameters
:param deployment_id: deployment id which policy will be changed
:param community_value: community value to match for the updated policy
:param prefixes_v4: a list of v4 prefixes for the updated policy
:param prefixes_v6: a list of v6 prefixes for the updated policy
:param default_action: the default action for the policy. should be either 'permit' or 'deny'
"""
# update all related entries with the information
info = deployment_id, community_value, str(prefixes_v4), str(prefixes_v6)
@ -146,6 +149,8 @@ class BGPAllowListMgr(Manager):
cmds += self.__update_community(names['community'], community_value)
cmds += self.__update_allow_route_map_entry(self.V4, names['pl_v4'], names['community'], names['rm_v4'])
cmds += self.__update_allow_route_map_entry(self.V6, names['pl_v6'], names['community'], names['rm_v6'])
cmds += self.__update_default_route_map_entry(names['rm_v4'], default_action)
cmds += self.__update_default_route_map_entry(names['rm_v6'], default_action)
if cmds:
self.cfg_mgr.push_list(cmds)
peer_groups = self.__find_peer_group_by_deployment_id(deployment_id)
@ -365,6 +370,52 @@ class BGPAllowListMgr(Manager):
cmds.append(" match community %s" % community_name)
return cmds
def __update_default_route_map_entry(self, route_map_name, default_action_community):
"""
Add or update default action rule for the route-map.
Default action rule is hardcoded into route-map permit 65535
:param route_map_name: name of the target route_map
:param default_action_community: community value to mark not-matched prefixes
"""
info = route_map_name, default_action_community
log_debug("BGPAllowListMgr::__update_default_route_map_entry. rm='%s' set_community='%s'" % info)
current_default_action_value = self.__parse_default_action_route_map_entry(route_map_name)
if current_default_action_value != default_action_community:
return [
'route-map %s permit 65535' % route_map_name,
' set community %s additive' % default_action_community
]
else:
return []
def __parse_default_action_route_map_entry(self, route_map_name):
"""
Parse default-action route-map entry
:param route_map_name: Name of the route-map to parse
:return: a community value used for default action
"""
log_debug("BGPAllowListMgr::__parse_default_action_route_map_entries. rm='%s'" % route_map_name)
match_string = 'route-map %s permit 65535' % route_map_name
match_community = re.compile(r'^set community (\S+) additive$')
inside_route_map = False
community_value = ""
conf = self.cfg_mgr.get_text()
for line in conf + [""]:
s_line = line.strip()
if inside_route_map:
matched = match_community.match(s_line)
if matched:
community_value = matched.group(1)
break
else:
log_err("BGPAllowListMgr::Found incomplete route-map '%s' entry. seq_no=65535" % route_map_name)
inside_route_map = False
elif s_line == match_string:
inside_route_map = True
if community_value == "":
log_err("BGPAllowListMgr::Default action community value is not found. route-map '%s' entry. seq_no=65535" % route_map_name)
return community_value
def __remove_allow_route_map_entry(self, af, allow_address_pl_name, community_name, route_map_name):
"""
Add or update a "Allow address" route-map entry with the parameters
@ -624,3 +675,26 @@ class BGPAllowListMgr(Manager):
:return: prefix list ip family
"""
return 'ip' if af == self.V4 else 'ipv6'
def __get_default_action_community(self, data):
"""
Determine the default action community based on the request.
If request doesn't contain "default_action" field - the default_action value
from the constants is being used
:param data: SET request data
:return: returns community value for "default_action"
"""
drop_community = self.constants["bgp"]["allow_list"]["drop_community"]
if "default_action" in data:
if data["default_action"] == "deny":
return "no-export"
else: # "permit"
return drop_community
else:
if "default_action" in self.constants["bgp"]["allow_list"]:
if self.constants["bgp"]["allow_list"]["default_action"].strip() == "deny":
return "no-export"
else:
return drop_community
else:
return drop_community

4
src/sonic-bgpcfgd/tests/data/general/policies.conf/param_all.json
View File

@ -4,9 +4,9 @@
"bgp": {
"allow_list": {
"enabled": true,
"default_action": "permit",
"drop_community": "12345:12345"
}
}
}
},
"allow_list_default_action": "permit"
}

4
src/sonic-bgpcfgd/tests/data/general/policies.conf/param_deny.json
View File

@ -4,9 +4,9 @@
"bgp": {
"allow_list": {
"enabled": true,
"default_action": "deny",
"drop_community": "12345:12345"
}
}
}
},
"allow_list_default_action": "deny"
}

17
src/sonic-bgpcfgd/tests/data/general/policies.conf/result_all.conf
View File

@ -1,20 +1,33 @@
!
! template: bgpd/templates/general/policies.conf.j2
!
! please don't remove. 65535 entries are default rules
! which works when allow_list is enabled, but new configuration
! is not applied
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
set community 12345:12345 additive
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
set community 12345:12345 additive
!
route-map FROM_BGP_PEER_V4 permit 2
bgp community-list standard allow_list_default_community permit no-export
bgp community-list standard allow_list_default_community permit 12345:12345
!
route-map FROM_BGP_PEER_V4 permit 10
call ALLOW_LIST_DEPLOYMENT_ID_0_V4
on-match next
!
route-map FROM_BGP_PEER_V6 permit 2
route-map FROM_BGP_PEER_V4 permit 11
match community allow_list_default_community
!
route-map FROM_BGP_PEER_V6 permit 10
call ALLOW_LIST_DEPLOYMENT_ID_0_V6
on-match next
!
route-map FROM_BGP_PEER_V6 permit 11
match community allow_list_default_community
!
route-map FROM_BGP_PEER_V4 permit 100
!
route-map TO_BGP_PEER_V4 permit 100

17
src/sonic-bgpcfgd/tests/data/general/policies.conf/result_deny.conf
View File

@ -1,20 +1,33 @@
!
! template: bgpd/templates/general/policies.conf.j2
!
! please don't remove. 65535 entries are default rules
! which works when allow_list is enabled, but new configuration
! is not applied
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V4 permit 65535
set community no-export additive
!
route-map ALLOW_LIST_DEPLOYMENT_ID_0_V6 permit 65535
set community no-export additive
!
route-map FROM_BGP_PEER_V4 permit 2
bgp community-list standard allow_list_default_community permit no-export
bgp community-list standard allow_list_default_community permit 12345:12345
!
route-map FROM_BGP_PEER_V4 permit 10
call ALLOW_LIST_DEPLOYMENT_ID_0_V4
on-match next
!
route-map FROM_BGP_PEER_V6 permit 2
route-map FROM_BGP_PEER_V4 permit 11
match community allow_list_default_community
!
route-map FROM_BGP_PEER_V6 permit 10
call ALLOW_LIST_DEPLOYMENT_ID_0_V6
on-match next
!
route-map FROM_BGP_PEER_V6 permit 11
match community allow_list_default_community
!
route-map FROM_BGP_PEER_V4 permit 100
!
route-map TO_BGP_PEER_V4 permit 100

4
src/sonic-bgpcfgd/tests/data/sonic-cfggen/tsa/isolate.conf
View File

@ -1,5 +1,5 @@
route-map test_rm_name permit 2
route-map test_rm_name permit 20
match ip address prefix-list PL_LoopbackV4
set community 12345:555
route-map test_rm_name deny 3
route-map test_rm_name deny 30
!

4
src/sonic-bgpcfgd/tests/data/sonic-cfggen/tsa/unisolate.conf
View File

@ -1,3 +1,3 @@
no route-map test_rm permit 2
no route-map test_rm deny 3
no route-map test_rm permit 20
no route-map test_rm deny 30
!

146
src/sonic-bgpcfgd/tests/test_allow_list.py
View File

@ -18,7 +18,9 @@ global_constants = {
"deny 0::/0 le 59",
"deny 0::/0 ge 65"
]
}
},
"default_action": "permit",
"drop_community": "123:123"
}
}
}
@ -64,7 +66,12 @@ def test_set_handler_with_community():
"prefixes_v4": "10.20.30.0/24,30.50.0.0/16",
"prefixes_v6": "fc00:20::/64,fc00:30::/64",
}),
[],
[
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 65535',
' set community 123:123 additive',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 65535',
' set community 123:123 additive'
],
[
'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 10 deny 0.0.0.0/0 le 17',
'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V4 seq 20 permit 10.20.30.0/24 le 32',
@ -90,7 +97,12 @@ def test_set_handler_no_community():
"prefixes_v4": "20.20.30.0/24,40.50.0.0/16",
"prefixes_v6": "fc01:20::/64,fc01:30::/64",
}),
[],
[
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 65535',
' set community 123:123 additive',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 65535',
' set community 123:123 additive',
],
[
'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 10 deny 0.0.0.0/0 le 17',
'ip prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4 seq 20 permit 20.20.30.0/24 le 32',
@ -184,6 +196,10 @@ def test_set_handler_with_community_data_is_already_presented():
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10',
' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6',
' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 65535',
' set community 123:123 additive',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 65535',
' set community 123:123 additive',
""
],
[]
@ -206,6 +222,10 @@ def test_set_handler_no_community_data_is_already_presented():
' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000',
' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 65535',
' set community 123:123 additive',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 65535',
' set community 123:123 additive',
""
]
common_objs = {
@ -259,6 +279,10 @@ def test_set_handler_with_community_update_prefixes_add():
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 10',
' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020_V6',
' match community COMMUNITY_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_1010:2020',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 65535',
' set community 123:123 additive',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 65535',
' set community 123:123 additive',
""
],
[
@ -295,6 +319,10 @@ def test_set_handler_no_community_update_prefixes_add():
' match ip address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V4',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 30000',
' match ipv6 address prefix-list PL_ALLOW_LIST_DEPLOYMENT_ID_5_COMMUNITY_empty_V6',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V4 permit 65535',
' set community 123:123 additive',
'route-map ALLOW_LIST_DEPLOYMENT_ID_5_V6 permit 65535',
' set community 123:123 additive',
""
],
[
@ -450,4 +478,114 @@ def test___to_prefix_list():
res_v6 = mgr._BGPAllowListMgr__to_prefix_list(mgr.V6, ["fc00::1/128", "fc00::/64"])
assert res_v6 == ["permit fc00::1/128", "permit fc00::/64 le 128"]
# FIXME: more testcases for coverage
@patch.dict("sys.modules", swsscommon=swsscommon_module_mock)
def construct_BGPAllowListMgr(constants):
from bgpcfgd.managers_allow_list import BGPAllowListMgr
cfg_mgr = MagicMock()
common_objs = {
'directory': Directory(),
'cfg_mgr': cfg_mgr,
'tf': TemplateFabric(),
'constants': constants,
}
mgr = BGPAllowListMgr(common_objs, "CONFIG_DB", "BGP_ALLOWED_PREFIXES")
return mgr
def test___get_enabled_enabled():
constants = {
"bgp": {
"allow_list": {
"enabled": True,
}
}
}
mgr = construct_BGPAllowListMgr(constants)
assert mgr._BGPAllowListMgr__get_enabled()
def test___get_enabled_disabled_1():
constants = {
"bgp": {
"allow_list": {
"enabled": False,
}
}
}
mgr = construct_BGPAllowListMgr(constants)
assert not mgr._BGPAllowListMgr__get_enabled()
def test___get_enabled_disabled_2():
constants = {
"bgp": {
"allow_list": {}
}
}
mgr = construct_BGPAllowListMgr(constants)
assert not mgr._BGPAllowListMgr__get_enabled()
def test___get_enabled_disabled_3():
constants = {
"bgp": {}
}
mgr = construct_BGPAllowListMgr(constants)
assert not mgr._BGPAllowListMgr__get_enabled()
def test___get_enabled_disabled_4():
constants = {}
mgr = construct_BGPAllowListMgr(constants)
assert not mgr._BGPAllowListMgr__get_enabled()
def test___get_default_action_deny():
constants = {
"bgp": {
"allow_list": {
"enabled": True,
"default_action": "deny",
"drop_community": "123:123"
}
}
}
data = {}
mgr = construct_BGPAllowListMgr(constants)
assert mgr._BGPAllowListMgr__get_default_action_community(data) == "no-export"
def test___get_default_action_permit_1():
constants = {
"bgp": {
"allow_list": {
"enabled": True,
"default_action": "permit",
"drop_community": "123:123"
}
}
}
data = {}
mgr = construct_BGPAllowListMgr(constants)
assert mgr._BGPAllowListMgr__get_default_action_community(data) == "123:123"
def test___get_default_action_permit_2():
constants = {
"bgp": {
"allow_list": {
"enabled": True,
"drop_community": "123:123"
}
}
}
data = {}
mgr = construct_BGPAllowListMgr(constants)
assert mgr._BGPAllowListMgr__get_default_action_community(data) == "123:123"
def test___get_default_action_permit_3():
constants = {
"bgp": {
"allow_list": {
"enabled": False,
"drop_community": "123:123"
}
}
}
data = {}
mgr = construct_BGPAllowListMgr(constants)
assert mgr._BGPAllowListMgr__get_default_action_community(data) == "123:123"
# FIXME: more testcases for coverage