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 336E3459CF; Wed, 18 Sep 2024 20:51:12 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C6A84026F; Wed, 18 Sep 2024 20:51:12 +0200 (CEST) Received: from mail-pj1-f52.google.com (mail-pj1-f52.google.com [209.85.216.52]) by mails.dpdk.org (Postfix) with ESMTP id 970AA4003C for ; Wed, 18 Sep 2024 20:51:10 +0200 (CEST) Received: by mail-pj1-f52.google.com with SMTP id 98e67ed59e1d1-2d889207d1aso42848a91.3 for ; Wed, 18 Sep 2024 11:51:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1726685470; x=1727290270; 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=l8CTf4HkGZa2gmvlXWJO9iKBGIR8mHdH/98y3SOkEgY=; b=bxHomaCUS95cRsriU3KnDz/eNsmjF6yPK3MeexPakajzl33dlV44fxrFS0MeueS55z 3/RH9lzqlGRIe2/aQ0XaHFajvVYckEigSdjIIS6gLYJtb7XOJF1Kh28yCSP+05c4NE/E TvWlfaIvn3+IE60EGjq3yeinxr6qItT6ne1tc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726685470; x=1727290270; 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=l8CTf4HkGZa2gmvlXWJO9iKBGIR8mHdH/98y3SOkEgY=; b=U4jGtmYK1g0IJ6c7JJUVSGYIRLeig11dd75Azt8ijd5+NJp39bo1luMte/ft6nNcz5 HkbVGCjdF6nnpaPIZtqa9Q5G7AMnnlnQulKiTtM/Z9Rthv378Xe2HN1Q83wcC0Yh3lwN omCThgy9czg3RMxL3q9RtE5uPDFZ62poAbQhkxQT5LlvF5eJJSkHfvdWXJd7x3sO70Q2 lyfoyqsky5tO37jyznokzTXcAo6KnFfGp9WdQLf72KaD6aaVtEtSooUc5osa8Lu+oD43 tM4aVk3IrLVWc0KhKgxHmsNBFKb/vof4aUYIOZCJ+VY/l+b3tp+wahoU4+HjHg1AKYlc AUvw== X-Forwarded-Encrypted: i=1; AJvYcCU82dBJo2Zz+PtoDg6IqhxugalgOsHKzk/s8BGGI0RYWPR34AAMrrNnjVAUBV1DutJdHBg=@dpdk.org X-Gm-Message-State: AOJu0YzxzJDGkBhFfXvxx5ZqfIQWst9ni5iyUM9A1J0e8lL1ThlNW+yC I1yM6sLFN7kRpRhhatSnN7h39baaH8Y6ypqmSRahyas9KKkBb3fHG018+X+dKyNJnJE66QLa8Gz fjjXHUiWxfQymQDfKm3iVvF2ONg+wFz0k2O9PjA== X-Google-Smtp-Source: AGHT+IGE+SJAhfbGsAPsML1ZCv/SD2iSRsDla4HtzpSErmd4ymvd8P2NOOmmMy1ro7Y/7A0X2rdod9R06l58Y1x6AHU= X-Received: by 2002:a17:90a:d34c:b0:2d8:87d3:903b with SMTP id 98e67ed59e1d1-2dba0083863mr24589075a91.35.1726685469438; Wed, 18 Sep 2024 11:51:09 -0700 (PDT) MIME-Version: 1.0 References: <20240812172251.41131-1-jspewock@iol.unh.edu> <20240812172251.41131-2-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Wed, 18 Sep 2024 14:50:58 -0400 Message-ID: Subject: Re: [PATCH 1/1] dts: add binding to different drivers to TG node To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: alex.chapman@arm.com, Luca.Vizzarro@arm.com, wathsala.vithanage@arm.com, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com, npratte@iol.unh.edu, 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 On Mon, Sep 16, 2024 at 6:04=E2=80=AFAM Juraj Linke=C5=A1 wrote: > > > > On 9. 9. 2024 17:55, Jeremy Spewock wrote: > > On Mon, Sep 9, 2024 at 8:16=E2=80=AFAM Juraj Linke=C5=A1 wrote: > >> > >> > >> > >> On 12. 8. 2024 19:22, 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 steps to copy the DPDK tarball into the node cla= ss > >>> rather than the SUT node class, and calls this function on the TG nod= e > >>> as well as the SUT. It also moves the driver binding step into the No= de > >>> class and triggers the same pattern of binding to ports that existed = on > >>> the SUT on the TG. > >>> > >> > >> This is a very inefficient way to do this. We'll have to build DPDK > >> twice and that's very time consuming. I was thinking in terms of just > > > > This patch shouldn't be compiling DPDK twice, are you referring to the > > process of copying the tarball over and extracting it taking too long? > > If so, that makes sense that it takes longer than we need for this one > > task. I figured it wouldn't hurt to have the whole DPDK directory > > there, and that it could even be potentially useful to have it if the > > TG ever needed it. That and it seemed like the most straightforward > > way that kept these two set up in a similar way. Extracting the > > tarball is obviously pretty quick, so I guess the real question here > > is whether it is fine to add the time of one extra SCP of the DPDK > > tarball around. > > > > Ah, I didn't look carefully at the split. This is fine, but there some > things I noticed. > > As Patrick mentioned, the docstrings in Node.set_up_build_target() and > SutNode.set_up_build_target() would need to be updated. > Why are we binding ports on the TG node? I figured that the assumption would be that whatever is in the config file is what the TG needs to be bound to in order to run the testing, similarly to how we always bind on the SUT assuming that we need to be using the DPDK driver to test DPDK. > This shouldn't really be part of set_up_build_target; set_up_test_run is > a better place to put this, as we don't need to copy it for each build > target. And, as I realized then thinking about the property (down > below), we don't need to do that even per test_run; once per TG node's > lifetime is enough. That makes sense to me actually considering the traffic generator being used cannot change throughout the node's lifetime. I will make that change. > > >> copying the script to the TG node and storing its location on the TG > >> node. We should have access to the script whether DTS is run from the > >> repository or a tarball. > > > > We should have access to it regardless, but extracting only that one > > script would be different based on if it was a tarball or a repository > > since, I believe at least, I would have to use the tarfile library to > > read and extract only this one file to copy over if it was a tarball. > > It would be faster I assume, so if you think it is worth it I could > > make the change. Unless you are saying that we wouldn't need to take > > the devbind script from the tarball that is passed into the DTS run > > and instead assume that we can just go one directory up from `dts/` on > > the runner host. That could be an interesting idea which would be > > faster, but I wasn't sure if that was something that was fine to do > > since (I don't think at least) there is anything that technically ties > > you to running from in a DPDK directory other than the docker > > container. > > You can run DTS from any directory, but currently DTS it's always going > to be in a DPDK tree (there's no other way to get DTS), so I think it's > safe to assume the script is there. We can put a variable pointing to > dpdk_root into utils.py and use that. > Fair enough, I don't see why it would be run outside of the DPDK directory in most cases. There is one situation where it could happen, which is the runner target for the Docker container copies only the DTS directory into the container when it "bakes DTS in." It does so because it really only needs the .toml file for the poetry install and, realistically, it seems like you should normally be mounting your local DPDK directory over the DTS directory, but maybe that isn't super clear. Out of scope for this patch, but just something to note. > My idea was copying that one file, nothing else (no tarball or anything > would be needed). > I think we'd only need to move _remote_tmp_dir and > _path_to_devbind_script to Node and then implement set_up_test_run() on > the TG node to copy just the script (with self.main_session.copy_to()) > and set _path_to_devbind_script. And I guess set _path_to_devbind_script > in SutNode.tear_down_build_target() and TGNode.tear_down_test_run() > since those seems to be missing. > > But there's actually one more thing we could improve on top of that. > _path_to_devbind_script could be a property which would be used the same > way in SutNode, but in TGNode, we could copy the script only if it's > None and set it back to None only when closing the Node (as we need to > copy the script only once per TG node lifespan). Sure, that makes sense to me. I'll make that change.