Skip to content

CLI for Configuring PFC Historical Statistics#3779

Merged
vmittal-msft merged 3 commits intosonic-net:masterfrom
peterbailey-arista:pfc-stat-hist-config
Aug 5, 2025
Merged

CLI for Configuring PFC Historical Statistics#3779
vmittal-msft merged 3 commits intosonic-net:masterfrom
peterbailey-arista:pfc-stat-hist-config

Conversation

@peterbailey-arista
Copy link
Contributor

@peterbailey-arista peterbailey-arista commented Feb 25, 2025

What I did

Added support for configuring pfc statistical history through following commands:

  • config pfcwd start --pfc-stat-history
  • config pfcwd pfc_stat_history <enable|disable>
  • show pfcwd config

These changes were made for the PFC Historical Statistics feature: sonic-net/SONiC#1904

How I did it

  • Added to the PFCWD script with a pfc-stat-history flag for starting the watchdog with history enabled
  • Added to the PFCWD script with a pfc_stat_history command for enabling/disabling history on selected ports

How to verify it

  • Run the sonic-utilities unit tests
  • Run config pfcwd start <ports> <detection-time> --pfc-stat-history on a switch, verify the configurations with show pfcwd config
  • Run config pfcwd pfc_stat_history enable|disable <ports> on a switch, verify the configurations with show pfcwd config

New command output (if the output of a command-line utility has changed)

Example output of show pfcwd config:

Changed polling interval to 600ms
     PORT    ACTION    DETECTION TIME    RESTORATION TIME    HISTORY
---------  --------  ----------------  ------------------  ---------
Ethernet0   forward               102                 101    disable
Ethernet4      drop               600                 600     enable
Ethernet8      drop               600                 600    disable

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

PFC_STAT_HISTORY_DEFAULT_STATUS = "disable"

#
# 'pfc-stat-history' group ('config pfc-stat-history ...')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you move new config command to config directory? I don't think utilities_common is a place to add new config command

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for reviewing this as well.
The implementation has since changed to use the PFCWD orchagent so I have also changed the CLI to be configured through the PFCWD config and show commands instead.

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@peterbailey-arista peterbailey-arista marked this pull request as ready for review April 8, 2025 22:16
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vmittal-msft vmittal-msft merged commit c0838d7 into sonic-net:master Aug 5, 2025
7 checks passed
@vaibhavhd
Copy link
Contributor

@peterbailey-arista I think this PR has introduced a test regression that is preventing submodule update PR from passing the PR tests.

I checked the failure logs and it seems like this PR from Arista is causing OA to not process the stats
Submodule update PR that is blocked - sonic-net/sonic-buildimage#23557

Example of the failed run - https://elastictest.org/scheduler/testplan/68c920ad288a03159090b019?testcase=generic_config_updater%2Ftest_pfcwd_status.py%7C%7C%7C2&type=console

Syslog errors from the failed run:

'2025 Sep 16 09:31:04.158401 vlab-01 ERR swss#orchagent: :- createEntry: Failed to parse PFC Watchdog Ethernet4 configuration. Unknown attribute pfc_stat_history.\n',
'2025 Sep 16 09:31:04.159063 vlab-01 ERR swss#orchagent: :- doTask: Failed to process PFC watchdog SET task, invalid entry\n',

There is another issue that I suspect here - None/ Null in the config that leads to config check failure:
extract_pfcwd_config = defaultdict(None, {'Ethernet4': {'action': 'drop', 'detect_time': '200', 'restore_time': '200'}

DEBUG:tests.conftest:core_dump_and_config_check failed, check_result:
"config_db_check": {"failed": true,
"inconsistent_config": {"vlab-01": {"null": {}}}}}

@peterbailey-arista
Copy link
Contributor Author

@vaibhavhd I see that sonic-net/sonic-buildimage#23491 which contains submodule bump up with the orchagent change to process the pfc stat history param is also unmerged.

Thank you for bringing this to my attention, the fix for the syslog errors is for the swss change to merge first. Is that possible?

For the generic_config_updater test I see sonic-net/sonic-mgmt#20667 has already been made to handle the problem. So the fix should be to include this commit (70648ef) in the submodule bump up as well

YairRaviv pushed a commit to YairRaviv/sonic-utilities that referenced this pull request Jan 12, 2026
* Add pfc stat history options to PFCWD CLI
* Unit tests for PFCWD pfc history cli
Includes modifications made to pass pre-commit (fixing import *).
* Add pfc-stat-history command and flag for PFCWD to docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants