[chassis] Added support of isolating given LC in Chassis with TSA mod…

…e (#16732)

What I did:
Added support when TSA is done on Line Card make sure it's completely
isolated from all e-BGP peer devices from this LC or remote LC

Why I did:
Currently when TSA is executed on LC routes are withdrawn from it's connected e-BGP peers only. e-BGP peers on remote LC can/will (via i-BGP) still have route pointing/attracting traffic towards this isolated LC.

How I did:

When TSA is applied on LC all the routes that are advertised via i-BGP are set with community tag of no-export so that when remote LC received these routes it does not send over to it's connected e-BGP peers.

Also once we receive the route with no-export  over iBGP match on it and and set the local preference of that route to lower value (80) so that we remove that route from the forwarding database. Below scenario explains why we do this:

- LC1 advertise R1 to LC3
- LC2 advertise R1 to LC3
- On LC3 we have multi-path/ECMP over both LC1 and LC2
- On LC3 R1 received from LC1 is consider best route over R1 over received from LC2 and is send to LC3 e-BGP peers
- Now we do TSA on LC2
- LC3 will receive R1 from LC2 with community no-export and from LC1 same as earlier (no change)
- LC3 will still get traffic for R1 since it is still advertised to e-BGP peers (since R1 from LC1 is best route)
- LC3 will forward to both LC1 and LC2 (ecmp) and this causes issue as LC2 is in TSA mode and should not receive traffic

To fix above scenario we change the preference to lower value of R1 received from LC2 so that it is removed from Multi-path/ECMP group.

How I verfiy:

UT has been added to make sure Template generation is correct
Manual Verification of the functionality
sonic-mgmt test case will be updated accordingly.
Please note this PR is on top of this :#16714 which needs to be merged first.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>

dockers/docker-fpm-frr/TS

@@ -4,7 +4,12 @@ switch_type=`sonic-cfggen -d -v "DEVICE_METADATA['localhost']['switch_type']"`
 # Check whether the routemap is for internal BGP sessions.
 function is_internal_route_map()
 {
-    [[ "$1" =~ .*"_INTERNAL_".* || "$1" =~ .*"VOQ_".* ]]
+    if [[ "$1" =~ .*"_INTERNAL_".* || "$1" =~ .*"VOQ_".* ]]
+    then
+         return 1
+    else
+         return 0
+    fi
 }
 
 function check_not_installed()
@@ -13,7 +18,10 @@ function check_not_installed()
   config=$(vtysh -c "show run")
   for route_map_name in $(echo "$config" | sed -ne 's/  neighbor \S* route-map \(\S*\) out/\1/p' | egrep 'V4|V6' | uniq);
   do
-    is_internal_route_map $route_map_name && continue
+    is_internal_route_map $route_map_name
+    if [[ $? -eq 1 ]]; then
+        continue
+    fi
     echo "$config" | egrep -q "^route-map $route_map_name permit 20$"
     c=$((c+$?))
     echo "$config" | egrep -q "^route-map $route_map_name permit 30$"
@@ -31,7 +39,10 @@ function check_installed()
   config=$(vtysh -c "show run")
   for route_map_name in $(echo "$config" | sed -ne 's/  neighbor \S* route-map \(\S*\) out/\1/p' | egrep 'V4|V6' | uniq);
   do
-    is_internal_route_map $route_map_name && continue
+    is_internal_route_map $route_map_name
+    if [[ $? -eq 1 ]]; then
+        continue
+    fi
     echo "$config" | egrep -q "^route-map $route_map_name permit 20$"
     c=$((c+$?))
     e=$((e+1))
@@ -51,7 +62,10 @@ function find_num_routemap()
   config=$(vtysh -c "show run")
   for route_map_name in $(echo "$config" | sed -ne 's/  neighbor \S* route-map \(\S*\) out/\1/p' | egrep 'V4|V6' | uniq);
   do
-    is_internal_route_map $route_map_name && continue
+    is_internal_route_map $route_map_name
+    if [[ $? -eq 1 ]]; then
+        continue
+    fi
     c=$((c+1))
   done
   return $c

dockers/docker-fpm-frr/TSA

@@ -16,7 +16,8 @@ then
   TSA_FILE=$(mktemp)
   for route_map_name in $(echo "$config" | sed -ne 's/  neighbor \S* route-map \(\S*\) out/\1/p' | uniq);
   do
-    is_internal_route_map $route_map_name && continue
+    is_internal_route_map $route_map_name
+    internal_route_map=$?
     case "$route_map_name" in
       *V4*)
         ip_version=V4
