-
Notifications
You must be signed in to change notification settings - Fork 273
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
Added query Enum value capability for vxlan EVPN feature #920
Conversation
check build errors |
/azpw run |
this test failure is not related to the PR |
@kcudnik Can we merge this PR? What is the approach? |
can you add unittests for this feature ? in sairedis/unittests/vslib/ |
vslib/SwitchStateBase.cpp
Outdated
|
||
if (object_type == SAI_OBJECT_TYPE_TUNNEL && attr_id == SAI_TUNNEL_ATTR_PEER_MODE) | ||
{ | ||
return queryVxlanTunnelPeerModeCapability(enum_values_capability); |
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.
why there is Vxlan in in name here? there is no vxlan object type nor attribute in name, this is just for tunnel
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.
Removed
Sure. Can you please share the steps on how to run these unit tests? |
yea, just type "make check" in test directory |
I ran the unit tests [----------] 5 tests from SwitchMLNX2700 [----------] 5 tests from SwitchBCM56850 |
@kcudnik I have added tests to validate new APIs |
fake_platfrom sholud not be used anywhere its a terrible idea to use that variable anywhere |
The infrastructure cleanup it's a big work item that will be done in some later point of time. For now fake_platform isn't used anywhere inside sai-redis |
sonic project is a classic book example for "will be done in some later point" translates to never |
int32_t list[2]; | ||
enum_val_cap.count = 2; | ||
enum_val_cap.list = list; | ||
EXPECT_EQ(sw.queryAttrEnumValuesCapability(0x2100000000, SAI_OBJECT_TYPE_TUNNEL, |
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.
please use 1 parameter per line if you want to break function params to multiple lines, also do this across this PR
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.
Done
Taking up the entire cleanup work is not within the timelines of the current feature. It's a new work item by itself. We have prioritized this work item as it is also required for us to specify platform and I agree fake_platform isn't a good approach. |
The test failures are not related to changes in the PR |
Added query enum value capability for some attributes which are required for EVPN VxLAN feature.