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 E49CD43866; Mon, 8 Jan 2024 17:37:58 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id CF0B140696; Mon, 8 Jan 2024 17:37:58 +0100 (CET) Received: from mail-pj1-f44.google.com (mail-pj1-f44.google.com [209.85.216.44]) by mails.dpdk.org (Postfix) with ESMTP id 8EBCE4064E for ; Mon, 8 Jan 2024 17:37:57 +0100 (CET) Received: by mail-pj1-f44.google.com with SMTP id 98e67ed59e1d1-28cba988d6bso833266a91.2 for ; Mon, 08 Jan 2024 08:37:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1704731877; x=1705336677; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=3/RZi/+edHtDJ8gK3W0WG2r3lmxqR/2iuBS5lJd2G5k=; b=A/1As7nl/2skIKUF9gN40pEy6xh7ZVhP2fFn4rrn99UhSCJsTQISUkRHu0Awyub6Rp TQoBRRHf4Og+j/8yn8kSrQO5vKcK7kHI0IYGRIGlz7kVP0nugNwmILnECvqzQqrBYDcb O7vjiaJOq1ca3zww6/ikCF50FcBwsy8J3I/TY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704731877; x=1705336677; h=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=3/RZi/+edHtDJ8gK3W0WG2r3lmxqR/2iuBS5lJd2G5k=; b=rcZAw/qjlc+V6oFzLWJjICp9k146r0qZFn2NL06fyIX+XIBObdQXvnBq8/YhPv/R6P P9n90hEhFjbFss7P971VSBtIleC2w1sfaPy8VZaOFJVBaqlM4cpTzXJgvTvx0JOczzFE seyzBYHY0Vusm2Q3mu2l+KzkirDkBWWxsC36GvPBpQ3KLAgq0lZ2DAvOxmkD/howOMCu ZzM3ZJHzM49Ojyk3KI2zq+IMTa6BjMVt8UMjKbdhSh9KK489zc2BJxlLjGyibWHVon8L cZ8wG3kLujpJtnJr1pFVLyOrRIhVofTL0wubXbPFd8U/OMWrjipPzOpXdcz8ifPXWptR 7Drw== X-Gm-Message-State: AOJu0YzZOqBZBSuu/+bIh1p3+bxmTZxmH6sKIQxCLKwaIwldc97KJt4K zTXlr9C9B9XWzCHcYDckb7czYl2W1SaoJtXl/l7MfbgIMmo1jA== X-Google-Smtp-Source: AGHT+IG49OF7qxHiE999eXKYBRi5mVwxTBHiaRi90/4l2NO3yomb/4tGD0t3mvb1+FqM1Blz8deuhfLUy3iWWaRW7nk= X-Received: by 2002:a17:90b:24d:b0:28b:ee54:1051 with SMTP id fz13-20020a17090b024d00b0028bee541051mr1286778pjb.81.1704731876743; Mon, 08 Jan 2024 08:37:56 -0800 (PST) MIME-Version: 1.0 References: <20240103221217.18954-1-jspewock@iol.unh.edu> <20240103223206.23129-1-jspewock@iol.unh.edu> <20240103223206.23129-3-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Mon, 8 Jan 2024 11:37:45 -0500 Message-ID: Subject: Re: [PATCH v6 2/7] dts: limit EAL parameters to DPDK apps and add parameters to all apps To: =?UTF-8?Q?Juraj_Linke=C5=A1?= 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, lylavoie@iol.unh.edu, ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, dev@dpdk.org Content-Type: multipart/alternative; boundary="000000000000f3708e060e71d1f4" 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 --000000000000f3708e060e71d1f4 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jan 8, 2024 at 6:52=E2=80=AFAM Juraj Linke=C5=A1 wrote: > On Wed, Jan 3, 2024 at 11:33=E2=80=AFPM wrote: > > > > From: Jeremy Spewock > > > > Changed the factory method for creating interactive apps in the SUT Nod= e > > so that EAL parameters would only be passed into DPDK apps since > > non-DPDK apps wouldn't be able to process them. Also modified > > interactive apps to allow for the ability to pass parameters into the > > app on startup so that the applications can be started with certain > > configuration steps passed on the command line. > > > > Signed-off-by: Jeremy Spewock > > --- > > > > I ended up reverting part of this back to making the argument for > > eal_parameters allowed to be a string. This was because it was casuing > > mypy errors where the method signatures of sut_node did not match with > > that of node. > > > > This is because the signatures don't actually match :-) > > The eal_parameters parameter is added on not top of what's in the base > methods. I suggest we move eal_parameters to the end and then we don't > need to allow str for eal_parameters. > > > dts/framework/remote_session/testpmd_shell.py | 2 +- > > dts/framework/testbed_model/sut_node.py | 14 +++++++++----- > > 2 files changed, 10 insertions(+), 6 deletions(-) > > > > diff --git a/dts/framework/remote_session/testpmd_shell.py > b/dts/framework/remote_session/testpmd_shell.py > > index f310705fac..8f40e8f40e 100644 > > --- a/dts/framework/remote_session/testpmd_shell.py > > +++ b/dts/framework/remote_session/testpmd_shell.py > > @@ -118,7 +118,7 @@ def _start_application(self, get_privileged_command= : > Callable[[str], str] | None > > Also find the number of pci addresses which were allowed on th= e > command line when the app > > was started. > > """ > > - self._app_args +=3D " -- -i --mask-event intr_lsc" > > + self._app_args +=3D " -i --mask-event intr_lsc" > > self.number_of_ports =3D self._app_args.count("-a ") > > super()._start_application(get_privileged_command) > > > > diff --git a/dts/framework/testbed_model/sut_node.py > b/dts/framework/testbed_model/sut_node.py > > index c4acea38d1..4df18bc183 100644 > > --- a/dts/framework/testbed_model/sut_node.py > > +++ b/dts/framework/testbed_model/sut_node.py > > @@ -431,6 +431,7 @@ def create_interactive_shell( > > timeout: float =3D SETTINGS.timeout, > > privileged: bool =3D False, > > eal_parameters: EalParameters | str | None =3D None, > > + app_parameters: str =3D "", > > What I meant above is we should move app_parameters before > eal_parameters and then we can remove the str type of eal_parameters. > Sounds good to me, I'll move these around in the next version. > > > ) -> InteractiveShellType: > > """Extend the factory for interactive session handlers. > > > > @@ -449,20 +450,23 @@ def create_interactive_shell( > > eal_parameters: List of EAL parameters to use to launch th= e > app. If this > > isn't provided or an empty string is passed, it will > default to calling > > :meth:`create_eal_parameters`. > > + app_parameters: Additional arguments to pass into the > application on the > > + command-line. > > > > Returns: > > An instance of the desired interactive application shell. > > """ > > - if not eal_parameters: > > - eal_parameters =3D self.create_eal_parameters() > > - > > - # We need to append the build directory for DPDK apps > > + # We need to append the build directory and add EAL parameters > for DPDK apps > > if shell_cls.dpdk_app: > > + if not eal_parameters: > > + eal_parameters =3D self.create_eal_parameters() > > + app_parameters =3D f"{eal_parameters} -- {app_parameters}" > > + > > shell_cls.path =3D self.main_session.join_remote_path( > > self.remote_dpdk_build_dir, shell_cls.path > > ) > > > > - return super().create_interactive_shell(shell_cls, timeout, > privileged, str(eal_parameters)) > > + return super().create_interactive_shell(shell_cls, timeout, > privileged, app_parameters) > > > > def bind_ports_to_driver(self, for_dpdk: bool =3D True) -> None: > > """Bind all ports on the SUT to a driver. > > -- > > 2.43.0 > > > --000000000000f3708e060e71d1f4 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


<= div dir=3D"ltr" class=3D"gmail_attr">On Mon, Jan 8, 2024 at 6:52=E2=80=AFAM= Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech> wrote:
On Wed, Jan 3, 2024 at 11:33= =E2=80=AFPM <j= spewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Changed the factory method for creating interactive apps in the SUT No= de
> so that EAL parameters would only be passed into DPDK apps since
> non-DPDK apps wouldn't be able to process them. Also modified
> interactive apps to allow for the ability to pass parameters into the<= br> > app on startup so that the applications can be started with certain > configuration steps passed on the command line.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>
> I ended up reverting part of this back to making the argument for
> eal_parameters allowed to be a string. This was because it was casuing=
> mypy errors where the method signatures of sut_node did not match with=
> that of node.
>

This is because the signatures don't actually match :-)

The eal_parameters parameter is added on not top of what's in the base<= br> methods. I suggest we move eal_parameters to the end and then we don't<= br> need to allow str for eal_parameters.

>=C2=A0 dts/framework/remote_session/testpmd_shell.py |=C2=A0 2 +-
>=C2=A0 dts/framework/testbed_model/sut_node.py=C2=A0 =C2=A0 =C2=A0 =C2= =A0| 14 +++++++++-----
>=C2=A0 2 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/frame= work/remote_session/testpmd_shell.py
> index f310705fac..8f40e8f40e 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -118,7 +118,7 @@ def _start_application(self, get_privileged_comman= d: Callable[[str], str] | None
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Also find the number of pci addresse= s which were allowed on the command line when the app
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 was started.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._app_args +=3D " -- -i --mask-e= vent intr_lsc"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._app_args +=3D " -i --mask-even= t intr_lsc"
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.number_of_ports =3D self._app_a= rgs.count("-a ")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 super()._start_application(get_privi= leged_command)
>
> diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/t= estbed_model/sut_node.py
> index c4acea38d1..4df18bc183 100644
> --- a/dts/framework/testbed_model/sut_node.py
> +++ b/dts/framework/testbed_model/sut_node.py
> @@ -431,6 +431,7 @@ def create_interactive_shell(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timeout: float =3D SETTINGS.timeout,=
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 privileged: bool =3D False,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 eal_parameters: EalParameters | str = | None =3D None,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 app_parameters: str =3D "",

What I meant above is we should move app_parameters before
eal_parameters and then we can remove the str type of eal_parameters.

Sounds good to me, I'll move these around in th= e next version.
=C2=A0

>=C2=A0 =C2=A0 =C2=A0 ) -> InteractiveShellType:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """Extend the factory= for interactive session handlers.
>
> @@ -449,20 +450,23 @@ def create_interactive_shell(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 eal_parameters: List o= f EAL parameters to use to launch the app. If this
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 isn'= t provided or an empty string is passed, it will default to calling
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 :meth:`c= reate_eal_parameters`.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 app_parameters: Additional = arguments to pass into the application on the
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 command-line.=
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Returns:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 An instance of the des= ired interactive application shell.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if not eal_parameters:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 eal_parameters =3D self.cre= ate_eal_parameters()
> -
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 # We need to append the build directory f= or DPDK apps
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 # We need to append the build directory a= nd add EAL parameters for DPDK apps
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if shell_cls.dpdk_app:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if not eal_parameters:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 eal_parameter= s =3D self.create_eal_parameters()
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 app_parameters =3D f"{= eal_parameters} -- {app_parameters}"
> +
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 shell_cls.path =3D sel= f.main_session.join_remote_path(
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.rem= ote_dpdk_build_dir, shell_cls.path
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
>
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 return super().create_interactive_shell(s= hell_cls, timeout, privileged, str(eal_parameters))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return super().create_interactive_shell(s= hell_cls, timeout, privileged, app_parameters)
>
>=C2=A0 =C2=A0 =C2=A0 def bind_ports_to_driver(self, for_dpdk: bool =3D = True) -> None:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """Bind all ports on = the SUT to a driver.
> --
> 2.43.0
>
--000000000000f3708e060e71d1f4--