Feature/ PFCWD Hardware Recovery HLD#2159
Conversation
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
/azp run |
|
No pipelines are associated with this pull request. |
- Comprehensive design document for hardware-based PFC recovery - Includes architecture refactoring proposal with PfcWdHwOrch - New CLI command: show pfcwd status with hardware-specific information - SAI attributes and implementation details for hardware recovery - Flow diagrams for hardware vs software recovery decision and workflow - Testing requirements and validation approach - Hardware timer granularity constraints and actual vs configured values - Comparison table highlighting differences between software and hardware recovery Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Changed 'CLI/YANG' to 'CLI-YANG' to fix markdown anchor link - Forward slash in header was breaking clickable navigation Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Updated TOC to reflect actual document structure - Removed non-existent subsections 12.2, 12.3, 12.4 - Only section 12.1 exists in the streamlined testing section Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Better accuracy in storm detection and restoration timing - Reduced latency compared to software polling-based approach - Lower CPU overhead by eliminating continuous software polling - Hardware operates at line rate without software delays Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Section 13 Open Points does not exist in the document - Cleaned up Table of Contents to match actual document structure Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Added CLI Data Flow subsection to TOC for proper navigation - Fixes parsing/navigation issue in section 5 Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Simplified the display table description in mermaid flowchart - Removed pipe characters that could cause parsing issues - Improved readability of the CLI data flow diagram Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
- Created doc/pfcwd/ directory for PFC Watchdog documentation - Moved pfc_hardware_recovery_hld.md from doc/qos/ to doc/pfcwd/ - Better organization following SONiC documentation structure - Dedicated directory allows for future PFC Watchdog related documents Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
d793e08 to
ee7c047
Compare
|
/azp run |
|
No pipelines are associated with this pull request. |
- Added STATUS column to show pfcwd status command output to indicate success/failed programming status - Updated CLI data flow diagram to include status validation - Added example showing failed configuration due to unsupported timer ranges - Updated hardware recovery workflow diagram to show event notification happening immediately after storm detection - Added conditional logic for app_managed_recovery flag: - If true: wait for SAI_QUEUE_ATTR_PFC_DLR_INIT programming - If false: hardware automatically applies recovery action Signed-off-by: Pinky Agrawal <pinky@nexthop.ai>
|
/azp run |
|
No pipelines are associated with this pull request. |
| No YANG model changes are required for this feature. The new `show pfcwd status` command displays operational state data, not configuration data. | ||
|
|
||
| **Rationale**: The new command displays runtime operational data (actual hardware values, granularity constraints, recovery type) rather than user configuration. This operational data is derived from: | ||
| - Platform hardware capabilities (granularity values) |
There was a problem hiding this comment.
Any SAI capability query to get the hardware granularity? If so, please include SAI attributes
There was a problem hiding this comment.
Added the relevant attributes to the section
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
9 similar comments
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
7dca809 to
cda7331
Compare
|
/azp run |
|
No pipelines are associated with this pull request. |
Drive the PfcWdHwOrch-vs-PfcWdSwOrch path selection solely from the SAI capability query. Remove the SKU-based path determination and the operator override for path selection, so bringing a platform onto hardware recovery is purely a function of its SAI advertising the capability. Addresses review feedback from @eddyk-nvidia, @kperumalbfn, @yunang-c. Signed-off-by: pinky-nexthop <pinky@nexthop.ai>
cda7331 to
2ebf808
Compare
|
/azp run |
|
No pipelines are associated with this pull request. |
|
@saksarav-nokia @ysmanman @arista-nwolfe please help review soon as we are planning to enable HW based PFCWD for upcoming SKUs and potentially for chassis as well. |
|
/easycla |
|
@prsunny @vmittal-msft |
|
@pinky-nexthop i am waiting for @kperumalbfn to approve as well. |
|
/azp run |
|
No pipelines are associated with this pull request. |
|
@kperumalbfn @eddyk-nvidia please help approve. |
Summary
This PR introduces a comprehensive High Level Design document for PFC Watchdog Hardware Recovery feature that provides better accuracy in storm detection and restoration compared to software-based approaches.
Key Features
show pfcwd statuswith hardware-specific information including actual timer values and granularityHardware Recovery Advantages
Document Structure
doc/pfcwd/directory.Technical Highlights
Review Focus Areas
Related Pull Requests