[bgp] Add explictly configure bgp router id for multi-asic (sonic-net…

…#18764) Why I did it HLD: sonic-net/SONiC#1643 Remove hard coupling between bgp router-id and IPv4 address of Loopback4096 Add support to explictly specify bgp router id for multi asic device Work item tracking Microsoft ADO (number only): 27801007 How I did it When bgp_router_id configured in DEVICE_METADATA, use it as bgp router-id and originator-id. Remove the hard dependency on loopback4096 ipv4 address when adding ibgp peer. Add UTs. Behavior of set bgp router-id To be clarified that when bgp router-id hasn't been explicitly set, bgp actions would totally like previous Loopback4096 IPv4 address exists Loopback4096 IPv4 address doesn't exist bgp_router_id configured Honor bgp_router_id Honor bgp_router_id bgp_router_id doesn't be configured Honor Loopback4096 IPv4 address FRR default router ID value is selected as the largest IP address of the device. When router zebra is not enabled bgpd can’t get interface information, so router-id is set to 0.0.0.0 Behavior of add bgp peer To be clarified that when bgp router-id hasn't been explicitly set, bgp actions would totally like previous Loopback4096 IPv4 address exists Loopback4096 IPv4 address doesn't exist bgp_router_id configured Add BGP peer Add BGP peer bgp_router_id doesn't be configured Add BGP peer Do not add iBGP peer How to verify it UT passed.
vadymhlushko-mlnx · May 16, 2024 · c48cc1e · c48cc1e
1 parent c5a2eba
commit c48cc1e
Show file tree

Hide file tree

Showing 17 changed files with 508 additions and 15 deletions.
diff --git a/dockers/docker-fpm-frr/frr/bgpd/bgpd.main.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/bgpd.main.conf.j2
@@ -9,6 +9,7 @@
 ! TSA configuration
 !
 {% set lo0_ipv4 = get_ipv4_loopback_address(LOOPBACK_INTERFACE, "Loopback0") | ip %}
+{% set lo4096_ipv4 = get_ipv4_loopback_address(LOOPBACK_INTERFACE, "Loopback4096") | ip %}
 {% if lo0_ipv4 is not none %}
 ip prefix-list PL_LoopbackV4 permit {{ lo0_ipv4 }}/32
 {% endif %}
@@ -89,7 +90,11 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
 !
 {# set router-id #}
 {% if DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' or DEVICE_METADATA['localhost']['switch_type'] == 'voq' or DEVICE_METADATA['localhost']['switch_type'] == 'chassis-packet' %}
-  bgp router-id {{ get_ipv4_loopback_address(LOOPBACK_INTERFACE, "Loopback4096") | ip }}
+{% if 'bgp_router_id' in DEVICE_METADATA['localhost'] %}
+  bgp router-id {{ DEVICE_METADATA["localhost"]["bgp_router_id"] }}
+{% elif lo4096_ipv4 is not none %}
+  bgp router-id {{ lo4096_ipv4 }}
+{% endif %}
 {% else %}
 {% if "localhost" in DEVICE_METADATA and "bgp_router_id" in DEVICE_METADATA["localhost"] %}
   bgp router-id {{ DEVICE_METADATA["localhost"]["bgp_router_id"] }}
@@ -102,8 +107,8 @@ router bgp {{ DEVICE_METADATA['localhost']['bgp_asn'] }}
 {% if lo0_ipv4 is not none %}
   network {{ lo0_ipv4 }}/32
 {% endif %}
-{% if ((multi_asic is defined and DEVICE_METADATA['localhost']['switch_type'] != 'chassis-packet') or (DEVICE_METADATA['localhost']['switch_type'] == 'voq')) %}
-  network {{ get_ipv4_loopback_address(LOOPBACK_INTERFACE, "Loopback4096") | ip }}/32 route-map HIDE_INTERNAL
+{% if lo4096_ipv4 is not none and ((multi_asic is defined and DEVICE_METADATA['localhost']['switch_type'] != 'chassis-packet') or (DEVICE_METADATA['localhost']['switch_type'] == 'voq')) %}
+  network {{ lo4096_ipv4 }}/32 route-map HIDE_INTERNAL
 {% endif %}
 !
 {% if get_ipv6_loopback_address(LOOPBACK_INTERFACE, "Loopback0") != 'None' %}

diff --git a/dockers/docker-fpm-frr/frr/bgpd/templates/internal/policies.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/templates/internal/policies.conf.j2
@@ -4,16 +4,25 @@
 !
 {% from "common/functions.conf.j2" import get_ipv4_loopback_address %}
 !
+{% set lo4096_ipv4 = get_ipv4_loopback_address(CONFIG_DB__LOOPBACK_INTERFACE, "Loopback4096") | ip %}
 {% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %}
 route-map FROM_BGP_INTERNAL_PEER_V4 permit 1
- set originator-id {{ get_ipv4_loopback_address(CONFIG_DB__LOOPBACK_INTERFACE, "Loopback4096") | ip }}
+{% if "localhost" in CONFIG_DB__DEVICE_METADATA and "bgp_router_id" in CONFIG_DB__DEVICE_METADATA["localhost"] %}
+ set originator-id {{ CONFIG_DB__DEVICE_METADATA["localhost"]["bgp_router_id"] }}
+{% elif lo4096_ipv4 is not none%}
+ set originator-id {{ lo4096_ipv4 }}
+{% endif %}
 !
 route-map FROM_BGP_INTERNAL_PEER_V6 permit 1
  set ipv6 next-hop prefer-global
  on-match next
 !
 route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
- set originator-id {{ get_ipv4_loopback_address(CONFIG_DB__LOOPBACK_INTERFACE, "Loopback4096") | ip }}
+{% if "localhost" in CONFIG_DB__DEVICE_METADATA and "bgp_router_id" in CONFIG_DB__DEVICE_METADATA["localhost"] %}
+ set originator-id {{ CONFIG_DB__DEVICE_METADATA["localhost"]["bgp_router_id"] }}
+ {% elif lo4096_ipv4 is not none%}
+ set originator-id {{ lo4096_ipv4 }}
+ {% endif %}
 {% 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

diff --git a/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py b/src/sonic-bgpcfgd/bgpcfgd/managers_bgp.py
@@ -128,9 +128,6 @@ def __init__(self, common_objs, db_name, table_name, peer_type, check_neig_meta)
         if self.check_deployment_id:
             deps.append(("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME, "localhost/deployment_id"))
 
-        if self.peer_type == 'internal':
-            deps.append(("CONFIG_DB", swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME, "Loopback4096"))
-
         super(BGPPeerMgrBase, self).__init__(
             common_objs,
             deps,
@@ -175,8 +172,9 @@ def add_peer(self, vrf, nbr, data):
         #
         if self.peer_type == 'internal':
             lo4096_ipv4 = self.get_lo_ipv4("Loopback4096|")
-            if lo4096_ipv4 is None:
-                log_warn("Loopback4096 ipv4 address is not presented yet")
+            if (lo4096_ipv4 is None and "bgp_router_id"
+                not in self.directory.get_slot("CONFIG_DB", swsscommon.CFG_DEVICE_METADATA_TABLE_NAME)["localhost"]):
+                log_warn("Loopback4096 ipv4 address is not presented yet and bgp_router_id not configured")
                 return False
 
         if "local_addr" not in data:

diff --git a/src/sonic-bgpcfgd/tests/data/internal/policies.conf/param_back_no_lo4096.json b/src/sonic-bgpcfgd/tests/data/internal/policies.conf/param_back_no_lo4096.json
@@ -0,0 +1,13 @@
+{
+    "CONFIG_DB__DEVICE_METADATA": {
+        "localhost": {
+            "sub_role": "BackEnd"
+        }
+    },
+    "constants": {
+        "bgp": {
+            "internal_community": "12345:556",
+	    "internal_community_match_tag": "101"
+        }
+    }
+}
diff --git a/src/sonic-bgpcfgd/tests/data/internal/policies.conf/param_back_router_id.json b/src/sonic-bgpcfgd/tests/data/internal/policies.conf/param_back_router_id.json
@@ -0,0 +1,17 @@
+{
+    "CONFIG_DB__DEVICE_METADATA": {
+        "localhost": {
+            "sub_role": "BackEnd",
+            "bgp_router_id": "8.8.8.8"
+        }
+    },
+    "CONFIG_DB__LOOPBACK_INTERFACE": {
+        "Loopback4096|10.10.10.10/32": {}
+    },
+    "constants": {
+        "bgp": {
+            "internal_community": "12345:556",
+	    "internal_community_match_tag": "101"
+        }
+    }
+}
diff --git a/src/sonic-bgpcfgd/tests/data/internal/policies.conf/param_back_router_id_no_lo4096.json b/src/sonic-bgpcfgd/tests/data/internal/policies.conf/param_back_router_id_no_lo4096.json
@@ -0,0 +1,14 @@
+{
+    "CONFIG_DB__DEVICE_METADATA": {
+        "localhost": {
+            "sub_role": "BackEnd",
+            "bgp_router_id": "8.8.8.8"
+        }
+    },
+    "constants": {
+        "bgp": {
+            "internal_community": "12345:556",
+	    "internal_community_match_tag": "101"
+        }
+    }
+}
diff --git a/src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_back_no_lo4096.conf b/src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_back_no_lo4096.conf
@@ -0,0 +1,21 @@
+!
+! template: bgpd/templates/internal/policies.conf.j2
+!
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 1
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 1
+ set ipv6 next-hop prefer-global
+ on-match next
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
+!
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 100
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 100
+!
+route-map TO_BGP_INTERNAL_PEER_V4 permit 100
+!
+route-map TO_BGP_INTERNAL_PEER_V6 permit 100
+!
+! end of template: bgpd/templates/internal/policies.conf.j2
+!
diff --git a/src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_back_router_id.conf b/src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_back_router_id.conf
@@ -0,0 +1,23 @@
+!
+! template: bgpd/templates/internal/policies.conf.j2
+!
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 1
+ set originator-id 8.8.8.8
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 1
+ set ipv6 next-hop prefer-global
+ on-match next
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
+ set originator-id 8.8.8.8
+!
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 100
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 100
+!
+route-map TO_BGP_INTERNAL_PEER_V4 permit 100
+!
+route-map TO_BGP_INTERNAL_PEER_V6 permit 100
+!
+! end of template: bgpd/templates/internal/policies.conf.j2
+!
diff --git a/src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_back_router_id_no_lo4096.conf b/src/sonic-bgpcfgd/tests/data/internal/policies.conf/result_back_router_id_no_lo4096.conf
@@ -0,0 +1,23 @@
+!
+! template: bgpd/templates/internal/policies.conf.j2
+!
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 1
+ set originator-id 8.8.8.8
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 1
+ set ipv6 next-hop prefer-global
+ on-match next
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 2
+ set originator-id 8.8.8.8
+!
+route-map FROM_BGP_INTERNAL_PEER_V4 permit 100
+!
+route-map FROM_BGP_INTERNAL_PEER_V6 permit 100
+!
+route-map TO_BGP_INTERNAL_PEER_V4 permit 100
+!
+route-map TO_BGP_INTERNAL_PEER_V6 permit 100
+!
+! end of template: bgpd/templates/internal/policies.conf.j2
+!
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis_ipv6_lo4096.conf b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis_ipv6_lo4096.conf
@@ -0,0 +1,72 @@
+!
+! template: bgpd/bgpd.main.conf.j2
+!
+! bgp multiple-instance
+!
+! BGP configuration
+!
+! TSA configuration
+!
+ip prefix-list PL_LoopbackV4 permit 55.55.55.55/32
+!
+ipv6 prefix-list PL_LoopbackV6 permit fc00::/64
+!
+ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 5 permit 10.10.10.0/24
+!
+ipv6 prefix-list LOCAL_VLAN_IPV6_PREFIX seq 10 permit fc01::/64
+!
+ip prefix-list V4_P2P_IP permit 0.0.0.0/0 ge 31 le 31
+!
+ipv6 prefix-list V6_P2P_IP permit ::/0 ge 126 le 126
+!
+route-map V4_CONNECTED_ROUTES permit 10
+  match ip address prefix-list V4_P2P_IP
+  call HIDE_INTERNAL
+!
+route-map V6_CONNECTED_ROUTES permit 10
+  match ipv6 address prefix-list V6_P2P_IP
+  call HIDE_INTERNAL
+!
+route-map HIDE_INTERNAL permit 10
+  set community no-export
+!
+!
+router bgp 55555
+!
+  bgp log-neighbor-changes
+  bgp suppress-fib-pending
+  no bgp default ipv4-unicast
+  no bgp ebgp-requires-policy
+!
+  bgp bestpath as-path multipath-relax
+!
+!
+!
+  network 55.55.55.55/32
+!
+  address-family ipv6
+    network fc00::1/64
+    network fc00::1/128 route-map HIDE_INTERNAL
+  exit-address-family
+!
+  network 10.10.10.1/24
+  address-family ipv6
+   network fc01::1/64
+  exit-address-family
+!
+  address-family ipv4
+    redistribute connected route-map V4_CONNECTED_ROUTES
+  exit-address-family
+  address-family ipv6
+    redistribute connected route-map V6_CONNECTED_ROUTES
+  exit-address-family
+!
+  address-family ipv4
+    maximum-paths 64
+  exit-address-family
+  address-family ipv6
+    maximum-paths 64
+  exit-address-family
+!
+! end of template: bgpd/bgpd.main.conf.j2
+!
diff --git a/src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis_ipv6_lo4096.json b/src/sonic-bgpcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis_ipv6_lo4096.json
@@ -0,0 +1,35 @@
+{
+    "DEVICE_METADATA": {
+        "localhost": {
+            "bgp_asn": "55555",
+            "sub_role": "FrontEnd",
+            "switch_type": "chassis-packet",
+            "type": "SpineRouter"
+        }
+    },
+    "LOOPBACK_INTERFACE": {
+        "Loopback0|55.55.55.55/32": {},
+        "Loopback0|fc00::1/128": {},
+        "Loopback4096|fc00::2/128": {}
+    },
+    "VLAN_INTERFACE": {
+        "Vlan10|10.10.10.1/24": {},
+        "Vlan10|fc01::1/64": {},
+        "Vlan20": {"vnet_name": "Vnet1"},
+        "Vlan20|20.20.20.1/24": {},
+        "Vlan20|fd01::1/64": {}
+    },
+    "constants": {
+        "bgp": {
+            "multipath_relax": {
+                "enabled": true
+            },
+            "graceful_restart": {
+                "enabled": true
+            },
+            "maximum_paths": {
+                "enabled": true
+            }
+        }
+    }
+}
diff --git a/...pcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis_ipv6_lo4096_router_id.conf b/...pcfgd/tests/data/sonic-cfggen/bgpd.main.conf.j2/packet_chassis_ipv6_lo4096_router_id.conf
@@ -0,0 +1,73 @@
+!
+! template: bgpd/bgpd.main.conf.j2
+!
+! bgp multiple-instance
+!
+! BGP configuration
+!
+! TSA configuration
+!
+ip prefix-list PL_LoopbackV4 permit 55.55.55.55/32
+!
+ipv6 prefix-list PL_LoopbackV6 permit fc00::/64
+!
+ip prefix-list LOCAL_VLAN_IPV4_PREFIX seq 5 permit 10.10.10.0/24
+!
+ipv6 prefix-list LOCAL_VLAN_IPV6_PREFIX seq 10 permit fc01::/64
+!
+ip prefix-list V4_P2P_IP permit 0.0.0.0/0 ge 31 le 31
+!
+ipv6 prefix-list V6_P2P_IP permit ::/0 ge 126 le 126
+!
+route-map V4_CONNECTED_ROUTES permit 10
+  match ip address prefix-list V4_P2P_IP
+  call HIDE_INTERNAL
+!
+route-map V6_CONNECTED_ROUTES permit 10
+  match ipv6 address prefix-list V6_P2P_IP
+  call HIDE_INTERNAL
+!
+route-map HIDE_INTERNAL permit 10
+  set community no-export
+!
+!
+router bgp 55555
+!
+  bgp log-neighbor-changes
+  bgp suppress-fib-pending
+  no bgp default ipv4-unicast
+  no bgp ebgp-requires-policy
+!
+  bgp bestpath as-path multipath-relax
+!
+!
+  bgp router-id 8.8.8.8
+!
+  network 55.55.55.55/32
+!
+  address-family ipv6
+    network fc00::1/64
+    network fc00::1/128 route-map HIDE_INTERNAL
+  exit-address-family
+!
+  network 10.10.10.1/24
+  address-family ipv6
+   network fc01::1/64
+  exit-address-family
+!
+  address-family ipv4
+    redistribute connected route-map V4_CONNECTED_ROUTES
+  exit-address-family
+  address-family ipv6
+    redistribute connected route-map V6_CONNECTED_ROUTES
+  exit-address-family
+!
+  address-family ipv4
+    maximum-paths 64
+  exit-address-family
+  address-family ipv6
+    maximum-paths 64
+  exit-address-family
+!
+! end of template: bgpd/bgpd.main.conf.j2
+!