Hi Luca, Thanks for the review. I will send a new version that addresses the comments shortly.
On Fri, Oct 16, 2026 at 12:28 PM Luca Vizzarro <[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 > > 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. > The patch will also require rebasing. > > On 11/08/2026 19:55, Koushik Bhargav Nimoji wrote: > > diff --git a/dts/framework/test_result.py b/dts/framework/test_result.py > > index 21faa55dc1..13ed8f671c 100644 > > --- a/dts/framework/test_result.py > > +++ b/dts/framework/test_result.py > > @@ -117,6 +117,7 @@ class ResultNode(BaseModel): > > label: str > > children: list[Union["ResultNode", ResultLeaf]] = > Field(default_factory=list) > > parent: Union["ResultNode", None] = None > > + ports: list[dict[str, str]] | None = None > > This attribute lacks the appropriate documentation. Also unless I missed > something, I am not sure how this belongs to this patch. Can you please > clarify? > This shouldn't be here, will be removed. > > > > def add_child(self, label: str) -> "ResultNode": > > """Creates and append a child node to the model. > > 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. > > + 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.
