[rfc1213] Interface MIB add l3 vlan interfaces & aggregate rif counters (sonic-net#133)

* [rfc1213] Interface MIB add l3 vlan interfaces
* [rfc1213] aggregate rifcounters on top of l2 counters
* fix issues
* add unittests
* add mock data
* cleanup
* lag rif fixes
* add more unittests
* update test_mibs.py
* fix comments
* update namespace/test_mibs.py unittest
* [rfc1213] add vlan interface admin_status
* only aggregate errors
* add vlan subinterface unitttests
* fix init_sync_d_lag
* update namespace mib test
* add namespace mock table data

Author: mykolaf

src/sonic_ax_impl/mibs/__init__.py

@@ -43,6 +43,20 @@
     "tx4power":     43,
 }
 
+RIF_COUNTERS_AGGR_MAP = {
+    b"SAI_PORT_STAT_IF_IN_OCTETS": b"SAI_ROUTER_INTERFACE_STAT_IN_OCTETS",
+    b"SAI_PORT_STAT_IF_IN_UCAST_PKTS": b"SAI_ROUTER_INTERFACE_STAT_IN_PACKETS",
+    b"SAI_PORT_STAT_IF_IN_ERRORS": b"SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS",
+    b"SAI_PORT_STAT_IF_OUT_OCTETS": b"SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS",
+    b"SAI_PORT_STAT_IF_OUT_UCAST_PKTS": b"SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS",
+    b"SAI_PORT_STAT_IF_OUT_ERRORS": b"SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS"
+}
+
+RIF_DROPS_AGGR_MAP = {
+    b"SAI_PORT_STAT_IF_IN_ERRORS": b"SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS",
+    b"SAI_PORT_STAT_IF_OUT_ERRORS": b"SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS"
+}
+
 # IfIndex to OID multiplier for transceiver
 IFINDEX_SUB_ID_MULTIPLIER = 1000
 
@@ -113,6 +127,14 @@ def if_entry_table(if_name):
     return b'PORT_TABLE:' + if_name
 
 
+def vlan_entry_table(if_name):
+    """
+    :param if_name: given interface to cast.
+    :return: VLAN_TABLE key.
+    """
+    return b'VLAN_TABLE:' + if_name
+
+
 def lag_entry_table(lag_name):
     """
     :param lag_name: given lag to cast.
@@ -244,6 +266,48 @@ def init_sync_d_interface_tables(db_conn):
 
     return if_name_map, if_alias_map, if_id_map, oid_sai_map, oid_name_map
 
+  
+def init_sync_d_rif_tables(db_conn):
+    """
+    Initializes map of RIF SAI oids to port SAI oid.
+    :return: dict
+    """
+    rif_port_map = port_util.get_rif_port_map(db_conn)
+
+    if not rif_port_map:
+        return {}, {}
+    port_rif_map = {port: rif for rif, port in rif_port_map.items()}
+    logger.debug("Rif port map:\n" + pprint.pformat(rif_port_map, indent=2))
+
+    return rif_port_map, port_rif_map
+
+
+def init_sync_d_vlan_tables(db_conn):
+    """
+    Initializes vlan interface maps for SyncD-connected MIB(s).
+    :return: tuple(vlan_name_map, oid_sai_map, oid_name_map)
+    """
+
+    vlan_name_map = port_util.get_vlan_interface_oid_map(db_conn)
+
+    logger.debug("Vlan oid map:\n" + pprint.pformat(vlan_name_map, indent=2))
+
+    # { OID -> sai_id }
+    oid_sai_map = {get_index(if_name): sai_id for sai_id, if_name in vlan_name_map.items()
+                   # only map the interface if it's a style understood to be a SONiC interface.
+                   if get_index(if_name) is not None}
+    logger.debug("OID sai map:\n" + pprint.pformat(oid_sai_map, indent=2))
+
+    # { OID -> if_name (SONiC) }
+    oid_name_map = {get_index(if_name): if_name for sai_id, if_name in vlan_name_map.items()
+                   # only map the interface if it's a style understood to be a SONiC interface.
+                   if get_index(if_name) is not None}
+
+    logger.debug("OID name map:\n" + pprint.pformat(oid_name_map, indent=2))
+
+    return vlan_name_map, oid_sai_map, oid_name_map
+
+
 def init_sync_d_lag_tables(db_conn):
     """
     Helper method. Connects to and initializes LAG interface maps for SyncD-connected MIB(s).
