Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

I have some thoughts for the future:
1a. The traffic generator is specified per-node, so maybe we could also 
change the binding to be for the whole lifetime of the TG node,
1b. But the same is true for the SUT node as well, right? After we do 
the port update (with kernel driver), we can just bind to DPDK driver. 
With SUT in the mix, this looks like a change for a different patch,
2. We could add a symlink to the devbind script with the target being in 
the dts directory. This way we don't have to go outside the dts 
directory and if DTS ever become a python package, we could just copy 
the script to the appropriate place. This is also something we don't 
really need to do.

And also two minor comments. A lot of suggestions for separate patches 
overall. :-)

On 19. 9. 2024 20:16, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> 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 location of the tmp directory as well as the method
> for binding ports into the node class rather than the SUT node class and
> adds an abstract method for getting the path to the devbind script into
> the node class. Then, binding ports to the correct drivers is moved into
> the build target setup and run on both nodes.
> 
> Bugzilla ID: 1420
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

With the two minor comments,
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

> diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py

> @@ -58,8 +65,10 @@ class Node(ABC):
>       lcores: list[LogicalCore]
>       ports: list[Port]
>       _logger: DTSLogger
> +    _remote_tmp_dir: PurePath
>       _other_sessions: list[OSSession]
>       _test_run_config: TestRunConfiguration
> +    _path_to_devbind_script: PurePath | None

A note on the naming. We have _remote_tmp_dir and 
_path_to_devbind_script. Both are pointing to a remote file/dir, but 
only one has the _remote prefix. They should probably be unified.

I've thought a bit about what the right name is. Dropping the prefix 
makes sense; sut_node.tmp_dir should mean the tmp dir on the SUT node 
(which would make it remote from the execution host's point of view, but 
not the node's view; the file is local to SUT node). This could be a 
good separate patch (improving the remote/local naming scheme to make it 
consistent and as sensible as possible).


> diff --git a/dts/framework/utils.py b/dts/framework/utils.py

> @@ -29,6 +29,8 @@
>   from .exception import ConfigurationError, InternalError
>   
>   REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
> +#: Path to DPDK directory on the host where DTS is being run.
> +LOCAL_DPDK_DIR: PurePath = PurePath(__file__).parents[2]

Local paths can be just pathlib.Path. PurePaths are for path 
manipulations only (useful for remote paths in RemoteSessions, 
OSSessions and Nodes), but for local existing paths we should use Path.

The OSSession and subclasses need a bit of an update in this regard - 
use Path for local paths and PurePaths for remote ones. We added this 
into our pre-built DPDK patch.

>   
>   
>   def expand_range(range_str: str) -> list[int]: