On Wed, Nov 8, 2023 at 6:03 AM Juraj Linkeš wrote: > On Fri, Oct 27, 2023 at 12:03 AM wrote: > > > > From: Jeremy Spewock > > > > 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 > > --- > > 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. > Agreed. > > 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. > > Jeremy and I discussed this and I'm not sure right now. With the SUT we have a known default behavior (bind to os_driver_for_dpdk), but for the TG we don't (really) know yet whether there will be any needs introduced by adding dperf, trex, ixia, etc. as traffic generators. I prefer to approach this question when we are adding TGs for perf testing and necessarily seeing if we need to allow for any configuration we aren't thinking of yet. > > + > > + 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. > I don't have a strong opinion on this other than I like the conf file being a source of truth, but 3 sounds fine too. > > > > > 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. > > I think I'm missing something but as we already depend on DPDK, we know we will have dpdk/usertools/dpdk-devbind.py, right?