|SUCCESS| [v1,8/8] dts: update API rst files for doc build

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/163270

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-23

I'll review this DPDK DTS patch series against the guidelines you provided. Let me analyze each commit carefully.

**Commit:** 20260423170331.33193-2-dmarx at iol.unh.edu
**Author:** Dean Marx <dmarx at iol.unh.edu>
**Subject:** dts: move test suite module from framework to API

This commit moves the test_suite module from framework to API and updates
all imports across the codebase.

> diff --git a/dts/api/capabilities.py b/dts/api/capabilities.py
> index 09bc538523..8569cacbd2 100644
> --- a/dts/api/capabilities.py
> +++ b/dts/api/capabilities.py
> @@ -47,7 +47,7 @@ def test_scatter_mbuf_2048(self):
>  from typing import TYPE_CHECKING, Callable
>  
>  if TYPE_CHECKING:
> -    from framework.test_suite import TestProtocol
> +    from api.test_suite import TestProtocol

The TYPE_CHECKING guard is correctly used here.

> diff --git a/dts/api/packet.py b/dts/api/packet.py
> index 094a1b7a9d..fc2be931fe 100644
> --- a/dts/api/packet.py
> +++ b/dts/api/packet.py
> @@ -175,7 +175,7 @@ def adjust_addresses(packets: list[Packet], expected: bool = False) -> list[Pack
>      Raises:
>          InternalError: If no tests are running.
>      """
> -    from framework.test_suite import TestSuite
> +    from api.test_suite import TestSuite

This import is inside a function body. Does the function need lazy importing
here, or could this import be moved to module level within the
TYPE_CHECKING block?

> diff --git a/dts/framework/test_suite.py b/dts/api/test_suite.py
> similarity index 99%
> rename from dts/framework/test_suite.py
> +++ b/dts/api/test_suite.py
> @@ -30,12 +30,11 @@
>  from typing_extensions import Self
>  
>  from framework.config.common import FrozenModel
> +from framework.exception import ConfigurationError, InternalError
> +from framework.logger import DTSLogger, get_dts_logger
>  from framework.testbed_model.capability import TestProtocol
>  from framework.testbed_model.topology import Topology
> -
> -from .exception import ConfigurationError, InternalError
> -from .logger import DTSLogger, get_dts_logger
> -from .utils import to_pascal_case
> +from framework.utils import to_pascal_case

The imports from framework.* here contradict the stated goal of this
series. If test_suite is moved to api/ but still imports from framework/,
does this create circular dependencies or break the intended API/framework
separation?

**Commit:** 20260423170331.33193-3-dmarx at iol.unh.edu
**Author:** Dean Marx <dmarx at iol.unh.edu>
**Subject:** dts: move testbed model from framework to API

> diff --git a/dts/api/testbed_model/capability.py b/dts/framework/testbed_model/capability.py
> similarity index 98%
> rename from dts/framework/testbed_model/capability.py
> +++ b/dts/api/testbed_model/capability.py
> @@ -64,11 +64,11 @@ def test_scatter_mbuf_2048(self):
>  from typing_extensions import Self
>  
>  from api.capabilities import LinkTopology, NicCapability
> -from framework.exception import ConfigurationError, InternalError, SkippedTestException
> +from api.exception import ConfigurationError, InternalError, SkippedTestException

Does api.exception exist at this point in the series? Checking commit 3/8
shows exception is moved later. Does this create a build breakage between
commits?

**Commit:** 20260423170331.33193-4-dmarx at iol.unh.edu
**Author:** Dean Marx <dmarx at iol.unh.edu>
**Subject:** dts: move exception module from framework to API

> diff --git a/dts/framework/exception.py b/dts/api/exception.py
> similarity index 100%
> rename from dts/framework/exception.py
> +++ b/dts/api/exception.py

The file is moved without any modifications. If commit 2/8 references
api.exception before this commit, does the series break bisectability?

**Commit:** 20260423170331.33193-5-dmarx at iol.unh.edu
**Author:** Dean Marx <dmarx at iol.unh.edu>
**Subject:** dts: move utils from framework to API

> diff --git a/dts/framework/utils.py b/dts/api/utils.py
> similarity index 100%
> rename from dts/framework/utils.py
> +++ b/dts/api/utils.py

Pure file move, no changes. Same question about ordering and commit
dependencies.

**Commit:** 20260423170331.33193-7-dmarx at iol.unh.edu
**Author:** Dean Marx <dmarx at iol.unh.edu>
**Subject:** dts: separate Linux session into interface and logic

> +class LinuxSession(ABC):
> +    """Abstract interface for Linux-specific OS session operations.
> +
> +    API consumers should type-hint against this class when they need access
> +    to Linux-only capabilities beyond the base :class:`~.os_session.OSSession` contract.
> +    """
> +
> +    @abstractmethod
> +    def configure_ipv4_forwarding(self, enable: bool) -> None:

The new ABC in api/testbed_model/linux_session.py has only one abstract
method. Is this abstract base necessary, or could it be integrated
differently? Does having a separate interface file with one method add
value?

> diff --git a/dts/framework/linux_session.py b/dts/framework/linux_session.py
> new file mode 100644
> index 0000000000..54c6370d53
> --- /dev/null
> +++ b/dts/framework/linux_session.py
> @@ -0,0 +1,375 @@
> +class LinuxSession(PosixSession, LinuxSessionBase):
> +    """Linux-specific implementation of the OS session interface.
> +
> +    Inherits POSIX-compliant operations from
> +    :class:`~.posix_session.PosixSession` and implements the
> +    :class:`~api.linux_session.LinuxSession` contract. All backend logic --
> +    ``lshw`` caching, hugepage sysfs interaction, VFIO module loading,
> +    SR-IOV management -- lives here in the framework.
> +    """
> +
> +    _devbind_script_path: PurePath | None
> +
> +    def __init__(self, *args, **kwargs) -> None:
> +        """Overrides :meth:`~.os_session.OSSession.__init__`."""
> +        self._devbind_script_path = None
> +        super().__init__(*args, **kwargs)

The __init__ override only sets _devbind_script_path to None, which
matches the annotation. Is this __init__ necessary, or would the class
attribute initialization be sufficient?

> +    @property
> +    def _lshw_net_info(self) -> list[LshwOutput]:
> +        """Cached lshw network info, fetched on first access."""
> +        if not hasattr(self, "_lshw_net_cache"):
> +            output = self.send_command("lshw -quiet -json -C network", verify=True)
> +            self._lshw_net_cache: list[LshwOutput] = json.loads(output.stdout)
> +        return self._lshw_net_cache
> +
> +    def _refresh_lshw(self) -> None:
> +        """Force refresh of cached lshw network info.
> +
> +        Called internally after operations that change port/driver state.
> +        """
> +        if hasattr(self, "_lshw_net_cache"):
> +            del self._lshw_net_cache
> +        _ = self._lshw_net_info

In _refresh_lshw(), after deleting _lshw_net_cache, the code assigns
self._lshw_net_info to _. Does this assignment to _ serve a purpose
beyond forcing the property getter to run? Could a comment clarify the
intent?

**Commit:** 20260423170331.33193-8-dmarx at iol.unh.edu
**Author:** Dean Marx <dmarx at iol.unh.edu>
**Subject:** dts: update API rst files for doc build

> diff --git a/dts/api/params/types.py b/dts/api/params/types.py
> index 55a0e546c1..280c87ec92 100644
> --- a/dts/api/params/types.py
> +++ b/dts/api/params/types.py
> @@ -12,8 +12,10 @@ def create_testpmd(**kwargs: Unpack[TestPmdParamsDict]):
>              params = TestPmdParams(**kwargs)
>  """
>  
> +from __future__ import annotations
> +
>  from pathlib import PurePath
> -from typing import TypedDict
> +from typing import TYPE_CHECKING, TypedDict
>  
>  from api.cryptodev.config import (
>      AeadAlgName,
> @@ -32,7 +34,9 @@ def create_testpmd(**kwargs: Unpack[TestPmdParamsDict]):
>      TestType,
>      TLSVersion,
>  )
> -from api.params import Switch, YesNoSwitch
> +
> +if TYPE_CHECKING:
> +    from api.params import Switch, YesNoSwitch

Does moving Switch and YesNoSwitch imports into TYPE_CHECKING break
runtime usage? Are these types used at runtime in this module?

**General Questions:**

1. Commit ordering: Do earlier commits in this series create import
   references before the modules are moved, breaking bisectability?

2. The series states it moves modules from framework to API, but several
   moved modules still import from framework.*. Does this preserve the
   circular dependency it aims to fix?

3. Several commits are pure file moves with no changes. Could these be
   combined, or does keeping them separate serve a documentation purpose?