Add functions to serialize and deserialize strict flags and use these functions for port_error_status by saksarav-nokia · Pull Request #1817 · sonic-net/sonic-sairedis

saksarav-nokia · 2026-03-26T21:02:49Z

Fixes the issue reported in sonic-net/sonic-buildimage#24895
This PR depends on opencomputeproject/SAI#2264

mssonicbld · 2026-03-26T21:02:57Z

/azp run

mssonicbld · 2026-03-27T01:01:52Z

/azp run

azure-pipelines · 2026-03-27T01:02:02Z

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik · 2026-03-27T07:41:07Z

    SWSS_LOG_ENTER();

-    sai_deserialize_enum(s, &sai_metadata_enum_sai_port_error_status_t, (int32_t&)status);
+    sai_deserialize_flags(s, &sai_metadata_enum_sai_port_error_status_t, (int32_t&)status);


please bring serialize enum here, and move sai_desrialize_flags to deserialize_enum and check if flags strict is set

kcudnik · 2026-03-27T07:41:24Z

    SWSS_LOG_ENTER();

-    return sai_serialize_enum(status, &sai_metadata_enum_sai_port_error_status_t);
+    return sai_serialize_flags(status, &sai_metadata_enum_sai_port_error_status_t);


same here as comment in deserialize

kcudnik · 2026-03-27T07:45:46Z

    return sai_serialize_number(value);
 }

+std::string sai_serialize_flags(


you will need unittests for both those methods

kcudnik · 2026-03-27T09:28:06Z

@saksarav-nokia hey, i took a look at your implementaion and it seems too complex for me, i made a proposal for serialization method for you: kcudnik@7ecbac8b
i forgot to add test case in Test where flags == 0

i assumed that meta->valuesnames[0] in each struct enum is always zero, but this will not be the case untill SAI submodule will be updated, so this extra check would need to be added in this serialieze, and i made fix here to support both case scenarios: kcudnik@3dc4b1c

i also added deserialize here:kcudnik@6749a44d its more flexible since it can support 0x token inside string not only at the end, but it don't support ignored values, and i didn't actually tested id xD you can do that :) its also flawed, if you give it empty string you will get 0 as return also if you give it strings with all pipes "|||" it will also return zero, it can be ether bug or feature :) for example "FOO||BAR" will be correctly deserialized, but serialize will not produce such string, and any missing value which is not in enum will be silently ignored which we probalby want to add warning or throw

saksarav-nokia · 2026-03-30T14:14:32Z

@saksarav-nokia hey, i took a look at your implementaion and it seems too complex for me, i made a proposal for serialization method for you: kcudnik@7ecbac8b i forgot to add test case in Test where flags == 0

i assumed that meta->valuesnames[0] in each struct enum is always zero, but this will not be the case untill SAI submodule will be updated, so this extra check would need to be added in this serialieze, and i made fix here to support both case scenarios: kcudnik@3dc4b1c

i also added deserialize here:kcudnik@6749a44d its more flexible since it can support 0x token inside string not only at the end, but it don't support ignored values, and i didn't actually tested id xD you can do that :) its also flawed, if you give it empty string you will get 0 as return also if you give it strings with all pipes "|||" it will also return zero, it can be ether bug or feature :) for example "FOO||BAR" will be correctly deserialized, but serialize will not produce such string, and any missing value which is not in enum will be silently ignored which we probalby want to add warning or throw

Thanks @kcudnik . Do you want me to create a PR and i can close mine. Or do you want me to take your changes, test it and update this PR with the new changes?. Anyway i am pulling your changes, update the unit test for deserialize and testing it. So let me know

mssonicbld · 2026-03-31T01:51:37Z

/azp run

azure-pipelines · 2026-03-31T01:51:49Z

Azure Pipelines successfully started running 1 pipeline(s).

saksarav-nokia · 2026-03-31T01:54:58Z

@kcudnik , i took your changes and fixed minor issues and also added LOG_WARNING in deserialize. also added unit test for deserialize. Tested in the BCM platform based chassis switch.

mssonicbld · 2026-03-31T13:37:49Z

/azp run

azure-pipelines · 2026-03-31T13:38:02Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-31T16:10:00Z

/azp run

azure-pipelines · 2026-03-31T16:10:14Z

Azure Pipelines successfully started running 1 pipeline(s).

kcudnik · 2026-03-31T17:54:32Z

        _In_ const int32_t value,
-        _In_ const sai_enum_metadata_t* meta);
+        _In_ const sai_enum_metadata_t* meta,
+        _In_ bool flagCheck = true);


why this flag is added it should not be used at all, current implementation should be backward compatible

kcudnik · 2026-03-31T17:55:03Z

        _In_ const int32_t value,
-        _In_ const sai_enum_metadata_t* meta)
+        _In_ const sai_enum_metadata_t* meta,
+        bool flagCheck)


plese remove that flag check, the entire purpose to serialize strict flags here in this enum is to not have this flag

I was about to message you in the comment regarding that change . The reason i added that flag is that the sai_stat_st_capability_t -> sai_stat_capability_t has sai_stat_id and uint32_t stat_modes. So when sai_serialize_stats_st_capability_list or sai_serialize_stat_st_capability is called with meta of type sai_stats_mode_t as by the unit_test, sai_serialize_stat_capability was serializing it incorrectly and unit test was failing. If you don't want a flag, i can fix the unit tests serialize_stat_st_capability_list . So sai_serialize_stats_st_capability_list should not be called with sai_metadata_enum_sai_stats_mode_t

