Orchagent validates mirror session queue parameter against maximum va… #2
Conversation
…lue from SAI Signed-off-by: Raphael Tryster <raphaelt@nvidia.com>
| { | ||
| SWSS_LOG_WARN("Failed to get switch attribute number of traffic classes. \ | ||
| Use default value. rv:%d", status); | ||
| m_maxNumTC = MIRROR_SESSION_DEFAULT_NUM_TC; |
There was a problem hiding this comment.
This MIRROR_SESSION_DEFAULT_NUM_TC is set to 15.
Could you please elaborate where this number comes from? Is it vendor specific? Is it theoretical maximum for all?
I get that we need to have some default number of TCs.
In example for a case when SAI_SWITCH_ATTR_QOS_MAX_NUMBER_OF_TRAFFIC_CLASSES is not implemented or just failed.
But just trying to understand if 15 is the right one.
There was a problem hiding this comment.
15 is the right number for Mellanox. I think it is vendor-specific, but I am not sure. Since it is right for one vendor, it is surely no worse than any other number.
There was a problem hiding this comment.
I discussed this with Marian the architect. He said that if the vendor's SAI does not supply the limit, we should use the largest possible value, effectively cancelling validation by orchagent. Since the queue parameter is 8 bits, I changed the default to 255. It is unlikely that anyone uses this value. Itai and Moshe commented respectively, "The industry standard are 8 pfc", "Default should be 15 but some vendors duplicate each 2 to 1 means 8".
… if vendor SAI does not specify limit Signed-off-by: Raphael Tryster <raphaelt@nvidia.com>
* [cbf] Added initial CBF support (#2) * Added CBF NHG support to NhgOrch * Added NhgMapOrch to handle DSCP_TO_FC and EXP_TO_FC tables * Added UT for the new NhgOrch function and NhgMapOrch Support sonic-net/SONiC#796 Co-authored-by: Alexandru Banu <v-albanu@microsoft.com>
…lue from SAI
Signed-off-by: Raphael Tryster raphaelt@nvidia.com
What I did
Orchagent queries SAI for the maximum value of the queue parameter. If the value requested by the user is invalid, it writes an error in the log and does not pass the request to SAI/SDK.
Why I did it
Fixes https://github.com/Azure/sonic-buildimage/issues/8189[mirroring] Missing per-vendor validation of mirror session queue parameter. Without this change, orchagent passes the invalid data to SAI/SDK which returns failure, causing orchagent to exit, effectively a system crash.
How I verified it
Manual and unit test: Create 3 mirror sessions with queue = 0, maximum valid value, lowest invalid value. First two cases, sessions are created. For last case, session not created in state DB and error in log.
Details if related