From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 446DC457A3; Mon, 12 Aug 2024 19:49:27 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2B937406B8; Mon, 12 Aug 2024 19:49:27 +0200 (CEST) Received: from mail-lj1-f177.google.com (mail-lj1-f177.google.com [209.85.208.177]) by mails.dpdk.org (Postfix) with ESMTP id D7A394028C for ; Mon, 12 Aug 2024 19:49:24 +0200 (CEST) Received: by mail-lj1-f177.google.com with SMTP id 38308e7fff4ca-2ef2e37a171so8763341fa.2 for ; Mon, 12 Aug 2024 10:49:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1723484964; x=1724089764; darn=dpdk.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=OyH07erK48w7Hvycqcl+/Lcb+uvaHV9elMsQtihpxS4=; b=SnWg9BVjxLCPOPWOmjkIXcVUBW+Np8zTS+54optyCguZfHZ1whgmWPcR7cini15nSl leo1YxOe8Px9SZ6oaNPjY0SmdBgk3pEFa3tVGLCHoZLIR36QqFJogoaZnQMVgrFSn6c+ LyRD7GstZ3elYwRQxjoAmmCDZNHUb1FD8tDSU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723484964; x=1724089764; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=OyH07erK48w7Hvycqcl+/Lcb+uvaHV9elMsQtihpxS4=; b=OMndhGl0B/klsGyP5Nxb9jj3XIGTRvHANI5S7zyHskMnCK655qXevXaxrUrDa1zmxc IZrCEY+MHVvSnedZ3yD1a2KslsDcTcXDJwyyS5czOAUe/NsBXykII2T1GayCZf6UNKXD 0lWr1OjZxyVgEMNmVrtsUqeFBtasV7W9c7q6AudSmj3xHmr7pZEG4BTFYIQITklnZsAm fwG1x/IQiyThW/DF1qBEoEc5eUiEz1dszSI5vGBq3DlZajTiG/XhzXpxps3l3ptViPRg IE5o5o8UYxRulPQYpSDr6/IUMvEjvKkzLwW25N5UGMgXSKR0SpoEPFbrp9/7Ym4w6ntz kczA== X-Forwarded-Encrypted: i=1; AJvYcCUHCj5LJrNNV+2bX6EjcSMbCSJ6mPPEjuiWfwNweb8aIzmq0T6PhbrNopXeH7AVJ1e/8os=@dpdk.org X-Gm-Message-State: AOJu0YwiZT568uUuKrCguQKOVtwl6Fo61esntqtK8nazXSNViddwikAu RY5r61i3QIkIz2RstVBBiTE0JaHARiOp1ltFz0E1rjNONTqJ+wmfn4A/DGOnSWe4jJtgkqJXJ5w wY/BFMNYhK4OHAN7ZETbp1rA5FhQKDQBkhZI/+A== X-Google-Smtp-Source: AGHT+IE+50oQMEIhGUT8bZ1i1pvYJBS1pFL2EEBo9McS/v4rA3Lb3lQ9Iz7As5Ven8CRvfNz6Oqbl5EziXwoZ/F8Vhk= X-Received: by 2002:a05:651c:2113:b0:2f0:23f7:8e6d with SMTP id 38308e7fff4ca-2f2b7285ad2mr4793361fa.8.1723484963958; Mon, 12 Aug 2024 10:49:23 -0700 (PDT) MIME-Version: 1.0 References: <20240812172251.41131-1-jspewock@iol.unh.edu> <20240812172251.41131-2-jspewock@iol.unh.edu> In-Reply-To: <20240812172251.41131-2-jspewock@iol.unh.edu> From: Nicholas Pratte Date: Mon, 12 Aug 2024 13:49:12 -0400 Message-ID: Subject: Re: [PATCH 1/1] dts: add binding to different drivers to TG node To: jspewock@iol.unh.edu Cc: alex.chapman@arm.com, Luca.Vizzarro@arm.com, wathsala.vithanage@arm.com, Honnappa.Nagarahalli@arm.com, juraj.linkes@pantheon.tech, paul.szczepanek@arm.com, thomas@monjalon.net, yoan.picchi@foss.arm.com, probb@iol.unh.edu, dev@dpdk.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Makes sense to me! Reviewed-by: Nicholas Pratte On Mon, Aug 12, 2024 at 1:23=E2=80=AFPM wrote: > > From: Jeremy Spewock > > The DTS framework in its current state supports binding ports to > different drivers on the SUT node but not the TG node. The TG node > already has the information that it needs about the different drivers > that it has available in the configuration file, but it did not > previously have access to the devbind script, so it did not use that > information for anything. > > This patch moves the steps to copy the DPDK tarball into the node class > rather than the SUT node class, and calls this function on the TG node > as well as the SUT. It also moves the driver binding step into the Node > class and triggers the same pattern of binding to ports that existed on > the SUT on the TG. > > Bugzilla ID: 1420 > > Signed-off-by: Jeremy Spewock > --- > dts/framework/runner.py | 2 + > dts/framework/testbed_model/node.py | 106 +++++++++++++++++++++++- > dts/framework/testbed_model/sut_node.py | 86 +------------------ > 3 files changed, 109 insertions(+), 85 deletions(-) > > diff --git a/dts/framework/runner.py b/dts/framework/runner.py > index 6b6f6a05f5..ed9e58b172 100644 > --- a/dts/framework/runner.py > +++ b/dts/framework/runner.py > @@ -484,6 +484,7 @@ def _run_build_target( > > try: > sut_node.set_up_build_target(build_target_config) > + tg_node.set_up_build_target(build_target_config) > self._result.dpdk_version =3D sut_node.dpdk_version > build_target_result.add_build_target_info(sut_node.get_build= _target_info()) > build_target_result.update_setup(Result.PASS) > @@ -498,6 +499,7 @@ def _run_build_target( > try: > self._logger.set_stage(DtsStage.build_target_teardown) > sut_node.tear_down_build_target() > + tg_node.tear_down_build_target() > build_target_result.update_teardown(Result.PASS) > except Exception as e: > self._logger.exception("Build target teardown failed.") > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_= model/node.py > index 12a40170ac..8e6181e424 100644 > --- a/dts/framework/testbed_model/node.py > +++ b/dts/framework/testbed_model/node.py > @@ -13,11 +13,19 @@ > The :func:`~Node.skip_setup` decorator can be used without subclassing. > """ > > +import os > +import tarfile > from abc import ABC > from ipaddress import IPv4Interface, IPv6Interface > +from pathlib import PurePath > from typing import Any, Callable, Union > > -from framework.config import OS, NodeConfiguration, TestRunConfiguration > +from framework.config import ( > + OS, > + BuildTargetConfiguration, > + NodeConfiguration, > + TestRunConfiguration, > +) > from framework.exception import ConfigurationError > from framework.logger import DTSLogger, get_dts_logger > from framework.settings import SETTINGS > @@ -58,8 +66,11 @@ class Node(ABC): > lcores: list[LogicalCore] > ports: list[Port] > _logger: DTSLogger > + _remote_tmp_dir: PurePath > + __remote_dpdk_dir: PurePath | None > _other_sessions: list[OSSession] > _test_run_config: TestRunConfiguration > + _path_to_devbind_script: PurePath | None > > def __init__(self, node_config: NodeConfiguration): > """Connect to the node and gather info during initialization. > @@ -88,6 +99,9 @@ def __init__(self, node_config: NodeConfiguration): > > self._other_sessions =3D [] > self._init_ports() > + self._remote_tmp_dir =3D self.main_session.get_remote_tmp_dir() > + self.__remote_dpdk_dir =3D None > + self._path_to_devbind_script =3D None > > def _init_ports(self) -> None: > self.ports =3D [Port(self.name, port_config) for port_config in = self.config.ports] > @@ -95,6 +109,34 @@ def _init_ports(self) -> None: > for port in self.ports: > self.configure_port_state(port) > > + def _guess_dpdk_remote_dir(self) -> PurePath: > + return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_= dir) > + > + @property > + def _remote_dpdk_dir(self) -> PurePath: > + """The remote DPDK dir. > + > + This internal property should be set after extracting the DPDK t= arball. 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 =3D self._guess_dpdk_remote_dir() > + return self.__remote_dpdk_dir > + > + @_remote_dpdk_dir.setter > + def _remote_dpdk_dir(self, value: PurePath) -> None: > + self.__remote_dpdk_dir =3D value > + > + @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 =3D self.main_session.join_remo= te_path( > + self._remote_dpdk_dir, "usertools", "dpdk-devbind.py" > + ) > + return self._path_to_devbind_script > + > def set_up_test_run(self, test_run_config: TestRunConfiguration) -> = None: > """Test run setup steps. > > @@ -114,6 +156,24 @@ def tear_down_test_run(self) -> None: > Additional steps can be added by extending the method in subclas= ses with the use of super(). > """ > > + def set_up_build_target(self, build_target_config: BuildTargetConfig= uration) -> None: > + """Set up DPDK the node and bind ports. > + > + DPDK setup includes setting all internals needed for the build, = the copying of DPDK tarball > + and then building DPDK. The drivers are bound to those that DPDK= needs. > + > + Args: > + build_target_config: The build target test run configuration= according to which > + the setup steps will be taken. > + """ > + self._copy_dpdk_tarball() > + self.bind_ports_to_driver() > + > + def tear_down_build_target(self) -> None: > + """Reset DPDK variables and bind port driver to the OS driver.""= " > + self.__remote_dpdk_dir =3D None > + self.bind_ports_to_driver(for_dpdk=3DFalse) > + > def create_session(self, name: str) -> OSSession: > """Create and return a new OS-aware remote session. > > @@ -228,6 +288,50 @@ def skip_setup(func: Callable[..., Any]) -> Callable= [..., Any]: > else: > return func > > + @skip_setup > + def _copy_dpdk_tarball(self) -> None: > + """Copy to and extract DPDK tarball on the node.""" > + self._logger.info(f"Copying DPDK tarball to {self.name}.") > + self.main_session.copy_to(SETTINGS.dpdk_tarball_path, self._remo= te_tmp_dir) > + > + # construct remote tarball path > + # the basename is the same on local host and on remote Node > + remote_tarball_path =3D self.main_session.join_remote_path( > + self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball= _path) > + ) > + > + # construct remote path after extracting > + with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar: > + dpdk_top_dir =3D dpdk_tar.getnames()[0] > + self._remote_dpdk_dir =3D self.main_session.join_remote_path( > + self._remote_tmp_dir, dpdk_top_dir > + ) > + > + self._logger.info( > + f"Extracting DPDK tarball on {self.name}: " > + f"'{remote_tarball_path}' into '{self._remote_dpdk_dir}'." > + ) > + # clean remote path where we're extracting > + self.main_session.remove_remote_dir(self._remote_dpdk_dir) > + > + # then extract to remote path > + self.main_session.extract_remote_tarball(remote_tarball_path, se= lf._remote_dpdk_dir) > + > + def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None: > + """Bind all ports on the node to a driver. > + > + Args: > + 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 =3D 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 {por= t.pci}", > + privileged=3DTrue, > + verify=3DTrue, > + ) > + > > def create_session(node_config: NodeConfiguration, name: str, logger: DT= SLogger) -> OSSession: > """Factory for OS-aware sessions. > diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/test= bed_model/sut_node.py > index 2855fe0276..f3fd4e2304 100644 > --- a/dts/framework/testbed_model/sut_node.py > +++ b/dts/framework/testbed_model/sut_node.py > @@ -13,7 +13,6 @@ > > > import os > -import tarfile > import time > from pathlib import PurePath > > @@ -26,7 +25,6 @@ > ) > from framework.params.eal import EalParams > from framework.remote_session.remote_session import CommandResult > -from framework.settings import SETTINGS > from framework.utils import MesonArgs > > from .node import Node > @@ -59,14 +57,11 @@ class SutNode(Node): > dpdk_timestamp: str > _build_target_config: BuildTargetConfiguration | None > _env_vars: dict > - _remote_tmp_dir: PurePath > - __remote_dpdk_dir: PurePath | None > _app_compile_timeout: float > _dpdk_kill_session: OSSession | None > _dpdk_version: str | None > _node_info: NodeInfo | None > _compiler_version: str | None > - _path_to_devbind_script: PurePath | None > > def __init__(self, node_config: SutNodeConfiguration): > """Extend the constructor with SUT node specifics. > @@ -79,8 +74,6 @@ def __init__(self, node_config: SutNodeConfiguration): > self.dpdk_prefix_list =3D [] > self._build_target_config =3D None > self._env_vars =3D {} > - self._remote_tmp_dir =3D self.main_session.get_remote_tmp_dir() > - self.__remote_dpdk_dir =3D None > self._app_compile_timeout =3D 90 > self._dpdk_kill_session =3D None > self.dpdk_timestamp =3D ( > @@ -89,25 +82,8 @@ def __init__(self, node_config: SutNodeConfiguration): > self._dpdk_version =3D None > self._node_info =3D None > self._compiler_version =3D None > - self._path_to_devbind_script =3D None > self._logger.info(f"Created node: {self.name}") > > - @property > - def _remote_dpdk_dir(self) -> PurePath: > - """The remote DPDK dir. > - > - This internal property should be set after extracting the DPDK t= arball. 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 =3D self._guess_dpdk_remote_dir() > - return self.__remote_dpdk_dir > - > - @_remote_dpdk_dir.setter > - def _remote_dpdk_dir(self, value: PurePath) -> None: > - self.__remote_dpdk_dir =3D value > - > @property > def remote_dpdk_build_dir(self) -> PurePath: > """The remote DPDK build directory. > @@ -151,15 +127,6 @@ def compiler_version(self) -> str: > return "" > return self._compiler_version > > - @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 =3D self.main_session.join_remo= te_path( > - self._remote_dpdk_dir, "usertools", "dpdk-devbind.py" > - ) > - return self._path_to_devbind_script > - > def get_build_target_info(self) -> BuildTargetInfo: > """Get additional build target information. > > @@ -170,9 +137,6 @@ def get_build_target_info(self) -> BuildTargetInfo: > dpdk_version=3Dself.dpdk_version, compiler_version=3Dself.co= mpiler_version > ) > > - def _guess_dpdk_remote_dir(self) -> PurePath: > - return self.main_session.guess_dpdk_remote_dir(self._remote_tmp_= dir) > - > def set_up_test_run(self, test_run_config: TestRunConfiguration) -> = None: > """Extend the test run setup with vdev config. > > @@ -199,19 +163,17 @@ def set_up_build_target(self, build_target_config: = BuildTargetConfiguration) -> > build_target_config: The build target test run configuration= according to which > the setup steps will be taken. > """ > + super().set_up_build_target(build_target_config) > self._configure_build_target(build_target_config) > - self._copy_dpdk_tarball() > self._build_dpdk() > - self.bind_ports_to_driver() > > def tear_down_build_target(self) -> None: > """Reset DPDK variables and bind port driver to the OS driver.""= " > + super().tear_down_build_target() > self._env_vars =3D {} > self._build_target_config =3D None > - self.__remote_dpdk_dir =3D None > self._dpdk_version =3D None > self._compiler_version =3D None > - self.bind_ports_to_driver(for_dpdk=3DFalse) > > def _configure_build_target(self, build_target_config: BuildTargetCo= nfiguration) -> None: > """Populate common environment variables and set build target co= nfig.""" > @@ -224,35 +186,6 @@ def _configure_build_target(self, build_target_confi= g: BuildTargetConfiguration) > f"'{build_target_config.compiler_wrapper} {build_target_= config.compiler.name}'" > ) # fmt: skip > > - @Node.skip_setup > - def _copy_dpdk_tarball(self) -> None: > - """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._remo= te_tmp_dir) > - > - # construct remote tarball path > - # the basename is the same on local host and on remote Node > - remote_tarball_path =3D self.main_session.join_remote_path( > - self._remote_tmp_dir, os.path.basename(SETTINGS.dpdk_tarball= _path) > - ) > - > - # construct remote path after extracting > - with tarfile.open(SETTINGS.dpdk_tarball_path) as dpdk_tar: > - dpdk_top_dir =3D dpdk_tar.getnames()[0] > - self._remote_dpdk_dir =3D self.main_session.join_remote_path( > - self._remote_tmp_dir, dpdk_top_dir > - ) > - > - self._logger.info( > - f"Extracting DPDK tarball on SUT: " > - f"'{remote_tarball_path}' into '{self._remote_dpdk_dir}'." > - ) > - # clean remote path where we're extracting > - self.main_session.remove_remote_dir(self._remote_dpdk_dir) > - > - # then extract to remote path > - self.main_session.extract_remote_tarball(remote_tarball_path, se= lf._remote_dpdk_dir) > - > @Node.skip_setup > def _build_dpdk(self) -> None: > """Build DPDK. > @@ -335,18 +268,3 @@ def configure_ipv4_forwarding(self, enable: bool) ->= None: > enable: If :data:`True`, enable the forwarding, otherwise di= sable it. > """ > self.main_session.configure_ipv4_forwarding(enable) > - > - def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None: > - """Bind all ports on the SUT to a driver. > - > - Args: > - 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 =3D 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 {por= t.pci}", > - privileged=3DTrue, > - verify=3DTrue, > - ) > -- > 2.45.2 >