Skip to content

Commit

Permalink
[bgpmon]: Fix exception in bgpmon caused by duplicate bgp neighbor ID (
Browse files Browse the repository at this point in the history
…#6546)

* Fix exception in bgpmon caused by duplicate keys
It is possible that BGP neighbors in IPv4 and IPv6 address families
share the same name (such as bgp monitor). However, such case is not
handled in bgpmon, and an Exception will be raised. This commit will
address the issue by Using set instead of list to avoid duplicate keys.
  • Loading branch information
bingwang-ms authored and lguohan committed Jan 28, 2021
1 parent a0fd862 commit be28153
Showing 1 changed file with 18 additions and 19 deletions.
37 changes: 18 additions & 19 deletions src/sonic-bgpcfgd/bgpmon/bgpmon.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,20 @@
Description: bgpmon.py -- populating bgp related information in stateDB.
script is started by supervisord in bgp docker when the docker is started.
Initial creation of this daemon is to assist SNMP agent in obtaining the
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
BGP related items that needs to be updated in a periodic manner in the
BGP related items that needs to be updated in a periodic manner in the
future, then more can be added into this process.
The script check if there are any bgp activities by monitoring the bgp
frr.log file timestamp. If activity is detected, then it will request bgp
neighbor state via vtysh cli interface. This bgp activity monitoring is
neighbor state via vtysh cli interface. This bgp activity monitoring is
done periodically (every 15 second). When triggered, it looks specifically
for the neighbor state in the json output of show ip bgp neighbors json
and update the state DB for each neighbor accordingly.
In order to not disturb and hold on to the State DB access too long and
removal of the stale neighbors (neighbors that was there previously on
removal of the stale neighbors (neighbors that was there previously on
previous get request but no longer there in the current get request), a
"previous" neighbor dictionary will be kept and used to determine if there
is a need to perform update or the peer is stale to be removed from the
Expand All @@ -34,13 +34,13 @@

class BgpStateGet:
def __init__(self):
# list peer_l stores the Neighbor peer Ip address
# set peer_l stores the Neighbor peer Ip address
# dic peer_state stores the Neighbor peer state entries
# list new_peer_l stores the new snapshot of Neighbor peer ip address
# set new_peer_l stores the new snapshot of Neighbor peer ip address
# dic new_peer_state stores the new snapshot of Neighbor peer states
self.peer_l = []
self.peer_l = set()
self.peer_state = {}
self.new_peer_l = []
self.new_peer_l = set()
self.new_peer_state = {}
self.cached_timestamp = 0
self.db = swsssdk.SonicV2Connector()
Expand All @@ -67,7 +67,7 @@ def bgp_activity_detected(self):

def update_new_peer_states(self, peer_dict):
peer_l = peer_dict["peers"].keys()
self.new_peer_l.extend(peer_l)
self.new_peer_l.update(peer_l)
for peer in peer_l:
self.new_peer_state[peer] = peer_dict["peers"][peer]["state"]

Expand All @@ -80,8 +80,8 @@ def get_all_neigh_states(self):
return

peer_info = json.loads(output)
# cmd ran successfully, safe to Clean the "new" lists/dic for new snapshot
del self.new_peer_l[:]
# cmd ran successfully, safe to Clean the "new" set/dict for new snapshot
self.new_peer_l.clear()
self.new_peer_state.clear()
for key, value in peer_info.items():
if key == "ipv4Unicast" or key == "ipv6Unicast":
Expand Down Expand Up @@ -115,8 +115,7 @@ def flush_pipe(self, data):

def update_neigh_states(self):
data = {}
for i in range (0, len(self.new_peer_l)):
peer = self.new_peer_l[i]
for peer in self.new_peer_l:
key = "NEIGH_STATE_TABLE|%s" % peer
if peer in self.peer_l:
# only update the entry if state changed
Expand All @@ -125,7 +124,7 @@ def update_neigh_states(self):
state = self.new_peer_state[peer]
data[key] = {'state':state}
self.peer_state[peer] = state
# remove this neighbor from old list since it is accounted for
# 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
Expand All @@ -135,19 +134,19 @@ def update_neigh_states(self):
if len(data) > PIPE_BATCH_MAX_COUNT:
self.flush_pipe(data)
# Check for stale state entries to be cleaned up
while len(self.peer_l) > 0:
for peer in self.peer_l:
# remove this from the stateDB and the current neighbor state entry
peer = self.peer_l.pop(0)
del_key = "NEIGH_STATE_TABLE|%s" % peer
data[del_key] = None
del self.peer_state[peer]
if peer in self.peer_state:
del self.peer_state[peer]
if len(data) > PIPE_BATCH_MAX_COUNT:
self.flush_pipe(data)
# If anything in the pipeline not yet flushed, flush them now
if len(data) > 0:
self.flush_pipe(data)
# Save the new List
self.peer_l = self.new_peer_l[:]
# Save the new set
self.peer_l = self.new_peer_l.copy()

def main():

Expand Down

0 comments on commit be28153

Please sign in to comment.