[Portstat]: Read counters from database #19

sihuihan88 · 2017-03-23T20:48:49Z

Signed-off-by: Sihui Han sihan@microsoft.com

Signed-off-by: Sihui Han <sihan@microsoft.com>

jleveque · 2017-03-23T21:02:46Z

scripts/portstat

+def natural_sort(l):
+    convert = lambda text: int(text) if text.isdigit() else text.lower()
+    alphanum_key = lambda key: [ convert(c) for c in re.split('([0-9]+)', key) ]
+    return sorted(l, key = alphanum_key)


This is not very portable (I know because I wrote pretty much the same function in minigraph_facts.py). Might be better to install the natsort library as a dependency and use natsort.natsorted(), which is what I wound up doing instead.

Do we need to make it portable?
All we're sorting here is name of the interface which is in our case most time 'EthernetX'

If we move up to Python 3 we might have issues. Also, the need for this functionality has arisen twice in the past month, thus it's likely to come up again. It's better to use a popular library than write duplicate code where needed.

I don't like natsort library because:

debian has outdated version of the library

this library break an api

Also I think this function is so small so it's ok to have it in our code.

@pavel-shirshov: I agree with 1. Could you expound on 2? Which API does it break?

natsort() in debian repo vs natsorted() in pypi.
See:
sonic-net/sonic-mgmt@957220e

Gotcha. You're just referring the change in their own API. I think we had a very outdated version on our internal machine. It was so old the package itself even had a different name, naturalsort version 1.5.1, I believe. Even though it was called naturalsort, the module you imported was still called natsort which caused the conflict.

It seems like the latest version of natsort in the Debian repos is 3.5.1, which has the newer natsorted() API. It is still a bit outdated, however, as the latest version is 5.0.2, but I don't think we need to worry about the API change unless we accidentally install naturalsort.

If the lib keeps changing API name, I am not sure whether it's a good idea to use it in our project.

As far as I can tell, the API changed only once, when the package changed its name from naturalsort to natsort.

Gotcha. Great I will use this lib then :)

jleveque · 2017-03-23T21:09:05Z

scripts/sonic_py.py

+CNT_DB = 2
+COUNTER_PORT_NAME_MAP = "COUNTERS_PORT_NAME_MAP"
+
+class sonic_py(object):


This module name should be more descriptive. Maybe sonic_db.py?

https://github.com/Azure/sonic-py-swsssdk - Python utility library for sonic/switch state service database access. Should we use them?

Thanks, Pavel. I was unaware that library existed. if it works, we should use it and not reinvent the wheel. Sihui, please take a look.

I think we should use that. It seems that the library is a little bit out of date, but we could still pick it up. For the other tool, the 'aclshow' in #18, I think we also need to use that.

Thanks for all the info. I will take a look and try to use this library

jleveque · 2017-03-24T23:38:39Z

setup.py

@@ -34,6 +34,7 @@
    install_requires=[
        'click',
        'click-default-group',
+        'natsort',
        'tabulate'


We should probably list sswsdk as a dependency here, also.

Unfortunately, we can't list sswsdk as a dependency here. Since we compile sonic-utilities into a .deb package, stdeb converts all Python dependencies here into Debian dependencies, thus it looks for installed .deb packages named "python-<python_package_name>". However, since we install sswsdk directly as a Python Wheel, it does not show up as an installed .deb package, so the sonic-utilities install will fail. We may want to consider unifying our package creation process.

lguohan · 2017-03-27T21:05:04Z

scripts/portstat

+    'SAI_PORT_STAT_IF_IN_NON_UCAST_PKTS': 0,
+    'SAI_PORT_STAT_IF_IN_ERRORS': 1,
+    'SAI_PORT_STAT_IF_IN_DISCARDS': 2,
+    'SAI_PORT_STAT_ETHER_TX_OVERSIZE_PKTS': 3,


-> RX_OVERSIZE_PKTS

lguohan · 2017-03-27T21:05:17Z

scripts/portstat

+    'SAI_PORT_STAT_IF_OUT_NON_UCAST_PKTS': 4,
+    'SAI_PORT_STAT_IF_OUT_ERRORS': 5,
+    'SAI_PORT_STAT_IF_OUT_DISCARDS': 6,
+    'SAI_PORT_STAT_ETHER_RX_OVERSIZE_PKTS': 7,


-> TX_OVERSIZE_PKTS

lguohan · 2017-03-27T21:13:38Z

scripts/portstat

+            fields = [0,0,0,0,0,0,0,0,0,0]
+            for counter_name, pos in counter_bucket_dict.iteritems():
+                full_table_id = COUNTER_TABLE_PREFIX + table_id
+                fields[pos] += int(self.db.get(self.db.COUNTERS_DB, full_table_id, counter_name))


can we use hmget to get all interested fields for a key in one shot?

https://redis.io/commands/hmget

We are using swssdk lib which hasn't implemented the method similar as hmget in redis-py. If we want to use the hmget, we need to update the swssdk first.

lguohan · 2017-03-27T21:16:39Z

scripts/portstat

+    'SAI_PORT_STAT_IF_OUT_UCAST_PKTS': 4,
+    'SAI_PORT_STAT_IF_OUT_NON_UCAST_PKTS': 4,
+    'SAI_PORT_STAT_IF_OUT_ERRORS': 5,
+    'SAI_PORT_STAT_IF_OUT_DISCARDS': 6,


do we know if OUT_DISCARDS map to all the counters we used to get from bshell? TDBGC0, 3, 5, 8 and the UCQ_DROP_PKT counters?

Signed-off-by: Sihui Han <sihan@microsoft.com>

stcheng · 2017-03-29T07:36:09Z

scripts/portstat

+
+class Portstat(object):
+    def __init__(self):
+        self.db = sswsdk.SonicV2Connector()


sonic-net/sonic-py-swsssdk#4 once this is merged. be sure to update all the related names.

Signed-off-by: Sihui Han <sihan@microsoft.com>

* [config_mgmt.py]: Redirect logs to syslog. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com * [config_mgmt.py]: Call openlog and closelog one time. Changes: 1.) Call openlog and closelog one time. 2.) call syslog directly. 3.) mask loglevel based on debug argument to class. 4.) Class destructor to call closelog. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com * [config_mgmt.py]: Fixing minor typo. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com * [config_mgmt.py]: Call openlog and closelog each time from a wrapper function. It is mandotary to call openlog and closelog eachtime while using syslog library. Since it is one class Instance shared among all python applications. Signed-off-by: Praveen Chaudhary pchaudhary@linkedin.com

