From b2d35e08f2c35c3f0151bb435f4bc68a5827a40c Mon Sep 17 00:00:00 2001 From: zegan Date: Fri, 13 Mar 2020 22:44:34 +0800 Subject: [PATCH 01/13] Fixbug: EVPN issue in FRR template Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 79 ++++++++++++++++++++++++ dockers/docker-fpm-frr/bgpd.peer.conf.j2 | 3 +- 2 files changed, 80 insertions(+), 2 deletions(-) mode change 100644 => 100755 dockers/docker-fpm-frr/bgpd.peer.conf.j2 diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 4348d22bcb8f..ee53b10323f3 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -272,6 +272,7 @@ class BGPPeerMgr(Manager): [ ("meta", "localhost/bgp_asn"), ("neigmeta", ""), + ("interface", "") ], "CONFIG_DB", swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME @@ -290,6 +291,24 @@ class BGPPeerMgr(Manager): vrf, nbr = key.split('|', 1) if key not in self.peers: cmd = None + + if "local_addr" not in data: + syslog.syslog(syslog.LOG_ERR, 'Peer {}. Error in missing required attribute "local_addr"'.format(key)) + return False + + # Check if this route is belong to a vnet + interfaces = self.directory.get_slot("interface") + target_interface = None + for metadata in interfaces.values(): + if metadata["ip"] == data["local_addr"]: + target_interface = metadata + if target_interface is None: + return False + elif target_interface.has_key("vnet_name") and target_interface["vnet_name"]: + # Ignore the route that is in a vnet + syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, data)) + return True + neigmeta = self.directory.get_slot("neigmeta") if 'name' in data and data["name"] not in neigmeta: return False @@ -390,6 +409,64 @@ class BGPPeerMgr(Manager): return peers +def prefix_attr(attr, value): + if not value: + return None + else: + try: + prefix = netaddr.IPNetwork(str(value)) + except: + return None + return str(getattr(prefix, attr)) + + +class InterfaceMgr(Manager): + def __init__(self, daemon, directory, interface_table = swsscommon.CFG_INTF_TABLE_NAME): + super(InterfaceMgr, self).__init__( + daemon, + directory, + [], + "CONFIG_DB", + interface_table + ) + self.interfaces = {} + + def set_handler(self, key, data): + # There are two entries for an interface in CFG_INTF_TABLE. + # One of them is to specify ip and prefix. + # In this case, the key contains '|', like "Ethernet0|192.168.0.6/30". + # Another one is to specify whether this interface belongs to a vnet + # In this case, the key only includes the interface name + if '|' in key: + key, network = key.split('|', 1) + data["ip"] = prefix_attr("ip", network) + data["prefixlen"] = prefix_attr("prefixlen", network) + + # Put the interface data into directory only if two entries have both been set + # Otherwise cache the first come entry + if key in self.interfaces: + self.interfaces[key].update(data) + self.directory.put("interface", key, self.interfaces[key]) + del self.interfaces[key] + else: + self.interfaces[key] = data + return True + + def del_handler(self, key): + if '|' in key: + key, _ = key.split('|', 1) + self.directory.remove("interface", key) + + +class LoopbackInterfaceMgr(InterfaceMgr): + def __init__(self, daemon, directory): + super(LoopbackInterfaceMgr, self).__init__( + daemon, + directory, + swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME + ) + + def wait_for_bgpd(): # wait for 20 seconds stop_time = datetime.datetime.now() + datetime.timedelta(seconds=20) @@ -408,6 +485,8 @@ def main(): BGPDeviceMetaMgr, BGPNeighborMetaMgr, BGPPeerMgr, + InterfaceMgr, + LoopbackInterfaceMgr, ] wait_for_bgpd() daemon = Daemon() diff --git a/dockers/docker-fpm-frr/bgpd.peer.conf.j2 b/dockers/docker-fpm-frr/bgpd.peer.conf.j2 old mode 100644 new mode 100755 index c3dc50449d35..bcc520f6b2df --- a/dockers/docker-fpm-frr/bgpd.peer.conf.j2 +++ b/dockers/docker-fpm-frr/bgpd.peer.conf.j2 @@ -26,8 +26,7 @@ neighbor {{ neighbor_addr }} next-hop-self {% endif %} {% if bgp_session["asn"] == DEVICE_METADATA['localhost']['bgp_asn'] - and DEVICE_METADATA['localhost']['type'] == "SpineChassisFrontendRouter" - and (not bgp_session.has_key("local_addr") or bgp_session["local_addr"] not in interfaces_in_vnets) %} + and DEVICE_METADATA['localhost']['type'] == "SpineChassisFrontendRouter" %} address-family l2vpn evpn neighbor {{ neighbor_addr }} activate advertise-all-vni From 08c294562ddb12c674a32fb4595ceae756e51c50 Mon Sep 17 00:00:00 2001 From: zegan Date: Fri, 13 Mar 2020 23:20:20 +0800 Subject: [PATCH 02/13] Fix typo Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index ee53b10323f3..c42d1a7b9972 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -306,7 +306,7 @@ class BGPPeerMgr(Manager): return False elif target_interface.has_key("vnet_name") and target_interface["vnet_name"]: # Ignore the route that is in a vnet - syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, data)) + syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, target_interface["vnet_name"])) return True neigmeta = self.directory.get_slot("neigmeta") From ba8488994fed5a63036645ed2e7434ce49e821a7 Mon Sep 17 00:00:00 2001 From: zegan Date: Sat, 14 Mar 2020 17:17:08 +0800 Subject: [PATCH 03/13] Add VLAN_INTERFACE Manager in BGPCFGD and polish code flow Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 39 ++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index c42d1a7b9972..64b962b34953 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -293,21 +293,22 @@ class BGPPeerMgr(Manager): cmd = None if "local_addr" not in data: - syslog.syslog(syslog.LOG_ERR, 'Peer {}. Error in missing required attribute "local_addr"'.format(key)) - return False + syslog.syslog(syslog.LOG_WARNING, 'Peer {}. Error in missing required attribute "local_addr"'.format(key)) + else: - # Check if this route is belong to a vnet - interfaces = self.directory.get_slot("interface") - target_interface = None - for metadata in interfaces.values(): - if metadata["ip"] == data["local_addr"]: - target_interface = metadata - if target_interface is None: - return False - elif target_interface.has_key("vnet_name") and target_interface["vnet_name"]: - # Ignore the route that is in a vnet - syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, target_interface["vnet_name"])) - return True + # Check if this route is belong to a vnet + interfaces = self.directory.get_slot("interface") + target_interface = None + for metadata in interfaces.values(): + if metadata["ip"] == data["local_addr"]: + target_interface = metadata + break + if target_interface is None: + return False + elif target_interface.has_key("vnet_name") and target_interface["vnet_name"]: + # Ignore the route that is in a vnet + syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, target_interface["vnet_name"])) + return True neigmeta = self.directory.get_slot("neigmeta") if 'name' in data and data["name"] not in neigmeta: @@ -467,6 +468,15 @@ class LoopbackInterfaceMgr(InterfaceMgr): ) +class VlanInterfaceMgr(InterfaceMgr): + def __init__(self, daemon, directory): + super(VlanInterfaceMgr, self).__init__( + daemon, + directory, + swsscommon.CFG_VLAN_INTF_TABLE_NAME + ) + + def wait_for_bgpd(): # wait for 20 seconds stop_time = datetime.datetime.now() + datetime.timedelta(seconds=20) @@ -487,6 +497,7 @@ def main(): BGPPeerMgr, InterfaceMgr, LoopbackInterfaceMgr, + VlanInterfaceMgr, ] wait_for_bgpd() daemon = Daemon() From 4c8a4c666c98a5f4c798d672160ad7c39c963cc6 Mon Sep 17 00:00:00 2001 From: zegan Date: Thu, 26 Mar 2020 02:59:23 +0800 Subject: [PATCH 04/13] Add PortChannelInterface Manager in BGPCFGD Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 64b962b34953..e0deaf66004f 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -477,6 +477,15 @@ class VlanInterfaceMgr(InterfaceMgr): ) +class PortChannelInterfaceMgr(InterfaceMgr): + def __init__(self, daemon, directory): + super(PortChannelInterfaceMgr, self).__init__( + daemon, + directory, + swsscommon.CFG_LAG_INTF_TABLE_NAME + ) + + def wait_for_bgpd(): # wait for 20 seconds stop_time = datetime.datetime.now() + datetime.timedelta(seconds=20) @@ -498,6 +507,7 @@ def main(): InterfaceMgr, LoopbackInterfaceMgr, VlanInterfaceMgr, + PortChannelInterfaceMgr, ] wait_for_bgpd() daemon = Daemon() From e99a2cfe371c8ddd04767a551daa9249d60c3289 Mon Sep 17 00:00:00 2001 From: zegan Date: Thu, 26 Mar 2020 10:03:17 +0800 Subject: [PATCH 05/13] Fixbug overwrite the interface in directory if IPv6 and IPv4 were both set in the same interface Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 54 +++++++++++++++------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index e0deaf66004f..d446918539dc 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -272,7 +272,8 @@ class BGPPeerMgr(Manager): [ ("meta", "localhost/bgp_asn"), ("neigmeta", ""), - ("interface", "") + ("local_addresses", ""), + ("interfaces", ""), ], "CONFIG_DB", swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME @@ -297,18 +298,19 @@ class BGPPeerMgr(Manager): else: # Check if this route is belong to a vnet - interfaces = self.directory.get_slot("interface") - target_interface = None - for metadata in interfaces.values(): - if metadata["ip"] == data["local_addr"]: - target_interface = metadata - break - if target_interface is None: + local_addresses = self.directory.get_slot("local_addresses") + if data["local_addr"] not in local_addresses: + return False + local_address = local_addresses[data["local_addr"]] + interfaces = self.directory.get_slot("interfaces") + if local_address.has_key("interface") and local_address["interface"] in interfaces: + interface = interfaces[local_address["interface"]] + if interface.has_key("vnet_name") and interface["vnet_name"]: + # Ignore the route that is in a vnet + syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, interface["vnet_name"])) + return True + else: return False - elif target_interface.has_key("vnet_name") and target_interface["vnet_name"]: - # Ignore the route that is in a vnet - syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, target_interface["vnet_name"])) - return True neigmeta = self.directory.get_slot("neigmeta") if 'name' in data and data["name"] not in neigmeta: @@ -430,33 +432,25 @@ class InterfaceMgr(Manager): "CONFIG_DB", interface_table ) - self.interfaces = {} def set_handler(self, key, data): - # There are two entries for an interface in CFG_INTF_TABLE. - # One of them is to specify ip and prefix. - # In this case, the key contains '|', like "Ethernet0|192.168.0.6/30". - # Another one is to specify whether this interface belongs to a vnet - # In this case, the key only includes the interface name if '|' in key: - key, network = key.split('|', 1) - data["ip"] = prefix_attr("ip", network) + data = {} + data["interface"], network = key.split('|', 1) data["prefixlen"] = prefix_attr("prefixlen", network) - - # Put the interface data into directory only if two entries have both been set - # Otherwise cache the first come entry - if key in self.interfaces: - self.interfaces[key].update(data) - self.directory.put("interface", key, self.interfaces[key]) - del self.interfaces[key] + ip = prefix_attr("ip", network) + self.directory.put("local_addresses", ip, data) else: - self.interfaces[key] = data + self.directory.put("interfaces", key, data) return True def del_handler(self, key): if '|' in key: - key, _ = key.split('|', 1) - self.directory.remove("interface", key) + _, network = key.split('|', 1) + ip = prefix_attr("ip", network) + self.directory.remove("local_addresses", ip) + else: + self.directory.remove("interfaces", key) class LoopbackInterfaceMgr(InterfaceMgr): From 244faf0704425c8ebff01a0ad3ac0bd720ce1bde Mon Sep 17 00:00:00 2001 From: zegan Date: Tue, 31 Mar 2020 12:20:37 +0800 Subject: [PATCH 06/13] Add comments for improving the readability Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index d446918539dc..84183c24eeed 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -297,14 +297,21 @@ class BGPPeerMgr(Manager): syslog.syslog(syslog.LOG_WARNING, 'Peer {}. Error in missing required attribute "local_addr"'.format(key)) else: - # Check if this route is belong to a vnet local_addresses = self.directory.get_slot("local_addresses") + # Check if the information for the local address of this route has been set + # Which means we can find the interface for this route by its local address + # We can only process this route message when the local address was set if data["local_addr"] not in local_addresses: return False local_address = local_addresses[data["local_addr"]] interfaces = self.directory.get_slot("interfaces") + # Check if the information for the interface of this local address has set + # Which means we can determine whether this route belongs to a vnet + # Because the vnet name will be set in the interface + # Ref:https://github.com/Azure/SONiC/blob/master/doc/vxlan/Vxlan_hld.md#212-vnetinterface-table if local_address.has_key("interface") and local_address["interface"] in interfaces: interface = interfaces[local_address["interface"]] + # Check if this route is belong to a vnet if interface.has_key("vnet_name") and interface["vnet_name"]: # Ignore the route that is in a vnet syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, interface["vnet_name"])) @@ -434,6 +441,11 @@ class InterfaceMgr(Manager): ) def set_handler(self, key, data): + # There are two types of entries for an interface. + # One of them is to specify ip and prefix. + # In this case, the key contains '|', like "Ethernet0|192.168.0.6/30". + # Another one is to specify whether this interface belongs to a vnet + # In this case, the key only includes the interface name. if '|' in key: data = {} data["interface"], network = key.split('|', 1) From 94ff75d6e8a6f3656120c0c372001433f0364d67 Mon Sep 17 00:00:00 2001 From: zegan Date: Tue, 31 Mar 2020 12:30:40 +0800 Subject: [PATCH 07/13] Add comments for improving the readability Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 84183c24eeed..5bd30be8b687 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -296,7 +296,8 @@ class BGPPeerMgr(Manager): if "local_addr" not in data: syslog.syslog(syslog.LOG_WARNING, 'Peer {}. Error in missing required attribute "local_addr"'.format(key)) else: - + # The route that belongs to a vnet cannot be advertised by the default BGP session. + # So we need to check whether this route belongs to a vnet. local_addresses = self.directory.get_slot("local_addresses") # Check if the information for the local address of this route has been set # Which means we can find the interface for this route by its local address From 6cac8ecef6eb2353f7352f08d7a8486e16e7fdcf Mon Sep 17 00:00:00 2001 From: zegan Date: Tue, 31 Mar 2020 17:03:56 +0800 Subject: [PATCH 08/13] Refactor functions to improve readability Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 77 ++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 5bd30be8b687..69130604222a 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -296,29 +296,16 @@ class BGPPeerMgr(Manager): if "local_addr" not in data: syslog.syslog(syslog.LOG_WARNING, 'Peer {}. Error in missing required attribute "local_addr"'.format(key)) else: - # The route that belongs to a vnet cannot be advertised by the default BGP session. - # So we need to check whether this route belongs to a vnet. - local_addresses = self.directory.get_slot("local_addresses") - # Check if the information for the local address of this route has been set - # Which means we can find the interface for this route by its local address - # We can only process this route message when the local address was set - if data["local_addr"] not in local_addresses: - return False - local_address = local_addresses[data["local_addr"]] - interfaces = self.directory.get_slot("interfaces") - # Check if the information for the interface of this local address has set - # Which means we can determine whether this route belongs to a vnet - # Because the vnet name will be set in the interface - # Ref:https://github.com/Azure/SONiC/blob/master/doc/vxlan/Vxlan_hld.md#212-vnetinterface-table - if local_address.has_key("interface") and local_address["interface"] in interfaces: - interface = interfaces[local_address["interface"]] - # Check if this route is belong to a vnet - if interface.has_key("vnet_name") and interface["vnet_name"]: - # Ignore the route that is in a vnet - syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, interface["vnet_name"])) - return True - else: + # The bgp session that belongs to a vnet cannot be advertised as the default BGP session. + # So we need to check whether this bgp session belongs to a vnet. + interface = self.get_local_interface(data["local_addr"]) + if not interface: return False + vnet = self.get_vnet(interface) + if vnet: + # Ignore the bgp session that is in a vnet + syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, vnet)) + return True neigmeta = self.directory.get_slot("neigmeta") if 'name' in data and data["name"] not in neigmeta: @@ -392,6 +379,25 @@ class BGPPeerMgr(Manager): os.remove(tmp_filename) return rc == 0 + def get_local_interface(self, local_addr): + local_addresses = self.directory.get_slot("local_addresses") + # Check if the local address of this bgp session has been set + if local_addr not in local_addresses: + return None + local_address = local_addresses[local_addr] + interfaces = self.directory.get_slot("interfaces") + # Check if the information for the interface of this local address has been set + if local_address.has_key("interface") and local_address["interface"] in interfaces: + return interfaces[local_address["interface"]] + else: + return None + + def get_vnet(self, interface): + if interface.has_key("vnet_name") and interface["vnet_name"]: + return interface["vnet_name"] + else: + return None + @staticmethod def normalize_key(key): if '|' not in key: @@ -420,17 +426,6 @@ class BGPPeerMgr(Manager): return peers -def prefix_attr(attr, value): - if not value: - return None - else: - try: - prefix = netaddr.IPNetwork(str(value)) - except: - return None - return str(getattr(prefix, attr)) - - class InterfaceMgr(Manager): def __init__(self, daemon, directory, interface_table = swsscommon.CFG_INTF_TABLE_NAME): super(InterfaceMgr, self).__init__( @@ -450,8 +445,13 @@ class InterfaceMgr(Manager): if '|' in key: data = {} data["interface"], network = key.split('|', 1) - data["prefixlen"] = prefix_attr("prefixlen", network) - ip = prefix_attr("ip", network) + try: + network = netaddr.IPNetwork(str(network)) + except: + syslog.syslog(syslog.LOG_WARNING, 'Subnet {} format is wrong'.format(network)) + return False + data["prefixlen"] = str(network.prefixlen) + ip = str(network.ip) self.directory.put("local_addresses", ip, data) else: self.directory.put("interfaces", key, data) @@ -460,7 +460,12 @@ class InterfaceMgr(Manager): def del_handler(self, key): if '|' in key: _, network = key.split('|', 1) - ip = prefix_attr("ip", network) + try: + network = netaddr.IPNetwork(str(network)) + except: + syslog.syslog(syslog.LOG_WARNING, 'Subnet {} format is wrong'.format(network)) + return False + ip = str(network.ip) self.directory.remove("local_addresses", ip) else: self.directory.remove("interfaces", key) From 1b04b9139e06d1a67691e19d89eddfbfdf986cab Mon Sep 17 00:00:00 2001 From: zegan Date: Wed, 1 Apr 2020 10:20:10 +0800 Subject: [PATCH 09/13] Move get_local_interface and get_vnet to InterfaceMgr Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 44 ++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 69130604222a..3054c7a109fb 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -298,10 +298,10 @@ class BGPPeerMgr(Manager): else: # The bgp session that belongs to a vnet cannot be advertised as the default BGP session. # So we need to check whether this bgp session belongs to a vnet. - interface = self.get_local_interface(data["local_addr"]) + interface = InterfaceMgr.get_local_interface(self.directory, data["local_addr"]) if not interface: return False - vnet = self.get_vnet(interface) + vnet = InterfaceMgr.get_vnet(interface) if vnet: # Ignore the bgp session that is in a vnet syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, vnet)) @@ -379,25 +379,6 @@ class BGPPeerMgr(Manager): os.remove(tmp_filename) return rc == 0 - def get_local_interface(self, local_addr): - local_addresses = self.directory.get_slot("local_addresses") - # Check if the local address of this bgp session has been set - if local_addr not in local_addresses: - return None - local_address = local_addresses[local_addr] - interfaces = self.directory.get_slot("interfaces") - # Check if the information for the interface of this local address has been set - if local_address.has_key("interface") and local_address["interface"] in interfaces: - return interfaces[local_address["interface"]] - else: - return None - - def get_vnet(self, interface): - if interface.has_key("vnet_name") and interface["vnet_name"]: - return interface["vnet_name"] - else: - return None - @staticmethod def normalize_key(key): if '|' not in key: @@ -470,6 +451,27 @@ class InterfaceMgr(Manager): else: self.directory.remove("interfaces", key) + @staticmethod + def get_local_interface(directory, local_addr): + local_addresses = directory.get_slot("local_addresses") + # Check if the local address of this bgp session has been set + if local_addr not in local_addresses: + return None + local_address = local_addresses[local_addr] + interfaces = directory.get_slot("interfaces") + # Check if the information for the interface of this local address has been set + if local_address.has_key("interface") and local_address["interface"] in interfaces: + return interfaces[local_address["interface"]] + else: + return None + + @staticmethod + def get_vnet(interface): + if interface.has_key("vnet_name") and interface["vnet_name"]: + return interface["vnet_name"] + else: + return None + class LoopbackInterfaceMgr(InterfaceMgr): def __init__(self, daemon, directory): From 6ba006636e90f7f7ed37d900e87c5dc1acb92d17 Mon Sep 17 00:00:00 2001 From: zegan Date: Wed, 1 Apr 2020 13:57:29 +0800 Subject: [PATCH 10/13] Polish comment Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 3054c7a109fb..98e0dae01521 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -418,11 +418,8 @@ class InterfaceMgr(Manager): ) def set_handler(self, key, data): - # There are two types of entries for an interface. - # One of them is to specify ip and prefix. - # In this case, the key contains '|', like "Ethernet0|192.168.0.6/30". - # Another one is to specify whether this interface belongs to a vnet - # In this case, the key only includes the interface name. + # Interface table can have two keys, + # one with ip prefix and one without ip prefix if '|' in key: data = {} data["interface"], network = key.split('|', 1) From 7e9ef261898316bb6a6bf82cf247137e81ff1e23 Mon Sep 17 00:00:00 2001 From: zegan Date: Wed, 1 Apr 2020 21:53:27 +0800 Subject: [PATCH 11/13] Polish log Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 98e0dae01521..c5ad8560cbe1 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -304,7 +304,14 @@ class BGPPeerMgr(Manager): vnet = InterfaceMgr.get_vnet(interface) if vnet: # Ignore the bgp session that is in a vnet - syslog.syslog(syslog.LOG_INFO, 'Peer {} in vnet {}'.format(key, vnet)) + syslog.syslog( + syslog.LOG_INFO, + 'Ignore the BGP peer {} as the interface {} is in vnet {}'.format( + key, + interface, + vnet + ) + ) return True neigmeta = self.directory.get_slot("neigmeta") @@ -426,7 +433,13 @@ class InterfaceMgr(Manager): try: network = netaddr.IPNetwork(str(network)) except: - syslog.syslog(syslog.LOG_WARNING, 'Subnet {} format is wrong'.format(network)) + syslog.syslog( + syslog.LOG_WARNING, + 'Subnet {} format is wrong for interface {}'.format( + network, + data["interface"] + ) + ) return False data["prefixlen"] = str(network.prefixlen) ip = str(network.ip) @@ -437,11 +450,17 @@ class InterfaceMgr(Manager): def del_handler(self, key): if '|' in key: - _, network = key.split('|', 1) + interface, network = key.split('|', 1) try: network = netaddr.IPNetwork(str(network)) except: - syslog.syslog(syslog.LOG_WARNING, 'Subnet {} format is wrong'.format(network)) + syslog.syslog( + syslog.LOG_WARNING, + 'Subnet {} format is wrong for interface {}'.format( + network, + interface + ) + ) return False ip = str(network.ip) self.directory.remove("local_addresses", ip) From e14028a4f9adfc0a761c7f7c871cd479888c1f73 Mon Sep 17 00:00:00 2001 From: zegan Date: Wed, 1 Apr 2020 22:41:33 +0800 Subject: [PATCH 12/13] Add log for failed messages Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index c5ad8560cbe1..48e496dd8826 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -300,6 +300,12 @@ class BGPPeerMgr(Manager): # So we need to check whether this bgp session belongs to a vnet. interface = InterfaceMgr.get_local_interface(self.directory, data["local_addr"]) if not interface: + syslog.syslog(syslog.LOG_INFO, + 'Peer {} with local address {} wait for the corresponding interface to be set'.format( + key, + data["local_addr"] + ) + ) return False vnet = InterfaceMgr.get_vnet(interface) if vnet: @@ -316,6 +322,12 @@ class BGPPeerMgr(Manager): neigmeta = self.directory.get_slot("neigmeta") if 'name' in data and data["name"] not in neigmeta: + syslog.syslog(syslog.LOG_INFO, + 'Peer {} with neighbor name {} wait for the corresponding neighbor metadata to be set'.format( + key, + data["name"] + ) + ) return False try: cmd = self.templates["add"].render( From 4b6a51aaa6602176090191ddd22390934948b07f Mon Sep 17 00:00:00 2001 From: zegan Date: Wed, 1 Apr 2020 23:05:57 +0800 Subject: [PATCH 13/13] Add descriptions for functions Signed-off-by: zegan --- dockers/docker-fpm-frr/bgpcfgd | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dockers/docker-fpm-frr/bgpcfgd b/dockers/docker-fpm-frr/bgpcfgd index 48e496dd8826..4e638def47b7 100755 --- a/dockers/docker-fpm-frr/bgpcfgd +++ b/dockers/docker-fpm-frr/bgpcfgd @@ -481,6 +481,13 @@ class InterfaceMgr(Manager): @staticmethod def get_local_interface(directory, local_addr): + """ + @summary: Get interface according to the local address from the directory + @param directory: Directory object that stored metadata of interfaces + @param local_addr: Local address of the interface + @return: Return the metadata of the interface with the local address + If the interface has not been set, return None + """ local_addresses = directory.get_slot("local_addresses") # Check if the local address of this bgp session has been set if local_addr not in local_addresses: @@ -495,6 +502,12 @@ class InterfaceMgr(Manager): @staticmethod def get_vnet(interface): + """ + @summary: Get the VNet name of the interface + @param interface: The metadata of the interface + @return: Return the vnet name of the interface if this interface belongs to a vnet, + Otherwise return None + """ if interface.has_key("vnet_name") and interface["vnet_name"]: return interface["vnet_name"] else: