From 2e6ad45c976ae3730d92c470a1e2cf8b22d1213e Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Fri, 12 Jul 2019 16:15:38 -0700 Subject: [PATCH 1/8] Subscribe to both ConfigDB and AppDB to get notifications to apply LLDP port config --- dockers/docker-lldp-sv2/lldpmgrd | 86 ++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 25 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index 62ed6904fb13..a2045de6c3bd 100755 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -101,20 +101,41 @@ class LldpManager(object): REDIS_TIMEOUT_MS = 0 def __init__(self): - # Open a handle to the State database - self.state_db = swsscommon.DBConnector(swsscommon.STATE_DB, - self.REDIS_HOSTNAME, - self.REDIS_PORT, - self.REDIS_TIMEOUT_MS) - # Open a handle to the Config database self.config_db = swsscommon.DBConnector(swsscommon.CONFIG_DB, self.REDIS_HOSTNAME, self.REDIS_PORT, self.REDIS_TIMEOUT_MS) + # Open a handle to the Application database + self.appl_db = swsscommon.DBConnector(swsscommon.APPL_DB, + self.REDIS_HOSTNAME, + self.REDIS_PORT, + self.REDIS_TIMEOUT_MS) + self.pending_cmds = {} + def is_port_up(self, port_name): + """ + Determine if a port is up or down by looking into the oper-status for the port in + PORT TABLE in the Application DB + """ + # Retrieve all entires for this port from the Port table + port_table = swsscommon.Table(self.appl_db, swsscommon.APP_PORT_TABLE_NAME) + (status, fvp) = port_table.get(port_name) + if status: + # Convert list of tuples to a dictionary + port_table_dict = dict(fvp) + + # Get the oper-status for the port + port_oper_status = port_table_dict.get("oper_status") + log_info("Port name {} oper status: {}".format(port_name, port_oper_status)) + return port_oper_status == "up" + + else: + log_error("Port '{}' not found in {} table in App DB".format(port_name, swsscommon.APP_PORT_TABLE_NAME)) + return False + def generate_pending_lldp_config_cmd_for_port(self, port_name): """ For port `port_name`, look up the description and alias in the Config database, @@ -185,37 +206,52 @@ class LldpManager(object): def run(self): """ Subscribes to notifications of changes in the PORT table - of the Redis State database. - Subscribe to STATE_DB - get notified of the creation of an interface + of the Redis Config/Application database. + Subscribe to APP_DB - get port oper status Subscribe to CONFIG_DB - get notified of port config changes Update LLDP configuration accordingly. """ # Set select timeout to 10 seconds SELECT_TIMEOUT_MS = 1000 * 10 - # Subscribe to PORT table notifications in the State DB sel = swsscommon.Select() - sst = swsscommon.SubscriberStateTable(self.state_db, swsscommon.STATE_PORT_TABLE_NAME) - sel.addSelectable(sst) - sst = swsscommon.SubscriberStateTable(self.config_db, swsscommon.CFG_PORT_TABLE_NAME) - sel.addSelectable(sst) - # Listen for changes to the PORT table in the STATE_DB and CONFIG_DB + # Subscribe to PORT table notifications in the Config DB + sst_confdb = swsscommon.SubscriberStateTable(self.config_db, swsscommon.CFG_PORT_TABLE_NAME) + sel.addSelectable(sst_confdb) + + # Subscribe to PORT table notifications in the App DB + sst_appdb = swsscommon.SubscriberStateTable(self.appl_db, swsscommon.APP_PORT_TABLE_NAME) + sel.addSelectable(sst_appdb) + + # Listen for changes to the PORT table in the CONFIG_DB and APP_DB while True: + state = None (state, c) = sel.select(SELECT_TIMEOUT_MS) if state == swsscommon.Select.OBJECT: - (key, op, fvp) = sst.pop() - - fvp_dict = dict(fvp) - - # handle creation - if op == "SET" and fvp_dict.get("state") == "ok": - self.generate_pending_lldp_config_cmd_for_port(key) - - # handle config change - if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : - self.generate_pending_lldp_config_cmd_for_port(key) + (key, op, fvp) = sst_confdb.pop() + if (key != "PortInitDone") and (key != "PortConfigDone"): + if fvp: + fvp_dict = dict(fvp) + + # handle creation + if op == "SET" and fvp_dict.get("state") == "ok": + self.generate_pending_lldp_config_cmd_for_port(key) + + # handle config change + if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : + if self.is_port_up(key): + self.generate_pending_lldp_config_cmd_for_port(key) + + (key, op, fvp) = sst_appdb.pop() + if (key != "PortInitDone") and (key != "PortConfigDone"): + if fvp: + fvp_dict = dict(fvp) + if self.is_port_up(key): + self.generate_pending_lldp_config_cmd_for_port(key) + else: + self.pending_cmds.pop(key, None) # Process all pending commands self.process_pending_cmds() From cd8d06cb94e0b9c09f7b0d258a8eff1d8aad1417 Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Fri, 12 Jul 2019 16:20:32 -0700 Subject: [PATCH 2/8] the operstate file is not consistent Removing this since it is not serving any purpose --- dockers/docker-lldp-sv2/lldpmgrd | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index a2045de6c3bd..1d774a5e012c 100755 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -71,18 +71,6 @@ def signal_handler(sig, frame): else: log_warning("Caught unhandled signal '" + sig + "'") -# ========================== Helpers ================================== - -def is_port_exist(port_name): - filename = "/sys/class/net/%s/operstate" % port_name - if os.path.exists(filename): - with open(filename) as fp: - state = fp.read() - return "up" in state - else: - filename = "/sys/class/net/%s/ifindex" % port_name - return os.path.exists(filename) - # ============================== Classes ============================== class LldpManager(object): @@ -178,11 +166,6 @@ class LldpManager(object): to_delete = [] for (port_name, cmd) in self.pending_cmds.iteritems(): - if not is_port_exist(port_name): - # it doesn't make any sense to configure lldpd if the target port does not exist - # let's postpone the command for the next iteration - continue - log_debug("Running command: '{}'".format(cmd)) proc = subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE) From b76b9de586ddd39298fe53008f6dcaa2e7bd37de Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Mon, 15 Jul 2019 09:26:20 -0700 Subject: [PATCH 3/8] Addressing review comments --- dockers/docker-lldp-sv2/lldpmgrd | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index 1d774a5e012c..69c0bc1e415b 100755 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -209,7 +209,6 @@ class LldpManager(object): # Listen for changes to the PORT table in the CONFIG_DB and APP_DB while True: - state = None (state, c) = sel.select(SELECT_TIMEOUT_MS) if state == swsscommon.Select.OBJECT: @@ -219,19 +218,21 @@ class LldpManager(object): fvp_dict = dict(fvp) # handle creation - if op == "SET" and fvp_dict.get("state") == "ok": + if op == "SET": self.generate_pending_lldp_config_cmd_for_port(key) # handle config change if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : if self.is_port_up(key): self.generate_pending_lldp_config_cmd_for_port(key) + else: + self.pending_cmds.pop(key, None) (key, op, fvp) = sst_appdb.pop() if (key != "PortInitDone") and (key != "PortConfigDone"): if fvp: fvp_dict = dict(fvp) - if self.is_port_up(key): + if "up" in fvp_dict.get("oper_status"): self.generate_pending_lldp_config_cmd_for_port(key) else: self.pending_cmds.pop(key, None) From 67ec30a58891c2ca5ae6c8f17ff0ed8e660d05db Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Mon, 15 Jul 2019 11:51:54 -0700 Subject: [PATCH 4/8] Remove check for PortInitDone and PortConfigDone This is not prteset in Config DB --- dockers/docker-lldp-sv2/lldpmgrd | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index 69c0bc1e415b..00b3350ddfd3 100755 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -213,20 +213,19 @@ class LldpManager(object): if state == swsscommon.Select.OBJECT: (key, op, fvp) = sst_confdb.pop() - if (key != "PortInitDone") and (key != "PortConfigDone"): - if fvp: - fvp_dict = dict(fvp) + if fvp: + fvp_dict = dict(fvp) - # handle creation - if op == "SET": - self.generate_pending_lldp_config_cmd_for_port(key) + # handle creation + if op == "SET": + self.generate_pending_lldp_config_cmd_for_port(key) - # handle config change - if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : - if self.is_port_up(key): - self.generate_pending_lldp_config_cmd_for_port(key) - else: - self.pending_cmds.pop(key, None) + # handle config change + if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : + if self.is_port_up(key): + self.generate_pending_lldp_config_cmd_for_port(key) + else: + self.pending_cmds.pop(key, None) (key, op, fvp) = sst_appdb.pop() if (key != "PortInitDone") and (key != "PortConfigDone"): From 70b36869731d3d9d0e9e61e348c410e3a8bfc87b Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Tue, 16 Jul 2019 11:13:28 -0700 Subject: [PATCH 5/8] Remove checking State DB for port creation --- dockers/docker-lldp-sv2/lldpmgrd | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index 00b3350ddfd3..f0846d6e384d 100755 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -216,10 +216,6 @@ class LldpManager(object): if fvp: fvp_dict = dict(fvp) - # handle creation - if op == "SET": - self.generate_pending_lldp_config_cmd_for_port(key) - # handle config change if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : if self.is_port_up(key): @@ -231,6 +227,8 @@ class LldpManager(object): if (key != "PortInitDone") and (key != "PortConfigDone"): if fvp: fvp_dict = dict(fvp) + + # handle port status change if "up" in fvp_dict.get("oper_status"): self.generate_pending_lldp_config_cmd_for_port(key) else: From 5ebac55dfaf36e469d35adf72356e36dc96d5807 Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Tue, 16 Jul 2019 13:14:52 -0700 Subject: [PATCH 6/8] Check for key to be present before fetching it --- dockers/docker-lldp-sv2/lldpmgrd | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) mode change 100755 => 100644 dockers/docker-lldp-sv2/lldpmgrd diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd old mode 100755 new mode 100644 index f0846d6e384d..543fbde66178 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -116,10 +116,13 @@ class LldpManager(object): port_table_dict = dict(fvp) # Get the oper-status for the port - port_oper_status = port_table_dict.get("oper_status") - log_info("Port name {} oper status: {}".format(port_name, port_oper_status)) - return port_oper_status == "up" - + if port_table_dict.has_key("oper_status"): + port_oper_status = port_table_dict.get("oper_status") + log_info("Port name {} oper status: {}".format(port_name, port_oper_status)) + return port_oper_status == "up" + else: + log_error("Port name {} does not have key oper_status".format(port_name)) + return False else: log_error("Port '{}' not found in {} table in App DB".format(port_name, swsscommon.APP_PORT_TABLE_NAME)) return False @@ -217,11 +220,12 @@ class LldpManager(object): fvp_dict = dict(fvp) # handle config change - if op in ["SET", "DEL"] and (fvp_dict.get("alias") or fvp_dict.get("description")) : - if self.is_port_up(key): - self.generate_pending_lldp_config_cmd_for_port(key) - else: - self.pending_cmds.pop(key, None) + if (fvp_dict.has_key("alias") or fvp_dict.has_key("description")): + if op in ["SET", "DEL"]: + if self.is_port_up(key): + self.generate_pending_lldp_config_cmd_for_port(key) + else: + self.pending_cmds.pop(key, None) (key, op, fvp) = sst_appdb.pop() if (key != "PortInitDone") and (key != "PortConfigDone"): From 21b17e099536992ea19d4691a22ae2ba96e6da98 Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Tue, 16 Jul 2019 13:40:58 -0700 Subject: [PATCH 7/8] Check if key is present before getting it --- dockers/docker-lldp-sv2/lldpmgrd | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index 543fbde66178..07c5bb7bef78 100644 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -233,10 +233,11 @@ class LldpManager(object): fvp_dict = dict(fvp) # handle port status change - if "up" in fvp_dict.get("oper_status"): - self.generate_pending_lldp_config_cmd_for_port(key) - else: - self.pending_cmds.pop(key, None) + if fvp_dict.has_key("oper_status"): + if "up" in fvp_dict.get("oper_status"): + self.generate_pending_lldp_config_cmd_for_port(key) + else: + self.pending_cmds.pop(key, None) # Process all pending commands self.process_pending_cmds() From 91704669a7f628ff748493e058f1d7bd25d5a4f1 Mon Sep 17 00:00:00 2001 From: Sumukha Tumkur Vani Date: Wed, 17 Jul 2019 13:14:26 -0700 Subject: [PATCH 8/8] Addressing review comments --- dockers/docker-lldp-sv2/lldpmgrd | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpmgrd b/dockers/docker-lldp-sv2/lldpmgrd index 07c5bb7bef78..d66072602a88 100644 --- a/dockers/docker-lldp-sv2/lldpmgrd +++ b/dockers/docker-lldp-sv2/lldpmgrd @@ -121,7 +121,6 @@ class LldpManager(object): log_info("Port name {} oper status: {}".format(port_name, port_oper_status)) return port_oper_status == "up" else: - log_error("Port name {} does not have key oper_status".format(port_name)) return False else: log_error("Port '{}' not found in {} table in App DB".format(port_name, swsscommon.APP_PORT_TABLE_NAME)) @@ -220,12 +219,11 @@ class LldpManager(object): fvp_dict = dict(fvp) # handle config change - if (fvp_dict.has_key("alias") or fvp_dict.has_key("description")): - if op in ["SET", "DEL"]: - if self.is_port_up(key): - self.generate_pending_lldp_config_cmd_for_port(key) - else: - self.pending_cmds.pop(key, None) + if (fvp_dict.has_key("alias") or fvp_dict.has_key("description")) and (op in ["SET", "DEL"]): + if self.is_port_up(key): + self.generate_pending_lldp_config_cmd_for_port(key) + else: + self.pending_cmds.pop(key, None) (key, op, fvp) = sst_appdb.pop() if (key != "PortInitDone") and (key != "PortConfigDone"):