…none presenct SFP eeprom (sonic-net#19)

* Changes of SONiC new platform API based on latest design * Moved get_change_event from DeviceBase to ChassisBase and ModuleBase * chassis_base.py : added get_change_event device_base.py : fixed typo fan_base.py : defined DEVICE_TYPE module_base.py : defined DEVICE_TYPE, fixed typo psu_base.py : defined DEVICE_TYPE sfp_base.py : defined DEVICE_TYPE, fixed typo, modified descriptions of set_power_override thermal_base.py : defined DEVICE_TYPE * module_base.py : modified get_change_event to return devices in nested dict * Added dom related status support for SFP, including temperature, voltage, TX bias, TX power and RX power * 1. Per channel return for TX bias, TX power and RX power in sfp_base.py. 2. Support system EEPROM and serial number in chassis_base.py. * 1. Added SYS EEPROM functions to module_base.py. 2. Fixed typos. 3. Merge get_watchdog to return self._watchdog in chassis_base * 1. Fixed various typos. 2. Changed get_ampere to get_current, get_watt to get_power in psu_base.py * Changed get_system_eeprom return to use type code defined in ONIE TlvInfo EEPROM as the keys

[Portstat]: read counters from database

8050287

Signed-off-by: Sihui Han <sihan@microsoft.com>

jleveque suggested changes Mar 23, 2017

View reviewed changes

jleveque added the Enhancement label Mar 24, 2017

[Portstat]:use sswsdk and natsort

8e740d8

jleveque suggested changes Mar 24, 2017

View reviewed changes

stcheng assigned sihuihan88 Mar 25, 2017

lguohan reviewed Mar 27, 2017

View reviewed changes

jleveque mentioned this pull request Mar 28, 2017

Rename module from 'sswsdk' to 'swsssdk' sonic-net/sonic-py-swsssdk#4

Merged

[portstat]: correct the counter being used

2505ea9

Signed-off-by: Sihui Han <sihan@microsoft.com>

lguohan approved these changes Mar 29, 2017

View reviewed changes

stcheng reviewed Mar 29, 2017

View reviewed changes

jleveque approved these changes Mar 29, 2017

View reviewed changes

[Portstat]: rename sswsdk to swsssdk

f8a9e8a

Signed-off-by: Sihui Han <sihan@microsoft.com>

jleveque mentioned this pull request Mar 29, 2017

portstat fails sonic-net/SONiC#60

Closed

stcheng merged commit 5c59ec6 into sonic-net:master Mar 29, 2017

sihuihan88 deleted the database branch March 30, 2017 21:16

This was referenced Apr 7, 2017

The 'portstat' doesn't support 'ce' interface #15

Closed

switch to read counter from database for portstat tool #11

Closed

vdahiya12 pushed a commit to vdahiya12/sonic-utilities that referenced this pull request Jul 23, 2021

[xcvrd] get hwsku from DB instead of from minigraph, avoid to access …

bc23ab0

…none presenct SFP eeprom (sonic-net#19)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Portstat]: Read counters from database #19

[Portstat]: Read counters from database #19

sihuihan88 commented Mar 23, 2017

jleveque Mar 23, 2017 •

edited

Loading

pavel-shirshov Mar 23, 2017

jleveque Mar 23, 2017 •

edited

Loading

pavel-shirshov Mar 24, 2017 •

edited

Loading

jleveque Mar 24, 2017 •

edited

Loading

pavel-shirshov Mar 24, 2017

jleveque Mar 24, 2017 •

edited

Loading

sihuihan88 Mar 24, 2017

jleveque Mar 24, 2017

sihuihan88 Mar 24, 2017

jleveque Mar 23, 2017

pavel-shirshov Mar 23, 2017

jleveque Mar 23, 2017

stcheng Mar 23, 2017

sihuihan88 Mar 24, 2017

jleveque Mar 24, 2017

jleveque Mar 27, 2017 •

edited

Loading

lguohan Mar 27, 2017

lguohan Mar 27, 2017

lguohan Mar 27, 2017

sihuihan88 Mar 28, 2017 •

edited

Loading

lguohan Mar 27, 2017

stcheng Mar 29, 2017

[Portstat]: Read counters from database #19

[Portstat]: Read counters from database #19

Conversation

sihuihan88 commented Mar 23, 2017

jleveque Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleveque Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

pavel-shirshov Mar 24, 2017 • edited Loading

Choose a reason for hiding this comment

jleveque Mar 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleveque Mar 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleveque Mar 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sihuihan88 Mar 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jleveque Mar 23, 2017 •

edited

Loading

jleveque Mar 23, 2017 •

edited

Loading

pavel-shirshov Mar 24, 2017 •

edited

Loading

jleveque Mar 24, 2017 •

edited

Loading

jleveque Mar 24, 2017 •

edited

Loading

jleveque Mar 27, 2017 •

edited

Loading

sihuihan88 Mar 28, 2017 •

edited

Loading