@@ -258,13 +322,18 @@ def init_sync_d_lag_tables(db_conn):
     if_name_lag_name_map = {}
     # { OID -> lag_name (SONiC) }
     oid_lag_name_map = {}
+    # { lag_name (SONiC) -> lag_oid (SAI) }
+    lag_sai_map = {}
 
     db_conn.connect(APPL_DB)
-
     lag_entries = db_conn.keys(APPL_DB, b"LAG_TABLE:*")
 
     if not lag_entries:
-        return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map
+        return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map
+
+    db_conn.connect(COUNTERS_DB)
+    lag_sai_map = db_conn.get_all(COUNTERS_DB, b"COUNTERS_LAG_NAME_MAP")
+    lag_sai_map = {name: sai_id.lstrip(b"oid:0x") for name, sai_id in lag_sai_map.items()}
 
     for lag_entry in lag_entries:
         lag_name = lag_entry[len(b"LAG_TABLE:"):]
@@ -286,7 +355,7 @@ def member_name_str(val, lag_name):
         if idx:
             oid_lag_name_map[idx] = if_name
 
-    return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map
+    return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map
 
 def init_sync_d_queue_tables(db_conn):
     """
@@ -540,6 +609,7 @@ def init_namespace_sync_d_lag_tables(dbs):
         lag_name_if_name_map = {}
         if_name_lag_name_map = {}
         oid_lag_name_map = {}
+        lag_sai_map = {}
 
         """
         all_ns_db - will have db_conn to all namespace DBs and
@@ -550,12 +620,43 @@ def init_namespace_sync_d_lag_tables(dbs):
         for db_conn in Namespace.get_non_host_dbs(dbs):
             lag_name_if_name_map_ns, \
             if_name_lag_name_map_ns, \
-            oid_lag_name_map_ns = init_sync_d_lag_tables(db_conn)
+            oid_lag_name_map_ns, \
+            lag_sai_map_ns = init_sync_d_lag_tables(db_conn)
             lag_name_if_name_map.update(lag_name_if_name_map_ns)
             if_name_lag_name_map.update(if_name_lag_name_map_ns)
             oid_lag_name_map.update(oid_lag_name_map_ns)
+            lag_sai_map.update(lag_sai_map_ns)
+
+        return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map
+
+    @staticmethod
+    def init_namespace_sync_d_rif_tables(dbs):
+        rif_port_map = {}
+        port_rif_map = {}
+
+        for db_conn in Namespace.get_non_host_dbs(dbs):
+            rif_port_map_ns, \
+            port_rif_map_ns = init_sync_d_rif_tables(db_conn)
+            rif_port_map.update(rif_port_map_ns)
+            port_rif_map.update(port_rif_map_ns)
+
+        return rif_port_map, port_rif_map
+
+    @staticmethod
+    def init_namespace_sync_d_vlan_tables(dbs):
+        vlan_name_map = {}
+        oid_sai_map = {}
+        oid_name_map = {}
+
+        for db_conn in Namespace.get_non_host_dbs(dbs):
+            vlan_name_map_ns, \
+            oid_sai_map_ns, \
+            oid_name_map_ns = init_sync_d_vlan_tables(db_conn)
+            vlan_name_map.update(vlan_name_map_ns)
+            oid_sai_map.update(oid_sai_map_ns)
+            oid_name_map.update(oid_name_map_ns)
 
-        return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map
+        return vlan_name_map, oid_sai_map, oid_name_map
 
     @staticmethod
     def init_namespace_sync_d_queue_tables(dbs):

src/sonic_ax_impl/mibs/ietf/rfc1213.py

@@ -4,6 +4,7 @@
 from bisect import bisect_right
 
 from sonic_ax_impl import mibs
+from sonic_ax_impl import logger
 from sonic_ax_impl.mibs import Namespace
 from ax_interface.mib import MIBMeta, ValueType, MIBUpdater, MIBEntry, SubtreeMIBEntry, OverlayAdpaterMIBEntry, OidMIBEntry
 from ax_interface.encodings import ObjectIdentifier
@@ -49,7 +50,8 @@ class DbTables(int, Enum):
 class IfTypes(int, Enum):
     """ IANA ifTypes """
     ethernetCsmacd = 6
-    ieee8023adLag = 161
+    l3ipvlan       = 136
+    ieee8023adLag  = 161
 
 class ArpUpdater(MIBUpdater):
     def __init__(self):
@@ -157,8 +159,13 @@ def __init__(self):
         self.lag_name_if_name_map = {}
         self.if_name_lag_name_map = {}
         self.oid_lag_name_map = {}
+        self.lag_sai_map = {}
         self.mgmt_oid_name_map = {}
         self.mgmt_alias_map = {}
+        self.vlan_oid_name_map = {}
+        self.vlan_name_map = {}
+        self.rif_port_map = {}
+        self.port_rif_map = {}
 
         # cache of interface counters
         self.if_counters = {}
@@ -168,6 +175,7 @@ def __init__(self):
         self.if_id_map = {}
         self.oid_sai_map = {}
         self.oid_name_map = {}
+        self.rif_counters = {}
 
     def reinit_data(self):
         """
@@ -186,6 +194,13 @@ def reinit_data(self):
         self.mgmt_oid_name_map, \
         self.mgmt_alias_map = mibs.init_mgmt_interface_tables(self.db_conn[0])
 
+        self.vlan_name_map, \
+        self.vlan_oid_sai_map, \
+        self.vlan_oid_name_map = Namespace.init_namespace_sync_d_vlan_tables(self.db_conn)
+
+        self.rif_port_map, \
+        self.port_rif_map = Namespace.init_namespace_sync_d_rif_tables(self.db_conn)
+
     def update_data(self):
         """
         Update redis (caches config)
@@ -195,13 +210,24 @@ def update_data(self):
             {sai_id: Namespace.dbs_get_all(self.db_conn, mibs.COUNTERS_DB, mibs.counter_table(sai_id), blocking=True)
             for sai_id in self.if_id_map}
 
+        rif_sai_ids = list(self.rif_port_map) + list(self.vlan_name_map)
+
+        self.rif_counters = \
+            {sai_id: Namespace.dbs_get_all(self.db_conn, mibs.COUNTERS_DB, mibs.counter_table(sai_id), blocking=True)
+             for sai_id in rif_sai_ids}
+
+        if self.rif_counters: 
+            self.aggregate_counters()
+
         self.lag_name_if_name_map, \
         self.if_name_lag_name_map, \
-        self.oid_lag_name_map = Namespace.init_namespace_sync_d_lag_tables(self.db_conn)
+        self.oid_lag_name_map, \
+        self.lag_sai_map = Namespace.init_namespace_sync_d_lag_tables(self.db_conn)
 
         self.if_range = sorted(list(self.oid_sai_map.keys()) +
                                list(self.oid_lag_name_map.keys()) +
-                               list(self.mgmt_oid_name_map.keys()))
+                               list(self.mgmt_oid_name_map.keys()) +
+                               list(self.vlan_oid_name_map.keys()))
         self.if_range = [(i,) for i in self.if_range]
 
     def get_next(self, sub_id):
