[bgpcfgd]: Changes to support sequential IDF isolation (sonic-net#19136)

Why I did it In order to isolate an IDF sequentially (one device at a time), need a mechanism to perform downstream isolation on T2 device. Adding support for two kinds of isolation - By setting no-export BGP community on all routes advertised to T1 (except loopback), so the routes are not advertised to T0s By withdrawing all routes from T1 How I did it Added 1. New configuration to support the changes described by Yang model changes for Sequential IDF isolation sonic-net#18597 Script to update configDB on each ASIC to configure no-export/withdraw-all isolation and unisolation of IDF. This is copied to host 2. BGPcfgd changes to handle configDB events and add/remove the required route-maps 3. UT cases and data file changes for BGPCfgd and jinja templates How to verify it Corresponding templates on T2 have to be updated to verify the functionality Configuration options- sudo idf_isolation withdraw_all sudo idf_isolation no-export sudo idf_isolation unisolated Validate the routes on T1 are either withdrawn, tagged with no-export community or re-advertised based on the configuration
arun1355492 · Jul 26, 2024 · 957a36c · 957a36c
1 parent a9299e5
commit 957a36c
Show file tree

Hide file tree

Showing 18 changed files with 382 additions and 7 deletions.
diff --git a/dockers/docker-fpm-frr/base_image_files/idf_isolation b/dockers/docker-fpm-frr/base_image_files/idf_isolation
@@ -0,0 +1,87 @@
+#!/bin/bash
+
+# Restrict command to sudo users
+if [ "$EUID" -ne 0 ] ; then
+    echo "Root priveleges are needed for this operation"
+    exit 1
+fi
+
+# Command is only supported on spine routers
+if [[ "$(sonic-cfggen -d -v DEVICE_METADATA.localhost.type)" != *"SpineRouter"* ]] ; then
+    echo "Operation is not supported on this platform"
+    exit 1
+fi
+
+# Skip operation on chassis supervisor
+if [ -f /etc/sonic/chassisdb.conf ]; then  
+    echo "Skipping Operation on chassis supervisor"
+    exit 0
+fi
+
+idf_isolation_states=("isolated_no_export" "isolated_withdraw_all" "unisolated" "status")
+valid=false
+for state in "${idf_isolation_states[@]}"; do
+    if [[ "$1" == "$state" ]]; then
+       valid=true
+       break
+    fi
+done
+if [ $valid == false ]; then
+    echo "Invalid parameter $1, Operation not supported"
+    echo ""
+    echo "Usage: sudo idf_isolation <isolated_no_export|isolated_withdraw_all|unisolated|status>"
+    exit 0
+fi
+
+IDF_ISOLATION_STATE="{\"BGP_DEVICE_GLOBAL\":{\"STATE\":{\"idf_isolation_state\": \"$1\"}}}"
+
+# read SONiC immutable variables
+[ -f /etc/sonic/sonic-environment ] && . /etc/sonic/sonic-environment
+
+PLATFORM=${PLATFORM:-`sonic-cfggen -H -v DEVICE_METADATA.localhost.platform`}
+
+# Parse the device specific asic conf file, if it exists
+ASIC_CONF=/usr/share/sonic/device/$PLATFORM/asic.conf
+[ -f $ASIC_CONF ] && . $ASIC_CONF
+
+if [[ ($NUM_ASIC -gt 1) ]]; then
+    asic=0
+    NAMESPACE_PREFIX='asic'
+
+    while [ $asic -lt $NUM_ASIC ]
+    do        
+        sub_role=`sonic-cfggen -d -v "DEVICE_METADATA['localhost']['sub_role']" -n $NAMESPACE_PREFIX$asic`
+        if [ $sub_role == 'FrontEnd' ] ; then
+            cur_idf_isolation_state="$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.idf_isolation_state -n $NAMESPACE_PREFIX$asic)"
+            if [ $1 == 'status' ] ; then
+                echo "BGP$asic: IDF isolation state: $cur_idf_isolation_state"
+            else
+                if [ $1 == $cur_idf_isolation_state ]; then
+                    echo "BGP$asic: IDF is already in $1 state"
+                else
+                    sonic-cfggen -a "$IDF_ISOLATION_STATE" -w -n $NAMESPACE_PREFIX$asic
+                    logger -t $1 -p user.info "IDF isolation state: $1"
+                    echo "BGP$asic: IDF isolation state: $1"
+                fi
+            fi
+        fi
+        asic=$[$asic+1]
+    done
+else
+    cur_idf_isolation_state="$(sonic-cfggen -d -v BGP_DEVICE_GLOBAL.STATE.idf_isolation_state)"
+    if [ $1 == 'status' ] ; then
+        echo "IDF isolation state: $cur_idf_isolation_state"
+    else
+        if [ $1 == $cur_idf_isolation_state ]; then
+            echo "IDF is already in $1 state"
+        else
+            sonic-cfggen -a "$IDF_ISOLATION_STATE" -w
+            logger -t $1 -p user.info "IDF isolation state: $1"
+            echo "IDF isolation state: $1"
+        fi
+    fi
+fi
+
+if [ $1 != 'status' ]; then
+    echo "Please execute 'sudo config save' to preserve IDF isolation state after reboot or config reload"
+fi
diff --git a/dockers/docker-fpm-frr/frr/bgpd/idf_isolate/idf_isolate.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/idf_isolate/idf_isolate.conf.j2
@@ -0,0 +1,20 @@
+route-map CHECK_IDF_ISOLATION permit 1
+    match ip address prefix-list PL_LoopbackV4
+    set community {{ constants.bgp.traffic_shift_community }}
+route-map CHECK_IDF_ISOLATION permit 2
+    match ipv6 address prefix-list PL_LoopbackV6
+    set community {{ constants.bgp.traffic_shift_community }}
+route-map CHECK_IDF_ISOLATION permit 3
+    match tag {{ constants.bgp.internal_community_match_tag }}
+    set community {{ constants.bgp.traffic_shift_community }}
+{# #}
+{%- if isolation_status == "isolated_withdraw_all" -%}
+    route-map CHECK_IDF_ISOLATION deny 4
+    route-map CHECK_IDF_ISOLATION permit 10
+        no set community no-export additive{# Added to clean up state, in case of transition from isolated_no_export (not expected) #}
+{%- elif isolation_status == "isolated_no_export" -%}
+    route-map CHECK_IDF_ISOLATION permit 10
+        set community no-export additive
+    no route-map CHECK_IDF_ISOLATION deny 4{# Added to clean up state, in case of transition from isolated_withdraw_all (not expected) #}
+{# #}
+{%- endif -%}
diff --git a/dockers/docker-fpm-frr/frr/bgpd/idf_isolate/idf_unisolate.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/idf_isolate/idf_unisolate.conf.j2
@@ -0,0 +1,6 @@
+no route-map CHECK_IDF_ISOLATION permit 1
+no route-map CHECK_IDF_ISOLATION permit 2
+no route-map CHECK_IDF_ISOLATION permit 3
+no route-map CHECK_IDF_ISOLATION deny 4
+route-map CHECK_IDF_ISOLATION permit 10
+    no set community no-export additive
diff --git a/rules/docker-fpm-frr.mk b/rules/docker-fpm-frr.mk
@@ -40,6 +40,7 @@ $(DOCKER_FPM_FRR)_BASE_IMAGE_FILES += TSA:/usr/bin/TSA
 $(DOCKER_FPM_FRR)_BASE_IMAGE_FILES += TSB:/usr/bin/TSB
 $(DOCKER_FPM_FRR)_BASE_IMAGE_FILES += TSC:/usr/bin/TSC
 $(DOCKER_FPM_FRR)_BASE_IMAGE_FILES += TS:/usr/bin/TS
+$(DOCKER_FPM_FRR)_BASE_IMAGE_FILES += idf_isolation:/usr/bin/idf_isolation
 
 SONIC_BULLSEYE_DOCKERS += $(DOCKER_FPM_FRR)
 SONIC_BULLSEYE_DBG_DOCKERS += $(DOCKER_FPM_FRR_DBG)
diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py
@@ -59,14 +59,15 @@ def update_pg(self, name, **kwargs):
         try:
             pg = self.peergroup_template.render(**kwargs)
             tsa_rm = self.device_global_cfgmgr.check_state_and_get_tsa_routemaps(pg)
+            idf_isolation_rm = self.device_global_cfgmgr.check_state_and_get_idf_isolation_routemaps()
         except jinja2.TemplateError as e:
             log_err("Can't render peer-group template: '%s': %s" % (name, str(e)))
             return False
 
         if kwargs['vrf'] == 'default':
-            cmd = ('router bgp %s\n' % kwargs['bgp_asn']) + pg + tsa_rm
+            cmd = ('router bgp %s\n' % kwargs['bgp_asn']) + pg + tsa_rm + idf_isolation_rm
         else:
-            cmd = ('router bgp %s vrf %s\n' % (kwargs['bgp_asn'], kwargs['vrf'])) + pg + tsa_rm
+            cmd = ('router bgp %s vrf %s\n' % (kwargs['bgp_asn'], kwargs['vrf'])) + pg + tsa_rm + idf_isolation_rm
         self.update_entity(cmd, "Peer-group for peer '%s'" % name)
         return True
 
@@ -108,6 +109,7 @@ def __init__(self, common_objs, db_name, table_name, peer_type, check_neig_meta)
             ("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/bgp_asn"),
             ("CONFIG_DB", swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME, "Loopback0"),
             ("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "tsa_enabled"),
+            ("CONFIG_DB", swsscommon.CFG_BGP_DEVICE_GLOBAL_TABLE_NAME, "idf_isolation_state"),
             ("LOCAL", "local_addresses", ""),
             ("LOCAL", "interfaces", ""),
         ]

diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py b/src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py
@@ -19,6 +19,8 @@ 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.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)
         super(DeviceGlobalCfgMgr, self).__init__(
             common_objs,
@@ -42,13 +44,31 @@ def set_handler(self, key, data):
             log_err("DeviceGlobalCfgMgr:: data is None")
             return False
 
+        tsa_status = "false"
+        idf_isolation_state = "unisolated"
+
+        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.cfg_mgr.commit()
-            self.cfg_mgr.update()
-            self.isolate_unisolate_device(data["tsa_enabled"])
             self.directory.put(self.db_name, self.table_name, "tsa_enabled", data["tsa_enabled"])
-            return True
-        return False
+            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")
@@ -113,3 +133,24 @@ def __extract_out_route_map_names(self, cmds):
                 route_map_names.add(result.group(1))
         return route_map_names
 
+    def downstream_isolate_unisolate(self, idf_isolation_state):
+        cmd = "\n"
+        if idf_isolation_state == "unisolated":
+            cmd += self.idf_unisolate_template.render(constants=self.constants)
+            log_notice("DeviceGlobalCfgMgr:: IDF un-isolated")
+        else:
+            cmd += self.idf_isolate_template.render(isolation_status=idf_isolation_state, constants=self.constants)
+            log_notice("DeviceGlobalCfgMgr:: IDF isolated, {} policy applied".format(idf_isolation_state))
+
+        self.cfg_mgr.push(cmd)
+        log_debug("DeviceGlobalCfgMgr::Done")
+
+    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":                
+                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
diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_idf_isolated_no_export.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_idf_isolated_no_export.conf
@@ -0,0 +1,34 @@
+!
+! template: bgpd/templates/general/peer-group.conf.j2
+!
+  neighbor PEER_V4 peer-group
+  neighbor PEER_V6 peer-group
+  address-family ipv4
+    neighbor PEER_V4 allowas-in 1
+    neighbor PEER_V4 soft-reconfiguration inbound
+    neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in
+    neighbor PEER_V4 route-map TO_BGP_PEER_V4 out
+  exit-address-family
+  address-family ipv6
+    neighbor PEER_V6 allowas-in 1
+    neighbor PEER_V6 soft-reconfiguration inbound
+    neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in
+    neighbor PEER_V6 route-map TO_BGP_PEER_V6 out
+  exit-address-family
+!
+! end of template: bgpd/templates/general/peer-group.conf.j2
+!
+
+route-map CHECK_IDF_ISOLATION permit 1
+    match ip address prefix-list PL_LoopbackV4
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 2
+    match ipv6 address prefix-list PL_LoopbackV6
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 3
+    match tag 1001
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 10
+        set community no-export additive
+    no route-map CHECK_IDF_ISOLATION deny 4
+
diff --git a/...onic-bgpcfgd/tests/data/general/peer-group.conf/result_all_idf_isolated_withdraw_all.conf b/...onic-bgpcfgd/tests/data/general/peer-group.conf/result_all_idf_isolated_withdraw_all.conf
@@ -0,0 +1,33 @@
+!
+! template: bgpd/templates/general/peer-group.conf.j2
+!
+  neighbor PEER_V4 peer-group
+  neighbor PEER_V6 peer-group
+  address-family ipv4
+    neighbor PEER_V4 allowas-in 1
+    neighbor PEER_V4 soft-reconfiguration inbound
+    neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in
+    neighbor PEER_V4 route-map TO_BGP_PEER_V4 out
+  exit-address-family
+  address-family ipv6
+    neighbor PEER_V6 allowas-in 1
+    neighbor PEER_V6 soft-reconfiguration inbound
+    neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in
+    neighbor PEER_V6 route-map TO_BGP_PEER_V6 out
+  exit-address-family
+!
+! end of template: bgpd/templates/general/peer-group.conf.j2
+!
+
+route-map CHECK_IDF_ISOLATION permit 1
+    match ip address prefix-list PL_LoopbackV4
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 2
+    match ipv6 address prefix-list PL_LoopbackV6
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 3
+    match tag 1001
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION deny 4
+    route-map CHECK_IDF_ISOLATION permit 10
+        no set community no-export additive
diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_idf_unisolated.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_all_idf_unisolated.conf
@@ -0,0 +1,27 @@
+!
+! template: bgpd/templates/general/peer-group.conf.j2
+!
+  neighbor PEER_V4 peer-group
+  neighbor PEER_V6 peer-group
+  address-family ipv4
+    neighbor PEER_V4 allowas-in 1
+    neighbor PEER_V4 soft-reconfiguration inbound
+    neighbor PEER_V4 route-map FROM_BGP_PEER_V4 in
+    neighbor PEER_V4 route-map TO_BGP_PEER_V4 out
+  exit-address-family
+  address-family ipv6
+    neighbor PEER_V6 allowas-in 1
+    neighbor PEER_V6 soft-reconfiguration inbound
+    neighbor PEER_V6 route-map FROM_BGP_PEER_V6 in
+    neighbor PEER_V6 route-map TO_BGP_PEER_V6 out
+  exit-address-family
+!
+! end of template: bgpd/templates/general/peer-group.conf.j2
+!
+
+no route-map CHECK_IDF_ISOLATION permit 1
+no route-map CHECK_IDF_ISOLATION permit 2
+no route-map CHECK_IDF_ISOLATION permit 3
+no route-map CHECK_IDF_ISOLATION deny 4
+route-map CHECK_IDF_ISOLATION permit 10
+    no set community no-export additive
diff --git a/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_idf_isolated.conf b/src/sonic-bgpcfgd/tests/data/general/peer-group.conf/result_idf_isolated.conf
@@ -0,0 +1,12 @@
+route-map CHECK_IDF_ISOLATION permit 1
+    match ip address prefix-list PL_LoopbackV4
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 2
+    match ipv6 address prefix-list PL_LoopbackV6
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 3
+    match tag 1001
+    set community 12345:12345
+route-map CHECK_IDF_ISOLATION permit 10
+        set community no-export additive
+    no route-map CHECK_IDF_ISOLATION deny 4
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_no_export.conf b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_no_export.conf
@@ -0,0 +1,12 @@
+route-map CHECK_IDF_ISOLATION permit 1
+    match ip address prefix-list PL_LoopbackV4
+    set community 12345:555
+route-map CHECK_IDF_ISOLATION permit 2
+    match ipv6 address prefix-list PL_LoopbackV6
+    set community 12345:555
+route-map CHECK_IDF_ISOLATION permit 3
+    match tag 1002
+    set community 12345:555
+route-map CHECK_IDF_ISOLATION permit 10
+        set community no-export additive
+    no route-map CHECK_IDF_ISOLATION deny 4
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_no_export.json b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_no_export.json
@@ -0,0 +1,9 @@
+{
+    "constants": {
+        "bgp": {
+            "traffic_shift_community": "12345:555",
+            "internal_community_match_tag": "1002"
+        }
+    },
+    "isolation_status": "isolated_no_export"
+}
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_withdraw_all.conf b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_withdraw_all.conf
@@ -0,0 +1,12 @@
+route-map CHECK_IDF_ISOLATION permit 1
+    match ip address prefix-list PL_LoopbackV4
+    set community 12345:555
+route-map CHECK_IDF_ISOLATION permit 2
+    match ipv6 address prefix-list PL_LoopbackV6
+    set community 12345:555
+route-map CHECK_IDF_ISOLATION permit 3
+    match tag 1002
+    set community 12345:555
+route-map CHECK_IDF_ISOLATION deny 4
+    route-map CHECK_IDF_ISOLATION permit 10
+        no set community no-export additive
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_withdraw_all.json b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_isolated_withdraw_all.json
@@ -0,0 +1,9 @@
+{
+    "constants": {
+        "bgp": {
+            "traffic_shift_community": "12345:555",
+            "internal_community_match_tag": "1002"
+        }
+    },
+    "isolation_status": "isolated_withdraw_all"
+}
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_unisolated.conf b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/idf_isolate/idf_unisolated.conf
@@ -0,0 +1,6 @@
+no route-map CHECK_IDF_ISOLATION permit 1
+no route-map CHECK_IDF_ISOLATION permit 2
+no route-map CHECK_IDF_ISOLATION permit 3
+no route-map CHECK_IDF_ISOLATION deny 4
+route-map CHECK_IDF_ISOLATION permit 10
+    no set community no-export additive