Skip to content

[recorder] Fix incorrect attribute enum value capability query#843

Merged
kcudnik merged 9 commits intosonic-net:masterfrom
stepanblyschak:fix-enum-value-cap-query-recorder
Jun 30, 2021
Merged

[recorder] Fix incorrect attribute enum value capability query#843
kcudnik merged 9 commits intosonic-net:masterfrom
stepanblyschak:fix-enum-value-cap-query-recorder

Conversation

@stepanblyschak
Copy link
Contributor

Recorder was assuming that enum value capability query is executed for an
attribute that has a value type of a s32 list.

When querying SAI_DEBUG_COUNTER_ATTR_TYPE using
sai_query_attribute_enum_values_capability a warning is printed in
syslog: "enum value 4 not found in enum sai_debug_counter_type".

This happens because SAI_DEBUG_COUNTER_ATTR_TYPE attrvaluetype is int32
(enum value) and sai_s32_list_t structure was casted to int32. Since we
initialize sai_s32_list with .count = 4 (the number of values in
sai_debug_counter_type_t enum) value 4 is printed in the syslog.

Signed-off-by: Stepan Blyschak [email protected]

Recorder was assuming that attribute query is executed for is has a
valuetype of a s32 list.

When querying SAI_DEBUG_COUNTER_ATTR_TYPE using
sai_query_attribute_enum_values_capability a warning is printed in
syslog: "enum value 4 not found in enum sai_debug_counter_type".

This happens because SAI_DEBUG_COUNTER_ATTR_TYPE attrvaluetype is int32
(enum value) and sai_s32_list_t structure was casted to int32. Since we
initialize sai_s32_list with .count = 4 (the number of values in
sai_debug_counter_type_t enum) value 4 is printed in the syslog.

Signed-off-by: Stepan Blyschak <[email protected]>
@kcudnik
Copy link
Collaborator

kcudnik commented Jun 17, 2021

int32 is exact same as s32 type, what's the problem here? if the problem i that count is treated as first value, then point it to list.list

@stepanblyschak
Copy link
Contributor Author

@kcudnik

This serialization works fine, because SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST has attrvaluetype of SAI_ATTR_VALUE_TYPE_INT32_LIST -> sai_serialize_enum_list(attr.value.s32list, meta.enummetadata, countOnly); will be called.:

attr.id = SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST;
attr.value.s32list = *enumValuesCapability;
auto values = SaiAttributeList::serialize_attr_list(objectType, 1, &attr, true);

And below code will not work and throw a warning in log, attrvaluetype for SAI_DEBUG_COUNTER_ATTR_TYPE is SAI_ATTR_VALUE_TYPE_INT32 -> sai_serialize_enum(attr.value.s32, meta.enummetadata); will be called :

attr.id = SAI_DEBUG_COUNTER_ATTR_TYPE;
attr.value.s32list = *enumValuesCapability;
auto values = SaiAttributeList::serialize_attr_list(objectType, 1, &attr, true);

Overall this looks strange because enum value capability serialization is handled as an attribute serialization but enum value capability is not an attribute.

[KC] if the problem i that count is treated as first value, then point it to list.list
Didn't get this part.

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 17, 2021

please provide unittest that will test both test cases

@stepanblyschak
Copy link
Contributor Author

@kcudnik I have thought about it, I wanted to use existing infra (sonic-sairedis/tests), but I found that even existing tests BCM56850/query_attr_enum_values_capability.rec are not checked because 'q' and 'Q' types of operations are skipped - https://github.com/Azure/sonic-sairedis/blob/master/saiplayer/SaiPlayer.cpp#L1980

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 17, 2021

could you then add explicit test in lib/src/tests.cpp for Recorder.cpp class for this api?

Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
Signed-off-by: Stepan Blyschak <[email protected]>
@kcudnik
Copy link
Collaborator

kcudnik commented Jun 22, 2021

 *** ./../lib/src/tests.cpp : method is missing SWSS_LOG_ENTER():
 std::vector<std::string> parseFirstRecordedAPI()
{

run "make check" locally to see if all test will pass

Signed-off-by: Stepan Blyschak <[email protected]>
@liat-grozovik
Copy link
Collaborator

@kcudnik could you please merge?
@stepanblyschak should it be taken for other active branches such as 201911 and 202012? can it be cleanly cherry picked?

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 24, 2021

something is broken here in tests, can you force push to this ? or add some dumy commit/squast, to retrigger build?

@stepanblyschak
Copy link
Contributor Author

@liat-grozovik Yes, cherry-pick into 202012 is clean.
201911 I don't think issues exists there, it is a pretty different code base.

@liat-grozovik
Copy link
Collaborator

@qiluo-msft or @kcudnik have no permission to run tests on this repo. Can you please retrigger?

@kcudnik
Copy link
Collaborator

kcudnik commented Jun 26, 2021

something is wrong with this tests, they get canceled, not sure whats happening on this pipeline

@qiluo-msft
Copy link
Contributor

It always timeout after 180 min. So something is hanging at "Install dependencies" step.

@kcudnik kcudnik merged commit 3c485e5 into sonic-net:master Jun 30, 2021
kcudnik pushed a commit to kcudnik/sonic-sairedis that referenced this pull request Jul 1, 2021
…-net#843)

Recorder was assuming that enum value capability query is executed for an
attribute that has a value type of a s32 list.

When querying SAI_DEBUG_COUNTER_ATTR_TYPE using
sai_query_attribute_enum_values_capability a warning is printed in
syslog: "enum value 4 not found in enum sai_debug_counter_type".

This happens because SAI_DEBUG_COUNTER_ATTR_TYPE attrvaluetype is int32
(enum value) and sai_s32_list_t structure was casted to int32. Since we
initialize sai_s32_list with .count = 4 (the number of values in
sai_debug_counter_type_t enum) value 4 is printed in the syslog.
qiluo-msft pushed a commit that referenced this pull request Jul 14, 2021
Recorder was assuming that enum value capability query is executed for an
attribute that has a value type of a s32 list.

When querying SAI_DEBUG_COUNTER_ATTR_TYPE using
sai_query_attribute_enum_values_capability a warning is printed in
syslog: "enum value 4 not found in enum sai_debug_counter_type".

This happens because SAI_DEBUG_COUNTER_ATTR_TYPE attrvaluetype is int32
(enum value) and sai_s32_list_t structure was casted to int32. Since we
initialize sai_s32_list with .count = 4 (the number of values in
sai_debug_counter_type_t enum) value 4 is printed in the syslog.
pettershao-ragilenetworks pushed a commit to pettershao-ragilenetworks/sonic-sairedis that referenced this pull request Nov 18, 2022
…-net#843)

Recorder was assuming that enum value capability query is executed for an
attribute that has a value type of a s32 list.

When querying SAI_DEBUG_COUNTER_ATTR_TYPE using
sai_query_attribute_enum_values_capability a warning is printed in
syslog: "enum value 4 not found in enum sai_debug_counter_type".

This happens because SAI_DEBUG_COUNTER_ATTR_TYPE attrvaluetype is int32
(enum value) and sai_s32_list_t structure was casted to int32. Since we
initialize sai_s32_list with .count = 4 (the number of values in
sai_debug_counter_type_t enum) value 4 is printed in the syslog.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants