On Fri, Oct 27, 2023 at 12:03 AM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Modifies the current process so that we bind to os_driver_for_dpdk from
> the configuration file before running test suites and bind back to the
> os_driver afterwards. This allows test suites to assume that the ports
> are bound to a DPDK supported driver or bind to either driver as needed.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/framework/remote_session/linux_session.py |  3 ++
>  dts/framework/remote_session/os_session.py    |  6 ++++
>  dts/framework/testbed_model/sut_node.py       | 34 +++++++++++++++++++
>  dts/tests/TestSuite_os_udp.py                 |  4 +++
>  dts/tests/TestSuite_smoke_tests.py            |  6 ++--
>  5 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/dts/framework/remote_session/linux_session.py b/dts/framework/remote_session/linux_session.py
> index a3f1a6bf3b..7f2453c44c 100644
> --- a/dts/framework/remote_session/linux_session.py
> +++ b/dts/framework/remote_session/linux_session.py
> @@ -199,3 +199,6 @@ def configure_port_ip_address(
>      def configure_ipv4_forwarding(self, enable: bool) -> None:
>          state = 1 if enable else 0
>          self.send_command(f"sysctl -w net.ipv4.ip_forward={state}", privileged=True)
> +
> +    def probe_driver(self, driver_name: str) -> None:
> +        self.send_command(f"modprobe {driver_name}", verify=True)

This may not be something we want to do in DTS, but rather assume it's
already been configured and maybe add a smoke test that would test
whether the driver is there.

> diff --git a/dts/framework/remote_session/os_session.py b/dts/framework/remote_session/os_session.py
> index 8a709eac1c..719e815ac8 100644
> --- a/dts/framework/remote_session/os_session.py
> +++ b/dts/framework/remote_session/os_session.py
> @@ -282,3 +282,9 @@ def configure_ipv4_forwarding(self, enable: bool) -> None:
>          """
>          Enable IPv4 forwarding in the underlying OS.
>          """
> +
> +    @abstractmethod
> +    def probe_driver(self, driver_name: str) -> None:
> +        """
> +        Load the module for the driver.
> +        """
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
> index 202aebfd06..5a7dd91cac 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -89,6 +89,7 @@ class SutNode(Node):
>      _dpdk_version: str | None
>      _node_info: NodeInfo | None
>      _compiler_version: str | None
> +    _path_to_devbind: PurePath | None

I'd add script to the variable so that it's clearer:
self._path_to_devbind_script

>
>      def __init__(self, node_config: SutNodeConfiguration):
>          super(SutNode, self).__init__(node_config)
> @@ -105,6 +106,7 @@ def __init__(self, node_config: SutNodeConfiguration):
>          self._dpdk_version = None
>          self._node_info = None
>          self._compiler_version = None
> +        self._path_to_devbind = None
>          self._logger.info(f"Created node: {self.name}")
>
>      @property
> @@ -155,6 +157,14 @@ def compiler_version(self) -> str:
>                  return ""
>          return self._compiler_version
>
> +    @property
> +    def path_to_devbind(self) -> PurePath:
> +        if self._path_to_devbind is None:
> +            self._path_to_devbind = self.main_session.join_remote_path(
> +                self._remote_dpdk_dir, "usertools", "dpdk-devbind.py"
> +            )
> +        return self._path_to_devbind
> +
>      def get_build_target_info(self) -> BuildTargetInfo:
>          return BuildTargetInfo(
>              dpdk_version=self.dpdk_version, compiler_version=self.compiler_version
> @@ -176,6 +186,14 @@ def _set_up_build_target(
>          self._configure_build_target(build_target_config)
>          self._copy_dpdk_tarball()
>          self._build_dpdk()
> +        self.bind_ports_to_driver()

This only binds the ports on the SUT node, should we also do this on
the TG node? We may not have access to the devbind script on the TG
node, but we can just copy it there.

> +
> +    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.
> +        """
> +        self.bind_ports_to_driver(for_dpdk=False)

There are three possibilities here:
1. We unbind the port
2. We bind the port to the OS driver (like you're suggesting)
3. We bind the port to the state before self.bind_ports_to_driver()

I don't think it's that big of a deal if we just bind it to the OS
driver, but 3 is probably the best.

>
>      def _configure_build_target(
>          self, build_target_config: BuildTargetConfiguration
> @@ -389,3 +407,19 @@ def create_interactive_shell(
>          return super().create_interactive_shell(
>              shell_cls, timeout, privileged, str(eal_parameters)
>          )
> +
> +    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 port in self.ports:
> +            driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
> +            self.main_session.probe_driver(driver)
> +            self.main_session.send_command(
> +                f"{self.path_to_devbind} -b {driver} --force {port.pci}",
> +                privileged=True,
> +                verify=True,
> +            )

Just a note: I was thinking about creating a dependency on the devbind
script which is outside DTS and it actually looks like a benefit. Not
only we don't need to implement anything, we're also testing the tool.