Skip to content

Enable LLDP-MED for DPUs#3692

Merged
stoo-davies merged 5 commits into
NVIDIA:mainfrom
stoo-davies:dpu-lldp-med-enable
Jul 20, 2026
Merged

Enable LLDP-MED for DPUs#3692
stoo-davies merged 5 commits into
NVIDIA:mainfrom
stoo-davies:dpu-lldp-med-enable

Conversation

@stoo-davies

Copy link
Copy Markdown
Contributor

Adds code to forge-dpu-agent to do two things..

  1. Enable LLDP-MED in the /etc/defaults/lldp file
  2. Removes the interface restriction that was preventing it from running on the management interface.

These changes are for automated cable validation.

Related issues

#3204

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

Tested on a BF3.

Signed-off-by: Stoo Davies <189670495+stoo-davies@users.noreply.github.com>
@stoo-davies
stoo-davies requested a review from a team as a code owner July 17, 2026 20:19
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary by CodeRabbit

  • New Features

    • Improved LLDP initialization during DPU-OS startup.
    • Automatically applies LLDP configuration and enables LLDP-MED inventory.
    • Added bounded retries and timeouts for LLDP commands and service restarts.
    • Added default LLDP configuration files for supported deployments.
  • Bug Fixes

    • Improved handling of missing or existing DAEMON_ARGS settings, including CRLF-formatted files.
    • Added clearer failure handling when LLDP configuration commands time out or fail.
  • Tests

    • Expanded coverage for LLDP configuration updates, parsing, and default file contents.

Walkthrough

LLDP preparation now configures lldpd, verifies LLDP-MED inventory with bounded retries, and runs during DPU-OS startup before asynchronously updating the LLDP system description.

Changes

LLDP preparation

Layer / File(s) Summary
LLDP configuration preparation
crates/agent/src/lldp.rs, crates/dpf/src/flavor.rs
Adds prepare_lldp(), disables the active interface configuration, rewrites DAEMON_ARGS with newline handling, and adds fixed LLDPd configuration overrides with updated tests.
LLDP-MED verification and restart retries
crates/agent/src/lldp.rs
Queries LLDP-MED configuration through bounded asynchronous lldpcli execution, restarts lldpd when necessary, and applies bounded retry delays and failure handling.
Asynchronous system description and startup wiring
crates/agent/src/lldp.rs, crates/agent/src/main_loop.rs
Makes system-description updates and daemon reads asynchronous, and runs preparation separately during DPU-OS startup with independent error logging. Unit tests cover argument rewriting and inventory parsing.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant setup_and_run
  participant prepare_lldp
  participant lldpcli
  participant systemctl
  setup_and_run->>prepare_lldp: prepare_lldp()
  prepare_lldp->>lldpcli: Query LLDP-MED configuration
  lldpcli-->>prepare_lldp: Configuration JSON
  prepare_lldp->>systemctl: Restart lldpd.service when required
  systemctl-->>prepare_lldp: Restart result
  prepare_lldp-->>setup_and_run: Preparation result
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: enabling LLDP-MED for DPUs.
Description check ✅ Passed The description is directly related to the LLDP-MED and interface-configuration changes in this PR.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/agent/src/lldp.rs`:
- Around line 81-103: Extract the configuration line-rewrite logic into a pure
helper and the inventory JSON value parsing into another pure helper, replacing
direct filesystem and command coupling in the tested paths. Add table-driven
tests using value_scenarios! and scenarios! covering absent and existing
DAEMON_ARGS, LF, CRLF, and missing-final-newline inputs, plus yes, no, missing,
and unexpected JSON values; preserve the current transformation behavior.
- Around line 119-122: Bound all LLDP-related child processes with deadlines:
update the lldpcli call in the LLDP configuration read path, the systemctl
restart path, and LldpdConfigFileWriter::daemon_read() to use timeout-capable
process handling. Ensure expired commands are terminated and reported as
retryable failures so agent startup and config-change handling cannot block
indefinitely.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f2e83359-f23d-45d5-af93-f2b125c2bd71

📥 Commits

Reviewing files that changed from the base of the PR and between 0dae3ca and 637edb1.

📒 Files selected for processing (2)
  • crates/agent/src/lldp.rs
  • crates/agent/src/main_loop.rs

Comment thread crates/agent/src/lldp.rs
Comment thread crates/agent/src/lldp.rs Outdated
Signed-off-by: Stoo Davies <189670495+stoo-davies@users.noreply.github.com>
@wminckler

Copy link
Copy Markdown
Contributor

Is there a plan to support this when DPF is used? (or does it do that already?)

Comment thread crates/agent/src/lldp.rs Outdated
@stoo-davies
stoo-davies marked this pull request as draft July 17, 2026 22:19
@copy-pr-bot

copy-pr-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@stoo-davies

Copy link
Copy Markdown
Contributor Author

Converting this back to draft until I can address the DPF question from @wminckler

The new code won't break anything since it's guarded by the is_dpu_os() check, however I need to get the same config applied under DPF provisioning.

Signed-off-by: Stoo Davies <189670495+stoo-davies@users.noreply.github.com>
@stoo-davies

Copy link
Copy Markdown
Contributor Author

/ok to test 1520e3a

Signed-off-by: Stoo Davies <189670495+stoo-davies@users.noreply.github.com>
Signed-off-by: Stoo Davies <189670495+stoo-davies@users.noreply.github.com>
@stoo-davies

Copy link
Copy Markdown
Contributor Author

/ok to test 175e6ad

@github-actions

Copy link
Copy Markdown

@stoo-davies
stoo-davies marked this pull request as ready for review July 20, 2026 19:31

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/dpf/src/flavor.rs`:
- Around line 282-297: The DPF flavor currently provisions the active
lldp-interfaces.conf that lldp::disable_interfaces_config() renames during
startup. Update the DpuFlavorConfigFiles entry for
/etc/lldpd.d/lldp-interfaces.conf to avoid provisioning an active DPF override,
or otherwise preserve the DPF-owned configuration after prepare_lldp() runs;
keep the /etc/default/lldpd entry unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: eb8256cf-68d7-4597-b022-b1c1120c059d

📥 Commits

Reviewing files that changed from the base of the PR and between 60023fe and 175e6ad.

📒 Files selected for processing (2)
  • crates/agent/src/lldp.rs
  • crates/dpf/src/flavor.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/agent/src/lldp.rs

Comment thread crates/dpf/src/flavor.rs
@stoo-davies
stoo-davies merged commit 6a42736 into NVIDIA:main Jul 20, 2026
63 checks passed
@stoo-davies
stoo-davies deleted the dpu-lldp-med-enable branch July 20, 2026 21:42
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.

2 participants