@@ -245,6 +271,8 @@ def interface_description(self, sub_id):
             return self.oid_lag_name_map[oid]
         elif oid in self.mgmt_oid_name_map:
             return self.mgmt_alias_map[self.mgmt_oid_name_map[oid]]
+        elif oid in self.vlan_oid_name_map:
+            return self.vlan_oid_name_map[oid]
 
         return self.if_alias_map[self.oid_name_map[oid]]
 
@@ -254,7 +282,13 @@ def _get_counter(self, oid, table_name):
         :param table_name: the redis table (either IntEnum or string literal) to query.
         :return: the counter for the respective sub_id/table.
         """
-        sai_id = self.oid_sai_map[oid]
+        sai_id = ''
+        if oid in self.oid_sai_map:
+            sai_id = self.oid_sai_map[oid]
+        elif oid in self.vlan_oid_sai_map:
+            sai_id = self.vlan_oid_sai_map[oid]
+        else:
+            logger.warning("Unexpected oid {}".format(oid))
         # Enum.name or table_name = 'name_of_the_table'
         _table_name = bytes(getattr(table_name, 'name', table_name), 'utf-8')
 
@@ -268,6 +302,29 @@ def _get_counter(self, oid, table_name):
             mibs.logger.warning("SyncD 'COUNTERS_DB' missing attribute '{}'.".format(e))
             return None
 
+    def aggregate_counters(self):
+        """
+        For ports with l3 router interfaces l3 drops may be counted separately (RIF counters)
+        add l3 drops to l2 drop counters cache according to mapping
+
+        For l3vlan map l3 counters to l2 counters
+        """
+        for rif_sai_id, port_sai_id in self.rif_port_map.items():
+            if port_sai_id in self.if_id_map:
+                for port_counter_name, rif_counter_name in mibs.RIF_DROPS_AGGR_MAP.items():
+                    self.if_counters[port_sai_id][port_counter_name] = \
+                    int(self.if_counters[port_sai_id][port_counter_name]) + \
+                    int(self.rif_counters[rif_sai_id][rif_counter_name])
+
+        for vlan_sai_id in self.vlan_name_map:
+            for port_counter_name, rif_counter_name in mibs.RIF_COUNTERS_AGGR_MAP.items():
+                try:
+                    self.if_counters.setdefault(vlan_sai_id, {})
+                    self.if_counters[vlan_sai_id][port_counter_name] = \
+                    int(self.rif_counters[vlan_sai_id][rif_counter_name])
+                except KeyError as e:
+                    logger.warning("Not able to aggregate counters for {}: {}\n {}".format(vlan_sai_id, rif_counter_name, e))
+
     def get_counter(self, sub_id, table_name):
         """
         :param sub_id: The 1-based sub-identifier query.
@@ -287,7 +344,13 @@ def get_counter(self, sub_id, table_name):
             counter_value = 0
             for lag_member in self.lag_name_if_name_map[self.oid_lag_name_map[oid]]:
                 counter_value += self._get_counter(mibs.get_index(lag_member), table_name)
-
+            sai_lag_id = self.lag_sai_map[self.oid_lag_name_map[oid]]
+            sai_lag_rif_id = self.port_rif_map[sai_lag_id]
+            if sai_lag_rif_id in self.rif_port_map:
+                table_name = bytes(getattr(table_name, 'name', table_name), 'utf-8')
+                if table_name in mibs.RIF_DROPS_AGGR_MAP:
+                    rif_table_name = mibs.RIF_DROPS_AGGR_MAP[table_name]
+                    counter_value += int(self.rif_counters[sai_lag_rif_id][rif_table_name])
             # truncate to 32-bit counter
             return counter_value & 0x00000000ffffffff
         else:
@@ -317,6 +380,8 @@ def _get_if_entry(self, sub_id):
         elif oid in self.mgmt_oid_name_map:
             if_table = mibs.mgmt_if_entry_table(self.mgmt_oid_name_map[oid])
             db = mibs.CONFIG_DB
