From f5c1f1eea3cd453e86d9063945e6e302a544bdf1 Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Tue, 30 May 2023 22:28:59 +0000 Subject: [PATCH 1/6] Added change to add 'peerType' as element in NEIGH_STATE_TABLE. 'peerType' can be i-BGP vs e-BGP determined by local and remote AS number. This is useful to filter neighbors in SONiC chassis use case Signed-off-by: Abhishek Dosi --- src/sonic-bgpcfgd/bgpmon/bgpmon.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/sonic-bgpcfgd/bgpmon/bgpmon.py b/src/sonic-bgpcfgd/bgpmon/bgpmon.py index c63c63b54769..f60f1c5eb1b6 100755 --- a/src/sonic-bgpcfgd/bgpmon/bgpmon.py +++ b/src/sonic-bgpcfgd/bgpmon/bgpmon.py @@ -6,7 +6,8 @@ Initial creation of this daemon is to assist SNMP agent in obtaining the BGP related information for its MIB support. The MIB that this daemon is - assisting is for the CiscoBgp4MIB (Neighbor state only). If there are other + assisting is for the CiscoBgp4MIB (Neighbor state only). Also for chassis use-case + it identify if the given BGP neighbors as i-BGP vs e-BGP. If there are other BGP related items that needs to be updated in a periodic manner in the future, then more can be added into this process. @@ -69,7 +70,9 @@ def update_new_peer_states(self, peer_dict): peer_l = peer_dict["peers"].keys() self.new_peer_l.update(peer_l) for peer in peer_l: - self.new_peer_state[peer] = peer_dict["peers"][peer]["state"] + self.new_peer_state[peer] = (peer_dict["peers"][peer]["state"], + peer_dict["peers"][peer]["remoteAs"], + peer_dict["peers"][peer]["localAs"]) # Get a new snapshot of BGP neighbors and store them in the "new" location def get_all_neigh_states(self): @@ -124,17 +127,19 @@ def update_neigh_states(self): key = "NEIGH_STATE_TABLE|%s" % peer if peer in self.peer_l: # only update the entry if state changed - if self.peer_state[peer] != self.new_peer_state[peer]: + if self.peer_state[peer] != self.new_peer_state[peer][0]: # state changed. Update state DB for this entry - state = self.new_peer_state[peer] - data[key] = {'state':state} + state = self.new_peer_state[peer][0] + peerType = "i-BGP" if self.new_peer_state[peer][1] == self.new_peer_state[peer][2] else "e-BGP" + data[key] = {'state':state, 'peerType':peerType} self.peer_state[peer] = state # remove this neighbor from old set since it is accounted for self.peer_l.remove(peer) else: # New neighbor found case. Add to dictionary and state DB - state = self.new_peer_state[peer] - data[key] = {'state':state} + state = self.new_peer_state[peer][0] + peerType = "i-BGP" if self.new_peer_state[peer][1] == self.new_peer_state[peer][2] else "e-BGP" + data[key] = {'state':state, 'peerType':peerType} self.peer_state[peer] = state if len(data) > PIPE_BATCH_MAX_COUNT: self.flush_pipe(data) From 3cc5531aea47c4494c1394af9dbbd1722ebf05a9 Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Tue, 6 Jun 2023 00:59:56 +0000 Subject: [PATCH 2/6] Workaround for FRR issue where sometime route remians in inactive state because of 2 level of recursive next-hop tracking not working as expected. Signed-off-by: Abhishek Dosi --- .../frr/bgpd/templates/internal/instance.conf.j2 | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dockers/docker-fpm-frr/frr/bgpd/templates/internal/instance.conf.j2 b/dockers/docker-fpm-frr/frr/bgpd/templates/internal/instance.conf.j2 index 44c70f49bd67..06eabb5fc7cc 100644 --- a/dockers/docker-fpm-frr/frr/bgpd/templates/internal/instance.conf.j2 +++ b/dockers/docker-fpm-frr/frr/bgpd/templates/internal/instance.conf.j2 @@ -10,18 +10,16 @@ address-family ipv4 neighbor {{ neighbor_addr }} peer-group INTERNAL_PEER_V4 ! -{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %} +{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' or CONFIG_DB__DEVICE_METADATA['localhost']['switch_type'] == 'chassis-packet' %} neighbor {{ neighbor_addr }} next-hop-self force - neighbor {{ neighbor_addr }} route-map FROM_BGP_INTERNAL_PEER_V4 in {% endif %} ! {% elif neighbor_addr | ipv6 %} address-family ipv6 neighbor {{ neighbor_addr }} peer-group INTERNAL_PEER_V6 ! -{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' %} +{% if CONFIG_DB__DEVICE_METADATA['localhost']['sub_role'] == 'BackEnd' or CONFIG_DB__DEVICE_METADATA['localhost']['switch_type'] == 'chassis-packet' %} neighbor {{ neighbor_addr }} next-hop-self force - neighbor {{ neighbor_addr }} route-map FROM_BGP_INTERNAL_PEER_V6 in {% endif %} {% endif %} ! From 65d3a791ef332b00224075467216a0ba41610b62 Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Tue, 6 Jun 2023 01:24:42 +0000 Subject: [PATCH 3/6] Updated UT. Signed-off-by: Abhishek Dosi --- .../instance.conf/chassis_packet_v4.conf | 15 ++++++++++++ .../instance.conf/chassis_packet_v4.json | 23 +++++++++++++++++++ .../instance.conf/chassis_packet_v6.conf | 15 ++++++++++++ .../instance.conf/chassis_packet_v6.json | 23 +++++++++++++++++++ .../instance.conf/result_back_v4.conf | 1 - .../instance.conf/result_back_v6.conf | 1 - 6 files changed, 76 insertions(+), 2 deletions(-) create mode 100644 src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.conf create mode 100644 src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.json create mode 100644 src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.conf create mode 100644 src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.json diff --git a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.conf b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.conf new file mode 100644 index 000000000000..f848ebd457d4 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.conf @@ -0,0 +1,15 @@ +! +! template: bgpd/templates/internal/instance.conf.j2 +! + neighbor 10.10.10.10 remote-as 555 + neighbor 10.10.10.10 description remote_peer + neighbor 10.10.10.10 timers 3 10 + neighbor 10.10.10.10 timers connect 10 + address-family ipv4 + neighbor 10.10.10.10 peer-group INTERNAL_PEER_V4 + neighbor 10.10.10.10 next-hop-self force + neighbor 10.10.10.10 activate + exit-address-family +! +! end of template: bgpd/templates/internal/instance.conf.j2 +! diff --git a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.json b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.json new file mode 100644 index 000000000000..41f4aab4f8af --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v4.json @@ -0,0 +1,23 @@ +{ + "CONFIG_DB__DEVICE_METADATA": { + "localhost": { + "sub_role": "FrontEnd", + "type": "SpineRouter", + "switch_type": "chassis-packet" + } + }, + "neighbor_addr": "10.10.10.10", + "bgp_session": { + "asn": "555", + "name": "remote_peer", + "keepalive": "5", + "holdtime": "30", + "admin_status": "down", + "ASIC": "something" + }, + "constants": { + "deployment_id_asn_map": { + "5": "51111" + } + } +} diff --git a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.conf b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.conf new file mode 100644 index 000000000000..09c021b9195b --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.conf @@ -0,0 +1,15 @@ +! +! template: bgpd/templates/internal/instance.conf.j2 +! + neighbor fc::10 remote-as 555 + neighbor fc::10 description remote_peer + neighbor fc::10 timers 3 10 + neighbor fc::10 timers connect 10 + address-family ipv6 + neighbor fc::10 peer-group INTERNAL_PEER_V6 + neighbor fc::10 next-hop-self force + neighbor fc::10 activate + exit-address-family +! +! end of template: bgpd/templates/internal/instance.conf.j2 +! diff --git a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.json b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.json new file mode 100644 index 000000000000..eb25b37c3206 --- /dev/null +++ b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/chassis_packet_v6.json @@ -0,0 +1,23 @@ +{ + "CONFIG_DB__DEVICE_METADATA": { + "localhost": { + "sub_role": "FrontEnd", + "type": "SpineRouter", + "switch_type": "chassis-packet" + } + }, + "neighbor_addr": "fc::10", + "bgp_session": { + "asn": "555", + "name": "remote_peer", + "keepalive": "5", + "holdtime": "30", + "admin_status": "down", + "ASIC": "something" + }, + "constants": { + "deployment_id_asn_map": { + "5": "51111" + } + } +} diff --git a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v4.conf b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v4.conf index 43fdc12921bc..90e0e1315831 100644 --- a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v4.conf +++ b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v4.conf @@ -8,7 +8,6 @@ address-family ipv4 neighbor 10.10.10.10 peer-group INTERNAL_PEER_V4 neighbor 10.10.10.10 next-hop-self force - neighbor 10.10.10.10 route-map FROM_BGP_INTERNAL_PEER_V4 in neighbor 10.10.10.10 route-reflector-client neighbor 10.10.10.10 activate exit-address-family diff --git a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v6.conf b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v6.conf index 811f7b808478..75f8fd0d136f 100644 --- a/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v6.conf +++ b/src/sonic-bgpcfgd/tests/data/internal/instance.conf/result_back_v6.conf @@ -8,7 +8,6 @@ address-family ipv6 neighbor fc::10 peer-group INTERNAL_PEER_V6 neighbor fc::10 next-hop-self force - neighbor fc::10 route-map FROM_BGP_INTERNAL_PEER_V6 in neighbor fc::10 route-reflector-client neighbor fc::10 activate exit-address-family From 0d8cbf14da7ac30939111176dee1b66608ea195b Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Wed, 7 Jun 2023 22:06:08 +0000 Subject: [PATCH 4/6] Enable BFD for Sattic Route for chassis-packet. Signed-off-by: Abhishek Dosi --- src/sonic-config-engine/minigraph.py | 3 +++ src/sonic-config-engine/tests/test_cfggen.py | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 8dc82b40d73e..21df24f22564 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -1944,6 +1944,9 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw port['mux_cable'] = "true" if static_routes: + # Enable static Route BFD by default for static route in chassis-packet + if switch_type == "chassis-packet": + static_routes['bfd'] = "true" results['STATIC_ROUTE'] = static_routes for nghbr in list(neighbors.keys()): diff --git a/src/sonic-config-engine/tests/test_cfggen.py b/src/sonic-config-engine/tests/test_cfggen.py index d159508e84c7..83edc11e4639 100644 --- a/src/sonic-config-engine/tests/test_cfggen.py +++ b/src/sonic-config-engine/tests/test_cfggen.py @@ -1011,14 +1011,14 @@ def test_minigraph_bgp_packet_chassis_static_route(self): output = self.run_script(argument) self.assertEqual( utils.to_dict(output.strip()), - utils.to_dict("{'8.0.0.1/32': {'nexthop': '192.168.1.2,192.168.2.2', 'ifname': 'PortChannel40,PortChannel50', 'advertise':'false'}}") + utils.to_dict("{'8.0.0.1/32': {'nexthop': '192.168.1.2,192.168.2.2', 'ifname': 'PortChannel40,PortChannel50', 'advertise':'false', 'bfd':'true'}}") ) argument = ['-m', self.packet_chassis_graph, '-p', self.packet_chassis_port_ini, '-n', "asic1", '-v', "STATIC_ROUTE"] output = self.run_script(argument) self.assertEqual( utils.to_dict(output.strip()), - utils.to_dict("{'8.0.0.1/32': {'nexthop': '192.168.1.2,192.168.2.2', 'ifname': 'PortChannel40,PortChannel50', 'advertise':'false'}}") + utils.to_dict("{'8.0.0.1/32': {'nexthop': '192.168.1.2,192.168.2.2', 'ifname': 'PortChannel40,PortChannel50', 'advertise':'false', 'bfd':'true'}}") ) def test_minigraph_bgp_packet_chassis_vlan_subintf(self): From 2a17bf427d9358de5732721c2bb740cca23e7191 Mon Sep 17 00:00:00 2001 From: abdosi <58047199+abdosi@users.noreply.github.com> Date: Sat, 1 Jul 2023 15:25:12 -0700 Subject: [PATCH 5/6] Update minigraph.py --- src/sonic-config-engine/minigraph.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 21df24f22564..5590492aa1a8 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -1946,7 +1946,8 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None, hw if static_routes: # Enable static Route BFD by default for static route in chassis-packet if switch_type == "chassis-packet": - static_routes['bfd'] = "true" + for pfx, data in static_routes.items(): + data.update({"bfd":"true"}) results['STATIC_ROUTE'] = static_routes for nghbr in list(neighbors.keys()): From 068ed09840c7d9e54d15a226392ba53bbfcc2112 Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Wed, 5 Jul 2023 21:47:50 +0000 Subject: [PATCH 6/6] Added yang model change Signed-off-by: Abhishek Dosi --- src/sonic-yang-models/yang-models/sonic-static-route.yang | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/sonic-yang-models/yang-models/sonic-static-route.yang b/src/sonic-yang-models/yang-models/sonic-static-route.yang index 019825b78881..0134d5fe6cd5 100644 --- a/src/sonic-yang-models/yang-models/sonic-static-route.yang +++ b/src/sonic-yang-models/yang-models/sonic-static-route.yang @@ -54,6 +54,12 @@ module sonic-static-route { } default "false"; } + leaf bfd { + type string { + pattern "((true|false),)*(true|false)"; + } + default "false"; + } } list STATIC_ROUTE_LIST { key "vrf_name prefix";