Restored snmp vlan support per RFC1213 and added the missing support …

…for RFC2863 (sonic-net#218)

- What I did

Restored snmp vlan support per RFC1213, which had been reverted due to inconsistency with RFC2863, and added the missing support for RFC2863. Added unit tests for RFC2863.

- How I did it

Reverted sonic-net/sonic-snmpagent#191, which was itself a revert of sonic-net/sonic-snmpagent#169. Then added code to support vlan in rfc2863.py and unit tests.

Since a long time has elapsed since the original commit and other code has been pushed in the meantime, great care is needed in merging. Note in particular that mibs.init_sync_d_lag_tables now returns five parameters, the last two of which were added: lag_sai_map and sai_lap_map. This PR needs one of those maps, and a concurrent commit supporting RFC4363 needs the other, so all the callers were updated and use the one they need.

- How to verify it

Unit tests are run via make target/python-wheels/asyncsnmp-2.1.0-py3-none-any.whl.
See also Azure#169.

- Description for the changelog

Restored support for aggregated router interface counters and L3 VLAN counters to RFC1213 MIB, and extended to RFC2863.

src/sonic_ax_impl/mibs/__init__.py

@@ -27,6 +27,20 @@
 
 HOST_NAMESPACE_DB_IDX = 0
 
+RIF_COUNTERS_AGGR_MAP = {
+    "SAI_PORT_STAT_IF_IN_OCTETS": "SAI_ROUTER_INTERFACE_STAT_IN_OCTETS",
+    "SAI_PORT_STAT_IF_IN_UCAST_PKTS": "SAI_ROUTER_INTERFACE_STAT_IN_PACKETS",
+    "SAI_PORT_STAT_IF_IN_ERRORS": "SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS",
+    "SAI_PORT_STAT_IF_OUT_OCTETS": "SAI_ROUTER_INTERFACE_STAT_OUT_OCTETS",
+    "SAI_PORT_STAT_IF_OUT_UCAST_PKTS": "SAI_ROUTER_INTERFACE_STAT_OUT_PACKETS",
+    "SAI_PORT_STAT_IF_OUT_ERRORS": "SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS"
+}
+
+RIF_DROPS_AGGR_MAP = {
+    "SAI_PORT_STAT_IF_IN_ERRORS": "SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS",
+    "SAI_PORT_STAT_IF_OUT_ERRORS": "SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS"
+}
+
 redis_kwargs = {'unix_socket_path': '/var/run/redis/redis.sock'}
 
 
@@ -136,6 +150,14 @@ def if_entry_table(if_name):
     return 'PORT_TABLE:' + if_name
 
 
+def vlan_entry_table(if_name):
+    """
+    :param if_name: given interface to cast.
+    :return: VLAN_TABLE key.
+    """
+    return 'VLAN_TABLE:' + if_name
+
+
 def lag_entry_table(lag_name):
     """
     :param lag_name: given lag to cast.
@@ -292,11 +314,52 @@ def init_sync_d_interface_tables(db_conn):
 
     return if_name_map, if_alias_map, if_id_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 = {get_sai_id_key(db_conn.namespace, rif): get_sai_id_key(db_conn.namespace, port)
+                    for rif, port in port_util.get_rif_port_map(db_conn).items()}
+    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_map = {}
+    oid_name_map = {}
+    for sai_id, if_name in vlan_name_map.items():
+        port_index = get_index_from_str(if_name)
+        if not port_index:
+            continue
+        # { OID -> sai_id }
+        oid_sai_map[port_index] = sai_id
+        # { OID -> if_name (SONiC) }
+        oid_name_map[port_index] = if_name
+
+    logger.debug("OID sai map:\n" + pprint.pformat(oid_sai_map, indent=2))
+    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).
     :param db_conn: database connector
-    :return: tuple(lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map)
+    :return: tuple(lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map, sai_lag_map)
     """
     # { lag_name (SONiC) -> [ lag_members (if_name) ] }
     # ex: { "PortChannel0" : [ "Ethernet0", "Ethernet4" ] }
@@ -316,7 +379,7 @@ def init_sync_d_lag_tables(db_conn):
     lag_entries = db_conn.keys(APPL_DB, "LAG_TABLE:*")
 
     if not lag_entries:
-        return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map
+        return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map, sai_lag_map
 
     db_conn.connect(COUNTERS_DB)
     lag_sai_map = db_conn.get_all(COUNTERS_DB, "COUNTERS_LAG_NAME_MAP")
@@ -345,7 +408,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, sai_lag_map
+    return lag_name_if_name_map, if_name_lag_name_map, oid_lag_name_map, lag_sai_map, sai_lag_map
 
 def init_sync_d_queue_tables(db_conn):
     """
@@ -360,7 +423,7 @@ def init_sync_d_queue_tables(db_conn):
 
     # Parse the queue_name_map and create the following maps:
     # port_queues_map -> {"port_index : queue_index" : sai_oid}
-    # queue_stat_map -> {"port_index : queue stat table name" : {counter name : value}} 
+    # queue_stat_map -> {"port_index : queue stat table name" : {counter name : value}}
     # port_queue_list_map -> {port_index: [sorted queue list]}
     port_queues_map = {}
     queue_stat_map = {}
@@ -424,7 +487,7 @@ class RedisOidTreeUpdater(MIBUpdater):
     def __init__(self, prefix_str):
         super().__init__()
 
-        self.db_conn = Namespace.init_namespace_dbs() 
+        self.db_conn = Namespace.init_namespace_dbs()
         if prefix_str.startswith('.'):
             prefix_str = prefix_str[1:]
         self.prefix_str = prefix_str
@@ -537,7 +600,7 @@ def dbs_get_all(dbs, db_name, _hash, *args, **kwargs):
         db get_all function executed on global and all namespace DBs.
         """
         result = {}
-        # If there are multiple namespaces, _hash might not be 
+        # If there are multiple namespaces, _hash might not be
         # present in all namespace, ignore if not present in a
         # specfic namespace.
         if len(dbs) > 1:

src/sonic_ax_impl/mibs/ietf/rfc1213.py

@@ -50,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):
@@ -192,8 +193,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 = {}
@@ -202,6 +208,8 @@ def __init__(self):
         self.if_alias_map = {}
         self.if_id_map = {}
         self.oid_name_map = {}
+        self.rif_counters = {}
+
         self.namespace_db_map = Namespace.get_namespace_db_map(self.db_conn)
 
     def reinit_data(self):
@@ -220,26 +228,56 @@ 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.get_sync_d_from_all_namespace(mibs.init_sync_d_vlan_tables, self.db_conn)
+
+        self.rif_port_map, \
+        self.port_rif_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_rif_tables, self.db_conn)
+
     def update_data(self):
         """
         Update redis (caches config)
         Pulls the table references for each interface.
         """
-        for sai_id_key in self.if_id_map:
-            namespace, sai_id = mibs.split_sai_id_key(sai_id_key)
-            if_idx = mibs.get_index_from_str(self.if_id_map[sai_id_key])
-            self.if_counters[if_idx] = self.namespace_db_map[namespace].get_all(mibs.COUNTERS_DB, \
-                    mibs.counter_table(sai_id), blocking=True)
+
+        self.update_if_counters()
+        self.update_rif_counters()
+
+        self.aggregate_counters()
 
         self.lag_name_if_name_map, \
         self.if_name_lag_name_map, \
-        self.oid_lag_name_map, _ = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_lag_tables, self.db_conn)
+        self.oid_lag_name_map, \
+        self.lag_sai_map, self.sai_lag_map = Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_lag_tables, self.db_conn)
 
         self.if_range = sorted(list(self.oid_name_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 update_if_counters(self):
+        for sai_id_key in self.if_id_map:
+            namespace, sai_id = mibs.split_sai_id_key(sai_id_key)
+            if_idx = mibs.get_index_from_str(self.if_id_map[sai_id_key])
+            counters_db_data = self.namespace_db_map[namespace].get_all(mibs.COUNTERS_DB,
+                                                                        mibs.counter_table(sai_id),
+                                                                        blocking=True)
+            self.if_counters[if_idx] = {
+                counter: int(value) for counter, value in counters_db_data.items()
+            }
+
+    def update_rif_counters(self):
+        rif_sai_ids = list(self.rif_port_map) + list(self.vlan_name_map)
+        for sai_id in rif_sai_ids:
+            counters_db_data = Namespace.dbs_get_all(self.db_conn, mibs.COUNTERS_DB,
+                                                     mibs.counter_table(mibs.split_sai_id_key(sai_id)[1]),
+                                                     blocking=False)
+            self.rif_counters[sai_id] = {
+                counter: int(value) for counter, value in counters_db_data.items()
+            }
+
     def get_next(self, sub_id):
         """
         :param sub_id: The 1-based sub-identifier query.
@@ -281,6 +319,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]]
 
@@ -291,18 +331,46 @@ def _get_counter(self, oid, table_name):
         :return: the counter for the respective sub_id/table.
         """
         # Enum.name or table_name = 'name_of_the_table'
+        # Example: 
+        # table_name = <DbTables.SAI_PORT_STAT_IF_OUT_ERRORS: 20>
+        # _table_name = 'SAI_PORT_STAT_IF_OUT_ERRORS'
         _table_name = getattr(table_name, 'name', table_name)
 
         try:
             counter_value = self.if_counters[oid][_table_name]
             # truncate to 32-bit counter (database implements 64-bit counters)
-            counter_value = int(counter_value) & 0x00000000ffffffff
+            counter_value = counter_value & 0x00000000ffffffff
             # done!
             return counter_value
         except KeyError as e:
             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:
+                port_idx = mibs.get_index_from_str(self.if_id_map[port_sai_id])
+                for port_counter_name, rif_counter_name in mibs.RIF_DROPS_AGGR_MAP.items():
+                    self.if_counters[port_idx][port_counter_name] = \
+                    self.if_counters[port_idx][port_counter_name] + \
+                    self.rif_counters[rif_sai_id][rif_counter_name]
+
+        for vlan_sai_id, vlan_name in self.vlan_name_map.items():
+            for port_counter_name, rif_counter_name in mibs.RIF_COUNTERS_AGGR_MAP.items():
+                vlan_idx = mibs.get_index_from_str(vlan_name)
+                vlan_rif_counters = self.rif_counters[vlan_sai_id]
+                if rif_counter_name in vlan_rif_counters:
+                    self.if_counters.setdefault(vlan_idx, {})
+                    self.if_counters[vlan_idx][port_counter_name] = \
+                        vlan_rif_counters[rif_counter_name]
+
+
     def get_counter(self, sub_id, table_name):
         """
         :param sub_id: The 1-based sub-identifier query.
@@ -320,9 +388,40 @@ def get_counter(self, sub_id, table_name):
             return 0
         elif oid in self.oid_lag_name_map:
             counter_value = 0
+            # Sum the values of this counter for all ports in the LAG.
+            # Example: 
+            # table_name = <DbTables.SAI_PORT_STAT_IF_OUT_ERRORS: 20>
+            # oid = 1001
+            # self.oid_lag_name_map = {1001: 'PortChannel01', 1002: 'PortChannel02', 1003: 'PortChannel03'}
+            # self.oid_lag_name_map[oid] = 'PortChannel01'
+            # self.lag_name_if_name_map = {'PortChannel01': ['Ethernet112'], 'PortChannel02': ['Ethernet116'], 'PortChannel03': ['Ethernet120']}
+            # self.lag_name_if_name_map['PortChannel01'] = ['Ethernet112']
+            # mibs.get_index_from_str('Ethernet112') = 113 (because Ethernet N = N + 1)
+            # self._get_counter retrieves the counter per oid and table.
             for lag_member in self.lag_name_if_name_map[self.oid_lag_name_map[oid]]:
                 counter_value += self._get_counter(mibs.get_index_from_str(lag_member), table_name)
-
+            # Check if we need to add a router interface count.
+            # Example:
+            # self.lag_sai_map = {'PortChannel01': '2000000000006', 'PortChannel02': '2000000000005', 'PortChannel03': '2000000000004'}
+            # self.port_rif_map = {'2000000000006': '6000000000006', '2000000000005': '6000000000005', '2000000000004': '6000000000004'}
+            # self.rif_port_map = {'6000000000006': '2000000000006', '6000000000005': '2000000000005', '6000000000004': '2000000000004'}
+            # self.lag_sai_map['PortChannel01'] = '2000000000006'
+            # self.port_rif_map['2000000000006'] = '6000000000006'
+            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:
+                # Extract the 'name' part of 'table_name'.
+                # Example: 
+                # table_name = <DbTables.SAI_PORT_STAT_IF_OUT_ERRORS: 20>
+                # _table_name = 'SAI_PORT_STAT_IF_OUT_ERRORS'
+                table_name = getattr(table_name, 'name', table_name)
+                # Find rif counter table if applicable and add the count for this table.
+                # Example:
+                # mibs.RIF_DROPS_AGGR_MAP = {'SAI_PORT_STAT_IF_IN_ERRORS': 'SAI_ROUTER_INTERFACE_STAT_IN_ERROR_PACKETS', 'SAI_PORT_STAT_IF_OUT_ERRORS': 'SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS'}
+                # self.rif_counters['6000000000006'] = {'SAI_ROUTER_INTERFACE_STAT_IN_PACKETS': 6, ... 'SAI_ROUTER_INTERFACE_STAT_OUT_ERROR_PACKETS': 6, ...} 
+                if table_name in mibs.RIF_DROPS_AGGR_MAP:
+                    rif_table_name = mibs.RIF_DROPS_AGGR_MAP[table_name]
+                    counter_value += self.rif_counters[sai_lag_rif_id].get(rif_table_name, 0)
             # truncate to 32-bit counter
             return counter_value & 0x00000000ffffffff
         else:
@@ -352,6 +451,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:
@@ -456,6 +557,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)
@@ -464,6 +566,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