Skip to content

Add dummy TX queue when no transmit is defined#1402

Merged
bhashemian merged 6 commits intomainfrom
dummy-tx-q
Mar 10, 2026
Merged

Add dummy TX queue when no transmit is defined#1402
bhashemian merged 6 commits intomainfrom
dummy-tx-q

Conversation

@cliffburdick
Copy link
Copy Markdown
Contributor

@cliffburdick cliffburdick commented Feb 11, 2026

DPDK 25.11 and higher will fail to start if no TX queues are defined. Just like we do on RX, we defined a dummy TX queue if the user is not transmitting.

Summary by CodeRabbit

  • New Features

    • Adds creation of required dummy transmit queues during advanced network interface setup to support newer DPDK versions.
  • Bug Fixes

    • Ensures transmit queues are created during initialization so interfaces start reliably.
  • Performance

    • Prevents launching worker threads for dummy transmit queues, reducing unnecessary resource use.

Signed-off-by: Cliff Burdick <30670611+cliffburdick@users.noreply.github.com>
Signed-off-by: Cliff Burdick <30670611+cliffburdick@users.noreply.github.com>
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Feb 11, 2026

Greptile Summary

This PR adds a create_dummy_tx_q() helper that injects a minimal dummy TX queue for each interface that has no user-configured TX queues. This is necessary because DPDK 25.11+ requires at least one TX queue to be set up before rte_eth_dev_start(), mirroring the existing create_dummy_rx_q() workaround for Hardware Steering. The implementation is correct: the dummy queue is created early in initialize() so it participates in adjust_memory_regions(), allocate_memory_regions(), and setup_pools_and_rings(), and the run() method skips launching a worker thread for it via the "UNUSED" name-prefix guard.

Key changes:

  • create_dummy_tx_q() creates a TxQueueConfig named "UNUSED_TX_P<port>_Q0" and a matching MemoryRegionConfig ("MR_Unused_TX_P<port>") with a minimal 64-byte buffer size.
  • run() gains an "UNUSED" prefix check for TX queues (consistent with the pre-existing RX guard) to prevent spawning a worker thread for the dummy queue.
  • The header declares the new private method alongside create_dummy_rx_q().

Two minor style points were found: the new run() comment incorrectly attributes the skip to HWS rather than the DPDK 25.11+ TX-queue requirement, and the initialization status log always prints "TX: ENABLED" after the dummy queue is injected (mirroring the same pre-existing behaviour on the RX side).

Confidence Score: 4/5

  • Safe to merge; the change is a targeted, well-scoped DPDK 25.11+ compatibility fix with only minor style issues.
  • The implementation correctly mirrors the pre-existing RX dummy-queue pattern, is placed in the right order within initialize(), and the worker-skip guard in run() works as intended. The only issues are a misleading comment and a log-message observability gap, neither of which affects runtime correctness.
  • No files require special attention.

Important Files Changed

Filename Overview
operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.cpp Adds create_dummy_tx_q() that mirrors the existing create_dummy_rx_q() pattern. Logic is sound: dummy queue is added before adjust_memory_regions/setup_pools_and_rings so it participates in normal resource allocation, and the run() worker-skip guard correctly uses the "UNUSED" prefix check. Two minor style issues: the run() comment incorrectly cites HWS as the reason, and the initialization status log will always print "TX: ENABLED" after the dummy queue is injected.
operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.h Adds the create_dummy_tx_q() private method declaration, consistent with the existing create_dummy_rx_q() declaration. No issues.

Sequence Diagram

