[bgpcfgd] ECMP overlay VxLan with BGP support (#10716)

Why I did it
https://github.com/Azure/SONiC/blob/master/doc/vxlan/Overlay%20ECMP%20with%20BFD.md
From the design, need to advertise the route with community string, the PR is to implement this.

How I did it
To use the route-map as the profile for the community string, all advertised routes can be associated with one route-map.
Add one file, mangers_rm.py, which is to add/update/del the route-map. Modified the managers_advertise_rt.py file to associate profile with IP route.

The route-map usage is very flexible, by this PR, we only support one fixed usage to add community string for route to simplify this design.

How to verify it
Implement new unit tests for mangers_rm.py and updated unit test for managers_advertise_rt.py.
Manually verified the test case in the test plan section, will add testcase in sonic-mgmt later. Azure/sonic-mgmt#5581

src/sonic-bgpcfgd/bgpcfgd/main.py

@@ -17,6 +17,7 @@ from .managers_db import BGPDataBaseMgr
 from .managers_intf import InterfaceMgr
 from .managers_setsrc import ZebraSetSrc
 from .managers_static_rt import StaticRouteMgr
+from .managers_rm import RouteMapMgr
 from .runner import Runner, signal_handler
 from .template import TemplateFabric
 from .utils import read_constants
@@ -62,6 +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),
+        RouteMapMgr(common_objs, "APPL_DB", swsscommon.APP_BGP_PROFILE_TABLE_NAME),
     ]
     runner = Runner(common_objs['cfg_mgr'])
     for mgr in managers:

src/sonic-bgpcfgd/bgpcfgd/managers_advertise_rt.py

@@ -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 @@ class AdvertiseRouteMgr(Manager):
             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_DELETE = "DELETE"
-    OP_ADD = 'ADD'
+    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 __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):
+    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.advertise_route_commands(ip_prefix, vrf, self.OP_ADD, data)
-
-        self.advertised_routes.setdefault(vrf, set()).add(ip_prefix)
 
+        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 ip_prefix not in self.advertised_routes.get(vrf, dict()):
-        if not self.advertised_routes.get(vrf, set()):
+            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, data=None):
-    def advertise_route_commands(self, ip_prefix, vrf, op):
         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))
+
+        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"
+                % (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 @@ class AdvertiseRouteMgr(Manager):
         :param key: key to split
         :return: vrf name extracted from the key, ip prefix extracted from the key
         """
-        if '|' not in key:
+        if "|" not in key:
-            return 'default', key
+            return "default", key
         else:
-            return tuple(key.split('|', 1))
+            return tuple(key.split("|", 1))

src/sonic-bgpcfgd/bgpcfgd/managers_rm.py

@@ -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")

src/sonic-bgpcfgd/tests/test_advertise_rt.py

@@ -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", {}),
+        ("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 test_set_del_bgp_asn_change():
     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"]
+        ]
+    )

src/sonic-bgpcfgd/tests/test_rm.py

@@ -0,0 +1,62 @@
+from unittest.mock import MagicMock
+from bgpcfgd.directory import Directory
+from bgpcfgd.managers_rm import RouteMapMgr
+from swsscommon import swsscommon
+
+def constructor():
+    cfg_mgr = MagicMock()
+
+    common_objs = {
+        'directory': Directory(),
+        'cfg_mgr':   cfg_mgr,
+        'constants': {},
+    }
+
+    mgr = RouteMapMgr(common_objs, "APPL_DB", "BGP_PROFILE_TABLE")
+    return mgr
+
+def set_del_test(mgr, op, args, expected_ret, expected_cmds):
+    set_del_test.push_list_called = False
+    def push_list(cmds):
+        set_del_test.push_list_called = True
+        assert cmds in expected_cmds
+        return True
+    mgr.cfg_mgr.push_list = push_list
+
+    if op == "SET":
+        ret = mgr.set_handler(*args)
+        assert ret == expected_ret
+    elif op == "DEL":
+        mgr.del_handler(*args)
+    else:
+        assert False, "Wrong operation"
+
+    if expected_cmds:
+        assert set_del_test.push_list_called, "cfg_mgr.push_list wasn't called"
+    else:
+        assert not set_del_test.push_list_called, "cfg_mgr.push_list was called"
+
+def test_set_del():
+    mgr = constructor()
+    set_del_test(
+        mgr,
+        "SET",
+        ("FROM_SDN_SLB_ROUTES", {
+            "community_id": "1234:1234"
+        }),
+        True,
+        [
+            ["route-map FROM_SDN_SLB_ROUTES_RM permit 100",
+             " set community 1234:1234"]
+        ]
+    )
+
+    set_del_test(
+        mgr,
+        "DEL",
+        ("FROM_SDN_SLB_ROUTES",),
+        True,
+        [
+            ["no route-map FROM_SDN_SLB_ROUTES_RM permit 100"]
+        ]
+    )