@@ -30,7 +31,7 @@ then
         continue
       ;;
     esac
-    sonic-cfggen -d -a "{\"route_map_name\":\"$route_map_name\", \"ip_version\": \"$ip_version\", \"ip_protocol\": \"$ip_protocol\"}" -y /etc/sonic/constants.yml -t /usr/share/sonic/templates/bgpd/tsa/bgpd.tsa.isolate.conf.j2 > "$TSA_FILE"
+    sonic-cfggen -d -a "{\"route_map_name\":\"$route_map_name\", \"ip_version\": \"$ip_version\", \"ip_protocol\": \"$ip_protocol\", \"internal_route_map\": \"$internal_route_map\"}" -y /etc/sonic/constants.yml -t /usr/share/sonic/templates/bgpd/tsa/bgpd.tsa.isolate.conf.j2 > "$TSA_FILE"
     vtysh -f "$TSA_FILE"
     rm -f "$TSA_FILE"
   done

dockers/docker-fpm-frr/TSB

@@ -16,7 +16,8 @@ then
   TSB_FILE=$(mktemp)
   for route_map_name in $(echo "$config" | sed -ne 's/  neighbor \S* route-map \(\S*\) out/\1/p' | uniq);
   do
-    is_internal_route_map $route_map_name && continue
+    is_internal_route_map $route_map_name
+    internal_route_map=$?
     case "$route_map_name" in
       *V4*)
       ;;
@@ -26,7 +27,7 @@ then
         continue
       ;;
     esac
-    sonic-cfggen -d -a "{\"route_map_name\":\"$route_map_name\"}" -t /usr/share/sonic/templates/bgpd/tsa/bgpd.tsa.unisolate.conf.j2 > "$TSB_FILE"
+    sonic-cfggen -d -a "{\"route_map_name\":\"$route_map_name\", \"internal_route_map\": \"$internal_route_map\"}" -t /usr/share/sonic/templates/bgpd/tsa/bgpd.tsa.unisolate.conf.j2 > "$TSB_FILE"
     vtysh -f "$TSB_FILE"
     rm -f "$TSB_FILE"
   done

dockers/docker-fpm-frr/frr/bgpd/templates/internal/policies.conf.j2

@@ -16,12 +16,17 @@ route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
  set originator-id {{ get_ipv4_loopback_address(CONFIG_DB__LOOPBACK_INTERFACE, "Loopback4096") | ip }}
 {% elif CONFIG_DB__DEVICE_METADATA['localhost']['switch_type'] == 'chassis-packet' %}
 bgp community-list standard DEVICE_INTERNAL_COMMUNITY permit {{ constants.bgp.internal_community }}
+bgp community-list standard NO_EXPORT permit no-export
 !
 route-map FROM_BGP_INTERNAL_PEER_V4 permit 1
   match community DEVICE_INTERNAL_COMMUNITY
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag {{ constants.bgp.internal_community_match_tag }}
 !
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 2
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map FROM_BGP_INTERNAL_PEER_V6 permit 1
  set ipv6 next-hop prefer-global
  on-match next
@@ -31,6 +36,10 @@ route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag {{ constants.bgp.internal_community_match_tag }}
 !
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 3
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map TO_BGP_INTERNAL_PEER_V4 permit 1
   match ip address prefix-list PL_LoopbackV4
   set community {{ constants.bgp.internal_community }}

dockers/docker-fpm-frr/frr/bgpd/templates/voq_chassis/policies.conf.j2

@@ -2,12 +2,17 @@
 ! template: bgpd/templates/voq_chassis/policies.conf.j2
 !
 bgp community-list standard DEVICE_INTERNAL_COMMUNITY permit {{ constants.bgp.internal_community }}
