[v4,1/5] dts: allow binding only a single port to a different driver
Checks
Commit Message
From: Jeremy Spewock <jspewock@iol.unh.edu>
Previously the DTS framework only included methods that bind all ports
that the test run was aware of to either the DPDK driver or the OS
driver. There are however some cases, like creating virtual functions,
where you would want some ports bound to the OS driver and others bound
to their DPDK driver. This patch adds the ability to bind individual
ports to their respective drviers to solve this problem.
Depends-on: patch-144318 ("dts: add binding to different drivers to TG
node")
Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
dts/framework/testbed_model/node.py | 21 ++++++++++++---------
dts/tests/TestSuite_os_udp.py | 4 ++--
2 files changed, 14 insertions(+), 11 deletions(-)
Comments
On 23. 9. 2024 20:42, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Previously the DTS framework only included methods that bind all ports
> that the test run was aware of to either the DPDK driver or the OS
> driver. There are however some cases, like creating virtual functions,
> where you would want some ports bound to the OS driver and others bound
> to their DPDK driver. This patch adds the ability to bind individual
> ports to their respective drviers to solve this problem.
Typo: drviers
>
> Depends-on: patch-144318 ("dts: add binding to different drivers to TG
> node")
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
> dts/framework/testbed_model/node.py | 21 ++++++++++++---------
> dts/tests/TestSuite_os_udp.py | 4 ++--
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
> @@ -266,12 +266,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
> 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
> - self.main_session.send_command(
> - f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
> - privileged=True,
> - verify=True,
> - )
> + self._bind_port_to_driver(port, for_dpdk)
> +
> + def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
> + driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
> + self.main_session.send_command(
> + f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
> + privileged=True,
> + verify=True,
> + )
I noticed we're doing this port by port, but we can bind multiple ports
in one devbind run - just pass all ports to it. I think this improvement
is well worth it.
Hi there,
I agree with Juraj's comments on the typo and running dpdk-devbind once.
Also noticed the commit subject is way too long, should be max 50
characters. Is it possible to make this more concise?
Best,
Luca
@@ -138,11 +138,11 @@ def set_up_build_target(self, build_target_config: BuildTargetConfiguration) ->
the setup steps will be taken. This is unused in this method, but subclasses that
extend this method may need it.
"""
- self.bind_ports_to_driver()
+ self.bind_all_ports_to_driver()
def tear_down_build_target(self) -> None:
"""Bind ports to their OS drivers."""
- self.bind_ports_to_driver(for_dpdk=False)
+ self.bind_all_ports_to_driver(for_dpdk=False)
def create_session(self, name: str) -> OSSession:
"""Create and return a new OS-aware remote session.
@@ -258,7 +258,7 @@ def skip_setup(func: Callable[..., Any]) -> Callable[..., Any]:
else:
return func
- def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
+ def bind_all_ports_to_driver(self, for_dpdk: bool = True) -> None:
"""Bind all ports on the node to a driver.
Args:
@@ -266,12 +266,15 @@ def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
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
- self.main_session.send_command(
- f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
- privileged=True,
- verify=True,
- )
+ self._bind_port_to_driver(port, for_dpdk)
+
+ def _bind_port_to_driver(self, port: Port, for_dpdk: bool = True) -> None:
+ driver = port.os_driver_for_dpdk if for_dpdk else port.os_driver
+ self.main_session.send_command(
+ f"{self.path_to_devbind_script} -b {driver} --force {port.pci}",
+ privileged=True,
+ verify=True,
+ )
def create_session(node_config: NodeConfiguration, name: str, logger: DTSLogger) -> OSSession:
@@ -23,7 +23,7 @@ def set_up_suite(self) -> None:
Bind the SUT ports to the OS driver, configure the ports and configure the SUT
to route traffic from if1 to if2.
"""
- self.sut_node.bind_ports_to_driver(for_dpdk=False)
+ self.sut_node.bind_all_ports_to_driver(for_dpdk=False)
self.configure_testbed_ipv4()
def test_os_udp(self) -> None:
@@ -50,4 +50,4 @@ def tear_down_suite(self) -> None:
"""
self.configure_testbed_ipv4(restore=True)
# Assume other suites will likely need dpdk driver
- self.sut_node.bind_ports_to_driver(for_dpdk=True)
+ self.sut_node.bind_all_ports_to_driver(for_dpdk=True)