Add W-ECMP to BGP OA (#18533)

- Why I did it
W-ECMP feature implementation is according to the HLD

- How I did it
Added W-ECMP YANG model implementation
Added W-ECMP BGP OA implementation

- How to verify it
Run UTs
Verified manually with the feature qualification

Signed-off-by: Nazarii Hnydyn <[email protected]>

Author: nazariig

dockers/docker-fpm-frr/frr/bgpd/wcmp/bgpd.wcmp.conf.j2

@@ -0,0 +1,21 @@
+!
+! template: bgpd/wcmp/bgpd.wcmp.conf.j2
+!
+route-map TO_BGP_PEER_V4 permit 100
+{%- if wcmp_enabled == 'true' %}
+  set extcommunity bandwidth num-multipaths
+{%- else %}
+  no set extcommunity bandwidth
+{%- endif %}
+exit
+!
+route-map TO_BGP_PEER_V6 permit 100
+{%- if wcmp_enabled == 'true' %}
+  set extcommunity bandwidth num-multipaths
+{%- else %}
+  no set extcommunity bandwidth
+{%- endif %}
+exit
+!
+! end of template: bgpd/wcmp/bgpd.wcmp.conf.j2
+!

files/build_templates/init_cfg.json.j2

@@ -31,6 +31,7 @@
     "BGP_DEVICE_GLOBAL": {
         "STATE": {
             "tsa_enabled": "false",
+            "wcmp_enabled": "false",
             "idf_isolation_state": "unisolated"
         }
     },

src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py

@@ -1,11 +1,17 @@
+import re
+import jinja2
+
 from .manager import Manager
 from .log import log_err, log_debug, log_notice
-import re
 from swsscommon import swsscommon
 
 class DeviceGlobalCfgMgr(Manager):
     """This class responds to change in device-specific state"""
 
+    TSA_DEFAULTS = "false"
+    WCMP_DEFAULTS = "false"
+    IDF_DEFAULTS = "unisolated"
+
     def __init__(self, common_objs, db, table):
         """
         Initialize the object
@@ -19,6 +25,7 @@ def __init__(self, common_objs, db, table):
         self.constants = common_objs['constants']
         self.tsa_template = common_objs['tf'].from_file("bgpd/tsa/bgpd.tsa.isolate.conf.j2")
         self.tsb_template = common_objs['tf'].from_file("bgpd/tsa/bgpd.tsa.unisolate.conf.j2")
+        self.wcmp_template = common_objs['tf'].from_file("bgpd/wcmp/bgpd.wcmp.conf.j2")
         self.idf_isolate_template = common_objs['tf'].from_file("bgpd/idf_isolate/idf_isolate.conf.j2")
         self.idf_unisolate_template = common_objs['tf'].from_file("bgpd/idf_isolate/idf_unisolate.conf.j2")
         self.directory.subscribe([("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/switch_type"),], self.on_switch_type_change)
@@ -29,49 +36,130 @@ def __init__(self, common_objs, db, table):
             table,
         )
 
+        # By default TSA feature is disabled
+        if not self.directory.path_exist(self.db_name, self.table_name, "tsa_enabled"):
+            self.directory.put(self.db_name, self.table_name, "tsa_enabled", self.TSA_DEFAULTS)
+        # By default W-ECMP feature is disabled
+        if not self.directory.path_exist(self.db_name, self.table_name, "wcmp_enabled"):
+            self.directory.put(self.db_name, self.table_name, "wcmp_enabled", self.WCMP_DEFAULTS)
+        # By default IDF feature is unisolated
+        if not self.directory.path_exist(self.db_name, self.table_name, "idf_isolation_state"):
+            self.directory.put(self.db_name, self.table_name, "idf_isolation_state", self.IDF_DEFAULTS)
+
     def on_switch_type_change(self):
         log_debug("DeviceGlobalCfgMgr:: Switch type update handler")
         if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/switch_type"):
             self.switch_type = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]["switch_type"]
         log_debug("DeviceGlobalCfgMgr:: Switch type: %s" % self.switch_type)
 
     def set_handler(self, key, data):
+        """ Handle device TSA/W-ECMP state change """
         log_debug("DeviceGlobalCfgMgr:: set handler")
+
         if self.switch_type:
             log_debug("DeviceGlobalCfgMgr:: Switch type: %s" % self.switch_type)
-        """ Handle device tsa_enabled state change """
         if not data:
             log_err("DeviceGlobalCfgMgr:: data is None")
             return False
 
-        tsa_status = "false"
-        idf_isolation_state = "unisolated"
+        # TSA configuration
+        self.configure_tsa(data)
+        # W-ECMP configuration
+        self.configure_wcmp(data)
+        # IDF configuration
+        self.configure_idf(data)
 
-        if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "tsa_enabled"):
-            tsa_status = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME)["tsa_enabled"]
-        if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "idf_isolation_state"):
-            idf_isolation_state = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME)["idf_isolation_state"]
-
-        if "tsa_enabled" in data:
-            self.directory.put(self.db_name, self.table_name, "tsa_enabled", data["tsa_enabled"])
-            if tsa_status != data["tsa_enabled"]:
-                self.cfg_mgr.commit()
-                self.cfg_mgr.update()
-                self.isolate_unisolate_device(data["tsa_enabled"])
-
-            
-        if "idf_isolation_state" in data:
-            self.directory.put(self.db_name, self.table_name, "idf_isolation_state", data["idf_isolation_state"])
-            if idf_isolation_state != data["idf_isolation_state"]:
-                if self.switch_type and self.switch_type != "SpineRouter":
-                    log_debug("DeviceGlobalCfgMgr:: Skipping IDF isolation configuration on Switch type: %s" % self.switch_type)
-                    return True
-                self.downstream_isolate_unisolate(data["idf_isolation_state"])
-            
         return True
 
     def del_handler(self, key):
         log_debug("DeviceGlobalCfgMgr:: del handler")
+
+        # TSA configuration
+        self.configure_tsa()
+        # W-ECMP configuration
+        self.configure_wcmp()
+        # IDF configuration
+        self.configure_idf()
+
+        return True
+
+    def is_update_required(self, key, value):
+        if self.directory.path_exist(self.db_name, self.table_name, key):
+            return value != self.directory.get(self.db_name, self.table_name, key)
+        return True
+
+    def configure_tsa(self, data=None):
+        """ Configure TSA feature"""
+
+        state = self.TSA_DEFAULTS
+
+        if data is not None:
+            if "tsa_enabled" in data:
+                state = data["tsa_enabled"]
+
+        if self.is_update_required("tsa_enabled", state):
+            self.cfg_mgr.commit()
+            self.cfg_mgr.update()
+            if self.isolate_unisolate_device(state):
+                self.directory.put(self.db_name, self.table_name, "tsa_enabled", state)
+        else:
+            log_notice("DeviceGlobalCfgMgr:: TSA configuration is up-to-date")
+
+    def configure_wcmp(self, data=None):
+        """ Configure W-ECMP feature"""
+
+        state = self.WCMP_DEFAULTS
+
+        if data is not None:
+            if "wcmp_enabled" in data:
+                state = data["wcmp_enabled"]
+
+        if self.is_update_required("wcmp_enabled", state):
+            if self.set_wcmp(state):
+                self.directory.put(self.db_name, self.table_name, "wcmp_enabled", state)
+        else:
+            log_notice("DeviceGlobalCfgMgr:: W-ECMP configuration is up-to-date")
+
+    def configure_idf(self, data=None):
+        """ Configure IDF feature"""
+
+        state = self.IDF_DEFAULTS
+
+        if data is not None:
+            if "idf_isolation_state" in data:
+                state = data["idf_isolation_state"]
+
+        if self.is_update_required("idf_isolation_state", state):
+            if self.downstream_isolate_unisolate(state):
+                self.directory.put(self.db_name, self.table_name, "idf_isolation_state", state)
+        else:
+            log_notice("DeviceGlobalCfgMgr:: IDF configuration is up-to-date")
+
+    def set_wcmp(self, status):
+        """ API to set/unset W-ECMP """
+
+        if status not in ["true", "false"]:
+            log_err("W-ECMP: invalid value({}) is provided".format(status))
+            return False
+
+        if status == "true":
+            log_notice("DeviceGlobalCfgMgr:: Enabling W-ECMP...")
+        else:
+            log_notice("DeviceGlobalCfgMgr:: Disabling W-ECMP...")
+
+        cmd = "\n"
+
+        try:
+            cmd += self.wcmp_template.render(wcmp_enabled=status)
+        except jinja2.TemplateError as e:
+            msg = "W-ECMP: error in template rendering"
+            log_err("%s: %s" % (msg, str(e)))
+            return False
+
+        self.cfg_mgr.push(cmd)
+
+        log_debug("DeviceGlobalCfgMgr::Done")
+
         return True
 
     def check_state_and_get_tsa_routemaps(self, cfg):
@@ -87,6 +175,11 @@ def check_state_and_get_tsa_routemaps(self, cfg):
 
     def isolate_unisolate_device(self, tsa_status):
         """ API to get TSA/TSB route-maps and apply configuration"""
+
+        if tsa_status not in ["true", "false"]:
+            log_err("TSA: invalid value({}) is provided".format(tsa_status))
+            return False
+
         cmd = "\n"
         if tsa_status == "true":
             log_notice("DeviceGlobalCfgMgr:: Device isolated. Executing TSA")
@@ -98,6 +191,8 @@ def isolate_unisolate_device(self, tsa_status):
         self.cfg_mgr.push(cmd)
         log_debug("DeviceGlobalCfgMgr::Done")
 
+        return True
+
     def get_ts_routemaps(self, cmds, ts_template):
         if not cmds:
             return ""
@@ -134,6 +229,16 @@ def __extract_out_route_map_names(self, cmds):
         return route_map_names
 
     def downstream_isolate_unisolate(self, idf_isolation_state):
+        """ API to apply IDF configuration """
+
+        if idf_isolation_state not in ["unisolated", "isolated_withdraw_all", "isolated_no_export"]:
+            log_err("IDF: invalid value({}) is provided".format(idf_isolation_state))
+            return False
+
+        if self.switch_type and self.switch_type != "SpineRouter":
+            log_debug("DeviceGlobalCfgMgr:: Skipping IDF isolation configuration on Switch type: %s" % self.switch_type)
+            return True
+
         cmd = "\n"
         if idf_isolation_state == "unisolated":
             cmd += self.idf_unisolate_template.render(constants=self.constants)
@@ -145,12 +250,16 @@ def downstream_isolate_unisolate(self, idf_isolation_state):
         self.cfg_mgr.push(cmd)
         log_debug("DeviceGlobalCfgMgr::Done")
 
+        return True
+
     def check_state_and_get_idf_isolation_routemaps(self):
         """ API to get TSA route-maps if device is isolated"""
+
         cmd = ""
         if self.directory.path_exist("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "idf_isolation_state"):
             idf_isolation_state = self.directory.get_slot("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME)["idf_isolation_state"]
-            if idf_isolation_state != "unisolated":                
+            if idf_isolation_state != "unisolated":
                 log_notice("DeviceGlobalCfgMgr:: IDF is isolated. Applying required route-maps")
-                cmd = self.idf_isolate_template.render(isolation_status=idf_isolation_state, constants=self.constants)           
-        return cmd
+                cmd = self.idf_isolate_template.render(isolation_status=idf_isolation_state, constants=self.constants)
+
+        return cmd

src/sonic-bgpcfgd/tests/data/wcmp/wcmp.set.conf

@@ -0,0 +1,14 @@
+
+!
+! template: bgpd/wcmp/bgpd.wcmp.conf.j2
+!
+route-map TO_BGP_PEER_V4 permit 100
+  set extcommunity bandwidth num-multipaths
+exit
+!
+route-map TO_BGP_PEER_V6 permit 100
+  set extcommunity bandwidth num-multipaths
+exit
+!
+! end of template: bgpd/wcmp/bgpd.wcmp.conf.j2
+!

src/sonic-bgpcfgd/tests/data/wcmp/wcmp.unset.conf

@@ -0,0 +1,14 @@
+
+!
+! template: bgpd/wcmp/bgpd.wcmp.conf.j2
+!
+route-map TO_BGP_PEER_V4 permit 100
+  no set extcommunity bandwidth
+exit
+!
+route-map TO_BGP_PEER_V6 permit 100
+  no set extcommunity bandwidth
+exit
+!
+! end of template: bgpd/wcmp/bgpd.wcmp.conf.j2
+!