On 20/10/2026 17:06, Koushik Bhargav Nimoji wrote:
On Fri, Oct 16, 2026 at 12:28 PM Luca Vizzarro <[email protected]
<mailto:[email protected]>> wrote:
Hi Koushik,
Apologies for the long wait. To start off, could you please address the
AI Code Review as it raised a few concerns:
http://mails.dpdk.org/archives/test-report/2026-August/1029040.html
<http://mails.dpdk.org/archives/test-report/2026-August/1029040.html>
In terms of the 'del self._lshw_net_Info' section, i added the 'if
self._lshw_net_Info' to check if the attribute exists before deleting,
as it's done in other sections of the file
For the error checking and TG resource teardown, this is already handled
by DTS.
Ack.
> diff --git a/dts/framework/test_run.py b/dts/framework/test_run.py
> index c20a0fa885..b23930b953 100644
> --- a/dts/framework/test_run.py
> +++ b/dts/framework/test_run.py
> @@ -115,7 +115,7 @@
> from framework.remote_session.dpdk import DPDKBuildEnvironment,
DPDKRuntimeEnvironment
> from framework.settings import SETTINGS
> from framework.test_result import Result, ResultNode, TestRunResult
> -from framework.test_suite import BaseConfig, TestCase,
TestCaseType, TestSuite
> +from framework.test_suite import BaseConfig, TestCase,
TestCaseType, TestSuite, TestSuiteType
> from framework.testbed_model.capability import (
> Capability,
> get_supported_capabilities,
> @@ -375,11 +375,6 @@ def next(self) -> State | None:
> ctx.topology.bind_cryptodevs("dpdk")
>
> ctx.topology.configure_ports("sut", "dpdk")
> - if ctx.func_tg and ctx.topology.type is not
LinkTopology.NO_LINK:
> - ctx.func_tg.setup(ctx.topology)
> - if ctx.perf_tg and ctx.topology.type is not
LinkTopology.NO_LINK:
> - ctx.perf_tg.setup(ctx.topology)
> -
> self.result.ports = [
> port.to_dict() for port in ctx.topology.sut_ports +
ctx.topology.tg_ports
> ]
> @@ -506,12 +501,34 @@ class TestSuiteSetup(TestSuiteState):
> """Test suite setup."""
>
> logger_name: ClassVar[str] = "test_suite_setup"
> + test_run: TestRun
Unsure why this is necessary, given it's already inherited by
TestSuiteState.
I kept it this way as it is the same style as TestRunSetup. TestRunSetup
inherits from State, which has a 'test_run: TestRun' attribute, but also
has its own test_run attribute. I will have it inherit test_run in my
upcoming version. as that's a better approach.
Oh odd, definitely my oversight! Looks like there was no original
intention to have test_run in State, but then I guess I changed it
without removing the duplications. You made a good point, thank you for
clarifying!
> + if (
> + ctx.topology.type is not LinkTopology.NO_LINK
> + and testsuite_type is not TestSuiteType.CRYPTO
> + ):
> + if testsuite_type == TestSuiteType.FUNC:
> + required_tg, other_tg = ctx.func_tg, ctx.perf_tg
> + else:
> + required_tg, other_tg = ctx.perf_tg, ctx.func_tg
> +
> + if other_tg and other_tg.is_setup:
> + other_tg.teardown()
> +
> + if testsuite_type == TestSuiteType.FUNC and
ctx.tg_node:
> + for port in ctx.topology.tg_ports:
> +
ctx.tg_node.main_session.bind_ports_to_driver([port],
port.config.os_driver)
I am a bit confused why this happens here. The driver to be bound to
should be governed by the TG that uses such ports. Could be os_driver,
could be dpdk_driver, regardless of Scapy and TRex.
This section exists because of how scapy and TRex handle port driver
bindings. TRex automatically binds ports to the required driver when
setup, but scapy requires it to be done manually. That's why if the next
test suite to be tested is a functional test suite, the TG ports are
rebound to the os_driver manually here.
So I think the original intention here was that TRex would exceptionally
change the driver as the normal behaviour was that the TG was not going
to be subject to driver binding. Therefore what would make sense is for
the TG to control driver binding accordingly. So if Scapy is setup that
it would be up to Scapy to "bind to kernel drivers" if needed. What do
you think?
