[snmpagent] Fix hardcoded qsfp lane count by reading sensor status from DB (#184)

**- What I did**

The current snmpagent implementation hardcoded qsfp lanes to 4, but qsfp-dd has 8 lanes. In that case snmp query only display the sensors of first 4 lanes. This PR is to fix it. 

**- How I did it**

1. read actual sensor status from redis db, now we only care temperature, voltage, txpower, rxpower, txbias.
2. sort sensor data to make sure it has a solid order
3. store the data to snmpagent data structure

**- How to verify it**

Manual test on master and run existing regression

Author: Junchao-Mellanox

src/sonic_ax_impl/mibs/ietf/physical_entity_sub_oid_generator.py

@@ -80,24 +80,6 @@
 CHASSIS_SUB_ID = 1
 CHASSIS_MGMT_SUB_ID = MODULE_TYPE_MGMT
 
-# This is used in both rfc2737 and rfc3433
-XCVR_SENSOR_PART_ID_MAP = {
-    "temperature":  SENSOR_TYPE_TEMP,
-    "tx1power":     SENSOR_TYPE_PORT_TX_POWER + 1,
-    "tx2power":     SENSOR_TYPE_PORT_TX_POWER + 2,
-    "tx3power":     SENSOR_TYPE_PORT_TX_POWER + 3,
-    "tx4power":     SENSOR_TYPE_PORT_TX_POWER + 4,
-    "rx1power":     SENSOR_TYPE_PORT_RX_POWER + 1,
-    "rx2power":     SENSOR_TYPE_PORT_RX_POWER + 2,
-    "rx3power":     SENSOR_TYPE_PORT_RX_POWER + 3,
-    "rx4power":     SENSOR_TYPE_PORT_RX_POWER + 4,
-    "tx1bias":      SENSOR_TYPE_PORT_TX_BIAS + 1,
-    "tx2bias":      SENSOR_TYPE_PORT_TX_BIAS + 2,
-    "tx3bias":      SENSOR_TYPE_PORT_TX_BIAS + 3,
-    "tx4bias":      SENSOR_TYPE_PORT_TX_BIAS + 4,
-    "voltage":      SENSOR_TYPE_VOLTAGE,
-}
-
 PSU_SENSOR_PART_ID_MAP = {
     'temperature': SENSOR_TYPE_TEMP,
     'power': SENSOR_TYPE_POWER,
@@ -175,14 +157,14 @@ def get_transceiver_sub_id(ifindex):
     """
     return (MODULE_TYPE_PORT + ifindex * PORT_IFINDEX_MULTIPLE, )
 
-def get_transceiver_sensor_sub_id(ifindex, sensor):
+def get_transceiver_sensor_sub_id(ifindex, offset):
     """
     Returns sub OID for transceiver sensor. Sub OID is calculated as folows:
-    sub OID = transceiver_oid + XCVR_SENSOR_PART_ID_MAP[sensor]
+    sub OID = transceiver_oid + offset
     :param ifindex: interface index
-    :param sensor: sensor key
+    :param offset: sensor OID offset
     :return: sub OID = {{index}} * 1000 + {{lane}} * 10 + sensor id
     """
 
     transceiver_oid, = get_transceiver_sub_id(ifindex)
-    return (transceiver_oid + XCVR_SENSOR_PART_ID_MAP[sensor],)
+    return (transceiver_oid + offset,)

src/sonic_ax_impl/mibs/ietf/rfc2737.py

@@ -21,6 +21,7 @@
 from .physical_entity_sub_oid_generator import get_psu_sensor_sub_id
 from .physical_entity_sub_oid_generator import get_transceiver_sub_id
 from .physical_entity_sub_oid_generator import get_transceiver_sensor_sub_id
+from .transceiver_sensor_data import TransceiverSensorData
 
 
 @unique
@@ -126,43 +127,7 @@ class ThermalInfoDB(str, Enum):
     'voltage'    : 4
 }
 
-# Map used to generate transceiver sensor description
-XCVR_SENSOR_NAME_MAP = {
-    "temperature" : "Temperature",
-    "voltage"     : "Voltage",
-    "rx1power"    : "RX Power",
-    "rx2power"    : "RX Power",
-    "rx3power"    : "RX Power",
-    "rx4power"    : "RX Power",
-    "tx1bias"     : "TX Bias",
-    "tx2bias"     : "TX Bias",
-    "tx3bias"     : "TX Bias",
-    "tx4bias"     : "TX Bias",
-    "tx1power"    : "TX Power",
-    "tx2power"    : "TX Power",
-    "tx3power"    : "TX Power",
-    "tx4power"    : "TX Power",
-}
-
-XCVR_SENSOR_INDEX_MAP = {
-    "temperature" : 1,
-    "tx1power"    : 2,
-    "tx2power"    : 3,
-    "tx3power"    : 4,
-    "tx4power"    : 5,
-    "rx1power"    : 6,
-    "rx2power"    : 7,
-    "rx3power"    : 8,
-    "rx4power"    : 9,
-    "tx1bias"     : 10,
-    "tx2bias"     : 11,
-    "tx3bias"     : 12,
-    "tx4bias"     : 13,
-    "voltage"     : 14,
-}
-
 NOT_AVAILABLE = 'N/A'
-QSFP_LANES = (1, 2, 3, 4)
 
 def is_null_str(value):
     """
@@ -195,29 +160,20 @@ def get_transceiver_description(sfp_type, if_alias):
 
     return "{} for {}".format(sfp_type, if_alias)
 
-def get_transceiver_sensor_description(sensor, if_alias):
+
+def get_transceiver_sensor_description(name, lane_number, if_alias):
     """
-    :param sensor: sensor key name
+    :param name: sensor name
+    :param lane_number: lane number of this sensor
     :param if_alias: interface alias
     :return: description string about sensor
     """
-
-    # assume sensors that is per channel in transceiver port
-    # has digit equals to channel number in the sensor's key name in DB
-    # e.g. rx3power (lane 3)
-    lane_number = list(filter(lambda c: c.isdigit(), sensor))
-
-    if len(lane_number) == 0:
+    if lane_number == 0:
         port_name = if_alias
-    elif len(lane_number) == 1 and int(lane_number[0]) in QSFP_LANES:
-        port_name = "{}/{}".format(if_alias, lane_number[0])
     else:
-        mibs.logger.warning("Tried to parse lane number from sensor name - {} ".format(sensor)
-                + "but parsed value is not a valid QSFP lane number")
-        # continue as with non per channel sensor
-        port_name = if_alias
+        port_name = "{}/{}".format(if_alias, lane_number)
 
-    return "DOM {} Sensor for {}".format(XCVR_SENSOR_NAME_MAP[sensor], port_name)
+    return "DOM {} Sensor for {}".format(name, port_name)
 
 
 class Callback(object):
@@ -845,19 +801,17 @@ def _update_transceiver_sensor_cache(self, interface, sub_id):
         if not transceiver_dom_entry:
             return
 
-        # go over transceiver sensors
-        for sensor in transceiver_dom_entry:
-            if sensor not in XCVR_SENSOR_NAME_MAP:
-                continue
-            sensor_sub_id = get_transceiver_sensor_sub_id(ifindex, sensor)
+        sensor_data_list = TransceiverSensorData.create_sensor_data(transceiver_dom_entry)
+        sensor_data_list = TransceiverSensorData.sort_sensor_data(sensor_data_list)
+        for index, sensor_data in enumerate(sensor_data_list):
+            sensor_sub_id = get_transceiver_sensor_sub_id(ifindex, sensor_data.get_oid_offset())
             self._add_entity_related_oid(interface, sensor_sub_id)
-            sensor_description = get_transceiver_sensor_description(sensor, ifalias)
-
             self.mib_updater.set_phy_class(sensor_sub_id, PhysicalClass.SENSOR)
+            sensor_description = get_transceiver_sensor_description(sensor_data.get_name(), sensor_data.get_lane_number(), ifalias)
             self.mib_updater.set_phy_descr(sensor_sub_id, sensor_description)
             self.mib_updater.set_phy_name(sensor_sub_id, sensor_description)
             self.mib_updater.set_phy_contained_in(sensor_sub_id, sub_id)
-            self.mib_updater.set_phy_parent_relative_pos(sensor_sub_id, XCVR_SENSOR_INDEX_MAP[sensor])
+            self.mib_updater.set_phy_parent_relative_pos(sensor_sub_id, index + 1)
             self.mib_updater.set_phy_fru(sensor_sub_id, False)
             # add to available OIDs list
             self.mib_updater.add_sub_id(sensor_sub_id)

src/sonic_ax_impl/mibs/ietf/rfc3433.py

@@ -11,6 +11,8 @@
 from sonic_ax_impl.mibs import Namespace
 
 from .physical_entity_sub_oid_generator import get_transceiver_sensor_sub_id
+from .transceiver_sensor_data import TransceiverSensorData
+
 
 @unique
 class EntitySensorDataType(int, Enum):
@@ -208,34 +210,13 @@ class XcvrTxPowerSensor(SensorInterface):
     CONVERTER = Converters.CONV_dBm_mW
 
 
-# mapping between DB key and Sensor object
-TRANSCEIVER_SENSOR_MAP = {
-    "temperature": XcvrTempSensor,
-    "voltage":     XcvrVoltageSensor,
-    "rx1power":    XcvrRxPowerSensor,
-    "rx2power":    XcvrRxPowerSensor,
-    "rx3power":    XcvrRxPowerSensor,
-    "rx4power":    XcvrRxPowerSensor,
-    "tx1bias":     XcvrTxBiasSensor,
-    "tx2bias":     XcvrTxBiasSensor,
-    "tx3bias":     XcvrTxBiasSensor,
-    "tx4bias":     XcvrTxBiasSensor,
-    "tx1power":    XcvrTxPowerSensor,
-    "tx2power":    XcvrTxPowerSensor,
-    "tx3power":    XcvrTxPowerSensor,
-    "tx4power":    XcvrTxPowerSensor,
-}
-
-
-def get_transceiver_sensor(sensor_key):
-    """
-    Gets transceiver sensor object
-    :param sensor_key: Sensor key from XcvrDomDB
-    :param ifindex: Interface index associated with transceiver
-    :return: Sensor object.
-    """
-
-    return TRANSCEIVER_SENSOR_MAP[sensor_key]
+TransceiverSensorData.bind_sensor_interface({
+    'temperature': XcvrTempSensor,
+    'voltage'    : XcvrVoltageSensor,
+    'rxpower'    : XcvrRxPowerSensor,
+    'txpower'    : XcvrTxPowerSensor,
+    'txbias'     : XcvrTxBiasSensor
+})
 
 
 class PhysicalSensorTableMIBUpdater(MIBUpdater):
@@ -313,14 +294,11 @@ def update_data(self):
             if not transceiver_dom_entry_data:
                 continue
 
-            for sensor_key in transceiver_dom_entry_data:
-                if sensor_key not in TRANSCEIVER_SENSOR_MAP:
-                    continue
-
-                raw_sensor_value = transceiver_dom_entry_data.get(sensor_key)
-
-                sensor = get_transceiver_sensor(sensor_key)
-                sub_id = get_transceiver_sensor_sub_id(ifindex, sensor_key)
+            sensor_data_list = TransceiverSensorData.create_sensor_data(transceiver_dom_entry_data)
+            for sensor_data in sensor_data_list:
+                raw_sensor_value = sensor_data.get_raw_value()
+                sensor = sensor_data.get_sensor_interface()
+                sub_id = get_transceiver_sensor_sub_id(ifindex, sensor_data.get_oid_offset())
 
                 try:
                     mib_values = sensor.mib_values(raw_sensor_value)

src/sonic_ax_impl/mibs/ietf/transceiver_sensor_data.py

@@ -0,0 +1,132 @@
+import re
+
+from .physical_entity_sub_oid_generator import SENSOR_TYPE_TEMP
+from .physical_entity_sub_oid_generator import SENSOR_TYPE_PORT_TX_POWER
+from .physical_entity_sub_oid_generator import SENSOR_TYPE_PORT_RX_POWER
+from .physical_entity_sub_oid_generator import SENSOR_TYPE_PORT_TX_BIAS
+from .physical_entity_sub_oid_generator import SENSOR_TYPE_VOLTAGE
+
+
+class TransceiverSensorData:
+    """
+    Base transceiver sensor data class. Responsible for:
+        1. Manage concrete sensor data class
+        2. Create concrete sensor data instances
+        3. Provide common logic for concrete sensor data class
+    """
+
+    sensor_attr_dict = {
+        'temperature': {
+            'pattern': 'temperature',
+            'name': 'Temperature',
+            'oid_offset_base': SENSOR_TYPE_TEMP,
+            'sort_factor': 0,
+            'lane_based_sensor': False
+        },
+        'voltage': {
+            'pattern': 'voltage',
+            'name': 'Voltage',
+            'oid_offset_base': SENSOR_TYPE_VOLTAGE,
+            'sort_factor': 9000,
+            'lane_based_sensor': False
+        },
+        'rxpower': {
+            'pattern': r'rx(\d+)power',
+            'name': 'RX Power',
+            'oid_offset_base': SENSOR_TYPE_PORT_RX_POWER,
+            'sort_factor': 2000,
+            'lane_based_sensor': True
+        },
+        'txpower': {
+            'pattern': r'tx(\d+)power',
+            'name': 'TX Power',
+            'oid_offset_base': SENSOR_TYPE_PORT_TX_POWER,
+            'sort_factor': 1000,
+            'lane_based_sensor': True
+        },
+        'txbias': {
+            'pattern': r'tx(\d+)bias',
+            'name': 'TX Bias',
+            'oid_offset_base': SENSOR_TYPE_PORT_TX_BIAS,
+            'sort_factor': 3000,
+            'lane_based_sensor': True
+        }
+    }
+
+    def __init__(self, key, value, sensor_attrs, match_result):
+        self._key = key
+        self._value = value
+        self._sensor_attrs = sensor_attrs
+        self._match_result = match_result
+
+    @classmethod
+    def create_sensor_data(cls, sensor_data_dict):
+        """
+        Create sensor data instances according to the sensor data got from redis
+        :param sensor_data_dict: sensor data got from redis
+        :return: A sorted sensor data instance list
+        """
+        sensor_data_list = []
+        for name, value in sensor_data_dict.items():
+            for sensor_attrs in cls.sensor_attr_dict.values():
+                match_result = re.match(sensor_attrs['pattern'], name)
+                if match_result:
+                    sensor_data = TransceiverSensorData(name, value, sensor_attrs, match_result)
+                    sensor_data_list.append(sensor_data)
+
+        return sensor_data_list
+
+    @classmethod
+    def sort_sensor_data(cls, sensor_data_list):
+        return sorted(sensor_data_list, key=lambda x: x.get_sort_factor())
+
+    @classmethod
+    def bind_sensor_interface(cls, sensor_interface_dict):
+        for name, sensor_attrs in cls.sensor_attr_dict.items():
+            if name in sensor_interface_dict:
+                sensor_attrs['sensor_interface'] = sensor_interface_dict[name]
+
+    def get_key(self):
+        """
+        Get the redis key of this sensor
+        """
+        return self._key
+
+    def get_raw_value(self):
+        """
+        Get raw redis value of this sensor
+        """
+        return self._value
+
+    def get_name(self):
+        """
+        Get the name of this sensor. Concrete sensor data class must override
+        this.
+        """
+        return self._sensor_attrs['name']
+
+    def get_sort_factor(self):
+        """
+        Get sort factor for this sensor. Concrete sensor data class must override
+        this.
+        """
+        return self._sensor_attrs['sort_factor'] + self.get_lane_number()
+
+    def get_lane_number(self):
+        """
+        Get lane number of this sensor. For example, some transceivers have more than one rx power sensor, the sub index
+        of rx1power is 1, the sub index of rx2power is 2.
+        """
+        return int(self._match_result.group(1)) if self._sensor_attrs['lane_based_sensor'] else 0
+
+    def get_oid_offset(self):
+        """
+        Get OID offset of this sensor.
+        """
+        return self._sensor_attrs['oid_offset_base'] + self.get_lane_number()
+
+    def get_sensor_interface(self):
+        """
+        Get sensor interface of this sensor. Used by rfc3433.
+        """
+        return self._sensor_attrs['sensor_interface']