-
Notifications
You must be signed in to change notification settings - Fork 1.5k
[Nokia-device][BCM-DNX] Set BCM soc variable to accumulate VOQ counters #21576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: saksarav <[email protected]>
/azp run Azure.sonic-buildimage |
@vmittal-msft @arlakshm @rlhui for viz |
Azure Pipelines successfully started running 1 pipeline(s). |
@saksarav-nokia how about fabric side ? |
@vmittal-msft , this soc property is not applicable to fabric switches. |
Thanks. so we don't need this soc property on fabric side for cumulative PACKET_INTEGRITY counters ? |
Yes Vineet. We already tested in SUP with triggering CRC errors on fabric ports |
@kenneth-arista we may need this change for Arista SKU as well. |
@vmittal-msft , This is what i see in SAI release notes. Eventhough DNX fabric asics do not need this flag for packet_integrity counter, we might need this flag for other counters in future. I will add the flag for Ramons as well. |
… the Voq counters Signed-off-by: saksarav <[email protected]>
/azp run Azure.sonic-buildimage |
Azure Pipelines successfully started running 1 pipeline(s). |
With this change, aren't there implications for orchagent/syncd? |
@saksarav-nokia, can you confirm if these changes will be applicable for the SAI_INTEGRITY counters only and other counters which are polled are not affected by this change? |
@vmittal-msft @kenneth-arista , The following switch and queue (voq) stats counters are the only ones use the sai_instru_stat_accum_enable soc flag. Please let me know if we use any of these switch drop stats and stats extension counters. i could not find these counters being polled. Switch drop stats: Switch Stats extension: Voq counters: |
@saksarav-nokia, thanks for the confirmation. @kenneth-arista, does this answer this question? |
How about via configurable drop counters? See https://github.com/sonic-net/SONiC/blob/master/doc/drop_counters/drop_counters_HLD.md. I'm not sure if the existing infrastructure expect the SAI counters to be clear on read or accummulating. Do you have a reason for changing this behavior? |
@kenneth-arista , Are you sure dnx supports SWITCH_INGRESS_DROPS & SWITCH_EGRESS_DROPS drop counter types?. PORT_INGRESS_DROPS 10 PORT_INGRESS_DROPS Test1 Test1 N/A PORT_INGRESS_DROPS SIP_LOOPBACK N/A sairedis logs: |q|attribute_enum_values_capability|SAI_OBJECT_TYPE_SWITCH:oid:0x21000000000000|SAI_DEBUG_COUNTER_ATTR_IN_DROP_REASON_LIST=100 |
@vmittal-msft , as we discussed with BCM, these soc variables are not required for 202405 since the commit which added the soc variables in SAI 12.X was not back ported to SAI 11.2 |
Does this new SOC property affect all DNX counters including ingress_drop counters ? This is the reason for my question because I don't see any product change in SONiC, whereas this SOC property seemingly affects how the DNX SDK will be handling counter data. Is my question clear? |
@kenneth-arista , this affects only the switch ingress and egress drop counters and not port ingress/egress drop counters as you see in the list of sai attributes i posted . As i mentioned in my previous comment . switch ingress/egress counters are not supported for dnx. |
I understand now that this soc property only affects a subset of counters, in particular SWITCH ingress and egress drops. Even though today the drop counters supports on DNX only enabled port related drop counters, there's not constraint that SWITCH ingress and egress counters won't be supported. In fact, it would be desirable to enable these drop counters. Can you explain why this behavior change is needed? Is there a feature that needs these counters to be accumulated? |
|
Please check the function FlexCounter::addCounter in syncd/FlexCounter.cpp. The statsMode is not SAI_STATS_MODE_READ_AND_CLEAR for SAI_OBJECT_TYPE_SWITCH, so the SAI should n't be clearing the stats on read. So the only way to stop the SAI clearing the stats is by setting this soc variable. |
@kenneth-arista we need these counters to be cumulative otherwise SAI polling was clearing it and we were losing history of these counters. In the absence of cumulative behavior, some poll intervals may see non zero count and next may see zero so our alerting mechanism may not work properly to take action. This is primarily to maintain history and inline with other drop counters which is are cumulative in behavior today and not clear on read. |
MSFT ADO - 31778657 |
…rs (sonic-net#21576) To accumulate the VOQ counters (CREDIT_WD_DELETE & PACKET_INTEGRITY) , the soc variable sai_instru_stat_accum_enable needs to be set --------- Signed-off-by: saksarav <[email protected]>
Why I did it
To accumulate the VOQ counters (CREDIT_WD_DELETE & PACKET_INTEGRITY) , the soc variable sai_instru_stat_accum_enable needs to be set
Work item tracking
How I did it
Added sai_instru_stat_accum_enable to Nokia device files
How to verify it
Verified that the voq counters are accumulated in SAI 12.X
Which release branch to backport (provide reason below if selected)
Tested branch (Please provide the tested image version)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)