Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 112 additions & 0 deletions dockers/docker-fpm-frr/bgpcfgd
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ class BGPPeerMgr(Manager):
[
("meta", "localhost/bgp_asn"),
("neigmeta", ""),
("local_addresses", ""),
("interfaces", ""),
],
"CONFIG_DB",
swsscommon.CFG_BGP_NEIGHBOR_TABLE_NAME
Expand All @@ -290,6 +292,21 @@ 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_WARNING, 'Peer {}. Error in missing required attribute "local_addr"'.format(key))
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
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what should be the log message here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, this is an intermediate state. Maybe it should add some log with INFO level to display this state. Same problem at here https://github.com/Azure/sonic-buildimage/blob/master/dockers/docker-fpm-frr/bgpcfgd#L295

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:
return False
Expand Down Expand Up @@ -362,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:
Expand Down Expand Up @@ -390,6 +426,78 @@ class BGPPeerMgr(Manager):
return peers


class InterfaceMgr(Manager):
def __init__(self, daemon, directory, interface_table = swsscommon.CFG_INTF_TABLE_NAME):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would also want to consider VLAN_INTERFACE as well - CFG_VLAN_INTF_TABLE_NAME

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your suggestion. Have added VlanInterfaceMgr.

super(InterfaceMgr, self).__init__(
daemon,
directory,
[],
"CONFIG_DB",
interface_table
)

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.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this true that vnet interface will only have interface name without ip addresses? @prsunny ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, vnet interface will also have ip address as any other regular entry. I don't think we need to mention about 'vnet' in this function as this is a generic handler. The comment can just state, "Interface table can have two keys, one with ip prefix and one without ip prefix".

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pterosaur , can you modify the comment accordingly, current description is not accurate.

if '|' in key:
data = {}
data["interface"], network = key.split('|', 1)
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)
return True

def del_handler(self, key):
if '|' in key:
_, network = key.split('|', 1)
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)


class LoopbackInterfaceMgr(InterfaceMgr):
def __init__(self, daemon, directory):
super(LoopbackInterfaceMgr, self).__init__(
daemon,
directory,
swsscommon.CFG_LOOPBACK_INTERFACE_TABLE_NAME
)


class VlanInterfaceMgr(InterfaceMgr):
def __init__(self, daemon, directory):
super(VlanInterfaceMgr, self).__init__(
daemon,
directory,
swsscommon.CFG_VLAN_INTF_TABLE_NAME
)


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)
Expand All @@ -408,6 +516,10 @@ def main():
BGPDeviceMetaMgr,
BGPNeighborMetaMgr,
BGPPeerMgr,
InterfaceMgr,
LoopbackInterfaceMgr,
VlanInterfaceMgr,
PortChannelInterfaceMgr,
]
wait_for_bgpd()
daemon = Daemon()
Expand Down
3 changes: 1 addition & 2 deletions dockers/docker-fpm-frr/bgpd.peer.conf.j2
100644 → 100755
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down