sequenceDiagram
    participant App
    participant DpdkMgr
    participant DPDK

    App->>DpdkMgr: initialize()
    DpdkMgr->>DpdkMgr: init_rx_core_q_map()
    DpdkMgr->>DpdkMgr: create_dummy_rx_q()
    Note over DpdkMgr: if no RX queues, inject UNUSED_P_Q0
    DpdkMgr->>DpdkMgr: create_dummy_tx_q() NEW
    Note over DpdkMgr: if no TX queues, inject UNUSED_TX_P_Q0
    DpdkMgr->>DpdkMgr: adjust_memory_regions()
    DpdkMgr->>DpdkMgr: allocate_memory_regions()
    DpdkMgr->>DPDK: rte_eth_tx_queue_setup() for all TX queues
    Note over DPDK: dummy queue satisfies DPDK 25.11+ requirement
    DpdkMgr->>DPDK: rte_eth_dev_start()
    DpdkMgr->>DpdkMgr: setup_pools_and_rings()
    Note over DpdkMgr: ring and burst pool created for dummy TX queue too

    App->>DpdkMgr: run()
    loop For each TX queue
        alt queue name starts with UNUSED
            DpdkMgr-->>DpdkMgr: skip, no worker thread launched
        else real TX queue
            DpdkMgr->>DPDK: rte_eal_remote_launch(tx_worker)
        end
    end
Loading

Comments Outside Diff (1)

  1. operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.cpp, line 458-461 (link)

    TX status log is always "ENABLED" after this PR

    create_dummy_tx_q() is called on line 419, before this log message. For ports where the user has not configured any TX queues, the dummy queue is injected at that point, so intf.tx_.queues_.size() > 0 will always be true here and the log will always print "TX: ENABLED", making it impossible to tell from the log whether real TX traffic was intended.

    The same issue already existed for the RX side (where create_dummy_rx_q() is called just before). Consider moving this diagnostic log to before the dummy-queue creation calls, or adding a separate flag/check that distinguishes the dummy queue case (e.g. checking the "UNUSED" prefix).

Last reviewed commit: de3744f

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60b32a0 and 376f0bd.

📒 Files selected for processing (1)
  • operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.cpp

Walkthrough

This PR adds creation of dummy TX queues (mirroring dummy RX logic) for interfaces with no TX queues, calls the new function during initialize(), and prevents launching TX worker threads for dummy (UNUSED) TX queues in run().

Changes

Cohort / File(s) Summary
Header Declaration
operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.h
Added private method declaration void create_dummy_tx_q() to DpdkMgr.
Implementation & Integration
operators/advanced_network/advanced_network/managers/dpdk/adv_network_dpdk_mgr.cpp
Implemented create_dummy_tx_q() to add dummy TX queue and corresponding MemoryRegionConfig. Updated initialize() to call it after create_dummy_rx_q(). Modified run() to skip launching TX workers for queues whose names start with the UNUSED prefix.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add dummy TX queue when no transmit is defined' directly and clearly summarizes the main change: adding a dummy TX queue for cases without user-defined transmit queues.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@bhashemian bhashemian requested a review from a team February 17, 2026 16:58
@bhashemian bhashemian added this to the Networking milestone Feb 17, 2026
@bhashemian bhashemian requested a review from agirault February 17, 2026 20:00
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

cliffburdick and others added 2 commits February 24, 2026 10:09
…network_dpdk_mgr.cpp

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Signed-off-by: Cliff Burdick <30670611+cliffburdick@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Member

@bhashemian bhashemian left a comment

Choose a reason for hiding this comment

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

LGTM!

@bhashemian bhashemian enabled auto-merge (squash) March 10, 2026 23:04
@bhashemian bhashemian merged commit a689251 into main Mar 10, 2026
18 checks passed
@bhashemian bhashemian deleted the dummy-tx-q branch March 10, 2026 23:04
@github-project-automation github-project-automation Bot moved this to Done in Holohub Mar 10, 2026
chengronglai pushed a commit to chengronglai/holohub that referenced this pull request Mar 19, 2026
DPDK 25.11 and higher will fail to start if no TX queues are defined.
Just like we do on RX, we defined a dummy TX queue if the user is not
transmitting.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Adds creation of required dummy transmit queues during advanced
network interface setup to support newer DPDK versions.

* **Bug Fixes**
* Ensures transmit queues are created during initialization so
interfaces start reliably.

* **Performance**
* Prevents launching worker threads for dummy transmit queues, reducing
unnecessary resource use.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Signed-off-by: Cliff Burdick <30670611+cliffburdick@users.noreply.github.com>
Co-authored-by: Bruce Hashemian <3968947+bhashemian@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants