[sonic-xcvrd] add new daemon sonic-xcvrd to platform monitor #17

keboliu · 2018-07-03T10:12:45Z

[sonic-xcvrd] add new daemon sonic-xcvrd to fetch Transceiver informations to DB

* This new daemon will periodcally read DOM information from eeprom and post to DB
* This new daemon will listen to the SFP plug in/out event and update the DB accordingly

* file change list

    new file:   scripts/xcvrd
    new file:   setup.py

Design doc: https://github.com/Azure/SONiC/blob/gh-pages/doc/OIDsforSensorandTransciver.MD

…tions to DB * This new daemon will periodcally read DOM information from eeprom and post to DB * This new daemon will listen to the SFP plug in/out event and update the DB accordingly * file change list new file: scripts/xcvrd new file: setup.py Signed-off-by Liu Kebo kebol@mellanox.com

jleveque · 2018-07-03T18:22:02Z

sonic-xcvrd/scripts/xcvrd

+PHYSICAL_PORT_NOT_EXIST = -1
+SFP_EEPROM_NOT_READY = -2
+
+TEMPE_UNIT = 'C'


Suggest rename to TEMP_UNIT as "temp" is a common abbreviation for "temperature." #Resolved

qiluo-msft · 2018-07-06T23:56:06Z

sonic-xcvrd/scripts/xcvrd

+REDIS_HOSTNAME = "localhost"
+REDIS_PORT = 6379
+REDIS_TIMEOUT_USECS = 0
+SELECT_TIMEOUT = 1000


SELECT_TIMEOUT [](start = 0, length = 14)

SELECT_TIMEOUT_USECS? #Closed

+1 - add units to name. #Resolved

qiluo-msft · 2018-07-06T23:58:12Z

sonic-xcvrd/scripts/xcvrd

+
+VERSION = '1.0'
+
+SYSLOG_IDENTIFIER = "xcvrd"


xcvrd [](start = 21, length = 5)

get the script basename programatically? #Closed

+1. This is copied from other daemons I've written, and I've also been meaning to go back over them and obtain the basename programmatically.

SYSLOG_IDENTIFIER = os.path.basename(__file__) should do the trick. #Resolved

qiluo-msft · 2018-07-06T23:58:22Z

sonic-xcvrd/scripts/xcvrd

+VERSION = '1.0'
+
+SYSLOG_IDENTIFIER = "xcvrd"
+XCVRD_MODULE_NAME = "xcvrd"


XCVRD_MODULE_NAME [](start = 0, length = 17)

not used. #Closed

qiluo-msft · 2018-07-06T23:58:36Z

sonic-xcvrd/scripts/xcvrd

+REDIS_TIMEOUT_USECS = 0
+SELECT_TIMEOUT = 1000
+
+DOM_INFO_UPDATE_PERIOD = 60


DOM_INFO_UPDATE_PERIOD [](start = 0, length = 22)

in seconds? #Closed

yes, and revised

qiluo-msft · 2018-07-07T00:00:16Z

sonic-xcvrd/scripts/xcvrd

+
+DOM_INFO_UPDATE_PERIOD = 60
+
+SFP_STATUS_PLUGIN = '1'


SFP_STATUS_PLUGIN [](start = 0, length = 17)

plugin->inserted
plugout->removed #Closed

Suggest rename, plug* are not good names.

In reply to: 200794543 [](ancestors = 200794543)

qiluo-msft · 2018-07-07T00:04:10Z

sonic-xcvrd/scripts/xcvrd

+        log_info("Caught SIGTERM - exiting...")
+        sys.exit(128 + sig)
+    else:
+        log_warning("Caught unhandled signal '" + sig + "'")


and return? #Closed

qiluo-msft · 2018-07-07T00:05:46Z

sonic-xcvrd/scripts/xcvrd

@@ -0,0 +1,430 @@
+#!/usr/bin/env python


python [](start = 15, length = 6)

python2 #Closed

qiluo-msft · 2018-07-07T00:08:21Z

sonic-xcvrd/scripts/xcvrd

+        stdout = proc.communicate()[0]
+        proc.wait()
+        hwsku = stdout.rstrip('\n')
+    except OSError, e:


except [](start = 4, length = 6)

try-except added no value except a message which may be not accurate. Suggest remove.

Update: actually you swallow the inner exception, which make it worse. #Closed

qiluo-msft · 2018-07-07T00:09:05Z

sonic-xcvrd/scripts/xcvrd

+    (platform, hwsku) = get_platform_and_hwsku()
+
+    # Load platform module from source
+    #platform_path = "/".join([PLATFORM_ROOT_PATH, platform])


#platform_path = "/".join([PLATFORM_ROOT_PATH, platform]) [](start = 4, length = 57)

remove all commented code. #Closed

qiluo-msft · 2018-07-07T00:28:08Z

sonic-xcvrd/scripts/xcvrd

+            continue
+
+        (key, op, fvp) = sst.pop()
+        if key in ["PortConfigDone", "PortInitDone"]:


PortConfigDone", "PortInitDone" [](start = 20, length = 31)

Will it be a problem if port configured down long before you subscribe? #Closed

No. I have tested, even Xcvrd started after port config already done, still can get this after subscribe.

qiluo-msft · 2018-07-07T00:30:03Z

sonic-xcvrd/scripts/xcvrd

+
+    #Spawn the task to handle the SFP plug in/out event
+    sfp_change_event_monitoring_thread = MonitoringThread(int_tbl, dom_tbl)
+    sfp_change_event_monitoring_thread.start()


start [](start = 39, length = 5)

after starting timer and thread, the main thread is doing nothing. maybe you can use one less thread. #Closed

moved the thread to the main task as the main loop.

qiluo-msft · 2018-07-07T00:39:18Z

sonic-xcvrd/scripts/xcvrd

+def monitor_sfp_change_task(int_tbl, dom_tbl):
+    log_info('Xcvrd sub thread started....')
+    while True:
+        status, port_dict = platform_sfputil.get_transceiver_change_event()


status [](start = 8, length = 6)

if some error happen, the while-loop will burn CPU. #Closed

qiluo-msft · 2018-07-07T00:43:41Z

sonic-xcvrd/scripts/xcvrd

+        post_port_dom_info_to_db(logical_port_name, table)
+
+    global timer
+    timer = threading.Timer(DOM_INFO_UPDATE_PERIOD, periodically_post_dom_info_to_db, [table])


periodically_post_dom_info_to_db [](start = 52, length = 32)

This is weird recursion. Lots of timers will be created quickly 1->2->4->... ? #Closed

No, if using a local variable 'timer' it will be the case as you said, here I am using a global 'timer' variable, so no this issue, timer triggered as expected.

hui-ma

Could you please add Txpower into database and SNMP? If you look at 8436.py and 8472.py the Sonic github (https://github.com/Azure/sonic-platform-common/blob/master/sonic_sfp), there both are a function for reading TX power calc_tx_poweron. Could you use it to get the TX power?

qiluo-msft

Updated and there are still open comments.

qiluo-msft · 2018-07-25T16:00:08Z

sonic-xcvrd/scripts/xcvrd

+
+    # Start main loop to listen to the SFP change event.
+    log_info("Start main loop")
+    while True:


while True: [](start = 4, length = 11)

if some error happen immediately in get_transceiver_change_event(), the while-loop will burn CPU #Closed

qiluo-msft · 2018-07-26T16:25:10Z

sonic-xcvrd/scripts/xcvrd

+        log_error("Error: thread_exit_flag is set and timer thread exit")
+        thread_exit_flag_lock.release()
+        return
+    thread_exit_flag_lock.release()


You may use threading.Event to reduce code. #Closed

qiluo-msft · 2018-07-26T16:26:50Z

sonic-xcvrd/scripts/xcvrd

+    for logical_port_name in logical_port_list:
+        post_port_dom_info_to_db(logical_port_name, table)
+
+    global timer


global [](start = 4, length = 6)

suggest define a class and move global var used by the thread inside the class. #Closed

qiluo-msft · 2018-07-26T16:28:07Z

sonic-xcvrd/scripts/xcvrd

+                        # TODO, SFP return error code, need handle accordingly.
+                        continue
+        else:
+            # If get_transceiver_change_event() return error, will clean up the DB and then exit


You may just simply break the while loop, and do the clean up with less indentation. #Closed

qiluo-msft · 2018-07-26T16:29:17Z

sonic-xcvrd/scripts/xcvrd

+            for logical_port_name in logical_port_list:
+                del_port_sfp_dom_info_to_db(logical_port_name, int_tbl, dom_tbl)
+            log_error("Error: got error from get_transceiver_change_event, exit")
+            return


return [](start = 12, length = 6)

return an non zero code to indicate daemon abnormal exit. #Closed

qiluo-msft · 2018-07-29T06:03:45Z

sonic-xcvrd/scripts/xcvrd

+
+    # Start the dom sensor info update timer thread
+    dom_update_threade_event = threading.Event()
+    dom_update_thread_timer = None


event and timer objects could be hidden in the update class. #Closed

qiluo-msft · 2018-07-29T06:05:23Z

sonic-xcvrd/scripts/xcvrd

+class dom_info_update_task:
+    def __init__(self, timer, event, table):
+        self.task_running_event = event
+        self.task_running_event.set()


task_running_event [](start = 13, length = 18)

How about a event for stopping. It is more intuitive and readable. #Closed

qiluo-msft · 2018-07-29T16:20:24Z

sonic-xcvrd/scripts/xcvrd

+class dom_info_update_task:
+    def __init__(self, table):
+        self.task_stopping_event = threading.Event()
+        self.task_stopping_event.set()


task_stopping_event [](start = 13, length = 19)

self.task_stopping_event.clear() #Closed

by default the event is set to false, so we don't need to call clear() here.

qiluo-msft · 2018-07-29T16:20:37Z

sonic-xcvrd/scripts/xcvrd

+        self.dom_table = table
+
+    def task_run(self):
+        if not self.task_stopping_event.wait(1):


if not self.task_stopping_event.wait(1): [](start = 8, length = 40)

if self.task_stopping_event.isSet(): #Closed

use isSet() to get the event status.

qiluo-msft · 2018-07-29T16:20:47Z

sonic-xcvrd/scripts/xcvrd

+        self.task_timer.start()
+
+    def task_stop(self):
+        self.task_stopping_event.clear()


self.task_stopping_event.clear() [](start = 8, length = 32)

self.task_stopping_event.set() #Closed

jleveque reviewed Jul 3, 2018

View reviewed changes

keboliu mentioned this pull request Jul 4, 2018

[docker-platform-monitor] make file and supervisord conf change for new xcvrd deamon sonic-net/sonic-buildimage#1840

Merged

fix typo

e8d343d

qiluo-msft reviewed Jul 6, 2018

View reviewed changes

qiluo-msft reviewed Jul 7, 2018

View reviewed changes

revise according to comments

8197bcb

hui-ma reviewed Jul 11, 2018

View reviewed changes

add tx_power support

49b45f2

qiluo-msft requested changes Jul 16, 2018

View reviewed changes

remove unnecessary try/except

41b2a69

qiluo-msft reviewed Jul 25, 2018

View reviewed changes

add error handle for get wrong status from common API

c8a713e

qiluo-msft reviewed Jul 26, 2018

View reviewed changes

fix review comments

a3c174d

qiluo-msft reviewed Jul 29, 2018

View reviewed changes

rename the event, move event and timer inside the class

f7fdcf5

qiluo-msft reviewed Jul 29, 2018

View reviewed changes

code readability fix

c9e9d0a

qiluo-msft approved these changes Jul 30, 2018

View reviewed changes

qiluo-msft merged commit b1b9169 into sonic-net:master Jul 30, 2018

keboliu deleted the sonic-xcvrd branch August 22, 2018 10:30

jleveque added Enhancement xcvrd labels Jul 10, 2020


		VERSION = '1.0'

		SYSLOG_IDENTIFIER = "xcvrd"

[sonic-xcvrd] add new daemon sonic-xcvrd to platform monitor #17

[sonic-xcvrd] add new daemon sonic-xcvrd to platform monitor #17

Conversation

keboliu commented Jul 3, 2018 • edited by qiluo-msft Loading

jleveque Jul 3, 2018 • edited by qiluo-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

jleveque Jul 7, 2018 • edited by qiluo-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

jleveque Jul 7, 2018 • edited by qiluo-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

qiluo-msft Jul 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hui-ma left a comment

Choose a reason for hiding this comment

qiluo-msft left a comment • edited Loading

Choose a reason for hiding this comment

qiluo-msft Jul 25, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 29, 2018 • edited Loading

Choose a reason for hiding this comment

keboliu commented Jul 3, 2018 •

edited by qiluo-msft

Loading

jleveque Jul 3, 2018 •

edited by qiluo-msft

Loading

qiluo-msft Jul 6, 2018 •

edited

Loading

jleveque Jul 7, 2018 •

edited by qiluo-msft

Loading

qiluo-msft Jul 6, 2018 •

edited

Loading

jleveque Jul 7, 2018 •

edited by qiluo-msft

Loading

qiluo-msft Jul 6, 2018 •

edited

Loading

qiluo-msft Jul 6, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft Jul 7, 2018 •

edited

Loading

qiluo-msft left a comment •

edited

Loading

qiluo-msft Jul 25, 2018 •

edited

Loading

qiluo-msft Jul 26, 2018 •

edited

Loading

qiluo-msft Jul 26, 2018 •

edited

Loading

qiluo-msft Jul 26, 2018 •

edited

Loading

qiluo-msft Jul 26, 2018 •

edited

Loading

qiluo-msft Jul 29, 2018 •

edited

Loading

qiluo-msft Jul 29, 2018 •

edited

Loading

qiluo-msft Jul 29, 2018 •

edited

Loading

qiluo-msft Jul 29, 2018 •

edited

Loading

qiluo-msft Jul 29, 2018 •

edited

Loading