-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[bgpcfgd] ECMP overlay VxLan with BGP support #10716
Changes from all commits
543dbf2
ea3cc7a
3e19a2c
70e6902
504d564
2bd2611
6956eb7
361053c
ec8f3f3
bb2911e
ec15a26
f7ffa6c
fb605a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,14 @@ | ||
from .manager import Manager | ||
from .template import TemplateFabric | ||
from swsscommon import swsscommon | ||
from .managers_rm import ROUTE_MAPS | ||
import ipaddress | ||
from .log import log_info, log_err, log_debug | ||
|
||
|
||
class AdvertiseRouteMgr(Manager): | ||
""" This class Advertises routes when ADVERTISE_NETWORK_TABLE in STATE_DB is updated """ | ||
|
||
def __init__(self, common_objs, db, table): | ||
""" | ||
Initialize the object | ||
|
@@ -18,82 +22,105 @@ def __init__(self, common_objs, db, table): | |
db, | ||
table, | ||
) | ||
|
||
self.directory.subscribe([("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),], self.on_bgp_asn_change) | ||
self.advertised_routes = dict() | ||
|
||
|
||
OP_DELETE = 'DELETE' | ||
OP_ADD = 'ADD' | ||
|
||
OP_DELETE = "DELETE" | ||
OP_ADD = "ADD" | ||
|
||
def set_handler(self, key, data): | ||
log_debug("AdvertiseRouteMgr:: set handler") | ||
if not self.__set_handler_validate(key, data): | ||
return True | ||
vrf, ip_prefix = self.split_key(key) | ||
self.add_route_advertisement(vrf, ip_prefix) | ||
self.add_route_advertisement(vrf, ip_prefix, data) | ||
|
||
return True | ||
|
||
|
||
def del_handler(self, key): | ||
log_debug("AdvertiseRouteMgr:: del handler") | ||
vrf, ip_prefix = self.split_key(key) | ||
self.remove_route_advertisement(vrf, ip_prefix) | ||
|
||
|
||
def add_route_advertisement(self, vrf, ip_prefix): | ||
def __set_handler_validate(self, key, data): | ||
if data: | ||
if ("profile" in data and data["profile"] in ROUTE_MAPS) or data == {"":""}: | ||
""" | ||
APP which config the data should be responsible to pass a valid IP prefix | ||
""" | ||
return True | ||
|
||
log_err("BGPAdvertiseRouteMgr:: Invalid data %s for advertised route %s" % (data, key)) | ||
return False | ||
|
||
def add_route_advertisement(self, vrf, ip_prefix, data): | ||
if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"): | ||
if not self.advertised_routes.get(vrf, set()): | ||
if not self.advertised_routes.get(vrf, dict()): | ||
self.bgp_network_import_check_commands(vrf, self.OP_ADD) | ||
self.advertise_route_commands(ip_prefix, vrf, self.OP_ADD) | ||
|
||
self.advertised_routes.setdefault(vrf, set()).add(ip_prefix) | ||
self.advertise_route_commands(ip_prefix, vrf, self.OP_ADD, data) | ||
|
||
self.advertised_routes.setdefault(vrf, dict()).update({ip_prefix: data}) | ||
|
||
def remove_route_advertisement(self, vrf, ip_prefix): | ||
self.advertised_routes.setdefault(vrf, set()).discard(ip_prefix) | ||
if not self.advertised_routes.get(vrf, set()): | ||
if ip_prefix not in self.advertised_routes.get(vrf, dict()): | ||
log_info("BGPAdvertiseRouteMgr:: %s|%s does not exist" % (vrf, ip_prefix)) | ||
return | ||
self.advertised_routes.get(vrf, dict()).pop(ip_prefix) | ||
if not self.advertised_routes.get(vrf, dict()): | ||
self.advertised_routes.pop(vrf, None) | ||
|
||
if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"): | ||
if not self.advertised_routes.get(vrf, set()): | ||
if not self.advertised_routes.get(vrf, dict()): | ||
self.bgp_network_import_check_commands(vrf, self.OP_DELETE) | ||
self.advertise_route_commands(ip_prefix, vrf, self.OP_DELETE) | ||
|
||
|
||
def advertise_route_commands(self, ip_prefix, vrf, op): | ||
def advertise_route_commands(self, ip_prefix, vrf, op, data=None): | ||
is_ipv6 = TemplateFabric.is_ipv6(ip_prefix) | ||
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"] | ||
|
||
cmd_list = [] | ||
if vrf == 'default': | ||
if vrf == "default": | ||
cmd_list.append("router bgp %s" % bgp_asn) | ||
else: | ||
cmd_list.append("router bgp %s vrf %s" % (bgp_asn, vrf)) | ||
|
||
cmd_list.append(" address-family %s unicast" % ("ipv6" if is_ipv6 else "ipv4")) | ||
cmd_list.append(" %snetwork %s" % ('no ' if op == self.OP_DELETE else '', ip_prefix)) | ||
|
||
self.cfg_mgr.push_list(cmd_list) | ||
if data and "profile" in data: | ||
cmd_list.append(" network %s route-map %s" % (ip_prefix, "%s_RM" % data["profile"])) | ||
log_debug( | ||
"BGPAdvertiseRouteMgr:: Update bgp %s network %s with route-map %s" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Based on scale, there could be significant number of route updates. So please reduce the log-level and ensure it won't show up in regular flow unless explicitly enabled There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good one, change to debug. |
||
% (bgp_asn, vrf + "|" + ip_prefix, "%s_RM" % data["profile"]) | ||
) | ||
else: | ||
cmd_list.append(" %snetwork %s" % ("no " if op == self.OP_DELETE else "", ip_prefix)) | ||
log_debug( | ||
"BGPAdvertiseRouteMgr:: %sbgp %s network %s" | ||
% ("Remove " if op == self.OP_DELETE else "Update ", bgp_asn, vrf + "|" + ip_prefix) | ||
) | ||
|
||
self.cfg_mgr.push_list(cmd_list) | ||
log_debug("BGPAdvertiseRouteMgr::Done") | ||
|
||
def bgp_network_import_check_commands(self, vrf, op): | ||
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"] | ||
cmd_list = [] | ||
if vrf == 'default': | ||
if vrf == "default": | ||
cmd_list.append("router bgp %s" % bgp_asn) | ||
else: | ||
cmd_list.append("router bgp %s vrf %s" % (bgp_asn, vrf)) | ||
cmd_list.append(" %sbgp network import-check" % ('' if op == self.OP_DELETE else 'no ')) | ||
cmd_list.append(" %sbgp network import-check" % ("" if op == self.OP_DELETE else "no ")) | ||
|
||
self.cfg_mgr.push_list(cmd_list) | ||
|
||
|
||
def on_bgp_asn_change(self): | ||
if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"): | ||
for vrf, ip_prefixes in self.advertised_routes.items(): | ||
self.bgp_network_import_check_commands(vrf, self.OP_ADD) | ||
for ip_prefix in ip_prefixes: | ||
self.add_route_advertisement(vrf, ip_prefix) | ||
|
||
self.add_route_advertisement(vrf, ip_prefix, ip_prefixes[ip_prefix]) | ||
|
||
@staticmethod | ||
def split_key(key): | ||
|
@@ -102,7 +129,7 @@ def split_key(key): | |
:param key: key to split | ||
:return: vrf name extracted from the key, ip prefix extracted from the key | ||
""" | ||
if '|' not in key: | ||
return 'default', key | ||
if "|" not in key: | ||
return "default", key | ||
else: | ||
return tuple(key.split('|', 1)) | ||
return tuple(key.split("|", 1)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
from .manager import Manager | ||
from .log import log_err, log_debug | ||
|
||
ROUTE_MAPS = ["FROM_SDN_SLB_ROUTES"] | ||
|
||
|
||
class RouteMapMgr(Manager): | ||
"""This class add route-map when BGP_PROFILE_TABLE in APPL_DB is updated""" | ||
|
||
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(RouteMapMgr, self).__init__( | ||
common_objs, | ||
[], | ||
db, | ||
table, | ||
) | ||
|
||
def set_handler(self, key, data): | ||
log_debug("BGPRouteMapMgr:: set handler") | ||
"""Only need a name as the key, and community id as the data""" | ||
if not self.__set_handler_validate(key, data): | ||
return True | ||
|
||
self.__update_rm(key, data) | ||
return True | ||
|
||
def del_handler(self, key): | ||
log_debug("BGPRouteMapMgr:: del handler") | ||
if not self.__del_handler_validate(key): | ||
return | ||
self.__remove_rm(key) | ||
|
||
def __remove_rm(self, rm): | ||
cmds = ["no route-map %s permit 100" % ("%s_RM" % rm)] | ||
log_debug("BGPRouteMapMgr:: remove route-map %s" % ("%s_RM" % rm)) | ||
self.cfg_mgr.push_list(cmds) | ||
log_debug("BGPRouteMapMgr::Done") | ||
|
||
def __set_handler_validate(self, key, data): | ||
if key not in ROUTE_MAPS: | ||
log_err("BGPRouteMapMgr:: Invalid key for route-map %s" % key) | ||
return False | ||
|
||
if not data: | ||
log_err("BGPRouteMapMgr:: data is None") | ||
return False | ||
community_ids = data["community_id"].split(":") | ||
try: | ||
if ( | ||
len(community_ids) != 2 | ||
or int(community_ids[0]) not in range(0, 65536) | ||
or int(community_ids[1]) not in range(0, 65536) | ||
): | ||
log_err("BGPRouteMapMgr:: data %s doesn't include valid community id %s" % (data, community_ids)) | ||
return False | ||
except ValueError: | ||
log_err("BGPRouteMapMgr:: data %s includes illegal input" % (data)) | ||
return False | ||
|
||
return True | ||
|
||
def __del_handler_validate(self, key): | ||
if key not in ROUTE_MAPS: | ||
log_err("BGPRouteMapMgr:: Invalid key for route-map %s" % key) | ||
return False | ||
return True | ||
|
||
def __update_rm(self, rm, data): | ||
cmds = ["route-map %s permit 100" % ("%s_RM" % rm), " set community %s" % data["community_id"]] | ||
log_debug("BGPRouteMapMgr:: update route-map %s community %s" % ("%s_RM" % rm, data["community_id"])) | ||
self.cfg_mgr.push_list(cmds) | ||
log_debug("BGPRouteMapMgr::Done") |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,7 +48,7 @@ def test_set_del(): | |
set_del_test( | ||
mgr, | ||
"SET", | ||
("10.1.0.0/24", {}), | ||
("10.1.0.0/24", {"":""}), | ||
True, | ||
[ | ||
["router bgp 65100", | ||
|
@@ -62,7 +62,7 @@ def test_set_del(): | |
set_del_test( | ||
mgr, | ||
"SET", | ||
("fc00:10::/64", {}), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we retain this case to cover empty data? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @prsunny I think the old unit test is inaccurate, from existing code of VNetRouteOrch, it send out {key:{"":""}} to the bgpcfgd for empty case as below. And also by manually testing, can't insert a key with a "empty" data to the redis. |
||
("fc00:10::/64", {"":""}), | ||
True, | ||
[ | ||
["router bgp 65100", | ||
|
@@ -103,7 +103,7 @@ def test_set_del_vrf(): | |
set_del_test( | ||
mgr, | ||
"SET", | ||
("vrfRED|10.2.0.0/24", {}), | ||
("vrfRED|10.2.0.0/24", {"":""}), | ||
True, | ||
[ | ||
["router bgp 65100 vrf vrfRED", | ||
|
@@ -117,7 +117,7 @@ def test_set_del_vrf(): | |
set_del_test( | ||
mgr, | ||
"SET", | ||
("vrfRED|fc00:20::/64", {}), | ||
("vrfRED|fc00:20::/64", {"":""}), | ||
True, | ||
[ | ||
["router bgp 65100 vrf vrfRED", | ||
|
@@ -158,7 +158,9 @@ def test_set_del_bgp_asn_change(): | |
set_del_test( | ||
mgr, | ||
"SET", | ||
("vrfRED|10.3.0.0/24", {}), | ||
("vrfRED|10.3.0.0/24", { | ||
"profile": "FROM_SDN_SLB_ROUTES" | ||
}), | ||
True, | ||
[] | ||
) | ||
|
@@ -170,7 +172,7 @@ def test_set_del_bgp_asn_change(): | |
" no bgp network import-check"], | ||
["router bgp 65100 vrf vrfRED", | ||
" address-family ipv4 unicast", | ||
" network 10.3.0.0/24"] | ||
" network 10.3.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM"] | ||
] | ||
def push_list(cmds): | ||
test_set_del_bgp_asn_change.push_list_called = True | ||
|
@@ -183,3 +185,61 @@ def push_list(cmds): | |
mgr.directory.put("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost", {"bgp_asn": "65100"}) | ||
|
||
assert test_set_del_bgp_asn_change.push_list_called | ||
|
||
def test_set_del_with_community(): | ||
mgr = constructor() | ||
set_del_test( | ||
mgr, | ||
"SET", | ||
("10.1.0.0/24", { | ||
"profile": "FROM_SDN_SLB_ROUTES" | ||
}), | ||
True, | ||
[ | ||
["router bgp 65100", | ||
" no bgp network import-check"], | ||
["router bgp 65100", | ||
" address-family ipv4 unicast", | ||
" network 10.1.0.0/24 route-map FROM_SDN_SLB_ROUTES_RM"] | ||
] | ||
) | ||
|
||
set_del_test( | ||
mgr, | ||
"SET", | ||
("fc00:10::/64", { | ||
"profile": "FROM_SDN_SLB_ROUTES" | ||
}), | ||
True, | ||
[ | ||
["router bgp 65100", | ||
" address-family ipv6 unicast", | ||
" network fc00:10::/64 route-map FROM_SDN_SLB_ROUTES_RM"] | ||
] | ||
) | ||
|
||
set_del_test( | ||
mgr, | ||
"DEL", | ||
("10.1.0.0/24",), | ||
True, | ||
[ | ||
["router bgp 65100", | ||
" address-family ipv4 unicast", | ||
" no network 10.1.0.0/24"] | ||
] | ||
) | ||
|
||
set_del_test( | ||
mgr, | ||
"DEL", | ||
("fc00:10::/64",), | ||
True, | ||
[ | ||
["router bgp 65100", | ||
" bgp network import-check"], | ||
["router bgp 65100", | ||
" address-family ipv6 unicast", | ||
" no network fc00:10::/64"] | ||
] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If data is not present, we need to advertise without a community string (for backward compatibility). Can you please confirm.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@prsunny confirmed, the check after the 'or' is for the case which doesn't have the community string.
if ("profile" in data and data["profile"] in ROUTE_MAPS) or data == {"":""}: