Fix switch capabilities enum query with metadata validation#3866
Fix switch capabilities enum query with metadata validation#3866dhanasekar-arista wants to merge 1 commit intosonic-net:masterfrom
Conversation
queryEnumCapabilitiesSai was using sai_query_attribute_enum_values_capability as a bait to fetch the actual count even though it was throwing an error SAI_STATUS_BUFFER_OVERFLOW. This was not logged as error in SAI until the latest release, hence it got overlooked. The recent SAI upgrade explicitly throws this as an error, hence i've fixed it. This patch improves the queryEnumCapabilitiesSai function by: - Adding metadata validation before making SAI calls - Pre-allocating buffer based on metadata to avoid overflow - Using proper error handling for invalid enum attributes - Following defensive programming practices
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
### |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
hi @kperumalbfn @prabhataravind could you take a look? |
| enumList.list = capList.data(); | ||
| SWSS_LOG_DEBUG("Capability size for objType:%d attrId:%d is count:%d", objType, attrId, meta->enummetadata->valuescount); | ||
| // Pre-allocate based on metadata to avoid buffer overflow | ||
| capList.resize(meta->enummetadata->valuescount); |
There was a problem hiding this comment.
@dhanasekar-arista what is the main difference between sai_query_attribute_enum_values_capability and sai_metadata_get_attr_metadata in getting the enum list?
If there are any custom enums inside vendor SAI library, how does sai_metadata_get_attr_metadata() work?
There was a problem hiding this comment.
The intention of this change is to not replace sai_query_attribute_enum_values_capability with sai_metadata_get_attr_metadata to get the enum list.
sai_query_attribute_enum_values_capability expects the maximum size of enum list to be provided while querying the asic, hence i have made a change to fetch the size using sai_metadata_get_attr_metadata.
I am not very sure about the impact custom enums would have here. do you believe that custom enums will not be returned in sai_metadata_get_attr_metadata?
There was a problem hiding this comment.
Could you pls verify that?
There was a problem hiding this comment.
@kperumalbfn thanks for pointing this out, i couldn't find any reference which mandates the requirement of custom enums should be published or exposed in statically compiled metadata. Hence we should not proceed with this change.
I’ve raised a CSP(CS00012428372) to explore a more suitable approach for handling this scenario. If anyone is already aware of existing guidance or precedents, please do share.
| if (!meta || !meta->isenum) | ||
| { | ||
| return status; | ||
| SWSS_LOG_ERROR("Failed to get enum metadata for objType:%d attrId:%d", objType, attrId); |
There was a problem hiding this comment.
The if condition says NOT enum? Is the error message accurate?
There was a problem hiding this comment.
I think this error message is correct. It means either the attribute is not found, or the attribute is found but not an enum.
|
hi @dhanasekar-arista , do you mind to help check Kumaresh's comment? with this fixed, it will help us fixing 21 different test cases. |
|
The fix posted is not generic enough to handle custom enums inside vendor SAI library. Hence im closing this issue for now. Will wait for update on the CS00012428372 from BRCM and fix it appropriately. FYI, we already have a fix in loganalyzer to ignore this error https://github.com/sonic-net/sonic-mgmt/pull/20605/files |
…ute_capability related errors (#766) #### How did you do it? Added 2 regex to loganalyzer_common_ignore.txt file to ignore 1. sai_query_attribute_enum_values_capability "rv=-8", ignoring non-functional error in swss PR pipeline till sonic-net/sonic-swss#3866 is merged 2. sai_query_attribute_capability "error -2", ignoring till it gets fixed from SAI, CS00012422823 is raised
8 participants
What I did
Fix the queryEnumCapabilitiesSai function to use sai_metadata_get_attr_metadata for fetching the size of the buffer.
Why I did it
queryEnumCapabilitiesSai was using sai_query_attribute_enum_values_capability as a bait to fetch the actual count even though it was throwing an error SAI_STATUS_BUFFER_OVERFLOW. This was not logged as error in SAI until the latest release, hence it got overlooked.
The recent SAI upgrade explicitly throws this as an error, hence i've fixed it.
How I verified it
Ensuring these error logs are not thrown post fix.
Details if related