Skip to content

[Celestica] Leh800bcls: Resolve buffer pool size config crash on dual-NPU platforms during PFC tests - #1499

Open
gang-tao wants to merge 1 commit into
facebook:mainfrom
gang-tao:leh800bcls_agent_fix20
Open

[Celestica] Leh800bcls: Resolve buffer pool size config crash on dual-NPU platforms during PFC tests#1499
gang-tao wants to merge 1 commit into
facebook:mainfrom
gang-tao:leh800bcls_agent_fix20

Conversation

@gang-tao

Copy link
Copy Markdown
Contributor

Pre-submission checklist

  • [ ✓ ] I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • [ ✓ ] pre-commit run
clang-format.............................................................Passed
shellcheck...........................................(no files to check)Skipped
shfmt................................................(no files to check)Skipped
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...........................................(no files to check)Skipped
check json...........................................(no files to check)Skipped
check for merge conflicts................................................Passed
ruff check...........................................(no files to check)Skipped
ruff format..........................................(no files to check)Skipped
Prevent sai_impl in fboss manifest.......................................Passed

Summary

During the execution of the AgentTrafficPfcTest suite (e.g., verifyPfcWithDefaultCfg) on Leh800bcls, the hardware agent process (fboss_hw_agent-sai_impl) crashes during initialization. The crash is triggered by a SAI_STATUS_INVALID_PARAMETER exception when attempting to assign a BufferProfileId to an egress queue.

W0809 06:28:33.630477 705682 SaiBufferManager.cpp:270] Overriding egress buffer pool size from 264594120 to 1505000
E0809 06:28:33.708355 705682 SaiApiError.h:67] [queue] Failed to set attribute QueueSaiId(282664682651649) to BufferProfileId: 107374182401: INVALID PARAMETER
Async logger exit with 134 bytes written to file
Terminated due to: [queue] Failed to set attribute QueueSaiId(282664682651649) to BufferProfileId: 107374182401: INVALID PARAMETER
*** Aborted at 1786271313 (Unix time, try 'date -d @1786271313') ***
*** Signal 6 (SIGABRT) (0xac27f) received by PID 705151 (pthread TID 0x7f1f19d3b640) (linux TID 705682) (maybe from PID 705151, UID 0) (code: sent by tkill or tgkill), stack trace: ***

Root Cause

Currently on the 2-NPU Leh800bcls platform, different NPUs use different YAML files, so the YAML files are placed under npuEntries as follows.

[root@localhost cfg]# tail -n 17 good/leh800bc.agent.lt0.evt1b.2npu.v11.conf   | cut -c 1-80
      "asicConfig": {
        "common": {
          "yamlConfig": ""
        },
        "npuEntries": {
          "0": {
            "yamlConfig": "---\ndevice:\n  0:\n    PC_PM_CORE:\n      ?\n
          },
          "1": {
            "yamlConfig": "---\ndevice:\n  0:\n    PC_PM_CORE:\n      ?\n
          }
        }

The test infrastructure helper modifyPlatformConfig() only apply overrides (including injecting the SKIP_BUFFER_RESERVATION: 1 entry via modifyYamlFunc) to the common block.
Since SKIP_BUFFER_RESERVATION: 1 was never injected into the NPU-specific configurations, the independent hardware agent processes parsed and applied the unmodified yamlConfig from npuEntries. During SDK initialization, the driver (unconditionally) accumulated the default egress queue minimum guarantees across all active physical lanes, resulting in a locked static reserved buffer size of 5745600 bytes (5.48MB). This physical reservation threshold exceeded the overridden pool capacity of 1.5MB, causing the SDK parameters validation to fail and crash the hardware process.

Solution

Refactor modifyPlatformConfig() in ConfigUtils.cpp to also check and traverse the npuEntries mapping inside asicConfig.
If npuEntries are populated, the helper dynamically applies the yaml config overrides (modifyYamlFunc and modifyMapFunc) to each individual NPU-specific entry in addition to the common block. This ensures that SKIP_BUFFER_RESERVATION: 1 is successfully injected into the respective NPU configurations

Test Plan

The AgentTrafficPfcTests related tests passed successfully on both npu0 and npu1.

[root@localhost log]#  grep " OK " AgentTxNpu[01]/mu*.log | sed 's/multi_switch_agent-AgentTraffic.*://g'
AgentTxNpu0/[       OK ] AgentTrafficPfcGenTest.verifyBufferPoolWatermarks (33639 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcGenTest.verifyIngressPriorityGroupWatermarks (33549 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcTest.verifyPfcWithDefaultCfg (33711 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcTest.verifyPfcWithMapChanges_0 (33727 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcTest.verifyPfcWithMapChanges_1 (33350 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcTest.verifyPfcWithZeroGlobalHeadRoomCfg (33871 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcWatchdogTest.PfcWatchdogDetection (38644 ms)
AgentTxNpu0/[       OK ] AgentTrafficPfcZeroPgHeadroomTest.verifyPfcWithZeroPgHeadRoomCfg (33364 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcGenTest.verifyBufferPoolWatermarks (33489 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcGenTest.verifyIngressPriorityGroupWatermarks (33194 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcTest.verifyPfcWithDefaultCfg (33445 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcTest.verifyPfcWithMapChanges_0 (34130 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcTest.verifyPfcWithMapChanges_1 (32504 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcTest.verifyPfcWithZeroGlobalHeadRoomCfg (33331 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcWatchdogTest.PfcWatchdogDetection (38157 ms)
AgentTxNpu1/[       OK ] AgentTrafficPfcZeroPgHeadroomTest.verifyPfcWithZeroPgHeadRoomCfg (34292 ms)

@gang-tao
gang-tao requested a review from a team as a code owner August 11, 2026 01:14
@meta-cla meta-cla Bot added the CLA Signed label Aug 11, 2026
@meta-codesync

meta-codesync Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

@jchallag has imported this pull request. If you are a Meta employee, you can view this in D115625388.

Comment thread fboss/agent/test/utils/ConfigUtils.cpp Outdated

if (asicConfig.npuEntries().has_value()) {
for (auto& [_, entry] : *asicConfig.npuEntries()) {
if (entry.getType() == cfg::AsicConfigEntry::Type::yamlConfig) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[fboss/agent/test/utils/ConfigUtils.cpp - Lines 1710-1718] The type-dispatch block inside the npuEntries loop (lines 1712-1716) duplicates the exact same pattern applied to common just above (lines 1703-1708). Per the fbcode C++ guideline "Do not repeat yourself. Always create a function for it", consider extracting this into a small local lambda (e.g., auto applyOverrides = [&](cfg::AsicConfigEntry& entry) { ... };) and invoking it for both common and each NPU entry. This keeps the two code paths in lock-step should another AsicConfigEntry::Type be added later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch, Thanks for the comment!
Extracted the duplicate type-dispatch logic into a local lambda applyOverrides and applied it to both common and npuEntries to eliminate redundancy and keep them perfectly in lock-step.

@gang-tao
gang-tao force-pushed the leh800bcls_agent_fix20 branch from 513d129 to 6939a92 Compare August 12, 2026 02:26
@facebook-github-tools

Copy link
Copy Markdown

@gang-tao has updated the pull request. You must reimport the pull request before landing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants