support to check SAI capability of ACL META_DATA fields #3274
+102
−2
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3173,7 +3173,6 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr | |
| m_L3V4V6Capability[ACL_STAGE_INGRESS] ? "yes" : "no", | ||
| m_L3V4V6Capability[ACL_STAGE_EGRESS] ? "yes" : "no"); | ||
|
|
||
| // In Mellanox platform, V4 and V6 rules are stored in different tables | ||
| // In Broadcom DNX platform also, V4 and V6 rules are stored in different tables | ||
| if (platform == MLNX_PLATFORM_SUBSTRING || | ||
|
|
@@ -3189,6 +3188,100 @@ void AclOrch::init(vector<TableConnector>& connectors, PortsOrch *portOrch, Mirr | |
| m_isCombinedMirrorV6Table = true; | ||
| } | ||
|
|
||
| if (platform == VS_PLATFORM_SUBSTRING) | ||
| { | ||
| // For testing on VS the following values will be used. | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_RANGE_CAPABLE] = "true"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MIN] = "1"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MAX] = "7"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ATTR_META_CAPABLE] = "true"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ACTION_META_CAPABLE] = "true"; | ||
| } | ||
| else | ||
| { | ||
| // check switch capability of Metadata attribute, action and range. | ||
| // SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE support and range values. | ||
| // SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA | ||
| // SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META | ||
|
|
||
| sai_status_t status = SAI_STATUS_SUCCESS; | ||
| sai_attr_capability_t capability; | ||
| uint16_t metadataMin = 0; | ||
| uint16_t metadataMax = 0; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MIN] = "0"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MAX] = "0"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_RANGE_CAPABLE] = "false"; | ||
| 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); | ||
| // Disabling these blocks to stop coverage analysis failure. The subsequent PR will enable these. | ||
| // if (status != SAI_STATUS_SUCCESS) | ||
|
Collaborator
There was a problem hiding this comment. No commented code per guidelines |
||
| // { | ||
| // SWSS_LOG_WARN("Could not query ACL_USER_META_DATA_RANGE %d", status); | ||
| // } | ||
| // else | ||
| if (status == SAI_STATUS_SUCCESS) | ||
|
Contributor
There was a problem hiding this comment. The code commented out is required. Is there any other way to resolve the code coverage issue?
Contributor
Author
There was a problem hiding this comment. I have not found a way to resolve this by other means. This is initialization code. The Vs environment does not populate these. There are VS tests in the 2nd PR which cover these values.
Contributor
There was a problem hiding this comment. If it can't be covered by UT in anyways, can you try to bypass the check as it's a test issue? It doesn't make sense to remove feature code for UT coverage.
Collaborator
There was a problem hiding this comment. agree, please refer to other PRs with capability query support added |
||
| { | ||
| if (capability.get_implemented) | ||
| { | ||
| sai_attribute_t attrs[1]; | ||
| attrs[0].id = SAI_SWITCH_ATTR_ACL_USER_META_DATA_RANGE; | ||
| sai_status_t status = sai_switch_api->get_switch_attribute(gSwitchId, 2, attrs); | ||
| // Disabling these blocks to stop coverage analysis failure. The subsequent PR will enable these. | ||
| // if (status != SAI_STATUS_SUCCESS) | ||
|
Collaborator
There was a problem hiding this comment. Same here. no commented code. I think this PR is still in draft state |
||
| // { | ||
| // SWSS_LOG_WARN("Could not get range for ACL_USER_META_DATA_RANGE %d", status); | ||
| // } | ||
| // else | ||
| if (status == SAI_STATUS_SUCCESS) | ||
| { | ||
| // SWSS_LOG_NOTICE("ACL_USER_META_DATA_RANGE, min: %u, max: %u", attrs[0].value.u32range.min, attrs[0].value.u32range.max); | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_RANGE_CAPABLE] = "true"; | ||
| m_switchMetaDataCapabilities[TABLE_ACL_USER_META_DATA_MIN] = std::to_string(attrs[0].value.u32range.min); | ||
| 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); | ||
| } | ||
|
|
||
| } | ||
| SWSS_LOG_NOTICE("ACL_USER_META_DATA_RANGE capability %d", capability.get_implemented); | ||
| } | ||
| status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_ACL_ENTRY, SAI_ACL_ENTRY_ATTR_FIELD_ACL_USER_META, &capability); | ||
| // Disabling these blocks to stop coverage analysis failure. The subsequent PR will enable these. | ||
| // if (status != SAI_STATUS_SUCCESS) | ||
| // { | ||
| // SWSS_LOG_WARN("Could not query ACL_ENTRY_ATTR_FIELD_ACL_USER_META %d", status); | ||
| // } | ||
| // else | ||
| if (status == SAI_STATUS_SUCCESS) | ||
| { | ||
| if (capability.set_implemented) | ||
| { | ||
| m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ATTR_META_CAPABLE] = "true"; | ||
| } | ||
| SWSS_LOG_NOTICE("ACL_ENTRY_ATTR_FIELD_ACL_USER_META capability %d", capability.set_implemented); | ||
| } | ||
|
|
||
| status = sai_query_attribute_capability(gSwitchId, SAI_OBJECT_TYPE_ACL_ENTRY, SAI_ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA, &capability); | ||
| // Disabling these blocks to stop coverage analysis failure. The subsequent PR will enable these. | ||
| // if (status != SAI_STATUS_SUCCESS) | ||
| // { | ||
| // SWSS_LOG_WARN("Could not query ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA %d", status); | ||
| // } | ||
| // else | ||
| if (status == SAI_STATUS_SUCCESS) | ||
| { | ||
| if (capability.set_implemented) | ||
| { | ||
| m_switchMetaDataCapabilities[TABLE_ACL_ENTRY_ACTION_META_CAPABLE] = "true"; | ||
| } | ||
| SWSS_LOG_NOTICE("ACL_ENTRY_ATTR_ACTION_SET_ACL_META_DATA capability %d", capability.set_implemented); | ||
| } | ||
| // The following lines would be replaced in the next PR where these values would be used to initialze the metadataMgr. | ||
| (void)metadataMin; | ||
|
Collaborator
There was a problem hiding this comment. Remove unused params. This can also fail the coverage. |
||
| (void)metadataMax; | ||
|
Contributor
There was a problem hiding this comment. Can we remove these two variables if they are not used?
Contributor
Author
There was a problem hiding this comment. They are used in the 2nd PR. which #3307.
Contributor
There was a problem hiding this comment. Then can we add these variables in the 2nd PR?
Contributor
Author
There was a problem hiding this comment. the 2nd PR is a child of this PR. In an effort to make the review easier, I created this PR first. the 2nd PR branch is a child of this branch. once this goes in, it would use these variables. |
||
| } | ||
|
|
||
| // Store the capabilities in state database | ||
| // TODO: Move this part of the code into syncd | ||
|
|
||
This file contains hidden or 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
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.
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 dont use magic numbers