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 973EB4337F; Mon, 20 Nov 2023 15:35:31 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 15E8A42DDE; Mon, 20 Nov 2023 15:35:31 +0100 (CET) Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) by mails.dpdk.org (Postfix) with ESMTP id 7068342DD2 for ; Mon, 20 Nov 2023 15:35:29 +0100 (CET) Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-548696eac92so3113080a12.3 for ; Mon, 20 Nov 2023 06:35:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700490929; x=1701095729; 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=T8Hj8SF8D8OJHIcmbscD4imRb/a6gVhuBGT8UQlOW/k=; b=ogrVVOzgWxHHR/65qDTco7sExyRMe0/xIf/G8RyA8F62ZKDd7SogRHX3b9Sm7FMLmx CFS97HyKibmBLI11+DLrzs/l1FNTUQM/P+1SHVikRMU6xhVvMk8WuBQueEu9/AXDZPdB EePSqixXrqzthjrHAuHK2zqbN6Rr75eUXPwNJvijPoOshL0fO63r17AKbCl1mx48O9m2 ckNmP3PKFCqEoqrCVP2jw37mjMBjHJqXUwafxvtzQ8+7goHrjm549WePTh3KQ4JlTnHe VGSl8WyU5TkUQjRWWquKicvjc1vECg4EQmhUuzT1p7L0mKebbKS2Rt8g+p3C0/Z2CeqV xAsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700490929; x=1701095729; 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=T8Hj8SF8D8OJHIcmbscD4imRb/a6gVhuBGT8UQlOW/k=; b=Y7pd+kx89N0Af5BD5+5r0hzHNyBXj/t5263ItQ7aBv5MCOOyJm6G/+HE0lRUY9j3w6 qRsi/aR/LU3vuvfwWc0um6z6BG6IO9eonxadKMUmkc5fNsDX0sqex8U4sJUFz4fxit5H H760jcnOSaKc5ZY6jQKUXOqVxTLDPZ92YwuGZGiXiSygzo2P36NS5qf+8zdq/b2daCTj QlYXb/WFe/1XV2qmVJ6Cl+PZqX8hTRGQJd1t2YW2Z6SMDP2kX1TZ0/oBfCJU6sBzPDkM w0jPIwXJoA0pKt5zOcBnnkfY6L0wsudU5uM3Rr9vnpEwQdjW5YtJoLfq3AmWQ3j0JAOz 8Drw== X-Gm-Message-State: AOJu0Yw3gNIYelHJW5L21ZJ+xP9IP+vhNx76doTIxHUlTAImzK9OhuCF +YwSRvyLrsY38pLcei6M5NtEVYPGni04m9yex7BQ2Q== X-Google-Smtp-Source: AGHT+IEj2P1uYTyPVx8Uhk5AvrhSPlIpDUq3fN0xRrF44AmhTIc+cpwWybWQARbMKNIqv+KMNCgz+jKIE3OlMWT1byc= X-Received: by 2002:a17:906:11:b0:9bd:a75a:5644 with SMTP id 17-20020a170906001100b009bda75a5644mr6293433eja.16.1700490929013; Mon, 20 Nov 2023 06:35:29 -0800 (PST) MIME-Version: 1.0 References: <20231113202833.12900-1-jspewock@iol.unh.edu> <20231113202833.12900-5-jspewock@iol.unh.edu> In-Reply-To: From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Mon, 20 Nov 2023 15:35:18 +0100 Message-ID: Subject: Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps To: Jeremy Spewock Cc: Honnappa.Nagarahalli@arm.com, thomas@monjalon.net, wathsala.vithanage@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, 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 Fri, Nov 17, 2023 at 7:08=E2=80=AFPM Jeremy Spewock wrote: > > > > On Thu, Nov 16, 2023 at 1:53=E2=80=AFPM Juraj Linke=C5=A1 wrote: >> >> On Mon, Nov 13, 2023 at 9:28=E2=80=AFPM wrote: >> > >> > From: Jeremy Spewock >> > >> > Modified interactive applications to allow for the ability to pass >> > parameters into the app on start up. Also modified the way EAL >> > parameters are handled so that the trailing "--" separator is added be >> > default after all EAL parameters. >> > >> > Signed-off-by: Jeremy Spewock >> > --- >> > dts/framework/remote_session/remote/testpmd_shell.py | 2 +- >> > dts/framework/testbed_model/sut_node.py | 11 +++++++---- >> > 2 files changed, 8 insertions(+), 5 deletions(-) >> > >> > diff --git a/dts/framework/remote_session/remote/testpmd_shell.py b/dt= s/framework/remote_session/remote/testpmd_shell.py >> > index 3ea16c7ab3..3f6a86aa35 100644 >> > --- a/dts/framework/remote_session/remote/testpmd_shell.py >> > +++ b/dts/framework/remote_session/remote/testpmd_shell.py >> > @@ -32,7 +32,7 @@ def _start_application( >> > self, get_privileged_command: Callable[[str], str] | None >> > ) -> None: >> > """See "_start_application" in InteractiveShell.""" >> > - self._app_args +=3D " -- -i" >> > + self._app_args +=3D " -i" >> > super()._start_application(get_privileged_command) >> > >> > def get_devices(self) -> list[TestPmdDevice]: >> > diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/t= estbed_model/sut_node.py >> > index 4161d3a4d5..bcac939e72 100644 >> > --- a/dts/framework/testbed_model/sut_node.py >> > +++ b/dts/framework/testbed_model/sut_node.py >> > @@ -377,7 +377,8 @@ def create_interactive_shell( >> > shell_cls: Type[InteractiveShellType], >> > timeout: float =3D SETTINGS.timeout, >> > privileged: bool =3D False, >> > - eal_parameters: EalParameters | str | None =3D None, >> > + eal_parameters: EalParameters | str =3D "", >> >> I think it makes more sense if the type is EalParameters | None. With >> this change, the str type of eal_parameters moved to app_parameters. >> >> > + app_parameters: str =3D "", >> > ) -> InteractiveShellType: >> > """Factory method for creating a handler for an interactive s= ession. >> > >> > @@ -392,12 +393,14 @@ def create_interactive_shell( >> > eal_parameters: List of EAL parameters to use to launch t= he app. If this >> > isn't provided or an empty string is passed, it will = default to calling >> > create_eal_parameters(). >> > + app_parameters: Additional arguments to pass into the app= lication on the >> > + command-line. >> > Returns: >> > Instance of the desired interactive application. >> > """ >> > - if not eal_parameters: >> > + if not eal_parameters and shell_cls.dpdk_app: >> > eal_parameters =3D self.create_eal_parameters() >> > - >> > + eal_parameters =3D f"{eal_parameters} --" >> >> I think this change is more complicated than it seems at first glance. >> >> Before we either passed EalParameters (meant for DPDK apps) or a >> string (meant for other apps). There was no additional check for these >> assumptions. >> Now that we've split it, I see some cases which seem to be not covered. >> >> 1. The app is a DPDK app and we pass EalParameters. The two hyphens >> are not added. >> 2. The app is not a DPDK app and we pass EalParameters. Similarly to >> current behavior (without the patch), we kinda assume that the caller >> won't do this, but now that we're modifying the behavior, let's add a >> check that won't allow EalParameters with non-DPDK apps. >> > > That is a good point that not all DPDk apps need the two hyphens, I can m= ake that just specific to testpmd I don't know whether all DPDK apps need EAL parameters (I thought they needed them). My point is about the condition: the two hyphens are only added for DPDK apps when *default* EAL parameters are used. When non-default EAL parameters are passed (i.e. when eal_parameters =3D=3D True), the hyphens are not added for DPDK apps. > and change it instead so that we don't pass EalParameters into DPDK appli= cations and I think that should cover these cases. > I think you meant non-DPDK applications here, right? > >> >> > # We need to append the build directory for DPDK apps >> > if shell_cls.dpdk_app: >> > shell_cls.path =3D self.main_session.join_remote_path( >> > @@ -405,7 +408,7 @@ def create_interactive_shell( >> > ) >> > >> > return super().create_interactive_shell( >> > - shell_cls, timeout, privileged, str(eal_parameters) >> > + shell_cls, timeout, privileged, f"{eal_parameters} {app_p= arameters}" >> > ) >> > >> > def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None: >> > -- >> > 2.42.0 >> >