-
Notifications
You must be signed in to change notification settings - Fork 98
[dash-p4] Add ENI mode and trusted vni stage #672
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
Conversation
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
@@ -16,6 +16,10 @@ sai_apis: | |||
name: SAI_DIRECTION_LOOKUP_ENTRY_ACTION_SET_OUTBOUND_DIRECTION |
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.
Will be great to update the title to cover all changes in 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.
Updated
dash-pipeline/bmv2/dash_pipeline.p4
Outdated
@@ -134,6 +137,9 @@ control dash_eni_stage( | |||
if (!eni.apply().hit) { | |||
UPDATE_COUNTER(eni_miss_drop, 0); | |||
} | |||
else if (meta.eni_data.eni_mode == dash_eni_mode_t.FNIC) { | |||
trusted_vni_stage.apply(hdr, meta); |
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.
this one should be in global, shared by all ENIs.
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.
Fixed
} | ||
|
||
@SaiTable[single_match_priority = "true", api = "dash_trusted_vni"] | ||
table trusted_vni { |
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.
eni_trusted_vni
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.
Fixed
{ | ||
action permit() {} | ||
|
||
action deny() { |
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.
iirc, we have a global deny function and maybe we can use that instead.
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.
Use global deny function
return; | ||
} | ||
|
||
trusted_vni.apply(); |
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.
need counter to track the drops.
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.
Add counter eni_trusted_vni_entry_miss_drop
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
} | ||
} | ||
|
||
@SaiTable[single_match_priority = "true", api = "dash_trusted_vni"] |
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.
Add order.
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.
Added
@@ -278,6 +277,7 @@ control dash_ingress( | |||
(meta.flow_sync_state == dash_flow_sync_state_t.FLOW_MISS && | |||
hdr.packet_meta.packet_source == dash_packet_source_t.EXTERNAL)) | |||
{ | |||
trusted_vni_stage.apply(hdr, meta); |
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 we missed the inbound routing table for the VNI matching, if we simply return it here.
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.
Add a TODO comment and will revisit it after inbound routing HLD done.
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
return; | ||
} | ||
|
||
if (!eni_trusted_vni.apply().hit) { |
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.
better add a TODO comment to make sure the pipeline can capture the behavior right.
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.
Have added TODO comment before trusted_vni_stage.apply(hdr, meta)
.
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
/azp run |
Commenter does not have sufficient privileges for PR 672 in repo sonic-net/DASH |
Referring to sonic-net/SONiC#1911 and #665, to support FNIC pipeline, this PR adds the followings:
eni_trusted_vni_entry_miss_drop
set_inbound_direction
is not defaultonly at tabledirection_lookup
global_trusted_vni
andeni_trusted_vni