+        elif oid in self.vlan_oid_name_map:
+            if_table = mibs.vlan_entry_table(self.vlan_oid_name_map[oid])
         elif oid in self.oid_name_map:
             if_table = mibs.if_entry_table(self.oid_name_map[oid])
         else:
@@ -421,6 +486,7 @@ def get_if_type(self, sub_id):
 
         ethernetCsmacd(6), -- for all ethernet-like interfaces,
                            -- regardless of speed, as per RFC3635
+        l3ipvlan(136)      -- Layer 3 Virtual LAN using IP 
         ieee8023adLag(161) -- IEEE 802.3ad Link Aggregate
         """
         oid = self.get_oid(sub_id)
@@ -429,6 +495,8 @@ def get_if_type(self, sub_id):
 
         if oid in self.oid_lag_name_map:
             return IfTypes.ieee8023adLag
+        elif oid in self.vlan_oid_name_map:
+            return IfTypes.l3ipvlan
         else:
             return IfTypes.ethernetCsmacd
 

src/sonic_ax_impl/mibs/ietf/rfc2863.py

@@ -79,7 +79,7 @@ def reinit_data(self):
 
         self.lag_name_if_name_map, \
         self.if_name_lag_name_map, \
-        self.oid_lag_name_map = Namespace.init_namespace_sync_d_lag_tables(self.db_conn)
+        self.oid_lag_name_map, _ = Namespace.init_namespace_sync_d_lag_tables(self.db_conn)
         """
         db_conn - will have db_conn to all namespace DBs and
         global db. First db in the list is global db.

src/sonic_ax_impl/mibs/vendor/cisco/ciscoPfcExtMIB.py

@@ -51,7 +51,7 @@ def update_data(self):
 
         self.lag_name_if_name_map, \
         self.if_name_lag_name_map, \
-        self.oid_lag_name_map = Namespace.init_namespace_sync_d_lag_tables(self.db_conn)
+        self.oid_lag_name_map, _ = Namespace.init_namespace_sync_d_lag_tables(self.db_conn)
 
         self.if_range = sorted(list(self.oid_sai_map.keys()) + list(self.oid_lag_name_map.keys()))
         self.if_range = [(i,) for i in self.if_range]

tests/mock_tables/asic0/counters_db.json

@@ -314,6 +314,12 @@
   "COUNTERS_LAG_NAME_MAP": {
     "PortChannel01": "oid:0x1000000000007"
   },
+  "COUNTERS_RIF_NAME_MAP": {
+    "PortChannel01": "oid:0x6000000000006"
+  },
+  "COUNTERS_RIF_TYPE_MAP": {
+    "oid:0x6000000000006": "SAI_ROUTER_INTERFACE_TYPE_PORT"
+  },
   "COUNTERS:oid:0x1000000000004": {
     "SAI_PORT_STAT_ETHER_STATS_TX_NO_ERRORS": "0",
     "SAI_PORT_STAT_ETHER_STATS_OVERSIZE_PKTS": "0",

tests/mock_tables/asic1/counters_db.json

@@ -110,6 +110,12 @@
   "COUNTERS_LAG_NAME_MAP": {
     "PortChannel02": "oid:0x1000000000012"
   },
+  "COUNTERS_RIF_TYPE_MAP": {
+    "oid:0x6000000000012": "SAI_ROUTER_INTERFACE_TYPE_PORT"
+  },
+  "COUNTERS_RIF_NAME_MAP": {
+    "PortChannel02": "oid:0x6000000000012"
+  },
   "COUNTERS:oid:0x1000000000009": {
     "SAI_PORT_STAT_ETHER_STATS_TX_NO_ERRORS": "0",
     "SAI_PORT_STAT_ETHER_STATS_OVERSIZE_PKTS": "0",