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 5CB9E43347; Thu, 16 Nov 2023 19:53:06 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4DBAC402E0; Thu, 16 Nov 2023 19:53:06 +0100 (CET) Received: from mail-ej1-f53.google.com (mail-ej1-f53.google.com [209.85.218.53]) by mails.dpdk.org (Postfix) with ESMTP id E116B402CB for ; Thu, 16 Nov 2023 19:53:04 +0100 (CET) Received: by mail-ej1-f53.google.com with SMTP id a640c23a62f3a-9db6cf8309cso162288266b.0 for ; Thu, 16 Nov 2023 10:53:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1700160784; x=1700765584; 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=lJvCI2bc3wTJeCUSlpLYJWbLXZgAwtqwpzEv/Xgzk20=; b=Gw4qFQj5NiOIa0B3H1H2Gs3h4HNozJI2ic1d3vyPq2a97s8n2SkPxIM06aXJjrcjle R2/cQeJG/hLzd97NlDQVt880ltuMFB0xiaHm/xOsf72AxTSOYHYDn9FNJPREUeQAi5VU wM3Aoy/YMyhKIhvJepFuAkZBCNLRDf3/z1vJzd3J45xdym3T+BOsvp90K1OhIYAMIl9l nWF7A0gJ8iGWfzEvJne1T+RkAKHRzmXKETeQit41YhrqRRPZL8EopuseldeeJ25LH+Rd nDnJOhE4ewqaYkwX2oytodgAgvUs6hANoBJfVZUWB+z4VsGjqzzWOYbT5bhB41XvzURU dItg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700160784; x=1700765584; 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=lJvCI2bc3wTJeCUSlpLYJWbLXZgAwtqwpzEv/Xgzk20=; b=TomLQw55pKq7lW7BcaF6Qvx9W2L3zndOt+eQ15f4yGG4HOgKQnCtgcfxw22I6g9dYB MCeP+sFXAJGui2TFTpDm/XoOXIeClk8H6l7J3TeCZXrfHpmedRDngpcBp0u8hATbeVxF /o7sLJBl1WktYomV2qpm4cqm8yaKEfDFL7Iw1e+mObxgScwNjKrrJO2tjDcNeqGXAs7u RU8Mzdrqpd6MucSdw4+DA094dmhVPxHI0m+gxkQgPRFnGuQw0Et56AXAj+JpfmbmGNol Nf9b75BiXK2So41Zx3bQY+HRIRc0hu9MfNLs6mJ1EeEKVKj8dCd07Z3sf2Yji4EBNNl/ A4zA== X-Gm-Message-State: AOJu0Yx6560Sd+tziJ8XBegor/1NWe5W5tdnMgAJnK9Ow3KZCOIL8Pmv GmVKCdaSF1GkVIrDLNzrG7e+DOsMhk/xc7FrXm/Qlg== X-Google-Smtp-Source: AGHT+IGNqZOOshReLkz0lqcOk6apgNbTtjPRqcvqJ+frJjRLYnhbX3OGT1a1IzwaYi/SarxZ6LYU+mIXNhrj3u+W4Tw= X-Received: by 2002:a17:907:76b0:b0:9e3:fb4d:dbe1 with SMTP id jw16-20020a17090776b000b009e3fb4ddbe1mr10330798ejc.44.1700160783679; Thu, 16 Nov 2023 10:53:03 -0800 (PST) MIME-Version: 1.0 References: <20231113202833.12900-1-jspewock@iol.unh.edu> <20231113202833.12900-5-jspewock@iol.unh.edu> In-Reply-To: <20231113202833.12900-5-jspewock@iol.unh.edu> From: =?UTF-8?Q?Juraj_Linke=C5=A1?= Date: Thu, 16 Nov 2023 19:52:52 +0100 Message-ID: Subject: Re: [PATCH v3 4/7] dts: allow passing parameters into interactive apps To: jspewock@iol.unh.edu 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 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/dts/f= ramework/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/test= bed_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 sess= ion. > > @@ -392,12 +393,14 @@ def create_interactive_shell( > eal_parameters: List of EAL parameters to use to launch the = app. If this > isn't provided or an empty string is passed, it will def= ault to calling > create_eal_parameters(). > + app_parameters: Additional arguments to pass into the applic= ation 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. > # 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_para= meters}" > ) > > def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None: > -- > 2.42.0 >