[rfc1213] Interface MIB add l3 vlan interfaces & aggregate rif counters by stepanblyschak · Pull Request #169 · sonic-net/sonic-snmpagent

stepanblyschak · 2020-10-16T12:17:50Z

- What I did

Rework based on #133 and feedback #148.

- How I did it

merge [rfc1213] Interface MIB add l3 vlan interfaces & aggregate rif counters #133 with upstream master.
Fix UnavailableDataError when RIF counters are not enabled.
Make RIF keys in maps unique in multi asic env.

- How to verify it

Disable rif counters
restart swss
snmpwalk
Verify no UnavailableDataError in logs
Verify port counters are returned
Enable rif counters
snmpwalk
Verify no error in logs
Verify error in/out counters are aggregated for rif ports and portchannels
Verify VLAN RIF counters are present in the MIB.

- Description for the changelog

Added support for aggregated router interface counters and L3 VLAN counters to RFC1213 MIB.

Signed-off-by: Mykola Faryma <[email protected]>

Fix for the following error in syslog: Oct 9 10:07:21.427210 mts-sonic-dut ERR snmp#snmp-subagent [ax_interface] ERROR: MIBUpdater.start() caught an unexpected exception during update_data()#012Traceback (most recent call last):sonic-net#12 File "/usr/local/lib/python3.7/dist-packa ges/ax_interface/mib.py", line 43, in start#012 self.update_data()sonic-net#12 File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/ietf/rfc1213.py", line 245, in update_data#012 self.update_rif_counters()sonic-net#12 File "/usr/local /lib/python3.7/dist-packages/sonic_ax_impl/mibs/ietf/rfc1213.py", line 271, in update_rif_counters#012 for sai_id in rif_sai_ids}sonic-net#12 File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/ietf/rfc1213.py", line 271, in <dic tcomp>sonic-net#12 for sai_id in rif_sai_ids}sonic-net#12 File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/__init__.py", line 627, in dbs_get_all#012 ns_result = db_conn.get_all(db_name, _hash, *args, **tmp_kwargs)sonic-net#12 File "/usr/ local/lib/python3.7/dist-packages/swsssdk/dbconnector.py", line 295, in get_all#012 return self.dbintf.get_all(db_name, _hash, *args, **kwargs)sonic-net#12 File "/usr/local/lib/python3.7/dist-packages/swsssdk/interface.py", line 38, in wrapp ed#012 ret_data = f(inst, db_name, *args, **kwargs)sonic-net#12 File "/usr/local/lib/python3.7/dist-packages/swsssdk/interface.py", line 314, in get_all#012 raise UnavailableDataError(message, _hash)#012swsssdk.exceptions.UnavailableDataE rror: Key 'b'COUNTERS:oid:0x60000000005d3'' unavailable in database 'COUNTERS_DB' Signed-off-by: Stepan Blyshchak <[email protected]>

Signed-off-by: Stepan Blyshchak <[email protected]>

qiluo-msft · 2020-10-26T04:26:37Z

Please resolve the conflicts. #Closed

qiluo-msft · 2020-10-26T04:38:35Z

src/sonic_ax_impl/mibs/__init__.py

-    NEIGH_TABLE:device:ipv4_address
-    """
-    _, device, ip = neigh_key.split(':')
-    return device, ip


de [](start = 11, length = 2)

If you merge latest master, you don't need to use byte array and just use string. #Closed

qiluo-msft · 2020-10-26T04:39:01Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

        self.mgmt_oid_name_map, \
        self.mgmt_alias_map = mibs.init_mgmt_interface_tables(self.db_conn[0])

+        self.vlan_name_map, \


bytes [](start = 29, length = 5)

If you merge latest master, you don't need to encoding to byte array.

qiluo-msft · 2020-10-26T04:44:49Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

 from bisect import bisect_right

 from sonic_ax_impl import mibs
+from sonic_ax_impl import logger


logger [](start = 26, length = 6)

Merge with above line #Closed

qiluo-msft

As comments

SuvarnaMeenakshi

similar to test_interfaces, can you add changes in tests/namespace/test_interfaces.
Also, mock_db changes for multi-asic mock-dbs as well.

Signed-off-by: Stepan Blyshchak <[email protected]>

lgtm-com · 2020-11-04T02:13:27Z

This pull request introduces 1 alert when merging 144658f into 57e54d9 - view on LGTM.com

new alerts:

1 for Unused import

qiluo-msft · 2020-11-04T03:38:16Z

Please resolve LGTM alert. Others look good to me. #Closed

Signed-off-by: Stepan Blyshchak <[email protected]>

…an_counters Signed-off-by: Stepan Blyshchak <[email protected]>

svc-acs · 2020-11-19T08:38:24Z

Can one of the admins verify this patch?

qiluo-msft

Please also check with other reviewers.

SuvarnaMeenakshi · 2020-11-20T17:10:37Z

tests/namespace/test_interfaces.py


    def test_getnextpdu_first_bp_ifindex(self):
-        oid = ObjectIdentifier(11, 0, 0, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 1,1004))
+        oid = ObjectIdentifier(11, 0, 0, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 1, 8999))


Is this change required? There is no new interface added in namespace mock_dbs, is that right? #Closed

@stepanblyschak This comment is still applicable.

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

I have added Ethernet16 https://github.com/Azure/sonic-snmpagent/pull/169/files/a00e69e25dfc7c17f2554a625b57c447aff477c0#diff-fe4c39c260164f04bfe84f3d68d6f6a5a28850a840667a26aaba67a9fc1afacbR48.
Now, different asic dbs have different port table.

SuvarnaMeenakshi · 2020-11-20T17:11:40Z

similar to test_interfaces, can you add changes in tests/namespace/test_interfaces.
Also, mock_db changes for multi-asic mock-dbs as well.

Closed.

liat-grozovik · 2020-12-08T07:32:05Z

@qiluo-msft can you please merge so we can proceed with the submodule update?

qiluo-msft · 2020-12-11T00:24:24Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

+            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 [](start = 20, length = 16)

I see you change self.if_counters[][] type from str to int.
Although this is working in python, however we are moving the library to sonic-swss-common SWIG wrapper, and this will make SWIG confused.
Please make type fixed. #Closed

…f counters (sonic-net#169)" This reverts commit e54036c.

…f counters (#169)" (#191) **- What I did** Revert all drop SNMP counters issue. Because it introduces inconsistency with RFC2863 **- How I did it** Revert 3 commits related to this feature. **- How to verify it** The RFC1213 MIB does not have VLAN interfaces and does not perform counters accumulation.

…egate rif counters (sonic-net#169)" (sonic-net#191)" This reverts commit dfde06e.

…for RFC2863 (#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 #191, which was itself a revert of #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.

…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#191, which was itself a revert of sonic-net#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. (cherry picked from commit 266bd15)

…rs (sonic-net#169) **- What I did** Rework based on sonic-net#133 and feedback sonic-net#148. **- How I did it** - merge sonic-net#133 with upstream master. - Fix UnavailableDataError when RIF counters are not enabled. - Make RIF keys in maps unique in multi asic env. **- How to verify it** - Disable rif counters - restart swss - snmpwalk - Verify no UnavailableDataError in logs - Verify port counters are returned - Enable rif counters - snmpwalk - Verify no error in logs - Verify error in/out counters are aggregated for rif ports and portchannels - Verify VLAN RIF counters are present in the MIB. **- Description for the changelog** Added support for aggregated router interface counters and L3 VLAN counters to RFC1213 MIB.

…f counters (sonic-net#169)" (sonic-net#191) **- What I did** Revert all drop SNMP counters issue. Because it introduces inconsistency with RFC2863 **- How I did it** Revert 3 commits related to this feature. **- How to verify it** The RFC1213 MIB does not have VLAN interfaces and does not perform counters accumulation.

…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#191, which was itself a revert of sonic-net#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.

Mykola Faryma and others added 21 commits May 15, 2020 12:32

[rfc1213] Interface MIB add l3 vlan interfaces

f1b5ec1

Signed-off-by: Mykola Faryma <[email protected]>

[rfc1213] aggregate rifcounters on top of l2 counters

029a992

Signed-off-by: Mykola Faryma <[email protected]>

fix issues

5d3421f

Signed-off-by: Mykola Faryma <[email protected]>

add unittests

f0e4048

Signed-off-by: Mykola Faryma <[email protected]>

add mock data

2a35053

Signed-off-by: Mykola Faryma <[email protected]>

cleanup

0999b4b

Signed-off-by: Mykola Faryma <[email protected]>

lag rif fixes

cbf06e3

Signed-off-by: Mykola Faryma <[email protected]>

add more unittests

7c969b7

Signed-off-by: Mykola Faryma <[email protected]>

update test_mibs.py

b5c4632

Signed-off-by: Mykola Faryma <[email protected]>

Merge branch 'master' into drops

20afa18

fix comments

167ece7

Signed-off-by: Mykola Faryma <[email protected]>

update namespace/test_mibs.py unittest

9124833

Signed-off-by: Mykola Faryma <[email protected]>

[rfc1213] add vlan interface admin_status

ed9409f

Signed-off-by: Mykola Faryma <[email protected]>

only aggregate errors

b141e51

Signed-off-by: Mykola Faryma <[email protected]>

add vlan subinterface unitttests

80935af

Signed-off-by: Mykola Faryma <[email protected]>

fix init_sync_d_lag

89616da

Signed-off-by: Mykola Faryma <[email protected]>

update namespace mib test

0116f85

Signed-off-by: Mykola Faryma <[email protected]>

add namespace mock table data

5d2110d

Signed-off-by: Mykola Faryma <[email protected]>

Merge commit 'refs/pull/133/head' of github.com:azure/sonic-snmpagent

2721b32

make rif key unique in multi-asic env

b22236b

Signed-off-by: Stepan Blyshchak <[email protected]>

stepanblyschak changed the title ~~Rif vlan counters~~ [rfc1213] Interface MIB add l3 vlan interfaces & aggregate rif counters Oct 16, 2020

qiluo-msft requested review from SuvarnaMeenakshi and qiluo-msft October 20, 2020 16:28

qiluo-msft reviewed Oct 26, 2020

View reviewed changes

SuvarnaMeenakshi reviewed Oct 27, 2020

View reviewed changes

stepanblyschak added 3 commits November 4, 2020 02:02

Merge remote-tracking branch 'origin/master' into rif_vlan_counters

8a741fb

Signed-off-by: Stepan Blyshchak <[email protected]>

support multi-asic devices & unit test for multi-asic

068f530

Signed-off-by: Stepan Blyshchak <[email protected]>

merge import lines

144658f

Signed-off-by: Stepan Blyshchak <[email protected]>

stepanblyschak added 2 commits November 4, 2020 10:28

remove unused report

196033f

Signed-off-by: Stepan Blyshchak <[email protected]>

Merge branch 'master' of github.com:azure/sonic-snmpagent into rif_vl…

a00e69e

…an_counters Signed-off-by: Stepan Blyshchak <[email protected]>

qiluo-msft approved these changes Nov 19, 2020

View reviewed changes

SuvarnaMeenakshi reviewed Nov 20, 2020

View reviewed changes

SuvarnaMeenakshi approved these changes Dec 8, 2020

View reviewed changes

qiluo-msft merged commit e54036c into sonic-net:master Dec 8, 2020

qiluo-msft reviewed Dec 11, 2020

View reviewed changes

stepanblyschak added a commit to stepanblyschak/sonic-snmpagent that referenced this pull request Jan 12, 2021

Revert "[rfc1213] Interface MIB add l3 vlan interfaces & aggregate ri…

71ddd56

…f counters (sonic-net#169)" This reverts commit e54036c.

raphaelt-nvidia added a commit to raphaelt-nvidia/sonic-snmpagent that referenced this pull request Apr 8, 2021

Revert "Revert "[rfc1213] Interface MIB add l3 vlan interfaces & aggr…

cbd0c8e

…egate rif counters (sonic-net#169)" (sonic-net#191)" This reverts commit dfde06e.

raphaelt-nvidia mentioned this pull request Apr 8, 2021

Vlan snmp raphaelt-nvidia/sonic-snmpagent#1

Closed

raphaelt-nvidia added a commit to raphaelt-nvidia/sonic-snmpagent that referenced this pull request May 25, 2021

Revert "Revert "[rfc1213] Interface MIB add l3 vlan interfaces & aggr…

c467dc8

…egate rif counters (sonic-net#169)" (sonic-net#191)" This reverts commit dfde06e.

This was referenced May 26, 2021

Vlan snmp raphaelt-nvidia/sonic-snmpagent#2

Closed

Restored snmp vlan support per RFC1213 and added the missing support for RFC2863 #218

Merged

Conversation

stepanblyschak commented Oct 16, 2020

Uh oh!

qiluo-msft commented Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Oct 26, 2020

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

SuvarnaMeenakshi left a comment

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Nov 4, 2020

Uh oh!

qiluo-msft commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

svc-acs commented Nov 19, 2020

Uh oh!

qiluo-msft left a comment

Choose a reason for hiding this comment

Uh oh!

SuvarnaMeenakshi Nov 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Nov 26, 2020

Choose a reason for hiding this comment

Uh oh!

stepanblyschak Dec 8, 2020

Choose a reason for hiding this comment

Uh oh!

SuvarnaMeenakshi commented Nov 20, 2020

Uh oh!

liat-grozovik commented Dec 8, 2020

Uh oh!

qiluo-msft Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

qiluo-msft commented Oct 26, 2020 •

edited

Loading

qiluo-msft Oct 26, 2020 •

edited

Loading

qiluo-msft Oct 26, 2020 •

edited

Loading

qiluo-msft commented Nov 4, 2020 •

edited

Loading

SuvarnaMeenakshi Nov 20, 2020 •

edited

Loading

qiluo-msft Dec 11, 2020 •

edited

Loading