+bgp community-list standard NO_EXPORT permit no-export
 !
 route-map FROM_VOQ_CHASSIS_V4_PEER permit 1
   match community DEVICE_INTERNAL_COMMUNITY
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag {{ constants.bgp.internal_community_match_tag }}
 !
+route-map FROM_VOQ_CHASSIS_V4_PEER permit 2
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map FROM_VOQ_CHASSIS_V4_PEER permit 100
 !
 route-map TO_VOQ_CHASSIS_V4_PEER permit 1
@@ -26,6 +31,10 @@ route-map FROM_VOQ_CHASSIS_V6_PEER permit 2
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag {{ constants.bgp.internal_community_match_tag }}
 !
+route-map FROM_VOQ_CHASSIS_V6_PEER permit 3
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map FROM_VOQ_CHASSIS_V6_PEER permit 100
 !
 route-map TO_VOQ_CHASSIS_V6_PEER permit 1

dockers/docker-fpm-frr/frr/bgpd/tsa/bgpd.tsa.isolate.conf.j2

@@ -1,8 +1,15 @@
+{%- if internal_route_map == '1' -%}
+route-map {{ route_map_name }} permit 20
+  set community no-export additive
+{# #}
+{%- else -%}
 route-map {{ route_map_name }} permit 20
   match {{ ip_protocol }} address prefix-list PL_Loopback{{ ip_version }}
   set community {{ constants.bgp.traffic_shift_community }}
 route-map {{ route_map_name }} permit 30
   match tag {{ constants.bgp.internal_community_match_tag }}
   set community {{ constants.bgp.traffic_shift_community }}
 route-map {{ route_map_name }} deny 40
+{# #}
+{%- endif -%}
 !

dockers/docker-fpm-frr/frr/bgpd/tsa/bgpd.tsa.unisolate.conf.j2

@@ -1,4 +1,10 @@
+{%- if internal_route_map == '1' -%}
+no route-map {{ route_map_name }} permit 20
+{# #}
+{%- else -%}
 no route-map {{ route_map_name }} permit 20
 no route-map {{ route_map_name }} permit 30
 no route-map {{ route_map_name }} deny 40
+{# #}
+{%- endif -%}
 !

src/sonic-bgpcfgd/bgpcfgd/managers_device_global.py

@@ -91,14 +91,16 @@ def __generate_routemaps_from_template(self, route_map_names, template):
             # For packet-based chassis, the bgp session between the linecards are also considered internal sessions 
             # While isolating a single linecard, these sessions should not be skipped
             if "_INTERNAL_" in rm or "VOQ_" in rm:
-                continue            
+                is_internal="1"
+            else:
+                is_internal="0"
             if "V4" in rm:
                 ipv="V4" ; ipp="ip"
             elif "V6" in rm:
                 ipv="V6" ; ipp="ipv6"                
             else:
-                continue                        
-            cmd += template.render(route_map_name=rm,ip_version=ipv,ip_protocol=ipp, constants=self.constants)
+                continue
+            cmd += template.render(route_map_name=rm,ip_version=ipv,ip_protocol=ipp,internal_route_map=is_internal, constants=self.constants)
             cmd += "\n"
         return cmd
 

src/sonic-bgpcfgd/tests/data/internal/peer-group.conf/result_chassis_packet_isolate.conf

@@ -0,0 +1,35 @@
+!
+! template: bgpd/templates/internal/peer-group.conf.j2
+!
+  neighbor INTERNAL_PEER_V4 peer-group
+  neighbor INTERNAL_PEER_V6 peer-group
+  neighbor INTERNAL_PEER_V4 update-source Loopback4096 
+  address-family ipv4
+    neighbor INTERNAL_PEER_V4 soft-reconfiguration inbound
+    neighbor INTERNAL_PEER_V4 allowas-in 1
+    neighbor INTERNAL_PEER_V4 route-map FROM_BGP_INTERNAL_PEER_V4 in
+    neighbor INTERNAL_PEER_V4 route-map TO_BGP_INTERNAL_PEER_V4 out
+    neighbor INTERNAL_PEER_V4 send-community
+    neighbor INTERNAL_PEER_V4 ttl-security hops 1
+  exit-address-family
+  neighbor INTERNAL_PEER_V6 update-source Loopback4096 
+  address-family ipv6
+    neighbor INTERNAL_PEER_V6 soft-reconfiguration inbound
+    neighbor INTERNAL_PEER_V6 allowas-in 1
+    neighbor INTERNAL_PEER_V6 route-map FROM_BGP_INTERNAL_PEER_V6 in
+    neighbor INTERNAL_PEER_V6 route-map TO_BGP_INTERNAL_PEER_V6 out
+    neighbor INTERNAL_PEER_V6 send-community
+    neighbor INTERNAL_PEER_V6 ttl-security hops 1
+  exit-address-family
+!
+! end of template: bgpd/templates/internal/peer-group.conf.j2
+!
+
+
+route-map TO_BGP_INTERNAL_PEER_V4 permit 20
+  set community no-export additive
+!
+route-map TO_BGP_INTERNAL_PEER_V6 permit 20
+  set community no-export additive
+!
+

src/sonic-bgpcfgd/tests/data/internal/peer-group.conf/result_chassis_packet_unisolate.conf

@@ -0,0 +1,33 @@
+!
+! template: bgpd/templates/internal/peer-group.conf.j2
+!
+  neighbor INTERNAL_PEER_V4 peer-group
+  neighbor INTERNAL_PEER_V6 peer-group
+  neighbor INTERNAL_PEER_V4 update-source Loopback4096 
+  address-family ipv4
+    neighbor INTERNAL_PEER_V4 soft-reconfiguration inbound
+    neighbor INTERNAL_PEER_V4 allowas-in 1
+    neighbor INTERNAL_PEER_V4 route-map FROM_BGP_INTERNAL_PEER_V4 in
+    neighbor INTERNAL_PEER_V4 route-map TO_BGP_INTERNAL_PEER_V4 out
+    neighbor INTERNAL_PEER_V4 send-community
+    neighbor INTERNAL_PEER_V4 ttl-security hops 1
+  exit-address-family
+  neighbor INTERNAL_PEER_V6 update-source Loopback4096 
+  address-family ipv6
+    neighbor INTERNAL_PEER_V6 soft-reconfiguration inbound
+    neighbor INTERNAL_PEER_V6 allowas-in 1
+    neighbor INTERNAL_PEER_V6 route-map FROM_BGP_INTERNAL_PEER_V6 in
+    neighbor INTERNAL_PEER_V6 route-map TO_BGP_INTERNAL_PEER_V6 out
+    neighbor INTERNAL_PEER_V6 send-community
+    neighbor INTERNAL_PEER_V6 ttl-security hops 1
+  exit-address-family
+!
+! end of template: bgpd/templates/internal/peer-group.conf.j2
+!
+
+
+no route-map TO_BGP_INTERNAL_PEER_V4 permit 20
+!
+no route-map TO_BGP_INTERNAL_PEER_V6 permit 20
+!
+

src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_chasiss_packet.conf

@@ -2,12 +2,17 @@
 ! template: bgpd/templates/internal/policies.conf.j2
 !
 bgp community-list standard DEVICE_INTERNAL_COMMUNITY permit 12345:556
+bgp community-list standard NO_EXPORT permit no-export
 !
 route-map FROM_BGP_INTERNAL_PEER_V4 permit 1
   match community DEVICE_INTERNAL_COMMUNITY
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag 101
 !
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 2
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map FROM_BGP_INTERNAL_PEER_V6 permit 1
  set ipv6 next-hop prefer-global
  on-match next
@@ -17,6 +22,10 @@ route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag 101
 !
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 3
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map TO_BGP_INTERNAL_PEER_V4 permit 1
   match ip address prefix-list PL_LoopbackV4
   set community 12345:556

src/sonic-bgpcfgd/tests/data/voq_chassis/policies.conf/result_base.conf

@@ -2,12 +2,17 @@
 ! template: bgpd/templates/voq_chassis/policies.conf.j2
 !
 bgp community-list standard DEVICE_INTERNAL_COMMUNITY permit 12345:556
+bgp community-list standard NO_EXPORT permit no-export
 !
 route-map FROM_VOQ_CHASSIS_V4_PEER permit 1
   match community DEVICE_INTERNAL_COMMUNITY
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag 101
 !
+route-map FROM_VOQ_CHASSIS_V4_PEER permit 2
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map FROM_VOQ_CHASSIS_V4_PEER permit 100
 !
 route-map TO_VOQ_CHASSIS_V4_PEER permit 1
@@ -25,6 +30,10 @@ route-map FROM_VOQ_CHASSIS_V6_PEER permit 2
   set comm-list DEVICE_INTERNAL_COMMUNITY delete
   set tag 101
 !
+route-map FROM_VOQ_CHASSIS_V6_PEER permit 3
+  match community NO_EXPORT
+  set local-preference 80
+!
 route-map FROM_VOQ_CHASSIS_V6_PEER permit 100
 !
 route-map TO_VOQ_CHASSIS_V6_PEER permit 1

src/sonic-bgpcfgd/tests/test_device_global.py

@@ -11,14 +11,15 @@
 
 TEMPLATE_PATH = os.path.abspath('../../dockers/docker-fpm-frr/frr')
 BASE_PATH = os.path.abspath('../sonic-bgpcfgd/tests/data/general/peer-group.conf/')
+INTERNAL_BASE_PATH = os.path.abspath('../sonic-bgpcfgd/tests/data/internal/peer-group.conf/')
 global_constants = {
     "bgp":  {
         "traffic_shift_community" :"12345:12345",
         "internal_community_match_tag" : "1001"
     }
 }
 
-def constructor():
+def constructor(check_internal=False):
     cfg_mgr = MagicMock()
     def get_text():
         text = []
@@ -29,7 +30,10 @@ def get_text():
         text += ["     "]
         return text
     def update():
-        cfg_mgr.changes = get_string_from_file("/result_all.conf")
+        if check_internal:
+            cfg_mgr.changes = get_string_from_file("/result_chasiss_packet.conf", INTERNAL_BASE_PATH)
+        else:
+            cfg_mgr.changes = get_string_from_file("/result_all.conf")
     def push(cfg):
         cfg_mgr.changes += cfg + "\n"
     def get_config():
@@ -60,13 +64,31 @@ def test_isolate_device(mocked_log_info):
     assert m.cfg_mgr.get_config() == get_string_from_file("/result_all_isolate.conf")
 
 @patch('bgpcfgd.managers_device_global.log_debug')
-def test_unisolate_device(mocked_log_info): 
+def test_isolate_device_internal_session(mocked_log_info):
+    m = constructor(check_internal=True)
+    res = m.set_handler("STATE", {"tsa_enabled": "true"})
+    assert res, "Expect True return value for set_handler"
+    mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done")
+    assert m.cfg_mgr.get_config() == get_string_from_file("/result_chassis_packet_isolate.conf", INTERNAL_BASE_PATH)
+
+
+@patch('bgpcfgd.managers_device_global.log_debug')
+def test_unisolate_device(mocked_log_info):
     m = constructor()
     res = m.set_handler("STATE", {"tsa_enabled": "false"})
     assert res, "Expect True return value for set_handler"
     mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done")
     assert m.cfg_mgr.get_config() == get_string_from_file("/result_all_unisolate.conf")
 
+@patch('bgpcfgd.managers_device_global.log_debug')
+def test_unisolate_device_internal_session(mocked_log_info):
+    m = constructor(check_internal=True)
+    res = m.set_handler("STATE", {"tsa_enabled": "false"})
+    assert res, "Expect True return value for set_handler"
+    mocked_log_info.assert_called_with("DeviceGlobalCfgMgr::Done")
+    assert m.cfg_mgr.get_config() == get_string_from_file("/result_chassis_packet_unisolate.conf", INTERNAL_BASE_PATH)
+
+
 def test_check_state_and_get_tsa_routemaps():
     m = constructor()
     m.set_handler("STATE", {"tsa_enabled": "true"})
@@ -93,8 +115,8 @@ def test_get_tsb_routemaps():
     expected_res = get_string_from_file("/result_unisolate.conf")
     assert res == expected_res
 
-def get_string_from_file(filename):
-    fp = open(BASE_PATH + filename, "r")
+def get_string_from_file(filename, base_path=BASE_PATH):
+    fp = open(base_path + filename, "r")
     cfg = fp.read()
     fp.close()