yea so this is special case since it's uint32_t field, maybe it should be converetd in SAI to enum,, but currently i see that stat_mode field is serialized as array, so i think it would be ahrd to make it backward compatible with new serialization, maybe just leave satt_mode as is ?

also i just noticed, that having pipe (|) in serializing enum as string, could mess some issues with recording/reading recording file, just wondering on that, didnt hit that caase yet, but then saiplayer parsing would beed to be updated

ok. let me take the sairedis.rec with these changes and paly it in saiplayer and test it.

mssonicbld · 2026-03-31T19:14:49Z

/azp run

azure-pipelines · 2026-03-31T19:15:00Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-31T19:15:06Z

/azp run

azure-pipelines · 2026-03-31T19:15:19Z

Azure Pipelines successfully started running 1 pipeline(s).

mssonicbld · 2026-03-31T20:31:52Z

/azp run

azure-pipelines · 2026-03-31T20:32:06Z

Azure Pipelines successfully started running 1 pipeline(s).

saksarav-nokia · 2026-04-01T01:31:29Z

/azpw run

mssonicbld · 2026-04-01T01:31:32Z

⚠️ Notice: /azpw run only runs failed jobs now. If you want to trigger a whole pipline run, please rebase your branch or close and reopen the PR.
💡 Tip: You can also use /azpw retry to retry failed jobs directly.

Retrying failed(or canceled) jobs...

mssonicbld · 2026-04-01T01:31:34Z

Retrying failed(or canceled) stages in build 1076076:

✅Stage BuildAsan:

Job amd64: retried.

kcudnik · 2026-04-01T06:18:01Z

+            continue;
+        }
+
+        for (i = 0; i < meta->valuescount; ++i)


declare size_t in this ilie in forloop this is not C

@kcudnik , I had it in the for loop first, but when i added the check and SYS_LOG_WARN message after the for loop, i had to use the variable "i" and hence moved it out.
if (i == meta->valuescount)
{
// v is empty or doesn't match any enum
SWSS_LOG_WARN("%s in %s has invalid enum for %s", v.c_str(), s.c_str(), meta->name);
}

@kcudnik , Could you please check my response.

ok then declare it at lease close to for

ok. Thanks. I addressed your comment. Pls check

mssonicbld · 2026-04-13T14:54:24Z

/azp run

azure-pipelines · 2026-04-13T14:54:38Z

Azure Pipelines successfully started running 1 pipeline(s).

… functions for port_error_status Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

… the correct meta Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

mssonicbld · 2026-05-01T00:28:21Z

/azp run

azure-pipelines · 2026-05-01T00:28:31Z

Azure Pipelines successfully started running 1 pipeline(s).

saksarav-nokia mentioned this pull request Mar 26, 2026

Bug: sai_serialize_enum: enum value 3 not found in enum sai_port_error_status_t sonic-net/sonic-buildimage#24895

Open

saksarav-nokia force-pushed the saksarav-nokia-strict-flags branch from 5ab8628 to c8d1f70 Compare March 27, 2026 01:01

kcudnik reviewed Mar 27, 2026

View reviewed changes

saksarav-nokia force-pushed the saksarav-nokia-strict-flags branch from 5e880ad to 99d354b Compare March 31, 2026 13:37

kcudnik reviewed Mar 31, 2026

View reviewed changes

saksarav-nokia force-pushed the saksarav-nokia-strict-flags branch from 06ddd6f to dff4f0c Compare March 31, 2026 19:14

saksarav-nokia force-pushed the saksarav-nokia-strict-flags branch from 90ec43b to 904a266 Compare March 31, 2026 20:31

kcudnik reviewed Apr 1, 2026

View reviewed changes

saksarav-nokia marked this pull request as ready for review April 1, 2026 14:56

mlok-nokia added the Request for 202511 Branch label Apr 17, 2026

saksarav-nokia added 5 commits April 30, 2026 20:05

Add functions to serialize and deserialize strict flags and use these…

4b21aaa

… functions for port_error_status Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

Updated the code with Kamil's code proposal

7f8da35

Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

Fix the pipeline failures

5e878c2

Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

FIx the TEST(SaiSerialize, serialize_stat_st_capability_list) to pass…

5cae053

… the correct meta Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

Fixed the review comment

025f0f8

Signed-off-by: saksarav <sakthivadivu.saravanaraj@nokia.com>

saksarav-nokia force-pushed the saksarav-nokia-strict-flags branch from e45645c to 025f0f8 Compare May 1, 2026 00:28

mssonicbld added the Cherry Pick Conflict_202511 label May 1, 2026

Conversation

saksarav-nokia commented Mar 26, 2026

Uh oh!

mssonicbld commented Mar 26, 2026

Uh oh!

mssonicbld commented Mar 27, 2026

Uh oh!

azure-pipelines Bot commented Mar 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kcudnik commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saksarav-nokia commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines Bot commented Mar 31, 2026

Uh oh!

saksarav-nokia commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines Bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines Bot commented Mar 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines Bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines Bot commented Mar 31, 2026

Uh oh!

mssonicbld commented Mar 31, 2026

Uh oh!

azure-pipelines Bot commented Mar 31, 2026

Uh oh!

saksarav-nokia commented Apr 1, 2026

Uh oh!

mssonicbld commented Apr 1, 2026

Uh oh!

mssonicbld commented Apr 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Apr 13, 2026

Uh oh!

azure-pipelines Bot commented Apr 13, 2026

Uh oh!

mssonicbld commented May 1, 2026

Uh oh!

azure-pipelines Bot commented May 1, 2026

kcudnik commented Mar 27, 2026 •

edited

Loading

saksarav-nokia commented Mar 30, 2026 •

edited

Loading