Skip to content

Commit

Permalink
address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@StormLiangMS
StormLiangMS committed May 14, 2022
1 parent f7ffa6c commit fb605a1
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 70 deletions.
3 changes: 1 addition & 2 deletions src/sonic-bgpcfgd/bgpcfgd/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ def do_work():
StaticRouteMgr(common_objs, "CONFIG_DB", "STATIC_ROUTE"),
# Route Advertisement Managers
AdvertiseRouteMgr(common_objs, "STATE_DB", swsscommon.STATE_ADVERTISE_NETWORK_TABLE_NAME),
# TBD, Change to schema name from swsscommon (swsscommon.APP_BGP_PROFILE_TABLE_NAME) when the submodule is advanced.
RouteMapMgr(common_objs, "APPL_DB", "BGP_PROFILE_TABLE"),
RouteMapMgr(common_objs, "APPL_DB", swsscommon.APP_BGP_PROFILE_TABLE_NAME),
]
runner = Runner(common_objs['cfg_mgr'])
for mgr in managers:
Expand Down
71 changes: 18 additions & 53 deletions src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
from swsscommon import swsscommon
from .managers_rm import ROUTE_MAPS
import ipaddress
from .log import log_info, log_err
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"""
""" This class Advertises routes when ADVERTISE_NETWORK_TABLE in STATE_DB is updated """

def __init__(self, common_objs, db, table):
"""
Expand All @@ -23,70 +23,38 @@ def __init__(self, common_objs, db, table):
table,
)

self.directory.subscribe(
[
("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),
],
self.on_bgp_asn_change,
)
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"

def set_handler(self, key, data):
log_info("AdvertiseRouteMgr:: set handler")
if not self._set_handler_validate(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, data)

return True

def del_handler(self, key):
log_info("AdvertiseRouteMgr:: del handler")
if not self._del_handler_validate(key):
return
log_debug("AdvertiseRouteMgr:: del handler")
vrf, ip_prefix = self.split_key(key)
self.remove_route_advertisement(vrf, ip_prefix)

def _ip_addr_validate(self, key):
if key:
_, ip_prefix = self.split_key(key)
ip_prefix = ip_prefix.split("/")
if len(ip_prefix) != 2:
log_err("BGPAdvertiseRouteMgr:: No valid ip prefix for advertised route %s" % key)
return False
try:
ip = ipaddress.ip_address(ip_prefix[0])
if ip.version == 4 and int(ip_prefix[1]) not in range(0, 33):
log_err(
"BGPAdvertiseRouteMgr:: ipv4 prefix %s is illegal for advertised route %s" % (ip_prefix[1], key)
)
return False
if ip.version == 6 and int(ip_prefix[1]) not in range(0, 129):
log_err(
"BGPAdvertiseRouteMgr:: ipv6 prefix %s is illegal for advertised route %s" % (ip_prefix[1], key)
)
return False
except ValueError:
log_err("BGPAdvertiseRouteMgr:: No valid ip %s for advertised route %s" % (ip_prefix[0], key))
return False
else:
return False
return True

def _set_handler_validate(self, key, data):
def __set_handler_validate(self, key, data):
if data:
if ("profile" in data and data["profile"] in ROUTE_MAPS) or data == {"":""}:
return self._ip_addr_validate(key)
"""
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 _del_handler_validate(self, key):
return self._ip_addr_validate(key)

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, dict()):
Expand All @@ -110,9 +78,8 @@ def remove_route_advertisement(self, vrf, ip_prefix):

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"
]
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]

cmd_list = []
if vrf == "default":
cmd_list.append("router bgp %s" % bgp_asn)
Expand All @@ -123,24 +90,22 @@ def advertise_route_commands(self, ip_prefix, vrf, op, data=None):

if data and "profile" in data:
cmd_list.append(" network %s route-map %s" % (ip_prefix, "%s_RM" % data["profile"]))
log_info(
log_debug(
"BGPAdvertiseRouteMgr:: Update bgp %s network %s with route-map %s"
% (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_info(
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_info("BGPAdvertiseRouteMgr::Done")
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"
]
bgp_asn = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["bgp_asn"]
cmd_list = []
if vrf == "default":
cmd_list.append("router bgp %s" % bgp_asn)
Expand Down
30 changes: 15 additions & 15 deletions src/sonic-bgpcfgd/bgpcfgd/managers_rm.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from .manager import Manager
from .log import log_info, log_err
from .log import log_err, log_debug

ROUTE_MAPS = ["FROM_SDN_SLB_ROUTES"]

Expand All @@ -22,27 +22,27 @@ def __init__(self, common_objs, db, table):
)

def set_handler(self, key, data):
log_info("BGPRouteMapMgr:: set handler")
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):
if not self.__set_handler_validate(key, data):
return True

self._update_rm(key, data)
self.__update_rm(key, data)
return True

def del_handler(self, key):
log_info("BGPRouteMapMgr:: del handler")
if not self._del_handler_validate(key):
log_debug("BGPRouteMapMgr:: del handler")
if not self.__del_handler_validate(key):
return
self._remove_rm(key)
self.__remove_rm(key)

def _remove_rm(self, rm):
def __remove_rm(self, rm):
cmds = ["no route-map %s permit 100" % ("%s_RM" % rm)]
log_info("BGPRouteMapMgr:: remove route-map %s" % ("%s_RM" % rm))
log_debug("BGPRouteMapMgr:: remove route-map %s" % ("%s_RM" % rm))
self.cfg_mgr.push_list(cmds)
log_info("BGPRouteMapMgr::Done")
log_debug("BGPRouteMapMgr::Done")

def _set_handler_validate(self, key, data):
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
Expand All @@ -65,14 +65,14 @@ def _set_handler_validate(self, key, data):

return True

def _del_handler_validate(self, key):
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):
def __update_rm(self, rm, data):
cmds = ["route-map %s permit 100" % ("%s_RM" % rm), " set community %s" % data["community_id"]]
log_info("BGPRouteMapMgr:: update route-map %s community %s" % ("%s_RM" % rm, 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_info("BGPRouteMapMgr::Done")
log_debug("BGPRouteMapMgr::Done")

0 comments on commit fb605a1

Please sign in to comment.