[v8,18/21] dts: sut and tg nodes docstring update

Message ID 20231123151344.162812-19-juraj.linkes@pantheon.tech (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers
Series dts: docstrings update |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Juraj Linkeš Nov. 23, 2023, 3:13 p.m. UTC
  Format according to the Google format and PEP257, with slight
deviations.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 dts/framework/testbed_model/sut_node.py | 230 ++++++++++++++++--------
 dts/framework/testbed_model/tg_node.py  |  42 +++--
 2 files changed, 176 insertions(+), 96 deletions(-)
  

Comments

Jeremy Spewock Dec. 1, 2023, 6:06 p.m. UTC | #1
On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:

> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  dts/framework/testbed_model/sut_node.py | 230 ++++++++++++++++--------
>  dts/framework/testbed_model/tg_node.py  |  42 +++--
>  2 files changed, 176 insertions(+), 96 deletions(-)
>
> diff --git a/dts/framework/testbed_model/sut_node.py
> b/dts/framework/testbed_model/sut_node.py
> index 5ce9446dba..c4acea38d1 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -3,6 +3,14 @@
>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
>  # Copyright(c) 2023 University of New Hampshire
>
> +"""System under test (DPDK + hardware) node.
> +
> +A system under test (SUT) is the combination of DPDK
> +and the hardware we're testing with DPDK (NICs, crypto and other devices).
> +An SUT node is where this SUT runs.
> +"""
>

I think this should just be "A SUT node"


> +
> +
>  import os
>  import tarfile
>  import time
> @@ -26,6 +34,11 @@
>
>
>  class EalParameters(object):
> +    """The environment abstraction layer parameters.
> +
> +    The string representation can be created by converting the instance
> to a string.
> +    """
> +
>      def __init__(
>          self,
>          lcore_list: LogicalCoreList,
> @@ -35,21 +48,23 @@ def __init__(
>          vdevs: list[VirtualDevice],
>          other_eal_param: str,
>      ):
> -        """
> -        Generate eal parameters character string;
> -        :param lcore_list: the list of logical cores to use.
> -        :param memory_channels: the number of memory channels to use.
> -        :param prefix: set file prefix string, eg:
> -                        prefix='vf'
> -        :param no_pci: switch of disable PCI bus eg:
> -                        no_pci=True
> -        :param vdevs: virtual device list, eg:
> -                        vdevs=[
> -                            VirtualDevice('net_ring0'),
> -                            VirtualDevice('net_ring1')
> -                        ]
> -        :param other_eal_param: user defined DPDK eal parameters, eg:
> -                        other_eal_param='--single-file-segments'
> +        """Initialize the parameters according to inputs.
> +
> +        Process the parameters into the format used on the command line.
> +
> +        Args:
> +            lcore_list: The list of logical cores to use.
> +            memory_channels: The number of memory channels to use.
> +            prefix: Set the file prefix string with which to start DPDK,
> e.g.: ``prefix='vf'``.
> +            no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``.
> +            vdevs: Virtual devices, e.g.::
> +
> +                vdevs=[
> +                    VirtualDevice('net_ring0'),
> +                    VirtualDevice('net_ring1')
> +                ]
> +            other_eal_param: user defined DPDK EAL parameters, e.g.:
> +                ``other_eal_param='--single-file-segments'``
>          """
>          self._lcore_list = f"-l {lcore_list}"
>          self._memory_channels = f"-n {memory_channels}"
> @@ -61,6 +76,7 @@ def __init__(
>          self._other_eal_param = other_eal_param
>
>      def __str__(self) -> str:
> +        """Create the EAL string."""
>          return (
>              f"{self._lcore_list} "
>              f"{self._memory_channels} "
> @@ -72,11 +88,21 @@ def __str__(self) -> str:
>
>
>  class SutNode(Node):
> -    """
> -    A class for managing connections to the System under Test, providing
> -    methods that retrieve the necessary information about the node (such
> as
> -    CPU, memory and NIC details) and configuration capabilities.
> -    Another key capability is building DPDK according to given build
> target.
> +    """The system under test node.
> +
> +    The SUT node extends :class:`Node` with DPDK specific features:
> +
> +        * DPDK build,
> +        * Gathering of DPDK build info,
> +        * The running of DPDK apps, interactively or one-time execution,
> +        * DPDK apps cleanup.
> +
> +    The :option:`--tarball` command line argument and the
> :envvar:`DTS_DPDK_TARBALL`
> +    environment variable configure the path to the DPDK tarball
> +    or the git commit ID, tag ID or tree ID to test.
> +
> +    Attributes:
> +        config: The SUT node configuration
>      """
>
>      config: SutNodeConfiguration
> @@ -94,6 +120,11 @@ class SutNode(Node):
>      _path_to_devbind_script: PurePath | None
>
>      def __init__(self, node_config: SutNodeConfiguration):
> +        """Extend the constructor with SUT node specifics.
> +
> +        Args:
> +            node_config: The SUT node's test run configuration.
> +        """
>          super(SutNode, self).__init__(node_config)
>          self._dpdk_prefix_list = []
>          self._build_target_config = None
> @@ -113,6 +144,12 @@ def __init__(self, node_config: SutNodeConfiguration):
>
>      @property
>      def _remote_dpdk_dir(self) -> PurePath:
> +        """The remote DPDK dir.
> +
> +        This internal property should be set after extracting the DPDK
> tarball. If it's not set,
> +        that implies the DPDK setup step has been skipped, in which case
> we can guess where
> +        a previous build was located.
> +        """
>          if self.__remote_dpdk_dir is None:
>              self.__remote_dpdk_dir = self._guess_dpdk_remote_dir()
>          return self.__remote_dpdk_dir
> @@ -123,6 +160,11 @@ def _remote_dpdk_dir(self, value: PurePath) -> None:
>
>      @property
>      def remote_dpdk_build_dir(self) -> PurePath:
> +        """The remote DPDK build directory.
> +
> +        This is the directory where DPDK was built.
> +        We assume it was built in a subdirectory of the extracted tarball.
> +        """
>          if self._build_target_config:
>              return self.main_session.join_remote_path(
>                  self._remote_dpdk_dir, self._build_target_config.name
> @@ -132,18 +174,21 @@ def remote_dpdk_build_dir(self) -> PurePath:
>
>      @property
>      def dpdk_version(self) -> str:
> +        """Last built DPDK version."""
>          if self._dpdk_version is None:
>              self._dpdk_version =
> self.main_session.get_dpdk_version(self._remote_dpdk_dir)
>          return self._dpdk_version
>
>      @property
>      def node_info(self) -> NodeInfo:
> +        """Additional node information."""
>          if self._node_info is None:
>              self._node_info = self.main_session.get_node_info()
>          return self._node_info
>
>      @property
>      def compiler_version(self) -> str:
> +        """The node's compiler version."""
>          if self._compiler_version is None:
>              if self._build_target_config is not None:
>                  self._compiler_version =
> self.main_session.get_compiler_version(
> @@ -158,6 +203,7 @@ def compiler_version(self) -> str:
>
>      @property
>      def path_to_devbind_script(self) -> PurePath:
> +        """The path to the dpdk-devbind.py script on the node."""
>          if self._path_to_devbind_script is None:
>              self._path_to_devbind_script =
> self.main_session.join_remote_path(
>                  self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
> @@ -165,6 +211,11 @@ def path_to_devbind_script(self) -> PurePath:
>          return self._path_to_devbind_script
>
>      def get_build_target_info(self) -> BuildTargetInfo:
> +        """Get additional build target information.
> +
> +        Returns:
> +            The build target information,
> +        """
>          return BuildTargetInfo(
>              dpdk_version=self.dpdk_version,
> compiler_version=self.compiler_version
>          )
> @@ -173,8 +224,9 @@ def _guess_dpdk_remote_dir(self) -> PurePath:
>          return
> self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
>
>      def _set_up_build_target(self, build_target_config:
> BuildTargetConfiguration) -> None:
> -        """
> -        Setup DPDK on the SUT node.
> +        """Setup DPDK on the SUT node.
> +
> +        Additional build target setup steps on top of those in
> :class:`Node`.
>          """
>          # we want to ensure that dpdk_version and compiler_version is
> reset for new
>          # build targets
> @@ -186,16 +238,14 @@ def _set_up_build_target(self, build_target_config:
> BuildTargetConfiguration) ->
>          self.bind_ports_to_driver()
>
>      def _tear_down_build_target(self) -> None:
> -        """
> -        This method exists to be optionally overwritten by derived
> classes and
> -        is not decorated so that the derived class doesn't have to use
> the decorator.
> +        """Bind ports to the operating system drivers.
> +
> +        Additional build target teardown steps on top of those in
> :class:`Node`.
>          """
>          self.bind_ports_to_driver(for_dpdk=False)
>
>      def _configure_build_target(self, build_target_config:
> BuildTargetConfiguration) -> None:
> -        """
> -        Populate common environment variables and set build target config.
> -        """
> +        """Populate common environment variables and set build target
> config."""
>          self._env_vars = {}
>          self._build_target_config = build_target_config
>
>  self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
> @@ -207,9 +257,7 @@ def _configure_build_target(self, build_target_config:
> BuildTargetConfiguration)
>
>      @Node.skip_setup
>      def _copy_dpdk_tarball(self) -> None:
> -        """
> -        Copy to and extract DPDK tarball on the SUT node.
> -        """
> +        """Copy to and extract DPDK tarball on the SUT node."""
>          self._logger.info("Copying DPDK tarball to SUT.")
>          self.main_session.copy_to(SETTINGS.dpdk_tarball_path,
> self._remote_tmp_dir)
>
> @@ -238,8 +286,9 @@ def _copy_dpdk_tarball(self) -> None:
>
>      @Node.skip_setup
>      def _build_dpdk(self) -> None:
> -        """
> -        Build DPDK. Uses the already configured target. Assumes that the
> tarball has
> +        """Build DPDK.
> +
> +        Uses the already configured target. Assumes that the tarball has
>          already been copied to and extracted on the SUT node.
>          """
>          self.main_session.build_dpdk(
> @@ -250,15 +299,19 @@ def _build_dpdk(self) -> None:
>          )
>
>      def build_dpdk_app(self, app_name: str, **meson_dpdk_args: str |
> bool) -> PurePath:
> -        """
> -        Build one or all DPDK apps. Requires DPDK to be already built on
> the SUT node.
> -        When app_name is 'all', build all example apps.
> -        When app_name is any other string, tries to build that example
> app.
> -        Return the directory path of the built app. If building all apps,
> return
> -        the path to the examples directory (where all apps reside).
> -        The meson_dpdk_args are keyword arguments
> -        found in meson_option.txt in root DPDK directory. Do not use -D
> with them,
> -        for example: enable_kmods=True.
> +        """Build one or all DPDK apps.
> +
> +        Requires DPDK to be already built on the SUT node.
> +
> +        Args:
> +            app_name: The name of the DPDK app to build.
> +                When `app_name` is ``all``, build all example apps.
> +            meson_dpdk_args: The arguments found in ``meson_options.txt``
> in root DPDK directory.
> +                Do not use ``-D`` with them.
> +
> +        Returns:
> +            The directory path of the built app. If building all apps,
> return
> +            the path to the examples directory (where all apps reside).
>          """
>          self.main_session.build_dpdk(
>              self._env_vars,
> @@ -277,9 +330,7 @@ def build_dpdk_app(self, app_name: str,
> **meson_dpdk_args: str | bool) -> PurePa
>          )
>
>      def kill_cleanup_dpdk_apps(self) -> None:
> -        """
> -        Kill all dpdk applications on the SUT. Cleanup hugepages.
> -        """
> +        """Kill all dpdk applications on the SUT, then clean up
> hugepages."""
>          if self._dpdk_kill_session and self._dpdk_kill_session.is_alive():
>              # we can use the session if it exists and responds
>
>  self._dpdk_kill_session.kill_cleanup_dpdk_apps(self._dpdk_prefix_list)
> @@ -298,33 +349,34 @@ def create_eal_parameters(
>          vdevs: list[VirtualDevice] | None = None,
>          other_eal_param: str = "",
>      ) -> "EalParameters":
> -        """
> -        Generate eal parameters character string;
> -        :param lcore_filter_specifier: a number of lcores/cores/sockets
> to use
> -                        or a list of lcore ids to use.
> -                        The default will select one lcore for each of two
> cores
> -                        on one socket, in ascending order of core ids.
> -        :param ascending_cores: True, use cores with the lowest numerical
> id first
> -                        and continue in ascending order. If False, start
> with the
> -                        highest id and continue in descending order. This
> ordering
> -                        affects which sockets to consider first as well.
> -        :param prefix: set file prefix string, eg:
> -                        prefix='vf'
> -        :param append_prefix_timestamp: if True, will append a timestamp
> to
> -                        DPDK file prefix.
> -        :param no_pci: switch of disable PCI bus eg:
> -                        no_pci=True
> -        :param vdevs: virtual device list, eg:
> -                        vdevs=[
> -                            VirtualDevice('net_ring0'),
> -                            VirtualDevice('net_ring1')
> -                        ]
> -        :param other_eal_param: user defined DPDK eal parameters, eg:
> -                        other_eal_param='--single-file-segments'
> -        :return: eal param string, eg:
> -                '-c 0xf -a 0000:88:00.0
> --file-prefix=dpdk_1112_20190809143420';
> -        """
> +        """Compose the EAL parameters.
> +
> +        Process the list of cores and the DPDK prefix and pass that along
> with
> +        the rest of the arguments.
>
> +        Args:
> +            lcore_filter_specifier: A number of lcores/cores/sockets to
> use
> +                or a list of lcore ids to use.
> +                The default will select one lcore for each of two cores
> +                on one socket, in ascending order of core ids.
> +            ascending_cores: Sort cores in ascending order (lowest to
> highest IDs).
> +                If :data:`False`, sort in descending order.
> +            prefix: Set the file prefix string with which to start DPDK,
> e.g.: ``prefix='vf'``.
> +            append_prefix_timestamp: If :data:`True`, will append a
> timestamp to DPDK file prefix.
> +            no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``.
> +            vdevs: Virtual devices, e.g.::
> +
> +                vdevs=[
> +                    VirtualDevice('net_ring0'),
> +                    VirtualDevice('net_ring1')
> +                ]
> +            other_eal_param: user defined DPDK EAL parameters, e.g.:
> +                ``other_eal_param='--single-file-segments'``.
> +
> +        Returns:
> +            An EAL param string, such as
> +            ``-c 0xf -a 0000:88:00.0
> --file-prefix=dpdk_1112_20190809143420``.
> +        """
>          lcore_list =
> LogicalCoreList(self.filter_lcores(lcore_filter_specifier, ascending_cores))
>
>          if append_prefix_timestamp:
> @@ -348,14 +400,29 @@ def create_eal_parameters(
>      def run_dpdk_app(
>          self, app_path: PurePath, eal_args: "EalParameters", timeout:
> float = 30
>      ) -> CommandResult:
> -        """
> -        Run DPDK application on the remote node.
> +        """Run DPDK application on the remote node.
> +
> +        The application is not run interactively - the command that
> starts the application
> +        is executed and then the call waits for it to finish execution.
> +
> +        Args:
> +            app_path: The remote path to the DPDK application.
> +            eal_args: EAL parameters to run the DPDK application with.
> +            timeout: Wait at most this long in seconds for `command`
> execution to complete.
> +
> +        Returns:
> +            The result of the DPDK app execution.
>          """
>          return self.main_session.send_command(
>              f"{app_path} {eal_args}", timeout, privileged=True,
> verify=True
>          )
>
>      def configure_ipv4_forwarding(self, enable: bool) -> None:
> +        """Enable/disable IPv4 forwarding on the node.
> +
> +        Args:
> +            enable: If :data:`True`, enable the forwarding, otherwise
> disable it.
> +        """
>          self.main_session.configure_ipv4_forwarding(enable)
>
>      def create_interactive_shell(
> @@ -365,9 +432,13 @@ def create_interactive_shell(
>          privileged: bool = False,
>          eal_parameters: EalParameters | str | None = None,
>      ) -> InteractiveShellType:
> -        """Factory method for creating a handler for an interactive
> session.
> +        """Extend the factory for interactive session handlers.
> +
> +        The extensions are SUT node specific:
>
> -        Instantiate shell_cls according to the remote OS specifics.
> +            * The default for `eal_parameters`,
> +            * The interactive shell path `shell_cls.path` is prepended
> with path to the remote
> +              DPDK build directory for DPDK apps.
>
>          Args:
>              shell_cls: The class of the shell.
> @@ -377,9 +448,10 @@ def create_interactive_shell(
>              privileged: Whether to run the shell with administrative
> privileges.
>              eal_parameters: List of EAL parameters to use to launch the
> app. If this
>                  isn't provided or an empty string is passed, it will
> default to calling
> -                create_eal_parameters().
> +                :meth:`create_eal_parameters`.
> +
>          Returns:
> -            Instance of the desired interactive application.
> +            An instance of the desired interactive application shell.
>          """
>          if not eal_parameters:
>              eal_parameters = self.create_eal_parameters()
> @@ -396,8 +468,8 @@ def bind_ports_to_driver(self, for_dpdk: bool = True)
> -> None:
>          """Bind all ports on the SUT to a driver.
>
>          Args:
> -            for_dpdk: Boolean that, when True, binds ports to
> os_driver_for_dpdk
> -            or, when False, binds to os_driver. Defaults to True.
> +            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
> +                If :data:`False`, binds to os_driver.
>          """
>          for port in self.ports:
>              driver = port.os_driver_for_dpdk if for_dpdk else
> port.os_driver
> diff --git a/dts/framework/testbed_model/tg_node.py
> b/dts/framework/testbed_model/tg_node.py
> index 8a8f0019f3..f269d4c585 100644
> --- a/dts/framework/testbed_model/tg_node.py
> +++ b/dts/framework/testbed_model/tg_node.py
> @@ -5,13 +5,8 @@
>
>  """Traffic generator node.
>
> -This is the node where the traffic generator resides.
> -The distinction between a node and a traffic generator is as follows:
> -A node is a host that DTS connects to. It could be a baremetal server,
> -a VM or a container.
> -A traffic generator is software running on the node.
> -A traffic generator node is a node running a traffic generator.
> -A node can be a traffic generator node as well as system under test node.
> +A traffic generator (TG) generates traffic that's sent towards the SUT
> node.
> +A TG node is where the TG runs.
>  """
>
>  from scapy.packet import Packet  # type: ignore[import]
> @@ -24,13 +19,16 @@
>
>
>  class TGNode(Node):
> -    """Manage connections to a node with a traffic generator.
> +    """The traffic generator node.
>
> -    Apart from basic node management capabilities, the Traffic Generator
> node has
> -    specialized methods for handling the traffic generator running on it.
> +    The TG node extends :class:`Node` with TG specific features:
>
> -    Arguments:
> -        node_config: The user configuration of the traffic generator node.
> +        * Traffic generator initialization,
> +        * The sending of traffic and receiving packets,
> +        * The sending of traffic without receiving packets.
> +
> +    Not all traffic generators are capable of capturing traffic, which is
> why there
> +    must be a way to send traffic without that.
>
>      Attributes:
>          traffic_generator: The traffic generator running on the node.
> @@ -39,6 +37,13 @@ class TGNode(Node):
>      traffic_generator: CapturingTrafficGenerator
>
>      def __init__(self, node_config: TGNodeConfiguration):
> +        """Extend the constructor with TG node specifics.
> +
> +        Initialize the traffic generator on the TG node.
> +
> +        Args:
> +            node_config: The TG node's test run configuration.
> +        """
>          super(TGNode, self).__init__(node_config)
>          self.traffic_generator = create_traffic_generator(self,
> node_config.traffic_generator)
>          self._logger.info(f"Created node: {self.name}")
> @@ -50,17 +55,17 @@ def send_packet_and_capture(
>          receive_port: Port,
>          duration: float = 1,
>      ) -> list[Packet]:
> -        """Send a packet, return received traffic.
> +        """Send `packet`, return received traffic.
>
> -        Send a packet on the send_port and then return all traffic
> captured
> -        on the receive_port for the given duration. Also record the
> captured traffic
> +        Send `packet` on `send_port` and then return all traffic captured
> +        on `receive_port` for the given duration. Also record the
> captured traffic
>          in a pcap file.
>
>          Args:
>              packet: The packet to send.
>              send_port: The egress port on the TG node.
>              receive_port: The ingress port in the TG node.
> -            duration: Capture traffic for this amount of time after
> sending the packet.
> +            duration: Capture traffic for this amount of time after
> sending `packet`.
>
>          Returns:
>               A list of received packets. May be empty if no packets are
> captured.
> @@ -70,6 +75,9 @@ def send_packet_and_capture(
>          )
>
>      def close(self) -> None:
> -        """Free all resources used by the node"""
> +        """Free all resources used by the node.
> +
> +        This extends the superclass method with TG cleanup.
> +        """
>          self.traffic_generator.close()
>          super(TGNode, self).close()
> --
> 2.34.1
>
>
  
Juraj Linkeš Dec. 4, 2023, 10:02 a.m. UTC | #2
On Fri, Dec 1, 2023 at 7:06 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
>
>
>
> On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> Format according to the Google format and PEP257, with slight
>> deviations.
>>
>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>> ---
>>  dts/framework/testbed_model/sut_node.py | 230 ++++++++++++++++--------
>>  dts/framework/testbed_model/tg_node.py  |  42 +++--
>>  2 files changed, 176 insertions(+), 96 deletions(-)
>>
>> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
>> index 5ce9446dba..c4acea38d1 100644
>> --- a/dts/framework/testbed_model/sut_node.py
>> +++ b/dts/framework/testbed_model/sut_node.py
>> @@ -3,6 +3,14 @@
>>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
>>  # Copyright(c) 2023 University of New Hampshire
>>
>> +"""System under test (DPDK + hardware) node.
>> +
>> +A system under test (SUT) is the combination of DPDK
>> +and the hardware we're testing with DPDK (NICs, crypto and other devices).
>> +An SUT node is where this SUT runs.
>> +"""
>
>
> I think this should just be "A SUT node"
>

I always spell it out which is why I used "an" (an es, ju:, ti: node).
From what I understand, the article is based on how the word is
pronounced. If it's an initialism (it's spelled), we should use "an"
and if it's an abbreviation (pronounced as the whole word), we should
use "a". It always made sense to me as an initialism - I think that's
the common usage.
  
Bruce Richardson Dec. 4, 2023, 11:02 a.m. UTC | #3
On Mon, Dec 04, 2023 at 11:02:21AM +0100, Juraj Linkeš wrote:
> On Fri, Dec 1, 2023 at 7:06 PM Jeremy Spewock <jspewock@iol.unh.edu> wrote:
> >
> >
> >
> > On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >>
> >> Format according to the Google format and PEP257, with slight
> >> deviations.
> >>
> >> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> >> ---
> >>  dts/framework/testbed_model/sut_node.py | 230 ++++++++++++++++--------
> >>  dts/framework/testbed_model/tg_node.py  |  42 +++--
> >>  2 files changed, 176 insertions(+), 96 deletions(-)
> >>
> >> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> >> index 5ce9446dba..c4acea38d1 100644
> >> --- a/dts/framework/testbed_model/sut_node.py
> >> +++ b/dts/framework/testbed_model/sut_node.py
> >> @@ -3,6 +3,14 @@
> >>  # Copyright(c) 2023 PANTHEON.tech s.r.o.
> >>  # Copyright(c) 2023 University of New Hampshire
> >>
> >> +"""System under test (DPDK + hardware) node.
> >> +
> >> +A system under test (SUT) is the combination of DPDK
> >> +and the hardware we're testing with DPDK (NICs, crypto and other devices).
> >> +An SUT node is where this SUT runs.
> >> +"""
> >
> >
> > I think this should just be "A SUT node"
> >
> 
> I always spell it out which is why I used "an" (an es, ju:, ti: node).
> From what I understand, the article is based on how the word is
> pronounced. If it's an initialism (it's spelled), we should use "an"
> and if it's an abbreviation (pronounced as the whole word), we should
> use "a". It always made sense to me as an initialism - I think that's
> the common usage.

+1 for using "an" instead of "a" in front of "SUT".
  

Patch

diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 5ce9446dba..c4acea38d1 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -3,6 +3,14 @@ 
 # Copyright(c) 2023 PANTHEON.tech s.r.o.
 # Copyright(c) 2023 University of New Hampshire
 
+"""System under test (DPDK + hardware) node.
+
+A system under test (SUT) is the combination of DPDK
+and the hardware we're testing with DPDK (NICs, crypto and other devices).
+An SUT node is where this SUT runs.
+"""
+
+
 import os
 import tarfile
 import time
@@ -26,6 +34,11 @@ 
 
 
 class EalParameters(object):
+    """The environment abstraction layer parameters.
+
+    The string representation can be created by converting the instance to a string.
+    """
+
     def __init__(
         self,
         lcore_list: LogicalCoreList,
@@ -35,21 +48,23 @@  def __init__(
         vdevs: list[VirtualDevice],
         other_eal_param: str,
     ):
-        """
-        Generate eal parameters character string;
-        :param lcore_list: the list of logical cores to use.
-        :param memory_channels: the number of memory channels to use.
-        :param prefix: set file prefix string, eg:
-                        prefix='vf'
-        :param no_pci: switch of disable PCI bus eg:
-                        no_pci=True
-        :param vdevs: virtual device list, eg:
-                        vdevs=[
-                            VirtualDevice('net_ring0'),
-                            VirtualDevice('net_ring1')
-                        ]
-        :param other_eal_param: user defined DPDK eal parameters, eg:
-                        other_eal_param='--single-file-segments'
+        """Initialize the parameters according to inputs.
+
+        Process the parameters into the format used on the command line.
+
+        Args:
+            lcore_list: The list of logical cores to use.
+            memory_channels: The number of memory channels to use.
+            prefix: Set the file prefix string with which to start DPDK, e.g.: ``prefix='vf'``.
+            no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``.
+            vdevs: Virtual devices, e.g.::
+
+                vdevs=[
+                    VirtualDevice('net_ring0'),
+                    VirtualDevice('net_ring1')
+                ]
+            other_eal_param: user defined DPDK EAL parameters, e.g.:
+                ``other_eal_param='--single-file-segments'``
         """
         self._lcore_list = f"-l {lcore_list}"
         self._memory_channels = f"-n {memory_channels}"
@@ -61,6 +76,7 @@  def __init__(
         self._other_eal_param = other_eal_param
 
     def __str__(self) -> str:
+        """Create the EAL string."""
         return (
             f"{self._lcore_list} "
             f"{self._memory_channels} "
@@ -72,11 +88,21 @@  def __str__(self) -> str:
 
 
 class SutNode(Node):
-    """
-    A class for managing connections to the System under Test, providing
-    methods that retrieve the necessary information about the node (such as
-    CPU, memory and NIC details) and configuration capabilities.
-    Another key capability is building DPDK according to given build target.
+    """The system under test node.
+
+    The SUT node extends :class:`Node` with DPDK specific features:
+
+        * DPDK build,
+        * Gathering of DPDK build info,
+        * The running of DPDK apps, interactively or one-time execution,
+        * DPDK apps cleanup.
+
+    The :option:`--tarball` command line argument and the :envvar:`DTS_DPDK_TARBALL`
+    environment variable configure the path to the DPDK tarball
+    or the git commit ID, tag ID or tree ID to test.
+
+    Attributes:
+        config: The SUT node configuration
     """
 
     config: SutNodeConfiguration
@@ -94,6 +120,11 @@  class SutNode(Node):
     _path_to_devbind_script: PurePath | None
 
     def __init__(self, node_config: SutNodeConfiguration):
+        """Extend the constructor with SUT node specifics.
+
+        Args:
+            node_config: The SUT node's test run configuration.
+        """
         super(SutNode, self).__init__(node_config)
         self._dpdk_prefix_list = []
         self._build_target_config = None
@@ -113,6 +144,12 @@  def __init__(self, node_config: SutNodeConfiguration):
 
     @property
     def _remote_dpdk_dir(self) -> PurePath:
+        """The remote DPDK dir.
+
+        This internal property should be set after extracting the DPDK tarball. If it's not set,
+        that implies the DPDK setup step has been skipped, in which case we can guess where
+        a previous build was located.
+        """
         if self.__remote_dpdk_dir is None:
             self.__remote_dpdk_dir = self._guess_dpdk_remote_dir()
         return self.__remote_dpdk_dir
@@ -123,6 +160,11 @@  def _remote_dpdk_dir(self, value: PurePath) -> None:
 
     @property
     def remote_dpdk_build_dir(self) -> PurePath:
+        """The remote DPDK build directory.
+
+        This is the directory where DPDK was built.
+        We assume it was built in a subdirectory of the extracted tarball.
+        """
         if self._build_target_config:
             return self.main_session.join_remote_path(
                 self._remote_dpdk_dir, self._build_target_config.name
@@ -132,18 +174,21 @@  def remote_dpdk_build_dir(self) -> PurePath:
 
     @property
     def dpdk_version(self) -> str:
+        """Last built DPDK version."""
         if self._dpdk_version is None:
             self._dpdk_version = self.main_session.get_dpdk_version(self._remote_dpdk_dir)
         return self._dpdk_version
 
     @property
     def node_info(self) -> NodeInfo:
+        """Additional node information."""
         if self._node_info is None:
             self._node_info = self.main_session.get_node_info()
         return self._node_info
 
     @property
     def compiler_version(self) -> str:
+        """The node's compiler version."""
         if self._compiler_version is None:
             if self._build_target_config is not None:
                 self._compiler_version = self.main_session.get_compiler_version(
@@ -158,6 +203,7 @@  def compiler_version(self) -> str:
 
     @property
     def path_to_devbind_script(self) -> PurePath:
+        """The path to the dpdk-devbind.py script on the node."""
         if self._path_to_devbind_script is None:
             self._path_to_devbind_script = self.main_session.join_remote_path(
                 self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
@@ -165,6 +211,11 @@  def path_to_devbind_script(self) -> PurePath:
         return self._path_to_devbind_script
 
     def get_build_target_info(self) -> BuildTargetInfo:
+        """Get additional build target information.
+
+        Returns:
+            The build target information,
+        """
         return BuildTargetInfo(
             dpdk_version=self.dpdk_version, compiler_version=self.compiler_version
         )
@@ -173,8 +224,9 @@  def _guess_dpdk_remote_dir(self) -> PurePath:
         return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_dir)
 
     def _set_up_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """
-        Setup DPDK on the SUT node.
+        """Setup DPDK on the SUT node.
+
+        Additional build target setup steps on top of those in :class:`Node`.
         """
         # we want to ensure that dpdk_version and compiler_version is reset for new
         # build targets
@@ -186,16 +238,14 @@  def _set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
         self.bind_ports_to_driver()
 
     def _tear_down_build_target(self) -> None:
-        """
-        This method exists to be optionally overwritten by derived classes and
-        is not decorated so that the derived class doesn't have to use the decorator.
+        """Bind ports to the operating system drivers.
+
+        Additional build target teardown steps on top of those in :class:`Node`.
         """
         self.bind_ports_to_driver(for_dpdk=False)
 
     def _configure_build_target(self, build_target_config: BuildTargetConfiguration) -> None:
-        """
-        Populate common environment variables and set build target config.
-        """
+        """Populate common environment variables and set build target config."""
         self._env_vars = {}
         self._build_target_config = build_target_config
         self._env_vars.update(self.main_session.get_dpdk_build_env_vars(build_target_config.arch))
@@ -207,9 +257,7 @@  def _configure_build_target(self, build_target_config: BuildTargetConfiguration)
 
     @Node.skip_setup
     def _copy_dpdk_tarball(self) -> None:
-        """
-        Copy to and extract DPDK tarball on the SUT node.
-        """
+        """Copy to and extract DPDK tarball on the SUT node."""
         self._logger.info("Copying DPDK tarball to SUT.")
         self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remote_tmp_dir)
 
@@ -238,8 +286,9 @@  def _copy_dpdk_tarball(self) -> None:
 
     @Node.skip_setup
     def _build_dpdk(self) -> None:
-        """
-        Build DPDK. Uses the already configured target. Assumes that the tarball has
+        """Build DPDK.
+
+        Uses the already configured target. Assumes that the tarball has
         already been copied to and extracted on the SUT node.
         """
         self.main_session.build_dpdk(
@@ -250,15 +299,19 @@  def _build_dpdk(self) -> None:
         )
 
     def build_dpdk_app(self, app_name: str, **meson_dpdk_args: str | bool) -> PurePath:
-        """
-        Build one or all DPDK apps. Requires DPDK to be already built on the SUT node.
-        When app_name is 'all', build all example apps.
-        When app_name is any other string, tries to build that example app.
-        Return the directory path of the built app. If building all apps, return
-        the path to the examples directory (where all apps reside).
-        The meson_dpdk_args are keyword arguments
-        found in meson_option.txt in root DPDK directory. Do not use -D with them,
-        for example: enable_kmods=True.
+        """Build one or all DPDK apps.
+
+        Requires DPDK to be already built on the SUT node.
+
+        Args:
+            app_name: The name of the DPDK app to build.
+                When `app_name` is ``all``, build all example apps.
+            meson_dpdk_args: The arguments found in ``meson_options.txt`` in root DPDK directory.
+                Do not use ``-D`` with them.
+
+        Returns:
+            The directory path of the built app. If building all apps, return
+            the path to the examples directory (where all apps reside).
         """
         self.main_session.build_dpdk(
             self._env_vars,
@@ -277,9 +330,7 @@  def build_dpdk_app(self, app_name: str, **meson_dpdk_args: str | bool) -> PurePa
         )
 
     def kill_cleanup_dpdk_apps(self) -> None:
-        """
-        Kill all dpdk applications on the SUT. Cleanup hugepages.
-        """
+        """Kill all dpdk applications on the SUT, then clean up hugepages."""
         if self._dpdk_kill_session and self._dpdk_kill_session.is_alive():
             # we can use the session if it exists and responds
             self._dpdk_kill_session.kill_cleanup_dpdk_apps(self._dpdk_prefix_list)
@@ -298,33 +349,34 @@  def create_eal_parameters(
         vdevs: list[VirtualDevice] | None = None,
         other_eal_param: str = "",
     ) -> "EalParameters":
-        """
-        Generate eal parameters character string;
-        :param lcore_filter_specifier: a number of lcores/cores/sockets to use
-                        or a list of lcore ids to use.
-                        The default will select one lcore for each of two cores
-                        on one socket, in ascending order of core ids.
-        :param ascending_cores: True, use cores with the lowest numerical id first
-                        and continue in ascending order. If False, start with the
-                        highest id and continue in descending order. This ordering
-                        affects which sockets to consider first as well.
-        :param prefix: set file prefix string, eg:
-                        prefix='vf'
-        :param append_prefix_timestamp: if True, will append a timestamp to
-                        DPDK file prefix.
-        :param no_pci: switch of disable PCI bus eg:
-                        no_pci=True
-        :param vdevs: virtual device list, eg:
-                        vdevs=[
-                            VirtualDevice('net_ring0'),
-                            VirtualDevice('net_ring1')
-                        ]
-        :param other_eal_param: user defined DPDK eal parameters, eg:
-                        other_eal_param='--single-file-segments'
-        :return: eal param string, eg:
-                '-c 0xf -a 0000:88:00.0 --file-prefix=dpdk_1112_20190809143420';
-        """
+        """Compose the EAL parameters.
+
+        Process the list of cores and the DPDK prefix and pass that along with
+        the rest of the arguments.
 
+        Args:
+            lcore_filter_specifier: A number of lcores/cores/sockets to use
+                or a list of lcore ids to use.
+                The default will select one lcore for each of two cores
+                on one socket, in ascending order of core ids.
+            ascending_cores: Sort cores in ascending order (lowest to highest IDs).
+                If :data:`False`, sort in descending order.
+            prefix: Set the file prefix string with which to start DPDK, e.g.: ``prefix='vf'``.
+            append_prefix_timestamp: If :data:`True`, will append a timestamp to DPDK file prefix.
+            no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``.
+            vdevs: Virtual devices, e.g.::
+
+                vdevs=[
+                    VirtualDevice('net_ring0'),
+                    VirtualDevice('net_ring1')
+                ]
+            other_eal_param: user defined DPDK EAL parameters, e.g.:
+                ``other_eal_param='--single-file-segments'``.
+
+        Returns:
+            An EAL param string, such as
+            ``-c 0xf -a 0000:88:00.0 --file-prefix=dpdk_1112_20190809143420``.
+        """
         lcore_list = LogicalCoreList(self.filter_lcores(lcore_filter_specifier, ascending_cores))
 
         if append_prefix_timestamp:
@@ -348,14 +400,29 @@  def create_eal_parameters(
     def run_dpdk_app(
         self, app_path: PurePath, eal_args: "EalParameters", timeout: float = 30
     ) -> CommandResult:
-        """
-        Run DPDK application on the remote node.
+        """Run DPDK application on the remote node.
+
+        The application is not run interactively - the command that starts the application
+        is executed and then the call waits for it to finish execution.
+
+        Args:
+            app_path: The remote path to the DPDK application.
+            eal_args: EAL parameters to run the DPDK application with.
+            timeout: Wait at most this long in seconds for `command` execution to complete.
+
+        Returns:
+            The result of the DPDK app execution.
         """
         return self.main_session.send_command(
             f"{app_path} {eal_args}", timeout, privileged=True, verify=True
         )
 
     def configure_ipv4_forwarding(self, enable: bool) -> None:
+        """Enable/disable IPv4 forwarding on the node.
+
+        Args:
+            enable: If :data:`True`, enable the forwarding, otherwise disable it.
+        """
         self.main_session.configure_ipv4_forwarding(enable)
 
     def create_interactive_shell(
@@ -365,9 +432,13 @@  def create_interactive_shell(
         privileged: bool = False,
         eal_parameters: EalParameters | str | None = None,
     ) -> InteractiveShellType:
-        """Factory method for creating a handler for an interactive session.
+        """Extend the factory for interactive session handlers.
+
+        The extensions are SUT node specific:
 
-        Instantiate shell_cls according to the remote OS specifics.
+            * The default for `eal_parameters`,
+            * The interactive shell path `shell_cls.path` is prepended with path to the remote
+              DPDK build directory for DPDK apps.
 
         Args:
             shell_cls: The class of the shell.
@@ -377,9 +448,10 @@  def create_interactive_shell(
             privileged: Whether to run the shell with administrative privileges.
             eal_parameters: List of EAL parameters to use to launch the app. If this
                 isn't provided or an empty string is passed, it will default to calling
-                create_eal_parameters().
+                :meth:`create_eal_parameters`.
+
         Returns:
-            Instance of the desired interactive application.
+            An instance of the desired interactive application shell.
         """
         if not eal_parameters:
             eal_parameters = self.create_eal_parameters()
@@ -396,8 +468,8 @@  def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the SUT to a driver.
 
         Args:
-            for_dpdk: Boolean that, when True, binds ports to os_driver_for_dpdk
-            or, when False, binds to os_driver. Defaults to True.
+            for_dpdk: If :data:`True`, binds ports to os_driver_for_dpdk.
+                If :data:`False`, binds to os_driver.
         """
         for port in self.ports:
             driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
index 8a8f0019f3..f269d4c585 100644
--- a/dts/framework/testbed_model/tg_node.py
+++ b/dts/framework/testbed_model/tg_node.py
@@ -5,13 +5,8 @@ 
 
 """Traffic generator node.
 
-This is the node where the traffic generator resides.
-The distinction between a node and a traffic generator is as follows:
-A node is a host that DTS connects to. It could be a baremetal server,
-a VM or a container.
-A traffic generator is software running on the node.
-A traffic generator node is a node running a traffic generator.
-A node can be a traffic generator node as well as system under test node.
+A traffic generator (TG) generates traffic that's sent towards the SUT node.
+A TG node is where the TG runs.
 """
 
 from scapy.packet import Packet  # type: ignore[import]
@@ -24,13 +19,16 @@ 
 
 
 class TGNode(Node):
-    """Manage connections to a node with a traffic generator.
+    """The traffic generator node.
 
-    Apart from basic node management capabilities, the Traffic Generator node has
-    specialized methods for handling the traffic generator running on it.
+    The TG node extends :class:`Node` with TG specific features:
 
-    Arguments:
-        node_config: The user configuration of the traffic generator node.
+        * Traffic generator initialization,
+        * The sending of traffic and receiving packets,
+        * The sending of traffic without receiving packets.
+
+    Not all traffic generators are capable of capturing traffic, which is why there
+    must be a way to send traffic without that.
 
     Attributes:
         traffic_generator: The traffic generator running on the node.
@@ -39,6 +37,13 @@  class TGNode(Node):
     traffic_generator: CapturingTrafficGenerator
 
     def __init__(self, node_config: TGNodeConfiguration):
+        """Extend the constructor with TG node specifics.
+
+        Initialize the traffic generator on the TG node.
+
+        Args:
+            node_config: The TG node's test run configuration.
+        """
         super(TGNode, self).__init__(node_config)
         self.traffic_generator = create_traffic_generator(self, node_config.traffic_generator)
         self._logger.info(f"Created node: {self.name}")
@@ -50,17 +55,17 @@  def send_packet_and_capture(
         receive_port: Port,
         duration: float = 1,
     ) -> list[Packet]:
-        """Send a packet, return received traffic.
+        """Send `packet`, return received traffic.
 
-        Send a packet on the send_port and then return all traffic captured
-        on the receive_port for the given duration. Also record the captured traffic
+        Send `packet` on `send_port` and then return all traffic captured
+        on `receive_port` for the given duration. Also record the captured traffic
         in a pcap file.
 
         Args:
             packet: The packet to send.
             send_port: The egress port on the TG node.
             receive_port: The ingress port in the TG node.
-            duration: Capture traffic for this amount of time after sending the packet.
+            duration: Capture traffic for this amount of time after sending `packet`.
 
         Returns:
              A list of received packets. May be empty if no packets are captured.
@@ -70,6 +75,9 @@  def send_packet_and_capture(
         )
 
     def close(self) -> None:
-        """Free all resources used by the node"""
+        """Free all resources used by the node.
+
+        This extends the superclass method with TG cleanup.
+        """
         self.traffic_generator.close()
         super(TGNode, self).close()