Added support for "UNDERLAY_SET_DSCP" and "UNDERLAY_SET_DSCPV6" tables #3307
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE
SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA
SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
8 tasks
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
bingwang-ms
reviewed
Nov 1, 2024
|
Creating the egress table implicitly when creating the ingress table still looks tricky to me. That also makes code logic more complicated. I still suggest separating them. |
bingwang-ms
previously approved these changes
Nov 6, 2024
|
Can you link the HLD to PR? |
prsunny
reviewed
Nov 7, 2024
prsunny
reviewed
Nov 7, 2024
kperumalbfn
reviewed
Nov 7, 2024
…in AddAclRule and removeAclRule
kperumalbfn
reviewed
Nov 12, 2024
orchagent/aclorch.cpp
Outdated
| matchData.data.u32 = to_uint<uint32_t>(attr_value); | ||
| matchData.mask.u32 = 0xFFFFFFFF; | ||
|
|
||
| if (matchData.data.u16 < m_pAclOrch->getAclMetaDataMin() || matchData.data.u16 > m_pAclOrch->getAclMetaDataMax()) |
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MAX] = std::to_string(attrs[0].value.u32range.max); | ||
| metadataMin = uint16_t(attrs[0].value.u32range.min); | ||
| metadataMax = uint16_t(attrs[0].value.u32range.max); | ||
| } |
There was a problem hiding this comment.
Do we need typecast here?
There was a problem hiding this comment.
the metadata values supproted by platforms are 1-7 , and 1-4095 bit. I dont think any platform would support a 32 bit for this field or any tcam would have 4 billion enties.
There was a problem hiding this comment.
i have changed this code to restrict the range to 0-4095
| m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ATTR_META_CAPABLE] = "false"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ACTION_META_CAPABLE] = "false"; | ||
|
|
||
| status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_SWITCH, SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE, &capability); |
There was a problem hiding this comment.
SImilar to validating USER_META_DATA capability, do we need to check ACL action set_dscp capability?
There was a problem hiding this comment.
that is already present on all platforms as we have tables using this capability.
orchagent/aclorch.cpp
Outdated
| bAllAttributesOk = false; | ||
| } | ||
| } | ||
| { |
There was a problem hiding this comment.
pls check indentation here
| uint16_t metaMin = 0; | ||
| uint16_t metaMax = 0; | ||
| list<uint16_t> m_freeMetadata; | ||
| map<uint8_t, uint16_t> m_dscpMetadata; |
There was a problem hiding this comment.
pls check for uint32 for all references to metadata.
There was a problem hiding this comment.
i have limited the metadata to be between 0-4095. beyond this range, i double we ll have a platform which would allow 4k acl entries in a group.
|
/azp run |
|
Commenter does not have sufficient privileges for PR 3307 in repo sonic-net/sonic-swss |
|
/azpw run |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
github-actions bot
pushed a commit
to bradh352/sonic-swss
that referenced
this pull request
Nov 20, 2024
sonic-net#3307) What I did This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6. This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing. These tables are only created on Cisco-8000, Mlnx and VS platforms. The PR also adds support to check if the platform supports the following SAI attributes. SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
mlok-nokia
pushed a commit
to mlok-nokia/sonic-swss
that referenced
this pull request
Nov 22, 2024
sonic-net#3307) What I did This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6. This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing. These tables are only created on Cisco-8000, Mlnx and VS platforms. The PR also adds support to check if the platform supports the following SAI attributes. SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
bradh352
pushed a commit
to bradh352/sonic-swss
that referenced
this pull request
Dec 4, 2024
sonic-net#3307) What I did This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6. This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing. These tables are only created on Cisco-8000, Mlnx and VS platforms. The PR also adds support to check if the platform supports the following SAI attributes. SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
divyachandralekha
pushed a commit
to divyachandralekha/sonic-swss
that referenced
this pull request
Dec 12, 2024
sonic-net#3307) What I did This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6. This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing. These tables are only created on Cisco-8000, Mlnx and VS platforms. The PR also adds support to check if the platform supports the following SAI attributes. SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
divyachandralekha
pushed a commit
to divyachandralekha/sonic-swss
that referenced
this pull request
Dec 12, 2024
sonic-net#3307) What I did This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6. This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing. These tables are only created on Cisco-8000, Mlnx and VS platforms. The PR also adds support to check if the platform supports the following SAI attributes. SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
This PR adds the logical table UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6.
This feature allows SONIC to match an ingress L4 packet and change the DSCP field of the outer header when the packet is egressing.
These tables are only created on Cisco-8000, Mlnx and VS platforms.
The PR also adds support to check if the platform supports the following SAI attributes.
SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE
SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA
SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META
Verified on Mlnx 2700
Why I did it
The match set for these tables are following. The action for both of these tables is SET_DSCP
UNDERLAY_SET_DSCP
UNDERLAY_SET_DSCPV6
Note: These tables are not created but only their names are used.
Since we require matching based on L4 parameters, all the mentioned match attributes are necessary. Merging these into a single table would result in a larger TCAM footprint which may be impact existing ACL usage scenarios.
These table names translate into MARK_META and MARK_METAV6 and EGR_SET_DSCP tables.
The translation is such that an attempt to create UNDERLAY_SET_DSCP or UNDERLAY_SET_DSCPV6 results into creation of
MARK_META & EGR_SET_DSCP or MARK_METAV6 & EGR_SET_DSCP .
The EGR_SET_DSCP table is shared and created only once. if both UNDERLAY_SET_DSCP and UNDERLAY_SET_DSCPV6 are created then only one instance of EGR_SET_DSCP is created to save tcam resource. The EGR_SET_DSCP is created in the Egress stage.
These internal MARK_META/V6 and EGR_SET_DSCP tables use SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA and SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META to set and match the metaData field.
For Example:
if the intent is to match an ingressing packet with fields
"DST_IP": "20.0.0.1/32",
"SRC_IP": "10.0.0.0/32",
"DSCP": "1"
"SRC_PORT" : "10"
"DST_PORT" : "20"
and set its outer DSCP to 12 after encapsulation,
This would be done so by MARK_META entry in ingress tcam matching based on above mentioned critera and setting a metaData value e.g. "1"
Another EGR_SET_DSCP entry in egress stage would be created with match criteria of metaData=1 and action of SET_DSCP=12.
Each entry in the EGR_SET_DSCP table is refcounted and shared among all the rules interested in same DSCP value.
There are 7 metadata values available [1...7]. This is currently hardcoded but this can be enhanced based on capibility check.
A Metadata Manager class governs the allcoation and freeing of each metadata value and they are shared based on DSCP value.
How I verified it
Added 16 Tests in sonic-mgmt. All passing.
Manual Test:
Tests added for the tables and Metadata Manager. 9 Test cases have been added which verfiy the table creation, deletion, EGR_SET_DSCP referencing, Entry creation/deletion, metadata value allcation/ release and exhausion scenarios.
Details if related
tested on mlnx 2700 with following rule
{ "ACL_RULE": {
"OVERLAY_MARK_META_TEST|RULE0": {
"PRIORITY": "999",
"DSCP_ACTION": "40",
"SRC_IP": "1.1.1.1/32"
},
"OVERLAY_MARK_META_TEST|RULE1": {
"PRIORITY": "998",
"DSCP_ACTION": "42",
"SRC_IP": "2.2.2.2/32"
}
} and a vxlan tunnel
reviewed HLD.sonic-net/SONiC#1743