Skip to content

DPDK: deprecate run_testpmd_concurrent for hotplug tests#4604

Draft
mcgov wants to merge 3 commits into
mainfrom
mcgov/hotplug-rework
Draft

DPDK: deprecate run_testpmd_concurrent for hotplug tests#4604
mcgov wants to merge 3 commits into
mainfrom
mcgov/hotplug-rework

Conversation

@mcgov

@mcgov mcgov commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Description

Gets rid of old abstraction that didn't allow direct monitoring of testpmd processes.

It is replaced with new functions specifically for each version of the test. We are violating the DRY ethos here in favor of readability and maintaining direct control over the processes.

This allows implementing the hotplug tests in a cleaner way, which also ends up being faster and more reliable in practice.

Related Issue

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring
  • Documentation update

Checklist

  • Description is filled in above
  • No credentials, secrets, or internal details are included
  • Peer review requested (if not, add required peer reviewers after raising PR)
  • Tests executed and results posted below

Test Validation

Key Test Cases:
verify_dpdk_testpmd_hotplug_sender_netvsc_pmd|verify_dpdk_testpmd_hotplug_receiver_netvsc_pmd|verify_dpdk_l3fwd_ntttcp_tcp_hotplug

Impacted LISA Features:
Dpdk

Tested Azure Marketplace Images:
Canonical:ubuntu-24_04-lts:server:latest

Test Results

Image VM Size Result
PASSED / FAILED / SKIPPED

Copilot AI review requested due to automatic review settings July 16, 2026 22:28

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings July 17, 2026 18:13

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread lisa/sut_orchestrator/azure/features.py
Comment on lines +224 to +229
# let it run for a bit
sleep(10)

# switch_sriov(... wait=True) has become really expensive,
# and it can easily timeout if things aren't perfect.
# So: we'll avoid all of that and just start processes and wait for output.
mcgov added 3 commits July 17, 2026 11:33
Gets rid of old abstraction that didn't allow direct monitoring
of testpmd processes.

It is replaced with new functions specifically for each version of the
test. We are violating the DRY ethos here in favor of readability and
maintaining direct control over the processes.

This allows implementing the hotplug tests in a cleaner way,
which also ends up being faster and more reliable in practice.
@mcgov
mcgov force-pushed the mcgov/hotplug-rework branch from 23b9284 to 85b4327 Compare July 17, 2026 18:34
Copilot AI review requested due to automatic review settings July 17, 2026 18:34

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

Comment on lines +236 to +238
processes[collect_from].wait_output(
"HN_DRIVER: netvsc_hotadd_callback(): Device notification type=1"
)
Comment on lines +224 to +226
# let it run for a bit
sleep(10)

Comment on lines +240 to +242
# let it run for a bit
sleep(10)

Comment on lines +253 to +255
# let it run for a bit
sleep(30)
# kill testpmd and process the output
Comment on lines 666 to 671
return (
f"{self._testpmd_install_path} {core_list} "
f"{nic_includes} {debug_logging} -- --forward-mode={mode} "
f"-a --stats-period 2 --nb-cores={forwarding_cores} {extra_args} "
f"-a --stats-period {stats_period} --nb-cores={forwarding_cores}"
f" {extra_args} --record-burst-stats"
)
f"into status [{enable}]"
).is_equal_to(enable)

@retry(HttpResponseError, tries=3, delay=30)
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