-
Notifications
You must be signed in to change notification settings - Fork 104
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement RFC4363 dot1qTpFdbTable Q-BRIDGE-MIB #3
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Summarizing my comments:
- double check that older UTs are passing
- simplify the CME class by making a new type of MIBEntry subclass
- general cleanup
src/ax_interface/mib.py
Outdated
@@ -127,10 +135,16 @@ def __init__(self, subtree, sub_ids, value_type, callable_, *args, updater=None) | |||
|
|||
def __iter__(self): | |||
for sub_id in self.sub_ids: | |||
yield self.sub_id + (sub_id,) | |||
if isinstance(sub_id, tuple): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider removing these if/else statements and moving the respective calls into their own class (avoiding any changes to CME)
class MIBSubtreeEntry(MIBEntry):
def __init__(...):
# impl
def __iter__(self):
# impl
def __call__(self, sub_id_group)
# impl
and remember to replace the type inspection for ContextualMIBEntry
in the metaclass __new__
with MIBSubtreeEntry
#Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a new MIBEntry subclass. Considering the dynamic OID feature, do some minor changes on existing CME class to facility the upper level implementation.
In reply to: 97460638 [](ancestors = 97460638)
@@ -0,0 +1,7 @@ | |||
def mac_decimals(mac): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider moving this into ax_interface/util.py
#Resolved
|
||
|
||
def fdb_ifindex(self, sub_id): | ||
assert sub_id is not None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider using mibs.logger
instead of assert
.
consider throwing a ValueError instead #WontFix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/test_fdb.py
Outdated
from ax_interface.constants import PduTypes | ||
from sonic_ax_impl.mibs import ieee802_1ab | ||
from sonic_ax_impl.mibs.ietf import rfc4363 | ||
from sonic_ax_impl.main import SonicMIB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleanup these imports #Resolved
from enum import unique, Enum | ||
|
||
from sonic_ax_impl import mibs | ||
from sonic_ax_impl.mibs.ietf import * |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't recommend import *
in any production code. For the mac_decimals function (see my other comment) and:
from ax_interface import ..., util
``` #Resolved
tests/test_fdb.py
Outdated
class TestFdbMIB(TestCase): | ||
@classmethod | ||
def setUpClass(cls): | ||
class FdbMIB(rfc4363.FdbMIB): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the intent of this declaration. #Resolved
assert sub_id is not None | ||
return self.vlanmac_ifindex_map[sub_id] | ||
|
||
class FdbMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.17.7.1.2.2.1'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update the readme with the RFC link
'Forwarding Database' https://tools.ietf.org/html/rfc4363 #Resolved
tests/test_fdb.py
Outdated
#mib_entry = self.lut[(1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2, 1000, 124, 254, 144, 128, 159, 92)] | ||
|
||
def test_getpdu(self): | ||
get_pdu = GetPDU( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes were made to getnext.
Verify that other getnext/get UTs weren't affected.
Then, add an equivalent test for getnext from
- (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 2) -- should find the first full vlan & mac
- (1, 3, 6, 1, 2, 1, 17, 7, 1, 2, 2, 1, 3) -- should report no such object
#Resolved
@tombo315 This PR is still open to any comments. |
Squashed commit of the following: commit 6c580806f962d508056e687108e04dc2a9df9276 Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Fri Jan 26 15:08:53 2018 -0800 Fix queue counter value type, and wrap queue index (sonic-net#4) * Fix queue counter value type * Wrap queue index commit 66d738c297f852a5d504de1405ae55ff7d45dd0f Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Fri Jan 26 02:44:32 2018 +0000 Added ciscoPfcExtMIB, ciscoSwitchQosMIB MIBs Squashed commit of the following: commit 0b68d9ed01ffe6a4b06345494e7bf7d3728c2371 Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Thu Jan 25 18:27:53 2018 -0800 Fix PFC queue index base and value type (sonic-net#3) * Fix PFC queue index base * Fix pfc counter64 commit e84d1d89b8925cb547dfd435a5c1e8b58c74dedc Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Tue Jan 23 23:59:51 2018 +0000 Added ciscoPfcExtMIB, ciscoSwitchQosMIB MIBs Squashed commit of the following: commit f0cbfff09066e372c1fe99983692a1dd719e5e80 Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Tue Jan 23 23:20:02 2018 +0000 Fix Pfc/Qos counter integer range commit c954e4e44d1e44fc5b4b2269a5bbcee63aeb07c7 Author: AndriiS <andriis@mellanox.com> Date: Thu Jan 11 21:51:35 2018 +0200 Added ciscoPfcExtMIB, ciscoSwitchQosMIB MIBs (#1) Remove lag from csqIfQosGroupStatsTable (sonic-net#5)
Squashed commit of the following: commit 6c580806f962d508056e687108e04dc2a9df9276 Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Fri Jan 26 15:08:53 2018 -0800 Fix queue counter value type, and wrap queue index (#4) * Fix queue counter value type * Wrap queue index commit 66d738c297f852a5d504de1405ae55ff7d45dd0f Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Fri Jan 26 02:44:32 2018 +0000 Added ciscoPfcExtMIB, ciscoSwitchQosMIB MIBs Squashed commit of the following: commit 0b68d9ed01ffe6a4b06345494e7bf7d3728c2371 Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Thu Jan 25 18:27:53 2018 -0800 Fix PFC queue index base and value type (#3) * Fix PFC queue index base * Fix pfc counter64 commit e84d1d89b8925cb547dfd435a5c1e8b58c74dedc Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Tue Jan 23 23:59:51 2018 +0000 Added ciscoPfcExtMIB, ciscoSwitchQosMIB MIBs Squashed commit of the following: commit f0cbfff09066e372c1fe99983692a1dd719e5e80 Author: Qi Luo <qiluo-msft@users.noreply.github.com> Date: Tue Jan 23 23:20:02 2018 +0000 Fix Pfc/Qos counter integer range commit c954e4e44d1e44fc5b4b2269a5bbcee63aeb07c7 Author: AndriiS <andriis@mellanox.com> Date: Thu Jan 11 21:51:35 2018 +0200 Added ciscoPfcExtMIB, ciscoSwitchQosMIB MIBs (#1) Remove lag from csqIfQosGroupStatsTable (#5)
No description provided.