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 01FBC459FE; Tue, 24 Sep 2024 15:57:59 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E57C340274; Tue, 24 Sep 2024 15:57:58 +0200 (CEST) Received: from mail-pl1-f181.google.com (mail-pl1-f181.google.com [209.85.214.181]) by mails.dpdk.org (Postfix) with ESMTP id 3762C400D5 for ; Tue, 24 Sep 2024 15:57:57 +0200 (CEST) Received: by mail-pl1-f181.google.com with SMTP id d9443c01a7336-205909afad3so63255465ad.2 for ; Tue, 24 Sep 2024 06:57:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1727186276; x=1727791076; 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=db5abDllRrPDmd0iE5pVTV1zd3dabn3xj0XihNwJS7w=; b=crrTNUxH3btujnRjR8Ls3xFA3tAyvDPEy6gDT68Z7uqiZTdYFDJaXZz7vJMSooDddW RGYFzDBNN66nXW5kRz0b5Ob2HXt3DcHBPAnvcxMc4errpaQ62VJc2gM7uMJvyUObwPiw TuLFzTgvBEfmWZnPlmB5kaP1GZHDEDMKU11VA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727186276; x=1727791076; 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=db5abDllRrPDmd0iE5pVTV1zd3dabn3xj0XihNwJS7w=; b=lBE/Lp0VJ1bcqIxDg4xICefL9RuxvyefTy7uu1xBf7ViffoeQqtSsIqA8jeZfLi8NY GACa6gWYhlY9Dez3ip231r/ZLD9LrsZEuoTet9FkFcEBt3njaBUIqJEEu1q9D7tdQQIb Kn0gGTyDosa49YnMTfAASXcYXWvXG7w9UUYw1AmHmLPKBaPUi4AhbVoRXJD0M9s8fK8F tsBmK6+eIRF8gHRRFLMR7xVtq+D/UMay7XbD2ityd/o6GcHZs+AIuvADXz4tTff/vhHh XmP8ktKZ5KJmfsI/sSekaSdYLoFpin9oC/eBviJRYIFgODlESL3tqZ3CXsg6mvBJrdQU iJog== X-Forwarded-Encrypted: i=1; AJvYcCU0b8c2a2qHnWIHa2ebiEANaN+shi+mGJfahynOtEwuFXmRb86S6oxtXQgSFJczmfcQatg=@dpdk.org X-Gm-Message-State: AOJu0YylDt89u2gMFjB1bBNlkYQoCJ4SfQJcONQgu23FiV2Qb9pSGpZi 6K3AOpZBcJNOz9nl5p+AWKka33XzdWFIuvxM8lhwINT79PtAcNt0s5mvZ92/bJh6C6imoOP95sT m9khZ7bShQ8e0NVH8HO7GQb3wIKeDo5r1pJnXIw== X-Google-Smtp-Source: AGHT+IGRjENZU7ZL2Lg53l9jJElZuES5W8l9tfb+ZwfyHiuel+9VNSJC3bzv5D35FAgXjppwTDaQ2XXqpNpArhBW/hw= X-Received: by 2002:a17:90b:691:b0:2de:ec70:837 with SMTP id 98e67ed59e1d1-2deec700e57mr10909124a91.1.1727186276309; Tue, 24 Sep 2024 06:57:56 -0700 (PDT) MIME-Version: 1.0 References: <20240812172251.41131-1-jspewock@iol.unh.edu> <20240919181611.20289-1-jspewock@iol.unh.edu> <20240919181611.20289-2-jspewock@iol.unh.edu> <0eeacec2-6b17-4ed6-8444-c3963378b7f5@pantheon.tech> In-Reply-To: <0eeacec2-6b17-4ed6-8444-c3963378b7f5@pantheon.tech> From: Jeremy Spewock Date: Tue, 24 Sep 2024 09:57:44 -0400 Message-ID: Subject: Re: [PATCH v2 1/1] dts: add binding to different drivers to TG node To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: alex.chapman@arm.com, npratte@iol.unh.edu, thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, yoan.picchi@foss.arm.com, Luca.Vizzarro@arm.com, paul.szczepanek@arm.com, wathsala.vithanage@arm.com, 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 On Tue, Sep 24, 2024 at 5:12=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > 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, Right, these are good points. A good observation too that we only really need the kernel driver at the start in both cases. You had mentioned in your previous comments as well that we should only be binding on the TG once per lifetime, but I ended up not adding it for that very reason of I still wanted the binding to be in Node, but I didn't want to change the process for the SUT. > 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. I like this idea a lot actually. It feels very weird to me having to step out of the DTS directory and I like the idea of keeping it together like it were a package (even if it isn't yet). > > 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 > > > > 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 metho= d > > for binding ports into the node class rather than the SUT node class an= d > > 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 int= o > > the build target setup and run on both nodes. > > > > Bugzilla ID: 1420 > > > > Signed-off-by: Jeremy Spewock > > --- > > With the two minor comments, > Reviewed-by: Juraj Linke=C5=A1 > > > diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbe= d_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 didn't think of this but you're right, the two are very similar but named differently. > > 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). I also like the sound of it without the prefix and how it actually has a more fitting meaning from the two perspectives. I agree that there is probably some other work to be done on this in another patch, but since I am moving the _remote_tmp_dir variable around anyway I think it wouldn't hurt for me to rename it. > > > > 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 =3D "/[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 =3D 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. Ack. I tried to search for the differences between the two actually and saw a similar answer that PurePaths are used for manipulation, but I didn't fully understand as I am still manipulated this path and it isn't referencing a real file on host where I am defining the variable (what ever host is running DTS that is). This is a good distinction though to use Paths when it is used as a local path somewhere. > > > > > > > def expand_range(range_str: str) -> list[int]: >