From 7043d38d1d89ea3eb50303f1978d3ed6ec3e5f21 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Wed, 1 Aug 2018 16:23:43 -0700 Subject: [PATCH 01/10] sonic-snmpagent: fixed interface speeds * ifSpeed now reports speed under 2^32 bps * ifSpeed returns 2^32 is speed is above * ifHighSpeed reports the interface speed in Mbps * ifHighSpeed defaults to 40000 if speed not available in APPL_DB * unit tests for rfc1213 and rfc2863 * appl_db.json modified to provide variety of speeds Signed-off-by: michel.moriniaux@gmail.com --- src/sonic_ax_impl/mibs/ietf/rfc1213.py | 21 ++++++--- src/sonic_ax_impl/mibs/ietf/rfc2863.py | 31 +++++++++++- tests/mock_tables/appl_db.json | 10 ++-- tests/test_hc_interfaces.py | 65 +++++++++++++++++++++++++- tests/test_interfaces.py | 63 +++++++++++++++++++++++++ 5 files changed, 174 insertions(+), 16 deletions(-) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc1213.py b/src/sonic_ax_impl/mibs/ietf/rfc1213.py index 3caea75c8..3a0394a81 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc1213.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc1213.py @@ -336,6 +336,19 @@ def get_mtu(self, sub_id): return int(entry.get(b"mtu", 0)) + def get_speed(self, sub_id): + """ + :param sub_id: The 1-based sub-identifier query. + :return: min of 4294967295 or speed value for the respective sub_id. + """ + entry = self._get_if_entry(sub_id) + if not entry: + return + + speed = int(entry.get(b"speed", 0)) + # speed is reported in Mbps in the db + return min(4294967295, speed * 1000000) + class InterfacesMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.2'): """ @@ -365,14 +378,8 @@ class InterfacesMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.2'): ifMtu = \ SubtreeMIBEntry('2.1.4', if_updater, ValueType.INTEGER, if_updater.get_mtu) - # FIXME Placeholder. - # "If the bandwidth of the interface is greater - # than the maximum value reportable by this object, - # then this object should report its maximum value - # (4.294,967,295) and ifHighSpeed must be used to - # report the interface's speed." ifSpeed = \ - SubtreeMIBEntry('2.1.5', if_updater, ValueType.GAUGE_32, lambda sub_id: 4294967295) + SubtreeMIBEntry('2.1.5', if_updater, ValueType.GAUGE_32, if_updater.get_speed) # FIXME Placeholder. ifPhysAddress = \ diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2863.py b/src/sonic_ax_impl/mibs/ietf/rfc2863.py index 90792cd27..c621e8272 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2863.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2863.py @@ -182,6 +182,34 @@ def _get_counter(self, oid, table_name, mask): mibs.logger.warning("SyncD 'COUNTERS_DB' missing attribute '{}'.".format(e)) return None + def _get_if_entry(self, sub_id): + """ + :param oid: The 1-based sub-identifier query. + :return: the DB entry for the respective sub_id. + """ + oid = self.get_oid(sub_id) + if not oid: + return + + table = "" + if oid in self.oid_lag_name_map: + table = mibs.lag_entry_table(self.oid_lag_name_map[oid]) + else: + table = mibs.if_entry_table(self.oid_name_map[oid]) + + return self.db_conn.get_all(mibs.APPL_DB, table, blocking=True) + + def get_high_speed(self, sub_id): + """ + :param sub_id: The 1-based sub-identifier query. + :return: speed value for the respective sub_id or 40000 if not defined. + """ + entry = self._get_if_entry(sub_id) + if not entry: + return + + return int(entry.get(b"speed", 40000)) + class InterfaceMIBObjects(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.31.1'): """ @@ -259,8 +287,7 @@ class InterfaceMIBObjects(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.31.1'): """ # FIXME: Placeholder (original impl reported 0) ifLinkUpDownTrapEnable = SubtreeMIBEntry('1.1.14', if_updater, ValueType.INTEGER, lambda sub_id: 2) - # FIXME: Placeholder - ifHighSpeed = SubtreeMIBEntry('1.1.15', if_updater, ValueType.GAUGE_32, lambda sub_id: 40000) + ifHighSpeed = SubtreeMIBEntry('1.1.15', if_updater, ValueType.GAUGE_32, if_updater.get_high_speed) """ ifPromiscuousMode OBJECT-TYPE diff --git a/tests/mock_tables/appl_db.json b/tests/mock_tables/appl_db.json index 5ed1d2104..dee1b4667 100644 --- a/tests/mock_tables/appl_db.json +++ b/tests/mock_tables/appl_db.json @@ -533,11 +533,10 @@ }, "PORT_TABLE:Ethernet116": { "description": "snowflake", - "speed": 100000 + "speed": 1000 }, "PORT_TABLE:Ethernet120": { - "description": "snowflake", - "speed": 100000 + "description": "snowflake" }, "PORT_TABLE:Ethernet124": { "description": "snowflake", @@ -661,11 +660,10 @@ }, "PORT_TABLE:Ethernet116": { "description": "snowflake", - "speed": 100000 + "speed": 1000 }, "PORT_TABLE:Ethernet120": { - "description": "snowflake", - "speed": 100000 + "description": "snowflake" }, "PORT_TABLE:Ethernet124": { "description": "snowflake", diff --git a/tests/test_hc_interfaces.py b/tests/test_hc_interfaces.py index 4cea860e6..10899e63c 100644 --- a/tests/test_hc_interfaces.py +++ b/tests/test_hc_interfaces.py @@ -71,4 +71,67 @@ def test_get_next3(self): payload = b'\x01\x06\x10\x00\x00\x00\x00\x17\x00\x00\x01V\x00\x00\x01W\x00\x00\x00,\x06\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x03\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x02\x01\x06\x10\x00\x00\x00\x00\x17\x00\x00\x01\\\x00\x00\x01]\x00\x00\x00,\x06\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x03\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x02\x01\x06\x10\x00\x00\x00\x00\x17\x00\x00\x01b\x00\x00\x01c\x00\x00\x00,\x06\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x03\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x02\x01\x06\x10\x00\x00\x00\x00\x17\x00\x00\x01h\x00\x00\x01i\x00\x00\x00,\x06\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x03\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x02' pdu = PDU.decode(payload) resp = pdu.make_response(self.lut) - print(resp) \ No newline at end of file + print(resp) + + def test_low_speed(self): + """ + For an interface with a speed inside the 32 bit counter returns the speed of the interface in Mbps + """ + oid = ObjectIdentifier(12, 0, 0, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 113)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.GAUGE_32) + self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 117)))) + self.assertEqual(value0.data, 1000) + + def test_high_speed(self): + """ + should return the speed of the interface + """ + oid = ObjectIdentifier(12, 0, 0, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 1)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.GAUGE_32) + self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 5)))) + self.assertEqual(value0.data, 100000) + + def test_no_speed(self): + """ + For a port with no speed in the db the result should be 40000 + """ + oid = ObjectIdentifier(12, 0, 0, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 117)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.GAUGE_32) + self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 121)))) + self.assertEqual(value0.data, 40000) diff --git a/tests/test_interfaces.py b/tests/test_interfaces.py index 3556de145..f638106a2 100644 --- a/tests/test_interfaces.py +++ b/tests/test_interfaces.py @@ -132,3 +132,66 @@ def test_missing_counter(self): pdu = PDU.decode(resp) resp = pdu.make_response(self.lut) print(resp) + + def test_low_speed(self): + """ + For an interface with a speed inside the 32 bit counter returns the speed of the interface in bps + """ + oid = ObjectIdentifier(11, 0, 0, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 113)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.GAUGE_32) + self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 117)))) + self.assertEqual(value0.data, 1000000000) + + def test_high_speed(self): + """ + For a speed higher than 4,294,967,295 the retrun should be 4294967295 + """ + oid = ObjectIdentifier(11, 0, 0, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 1)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.GAUGE_32) + self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 5)))) + self.assertEqual(value0.data, 4294967295) + + def test_no_speed(self): + """ + For a port with no speed in the db the result should be 0 + """ + oid = ObjectIdentifier(11, 0, 0, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 117)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.GAUGE_32) + self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 121)))) + self.assertEqual(value0.data, 0) From 287a077102c8d1bbcbbfb6ee369124c7b56c0c96 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Wed, 1 Aug 2018 16:23:43 -0700 Subject: [PATCH 02/10] sonic-snmpagent: fixed interface speeds * ifSpeed now reports speed under 2^32 bps * ifSpeed returns 2^32 is speed is above * ifHighSpeed reports the interface speed in Mbps * ifHighSpeed defaults to 40000 if speed not available in APPL_DB * unit tests for rfc1213 and rfc2863 * appl_db.json modified to provide variety of speeds Signed-off-by: michel.moriniaux@gmail.com --- src/sonic_ax_impl/mibs/ietf/rfc1213.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc1213.py b/src/sonic_ax_impl/mibs/ietf/rfc1213.py index 3a0394a81..27952dd24 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc1213.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc1213.py @@ -142,6 +142,9 @@ class IpMib(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.4'): SubtreeMIBEntry('22.1.2', arp_updater, ValueType.OCTET_STRING, arp_updater.arp_dest) class InterfacesUpdater(MIBUpdater): + + RFC1213_MAX_SPEED = 4294967295 + def __init__(self): super().__init__() self.db_conn = mibs.init_db() @@ -336,10 +339,10 @@ def get_mtu(self, sub_id): return int(entry.get(b"mtu", 0)) - def get_speed(self, sub_id): + def get_speed_bps(self, sub_id): """ :param sub_id: The 1-based sub-identifier query. - :return: min of 4294967295 or speed value for the respective sub_id. + :return: min of RFC1213_MAX_SPEED or speed value for the respective sub_id. """ entry = self._get_if_entry(sub_id) if not entry: @@ -347,8 +350,7 @@ def get_speed(self, sub_id): speed = int(entry.get(b"speed", 0)) # speed is reported in Mbps in the db - return min(4294967295, speed * 1000000) - + return min(RFC1213_MAX_SPEED, speed * 1000000) class InterfacesMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.2'): """ @@ -379,7 +381,7 @@ class InterfacesMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.2'): SubtreeMIBEntry('2.1.4', if_updater, ValueType.INTEGER, if_updater.get_mtu) ifSpeed = \ - SubtreeMIBEntry('2.1.5', if_updater, ValueType.GAUGE_32, if_updater.get_speed) + SubtreeMIBEntry('2.1.5', if_updater, ValueType.GAUGE_32, if_updater.get_speed_bps) # FIXME Placeholder. ifPhysAddress = \ From 7846a8ba37e819e0d5a5f4d6cbb5220b1bf3d1e1 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Wed, 1 Aug 2018 16:23:43 -0700 Subject: [PATCH 03/10] sonic-snmpagent: fixed interface speeds * ifSpeed now reports speed under 2^32 bps * ifSpeed returns 2^32 is speed is above * ifHighSpeed reports the interface speed in Mbps * ifHighSpeed defaults to 40000 if speed not available in APPL_DB * unit tests for rfc1213 and rfc2863 * appl_db.json modified to provide variety of speeds Signed-off-by: michel.moriniaux@gmail.com --- src/sonic_ax_impl/mibs/ietf/rfc1213.py | 12 ++++++------ src/sonic_ax_impl/mibs/ietf/rfc2863.py | 8 ++++---- tests/test_hc_interfaces.py | 6 ------ tests/test_interfaces.py | 6 ------ 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc1213.py b/src/sonic_ax_impl/mibs/ietf/rfc1213.py index 27952dd24..eae62f124 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc1213.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc1213.py @@ -144,7 +144,7 @@ class IpMib(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.4'): class InterfacesUpdater(MIBUpdater): RFC1213_MAX_SPEED = 4294967295 - + def __init__(self): super().__init__() self.db_conn = mibs.init_db() @@ -285,13 +285,13 @@ def _get_if_entry(self, sub_id): if not oid: return - table = "" + if_table = "" if oid in self.oid_lag_name_map: - table = mibs.lag_entry_table(self.oid_lag_name_map[oid]) + if_table = mibs.lag_entry_table(self.oid_lag_name_map[oid]) else: - table = mibs.if_entry_table(self.oid_name_map[oid]) + if_table = mibs.if_entry_table(self.oid_name_map[oid]) - return self.db_conn.get_all(mibs.APPL_DB, table, blocking=True) + return self.db_conn.get_all(mibs.APPL_DB, if_table, blocking=True) def _get_status(self, sub_id, key): """ @@ -349,7 +349,7 @@ def get_speed_bps(self, sub_id): return speed = int(entry.get(b"speed", 0)) - # speed is reported in Mbps in the db + # speed is reported in Mbps in the db return min(RFC1213_MAX_SPEED, speed * 1000000) class InterfacesMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.2'): diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2863.py b/src/sonic_ax_impl/mibs/ietf/rfc2863.py index c621e8272..c7b81a6bb 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2863.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2863.py @@ -191,13 +191,13 @@ def _get_if_entry(self, sub_id): if not oid: return - table = "" + if_table = "" if oid in self.oid_lag_name_map: - table = mibs.lag_entry_table(self.oid_lag_name_map[oid]) + if_table = mibs.lag_entry_table(self.oid_lag_name_map[oid]) else: - table = mibs.if_entry_table(self.oid_name_map[oid]) + if_table = mibs.if_entry_table(self.oid_name_map[oid]) - return self.db_conn.get_all(mibs.APPL_DB, table, blocking=True) + return self.db_conn.get_all(mibs.APPL_DB, if_table, blocking=True) def get_high_speed(self, sub_id): """ diff --git a/tests/test_hc_interfaces.py b/tests/test_hc_interfaces.py index 10899e63c..26de43a5a 100644 --- a/tests/test_hc_interfaces.py +++ b/tests/test_hc_interfaces.py @@ -87,8 +87,6 @@ def test_low_speed(self): response = get_pdu.make_response(self.lut) print(response) - n = len(response.values) - # self.assertEqual(n, 7) value0 = response.values[0] self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 117)))) @@ -108,8 +106,6 @@ def test_high_speed(self): response = get_pdu.make_response(self.lut) print(response) - n = len(response.values) - # self.assertEqual(n, 7) value0 = response.values[0] self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 5)))) @@ -129,8 +125,6 @@ def test_no_speed(self): response = get_pdu.make_response(self.lut) print(response) - n = len(response.values) - # self.assertEqual(n, 7) value0 = response.values[0] self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 121)))) diff --git a/tests/test_interfaces.py b/tests/test_interfaces.py index f638106a2..2e051c91a 100644 --- a/tests/test_interfaces.py +++ b/tests/test_interfaces.py @@ -147,8 +147,6 @@ def test_low_speed(self): response = get_pdu.make_response(self.lut) print(response) - n = len(response.values) - # self.assertEqual(n, 7) value0 = response.values[0] self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 117)))) @@ -168,8 +166,6 @@ def test_high_speed(self): response = get_pdu.make_response(self.lut) print(response) - n = len(response.values) - # self.assertEqual(n, 7) value0 = response.values[0] self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 5)))) @@ -189,8 +185,6 @@ def test_no_speed(self): response = get_pdu.make_response(self.lut) print(response) - n = len(response.values) - # self.assertEqual(n, 7) value0 = response.values[0] self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 2, 2, 1, 5, 121)))) From 080cd33a5752ff93a9f61ab12c87e4032ee27daa Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Thu, 2 Aug 2018 15:06:38 -0700 Subject: [PATCH 04/10] sonic-snmpagent: fixed interface descriptions * ifAlias should retrun the interface description not the SONiC name * unit tests Signed-off-by: michel.moriniaux@gmail.com --- src/sonic_ax_impl/mibs/ietf/rfc2863.py | 14 ++++++-------- tests/mock_tables/appl_db.json | 2 -- tests/test_hc_interfaces.py | 23 ++++++++++++++++++++++- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc2863.py b/src/sonic_ax_impl/mibs/ietf/rfc2863.py index c7b81a6bb..d34c1afab 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc2863.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc2863.py @@ -128,18 +128,16 @@ def interface_name(self, sub_id): def interface_alias(self, sub_id): """ - ifAlias specific - this is not the "Alias map". This simply returns the SONiC name. + ifAlias specific - this is not the "Alias map". + To be homogenous with what is available in the wild this should return the interface 'description'. :param sub_id: The 1-based sub-identifier query. - :return: The SONiC name. + :return: The SONiC interface description, empty string if not defined. """ - oid = self.get_oid(sub_id) - if not oid: + entry = self._get_if_entry(sub_id) + if not entry: return - if oid in self.oid_lag_name_map: - return self.oid_lag_name_map[oid] - - return self.oid_name_map[oid] + return entry.get(b"description", "") def get_counter32(self, sub_id, table_name): oid = self.get_oid(sub_id) diff --git a/tests/mock_tables/appl_db.json b/tests/mock_tables/appl_db.json index dee1b4667..897269ad2 100644 --- a/tests/mock_tables/appl_db.json +++ b/tests/mock_tables/appl_db.json @@ -532,7 +532,6 @@ "speed": 100000 }, "PORT_TABLE:Ethernet116": { - "description": "snowflake", "speed": 1000 }, "PORT_TABLE:Ethernet120": { @@ -659,7 +658,6 @@ "speed": 100000 }, "PORT_TABLE:Ethernet116": { - "description": "snowflake", "speed": 1000 }, "PORT_TABLE:Ethernet120": { diff --git a/tests/test_hc_interfaces.py b/tests/test_hc_interfaces.py index 26de43a5a..8dac142fb 100644 --- a/tests/test_hc_interfaces.py +++ b/tests/test_hc_interfaces.py @@ -47,7 +47,7 @@ def test_getnextpdu_firstifalias(self): value0 = response.values[0] self.assertEqual(value0.type_, ValueType.OCTET_STRING) self.assertEqual(str(value0.name), str(ObjectIdentifier(11, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 18, 1)))) - self.assertEqual(str(value0.data), 'Ethernet0') + self.assertEqual(str(value0.data), 'snowflake') def test_get_next_alias(self): if_alias = b'\x01\x06\x10\x00\x00\x00\x00o\x00\x01\xcc4\x00\x01\xcc5\x00\x00\x000\x07\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x01\x00\x00\x00\x11\x00\x00\x00}\x03\x02\x00\x00\x00\x00\x00\x01\x00\x00\x00\x1f\x00\x00\x00\x02' @@ -129,3 +129,24 @@ def test_no_speed(self): self.assertEqual(value0.type_, ValueType.GAUGE_32) self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 15, 121)))) self.assertEqual(value0.data, 40000) + + def test_no_description(self): + """ + For a port with no speed in the db the result should be 40000 + """ + oid = ObjectIdentifier(12, 0, 0, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 18, 113)) + get_pdu = GetNextPDU( + header=PDUHeader(1, PduTypes.GET, 16, 0, 42, 0, 0, 0), + oids=[oid] + ) + + encoded = get_pdu.encode() + response = get_pdu.make_response(self.lut) + print(response) + + n = len(response.values) + # self.assertEqual(n, 7) + value0 = response.values[0] + self.assertEqual(value0.type_, ValueType.OCTET_STRING) + self.assertEqual(str(value0.name), str(ObjectIdentifier(12, 0, 1, 0, (1, 3, 6, 1, 2, 1, 31, 1, 1, 1, 18, 117)))) + self.assertEqual(str(value0.data), '') From e88eca8a371edd614a2971514c17bbed3734fbe1 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Tue, 7 Aug 2018 12:30:02 -0700 Subject: [PATCH 05/10] Corrected referencing error in ifspeed PR * missing self. keyword Signed-off-by: michel.moriniaux@gmail.com --- src/sonic_ax_impl/mibs/ietf/rfc1213.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sonic_ax_impl/mibs/ietf/rfc1213.py b/src/sonic_ax_impl/mibs/ietf/rfc1213.py index eae62f124..127256771 100644 --- a/src/sonic_ax_impl/mibs/ietf/rfc1213.py +++ b/src/sonic_ax_impl/mibs/ietf/rfc1213.py @@ -350,7 +350,7 @@ def get_speed_bps(self, sub_id): speed = int(entry.get(b"speed", 0)) # speed is reported in Mbps in the db - return min(RFC1213_MAX_SPEED, speed * 1000000) + return min(self.RFC1213_MAX_SPEED, speed * 1000000) class InterfacesMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.2'): """ From 0e2c7510853333308ad4cfe47c472b3d07e31191 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Fri, 2 Nov 2018 15:18:17 -0700 Subject: [PATCH 06/10] snmp-snmpagent: allow the use of tcp sockets for ax * added code to read the snmpd.conf file for alternate agentx socket * added code to open the connection according to the type of socket This will allow external containers to use agentx to communicate with the snmp container. This is done by changing the default unix socket /var/agentx/master to a tcp socket via the /etc/snmp/snmpd.conf file. The agentx implementation reads the config directive from this file and adapts accordingly This allows export of SNMP metrics by the FRR container for eg. signed-off-by: Michel Moriniaux --- src/ax_interface/constants.py | 1 + src/ax_interface/socket_io.py | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/ax_interface/constants.py b/src/ax_interface/constants.py index 5577fd17c..1c5b8a8ec 100644 --- a/src/ax_interface/constants.py +++ b/src/ax_interface/constants.py @@ -1,6 +1,7 @@ from enum import Enum, unique AGENTX_SOCKET_PATH = '/var/agentx/master' +SNMPD_CONFIG_FILE = '/etc/snmp/snmpd.conf' # https://tools.ietf.org/html/rfc2741#section-6.1 -- PDU Header definition # Headers are fixed at 20 bytes. diff --git a/src/ax_interface/socket_io.py b/src/ax_interface/socket_io.py index 4fe31f5b8..4244b87c5 100644 --- a/src/ax_interface/socket_io.py +++ b/src/ax_interface/socket_io.py @@ -6,6 +6,7 @@ """ import asyncio import logging +import re from . import logger, constants from .protocol import AgentX @@ -25,6 +26,20 @@ def __init__(self, mib_table, run_event, loop): self.transport = self.ax_socket = None + self.ax_socket_path = constants.AGENTX_SOCKET_PATH + # open the snmpd config file and search for a agentx socke redefinition + pattern = re.compile("^agentxsocket\s+(.*)$") + try : + with open(constants.SNMPD_CONFIG_FILE,'rt') as config_file: + for line in config_file: + for match in pattern.finditer(line): + self.ax_socket_path = match.group(1) + except: + log_level = logging.WARNING + logger.log(log_level, "SNMPD config file not found, using default agentx file socket") + log_level = logging.INFO + logger.log(log_level, "Using agentx socket "+self.ax_socket_path) + async def connection_loop(self): """ Try/Retry connection coroutine to attach the socket. @@ -37,10 +52,21 @@ async def connection_loop(self): try: logger.info("Attempting AgentX socket bind...".format()) - connection_routine = self.loop.create_unix_connection( - protocol_factory=lambda: AgentX(self.mib_table, self.loop), - path=constants.AGENTX_SOCKET_PATH, - sock=self.ax_socket) + # Open the connection to the Agentx socket, we check the socket string to + # either open a tcp socket or a Unix domain socket + if self.ax_socket_path.startswith('tcp'): + myhost = self.ax_socket_path.split(':')[1] + myport = self.ax_socket_path.split(':')[2] + connection_routine = self.loop.create_connection( + protocol_factory=lambda: AgentX(self.mib_table, self.loop), + host=myhost, + port=myport, + sock=self.ax_socket) + else: + connection_routine = self.loop.create_unix_connection( + protocol_factory=lambda: AgentX(self.mib_table, self.loop), + path=self.ax_socket_path, + sock=self.ax_socket) # Initiate the socket connection self.transport, protocol = await connection_routine From c65307c1f82c797b131b06e979766f83ab7aecdc Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Fri, 2 Nov 2018 15:18:17 -0700 Subject: [PATCH 07/10] snmp-snmpagent: allow the use of tcp sockets for ax * added code to read the snmpd.conf file for alternate agentx socket * added code to open the connection according to the type of socket * corrections and coments from Qi Luo This will allow external containers to use agentx to communicate with the snmp container. This is done by changing the default unix socket /var/agentx/master to a tcp socket via the /etc/snmp/snmpd.conf file. The agentx implementation reads the config directive from this file and adapts accordingly This allows export of SNMP metrics by the FRR container for eg. signed-off-by: Michel Moriniaux --- src/ax_interface/socket_io.py | 56 +++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/src/ax_interface/socket_io.py b/src/ax_interface/socket_io.py index 4244b87c5..1018c1318 100644 --- a/src/ax_interface/socket_io.py +++ b/src/ax_interface/socket_io.py @@ -27,18 +27,25 @@ def __init__(self, mib_table, run_event, loop): self.transport = self.ax_socket = None self.ax_socket_path = constants.AGENTX_SOCKET_PATH - # open the snmpd config file and search for a agentx socke redefinition - pattern = re.compile("^agentxsocket\s+(.*)$") + + # The following code reads the snmp config file to see if the Agentx Socket path has been redefined + # from the default RFC value '/var/agentx/master'. We do not implement reading the SNMP daemon's + # command line to check if it has been redefined there, this should be an effort for the future + # perhaps via a psutil().cmdline() call + + # open the snmpd config file and search for a agentx socket redefinition. Exceptions will be raised + # if the constants.SNMPD_CONFIG_FILE or the file in itself do not exist + pattern = re.compile("^agentxsocket\s+(.*)$", re.IGNORECASE) try : with open(constants.SNMPD_CONFIG_FILE,'rt') as config_file: for line in config_file: - for match in pattern.finditer(line): + match = pattern.search(line) + if match: self.ax_socket_path = match.group(1) except: - log_level = logging.WARNING - logger.log(log_level, "SNMPD config file not found, using default agentx file socket") - log_level = logging.INFO - logger.log(log_level, "Using agentx socket "+self.ax_socket_path) + logger.warning("SNMPD config file not found, using default agentx file socket") + + logger.info("Using agentx socket " + self.ax_socket_path) async def connection_loop(self): """ @@ -54,7 +61,17 @@ async def connection_loop(self): # Open the connection to the Agentx socket, we check the socket string to # either open a tcp socket or a Unix domain socket - if self.ax_socket_path.startswith('tcp'): + if '/' in self.ax_socket_path: + # This looks like a filesystem path so lets open it as a domain socket + # but first lets remove 'unix' if it's in the spec + if self.ax_socket_path.startswith('unix'): + self.ax_socket_path = self.ax_socket_path.split(':')[1] + connection_routine = self.loop.create_unix_connection( + protocol_factory=lambda: AgentX(self.mib_table, self.loop), + path=self.ax_socket_path, + sock=self.ax_socket) + elif self.ax_socket_path.startswith('tcp'): + # This looks like a tcp connection myhost = self.ax_socket_path.split(':')[1] myport = self.ax_socket_path.split(':')[2] connection_routine = self.loop.create_connection( @@ -62,10 +79,31 @@ async def connection_loop(self): host=myhost, port=myport, sock=self.ax_socket) + elif self.ax_socket_path.startswith('udp'): + # This looks like a udp connection + myhost = self.ax_socket_path.split(':')[1] + myport = self.ax_socket_path.split(':')[2] + connection_routine = self.loop.create_datagram_endpoint( + protocol_factory=lambda: AgentX(self.mib_table, self.loop), + host=myhost, + port=myport, + sock=self.ax_socket) + elif self.ax_socket_path.isdigit(): + # if the socket path is just a number then it is treated as a udp port on localhost + myhost = 'localhost' + myport = self.ax_socket_path + connection_routine = self.loop.create_datagram_endpoint( + protocol_factory=lambda: AgentX(self.mib_table, self.loop), + host=myhost, + port=myport, + sock=self.ax_socket) else: + # We do not support 'ssh', 'dtlsudp', 'ipx', or 'aal5pvc' (ATM lol...) methods + # default to the snmp default /var/agentx/master and log a warning + logger.warning("Socket type " + self.ax_socket_path + " not supported, using default agentx file socket") connection_routine = self.loop.create_unix_connection( protocol_factory=lambda: AgentX(self.mib_table, self.loop), - path=self.ax_socket_path, + path=constants.AGENTX_SOCKET_PATH, sock=self.ax_socket) # Initiate the socket connection From 365de20d6abe27d1f0628c27ce1b72040e4ede84 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Fri, 2 Nov 2018 15:18:17 -0700 Subject: [PATCH 08/10] snmp-snmpagent: allow the use of tcp sockets for ax * added code to read the snmpd.conf file for alternate agentx socket * added code to open the connection according to the type of socket * corrections and coments from Qi Luo This will allow external containers to use agentx to communicate with the snmp container. This is done by changing the default unix socket /var/agentx/master to a tcp socket via the /etc/snmp/snmpd.conf file. The agentx implementation reads the config directive from this file and adapts accordingly This allows export of SNMP metrics by the FRR container for eg. signed-off-by: Michel Moriniaux --- src/ax_interface/socket_io.py | 116 +++++++++++++++++++++++----------- 1 file changed, 78 insertions(+), 38 deletions(-) diff --git a/src/ax_interface/socket_io.py b/src/ax_interface/socket_io.py index 1018c1318..1f9934965 100644 --- a/src/ax_interface/socket_io.py +++ b/src/ax_interface/socket_io.py @@ -45,7 +45,72 @@ def __init__(self, mib_table, run_event, loop): except: logger.warning("SNMPD config file not found, using default agentx file socket") - logger.info("Using agentx socket " + self.ax_socket_path) + self.parse_socket() + logger.info("Using agentx socket type " + self.ax_socket_type + " with path " + self.ax_socket_path) + + def parse_socket(self): + # Determine wether the socket method is supported + # extract the type and connection data + + # lets get the unsuported methods out of the way first + unsuported_list = ['ssh', 'dtlsudp', 'ipx', 'aal5pvc', 'udp'] + for method in unsuported_list: + if self.ax_socket_path.startswith(method): + # This is not a supported method + logger.warning("Socket type " + self.ax_socket_path + " not supported, using default agentx file socket") + self.ax_socket_path = constants.AGENTX_SOCKET_PATH + self.ax_socket_type = 'unix' + return + # Now the stuff that we are interested in + # First case: we have a simple number then its a local UDP port + # udp has been added to the unsuported_list because asynio throws a not implemented error with udp + # we leave the code here for when it will be implemented + if self.ax_socket_path.isdigit(): + self.ax_socket_type = 'udp' + self.host = 'localhost' + self.port = self.ax_socket_path + return + # if we have an explicit udp socket + if self.ax_socket_path.startswith('udp'): + self.ax_socket_type = 'udp' + self.host, self.port = self.get_ip_port(self.ax_socket_path.split(':',1)[1]) + return + # if we have an explicit tcp socket + if self.ax_socket_path.startswith('tcp'): + self.ax_socket_type = 'tcp' + self.host, self.port = self.get_ip_port(self.ax_socket_path.split(':',1)[1]) + return + # if we have an explicit unix domain socket + if self.ax_socket_path.startswith('unix'): + self.ax_socket_type = 'unix' + self.ax_socket_path = self.ax_socket_path.split(':',1)[1] + return + # unix is not compulsory so you can also have a plain path + if '/' in self.ax_socket_path: + self.ax_socket_type = 'unix' + return + # if at this point we haven't matched anything yet its that we are most likely left with a hort:port pair so UDP + if ':' in self.ax_socket_path: + self.ax_socket_type = 'udp' + self.host, self.port = self.get_ip_port(self.ax_socket_path) + return + # we should never get here but if we do it's that there is garbage so lets revert to the default of snmp + logger.warning("There's something weird with " + self.ax_socket_path + " , using default agentx file socket") + self.ax_socket_path = constants.AGENTX_SOCKET_PATH + self.ax_socket_type = 'unix' + return + + def get_ip_port(self,address): + # determine if we only have a port or a ip:port tuple, must work with IPv6 + address_list = address.split(':') + if len(address_list) == 1: + # we only have a port + return 'localhost', address_list[0] + else: + # if we get here then either: we've got garbage, an ip:port or ipv6:port or hostname:port + # an IP or IPv6 only is illegal + address_list = address.rsplit(':',1) + return address_list[0], address_list[1] async def connection_loop(self): """ @@ -60,50 +125,25 @@ async def connection_loop(self): logger.info("Attempting AgentX socket bind...".format()) # Open the connection to the Agentx socket, we check the socket string to - # either open a tcp socket or a Unix domain socket - if '/' in self.ax_socket_path: - # This looks like a filesystem path so lets open it as a domain socket - # but first lets remove 'unix' if it's in the spec - if self.ax_socket_path.startswith('unix'): - self.ax_socket_path = self.ax_socket_path.split(':')[1] + # lets open our socket according to its detected type + if self.ax_socket_type == 'unix': connection_routine = self.loop.create_unix_connection( protocol_factory=lambda: AgentX(self.mib_table, self.loop), - path=self.ax_socket_path, - sock=self.ax_socket) - elif self.ax_socket_path.startswith('tcp'): - # This looks like a tcp connection - myhost = self.ax_socket_path.split(':')[1] - myport = self.ax_socket_path.split(':')[2] - connection_routine = self.loop.create_connection( - protocol_factory=lambda: AgentX(self.mib_table, self.loop), - host=myhost, - port=myport, - sock=self.ax_socket) - elif self.ax_socket_path.startswith('udp'): - # This looks like a udp connection - myhost = self.ax_socket_path.split(':')[1] - myport = self.ax_socket_path.split(':')[2] - connection_routine = self.loop.create_datagram_endpoint( - protocol_factory=lambda: AgentX(self.mib_table, self.loop), - host=myhost, - port=myport, + path=constants.AGENTX_SOCKET_PATH, sock=self.ax_socket) - elif self.ax_socket_path.isdigit(): - # if the socket path is just a number then it is treated as a udp port on localhost - myhost = 'localhost' - myport = self.ax_socket_path + elif self.ax_socket_type == 'udp': + # we should not land here as the udp method is in the unsuported list + # testing shows that async_io throws a NotImplementedError when udp is used + # the code remains for when asyncio will implement it connection_routine = self.loop.create_datagram_endpoint( protocol_factory=lambda: AgentX(self.mib_table, self.loop), - host=myhost, - port=myport, + remote_addr=(self.host,self.port), sock=self.ax_socket) - else: - # We do not support 'ssh', 'dtlsudp', 'ipx', or 'aal5pvc' (ATM lol...) methods - # default to the snmp default /var/agentx/master and log a warning - logger.warning("Socket type " + self.ax_socket_path + " not supported, using default agentx file socket") - connection_routine = self.loop.create_unix_connection( + elif self.ax_socket_type == 'tcp': + connection_routine = self.loop.create_connection( protocol_factory=lambda: AgentX(self.mib_table, self.loop), - path=constants.AGENTX_SOCKET_PATH, + host=self.host, + port=self.port, sock=self.ax_socket) # Initiate the socket connection From 68ec9798b570a2e4ccaa653cc08c1a70d04b8e10 Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Fri, 2 Nov 2018 15:18:17 -0700 Subject: [PATCH 09/10] snmp-snmpagent: allow the use of tcp sockets for ax * added code to read the snmpd.conf file for alternate agentx socket * added code to open the connection according to the type of socket * corrections and coments from Qi Luo This will allow external containers to use agentx to communicate with the snmp container. This is done by changing the default unix socket /var/agentx/master to a tcp socket via the /etc/snmp/snmpd.conf file. The agentx implementation reads the config directive from this file and adapts accordingly This allows export of SNMP metrics by the FRR container for eg. signed-off-by: Michel Moriniaux --- src/ax_interface/socket_io.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/ax_interface/socket_io.py b/src/ax_interface/socket_io.py index 1f9934965..ec6c19cfd 100644 --- a/src/ax_interface/socket_io.py +++ b/src/ax_interface/socket_io.py @@ -69,11 +69,13 @@ def parse_socket(self): self.ax_socket_type = 'udp' self.host = 'localhost' self.port = self.ax_socket_path + self.unsuported_method() return # if we have an explicit udp socket if self.ax_socket_path.startswith('udp'): self.ax_socket_type = 'udp' self.host, self.port = self.get_ip_port(self.ax_socket_path.split(':',1)[1]) + self.unsuported_method() return # if we have an explicit tcp socket if self.ax_socket_path.startswith('tcp'): @@ -89,10 +91,11 @@ def parse_socket(self): if '/' in self.ax_socket_path: self.ax_socket_type = 'unix' return - # if at this point we haven't matched anything yet its that we are most likely left with a hort:port pair so UDP + # if at this point we haven't matched anything yet its that we are most likely left with a host:port pair so UDP if ':' in self.ax_socket_path: self.ax_socket_type = 'udp' self.host, self.port = self.get_ip_port(self.ax_socket_path) + self.unsuported_method() return # we should never get here but if we do it's that there is garbage so lets revert to the default of snmp logger.warning("There's something weird with " + self.ax_socket_path + " , using default agentx file socket") @@ -112,6 +115,11 @@ def get_ip_port(self,address): address_list = address.rsplit(':',1) return address_list[0], address_list[1] + def unsuported_method(self): + logger.warning("Socket type " + self.ax_socket_path + " not supported, using default agentx file socket") + self.ax_socket_path = constants.AGENTX_SOCKET_PATH + self.ax_socket_type = 'unix' + async def connection_loop(self): """ Try/Retry connection coroutine to attach the socket. @@ -129,7 +137,7 @@ async def connection_loop(self): if self.ax_socket_type == 'unix': connection_routine = self.loop.create_unix_connection( protocol_factory=lambda: AgentX(self.mib_table, self.loop), - path=constants.AGENTX_SOCKET_PATH, + path=self.ax_socket_path, sock=self.ax_socket) elif self.ax_socket_type == 'udp': # we should not land here as the udp method is in the unsuported list From 4a7fff1526d8bdbd5635cc8aceff7cc0aabe323d Mon Sep 17 00:00:00 2001 From: Michel Moriniaux Date: Tue, 20 Nov 2018 20:32:10 -0800 Subject: [PATCH 10/10] code removal for udp --- src/ax_interface/socket_io.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/src/ax_interface/socket_io.py b/src/ax_interface/socket_io.py index ec6c19cfd..96ca9ab16 100644 --- a/src/ax_interface/socket_io.py +++ b/src/ax_interface/socket_io.py @@ -57,24 +57,17 @@ def parse_socket(self): for method in unsuported_list: if self.ax_socket_path.startswith(method): # This is not a supported method - logger.warning("Socket type " + self.ax_socket_path + " not supported, using default agentx file socket") - self.ax_socket_path = constants.AGENTX_SOCKET_PATH - self.ax_socket_type = 'unix' + self.unsuported_method() return # Now the stuff that we are interested in # First case: we have a simple number then its a local UDP port - # udp has been added to the unsuported_list because asynio throws a not implemented error with udp + # udp has been added to the unsuported_list because asyncio throws a not implemented error with udp # we leave the code here for when it will be implemented if self.ax_socket_path.isdigit(): - self.ax_socket_type = 'udp' - self.host = 'localhost' - self.port = self.ax_socket_path self.unsuported_method() return # if we have an explicit udp socket if self.ax_socket_path.startswith('udp'): - self.ax_socket_type = 'udp' - self.host, self.port = self.get_ip_port(self.ax_socket_path.split(':',1)[1]) self.unsuported_method() return # if we have an explicit tcp socket @@ -93,8 +86,6 @@ def parse_socket(self): return # if at this point we haven't matched anything yet its that we are most likely left with a host:port pair so UDP if ':' in self.ax_socket_path: - self.ax_socket_type = 'udp' - self.host, self.port = self.get_ip_port(self.ax_socket_path) self.unsuported_method() return # we should never get here but if we do it's that there is garbage so lets revert to the default of snmp