-
Notifications
You must be signed in to change notification settings - Fork 93
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
Add routing groups #546
Add routing groups #546
Conversation
9f586cb
to
10fe8cf
Compare
@@ -6,12 +6,29 @@ | |||
control outbound_routing_stage(inout headers_t hdr, | |||
inout metadata_t meta) | |||
{ | |||
|
|||
action set_routing_group_version(bit<1> admin_state) { |
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.
since the parameter is not a version, do you mind to change this to set_routing_group_attr
?
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.
also do you mind to change admin_state to disabled?
there are 2 reasons:
- disabled is more intuitive than admin_state.
- admin_state should not be frequently used (otherwise, why programming it), hence it will be better to make use of the default value.
Making this field disabled with default value set to false seems to be a good way to achieve these 2 things.
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
@@ -33,8 +50,13 @@ control outbound_routing_stage(inout headers_t hdr, | |||
return; | |||
} | |||
|
|||
routing_group.apply(); | |||
if (meta.eni_data.routing_group_admin_state) { |
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.
maybe it is better to create a dedicated structure for routing group data, then we can use meta.routing_group_data.admin_state to access the related states.
As metadata becomes more and more complicated, it will be more future-proof.
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
routing.apply(); | ||
} else { |
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.
nit: drop and return first might be better then if else : D
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
6bc69eb
to
69d3d05
Compare
Make an intermediate stage between ENI and routing to be able to atomically bind/unbind an entire LPM table to/from ENI. Signed-off-by: Marian Pritsak <[email protected]>
PTF tests ok, SAI Challenger tests will be run locally on Marian's machine |
1ade41e
to
a0d995a
Compare
Signed-off-by: Marian Pritsak <[email protected]>
a97b5e0
to
b90351a
Compare
DPUGen to generate config; has hard-coded entries; ENI_ID is no longer a field; @mgheorghe will take a look at fixing the test, fix the outbound routing entry to use routing_group ID and struct instead of ENI. |
Signed-off-by: Marian Pritsak <[email protected]>
Signed-off-by: Marian Pritsak <[email protected]>
Make an intermediate stage between ENI and routing to be able to atomically bind/unbind an entire
LPM table to/from ENI.