Skip to content

[Nexthop] Fix getting SDK state dump in CLI - #1611

Open
vvasavada-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:fix-getting-sdk-state-dump-in-cli
Open

vvasavada-nexthop wants to merge 1 commit into
facebook:mainfrom
nexthop-ai:fix-getting-sdk-state-dump-in-cli

Conversation

@vvasavada-nexthop

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

Summary

fboss2 show sdk dump was broken by qsfp_service SDK-dump path-traversal guard added in Differential Revision: D109336251: the CLI sent an absolute temp-file path which the guard rejected with getSdkState: absolute fileName is not allowed. This PR fixes the CLI to work with the guard, ensures the service-owned dump directory exists, stops the agent from leaking a scratch directory per debug-dump, and surfaces empty agent SDK dumps to the user.

Changes

  • CLI
    • show sdk dump now sends a plain basename instead of absolute temp path, and reads the result back from the service-owned dir. This clears the absolute fileName is not allowed error.
    • show sdk agent now warns when the returned SDK state is empty instead of silently printing nothing.
  • qsfp_service: getSdkState now calls createDir(kSdkDumpDir) so the confined dump directory exists before SDK writes into it.
  • Agent: getDebugDump() now uses folly::test::TemporaryDirectory::Scope::DELETE_ON_DESTRUCTION, so the fboss_sdk_dump.<token> scratch dir is removed after the dump is read back - previously one dir leaked per invocation.
  • CredoMacsecUtil.cpp: updated the getSdkState caller to send a basename.

Test Plan

  • New SdkDumpPathTest (path sanitizer) and CmdShowSdkDumpTest (empty-dump warnings)
  • Thrift.getHwDebugDump extended to assert the scratch dir does not leak.

The unit tests failed without the fixes, passed with the fixes.

DUT verification

  1. Path fix

Before


# fboss2 show sdk dump
Thrift call failed: 'getSdkState: absolute fileName is not allowed: /tmp/2082-1b3a-8434-3a4a'

E0903 19:21:04 CmdHandler.cpp:324] localhost - Error in command execution: Thrift call failed:
  'getSdkState: absolute fileName is not allowed: /tmp/2082-1b3a-8434-3a4a'
localhost: Thrift call failed: 'getSdkState: absolute fileName is not allowed: /tmp/2082-...'

After


# fboss2 show sdk dump
Getting SDK state failed

Note that residual Getting SDK state failed is because of no xphy present on the box. The path error is gone and it can be verified from qsfp side.

  # qsfp_service.log — request now passes the path guard
  I0903 19:21:04.889  getSdkState thrift request ... failed in 0ms     # without fix (guard throws)
  I0903 19:59:29.228  getSdkState thrift request ... succeeded in 0ms   # with fix(guard passes)
  # confined dir created by the fixed server's createDir()
  $ ls -ld /var/facebook/fboss/sdk_dump/
  drwxr-xr-x. root root  /var/facebook/fboss/sdk_dump/     # did not exist before the call
  1. Scratch-dir leak fix

BEFORE (unpatched fboss_hw_agent-sai_impl):

leaked /var/facebook/fboss/fboss_sdk_dump.* count BEFORE: 3
# run `show sdk agent` x2
leaked ... count AFTER: 5        # +1 leaked dir per call

AFTER (patched fboss_hw_agent-sai_impl):

leaked /var/facebook/fboss/fboss_sdk_dump.* count BEFORE: 0
# run `show sdk agent` x5
leaked ... count AFTER: 0        # no leak
  1. Empty output fix
  fboss2 show sdk agent

  BEFORE (unpatched):
  Printing Agent SDK state:
  {
    "0": ""
  }

  AFTER (patched):
  Printing Agent SDK state:
  {
    "0": ""
  }
  Warning: no SDK state was captured for switch 0.
  Warning: no SDK state was captured for any switch.

Signed-off-by: Vishrant Vasavada <vvasavada@nexthop.ai>
@vvasavada-nexthop
vvasavada-nexthop requested review from a team as code owners September 8, 2026 15:01
@meta-cla meta-cla Bot added the CLA Signed label Sep 8, 2026
@vvasavada-nexthop vvasavada-nexthop changed the title Fix getting SDK state dump in CLI [Nexthop] Fix getting SDK state dump in CLI Sep 8, 2026
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.

1 participant