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 989FD43866; Mon, 8 Jan 2024 17:36:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1E2D64064E; Mon, 8 Jan 2024 17:36:26 +0100 (CET) Received: from mail-pf1-f175.google.com (mail-pf1-f175.google.com [209.85.210.175]) by mails.dpdk.org (Postfix) with ESMTP id D9C6040273 for ; Mon, 8 Jan 2024 17:36:24 +0100 (CET) Received: by mail-pf1-f175.google.com with SMTP id d2e1a72fcca58-6d9bbf71bc8so632283b3a.1 for ; Mon, 08 Jan 2024 08:36:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1704731784; x=1705336584; 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=7D9TZ0UJ4GUoeV5R1+ebWloK4wg2W/cBcqAs2PERuYY=; b=iv0rooHrGJiM+Vico57Jeuy2Kl6WAr9J3PE9XMub2YgZDpXqEpyzPfk5IGGQKlNzT0 T6M3lPOoDZ49SfMZ618Dq7dtV9scVpaUSqTluzsqz6W1pdWvzFzwB6KqNuSsIN7Uo7dU gtrMP5g3uEAcamYJqQ6vgopS0RpjI6Apkr4/g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704731784; x=1705336584; 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=7D9TZ0UJ4GUoeV5R1+ebWloK4wg2W/cBcqAs2PERuYY=; b=FUQF1T4aSChAA1sPNx0nBxLvCN3fu4LrIE0F6bP+ktf06clSPRJOqXoKUx0CwUhfMZ 6pEV96UHO5G4R6w4evTPiEtxYOCDF4P83T30IeEFItK6j5tvD84TjvQMk+VXoEoIExHR wIYxlphJWPfxpCCr/h4f/53GgQSYqJxXhltgNZOzgf5UgM1qesEkhuYUz0G0p0Lww+6B YooUDz8w/OvpyBdMZ/D7IN6bD3OokDG2IG6kt7M0IUn3YEXkByr/mtZudfi6WLNYY4d5 KI2oZ8Are6O+4MdJexqVs/wdZ1bV7Y8yPvkDYxVd5r1tWAJTTcvqf3xf75dSEKwMqkKe UjFQ== X-Gm-Message-State: AOJu0YzbBQ5KSYXxLSoQWZAEkLcjxhq3/RbPNsOmhZuFECg8aAy85CwN JP1QiZnkeyDzpHW4daxoEYWYKh1SCDuNOsGU7NC4tNxJJbV+8A== X-Google-Smtp-Source: AGHT+IHtnxw2CJMgfzsOR5oLd5Q+mtAbaX9SMtbLMcZZMlyTvLIAhBJ7KhiSG8n1wu3AoKHyBbDZF9K/s/tN622z3G0= X-Received: by 2002:a05:6a00:4f8e:b0:6da:b303:7f9c with SMTP id ld14-20020a056a004f8e00b006dab3037f9cmr1820095pfb.60.1704731783835; Mon, 08 Jan 2024 08:36:23 -0800 (PST) MIME-Version: 1.0 References: <20240103221217.18954-1-jspewock@iol.unh.edu> <20240103223206.23129-1-jspewock@iol.unh.edu> <20240103223206.23129-2-jspewock@iol.unh.edu> In-Reply-To: From: Jeremy Spewock Date: Mon, 8 Jan 2024 11:36:12 -0500 Message-ID: Subject: Re: [PATCH v6 1/7] dts: add startup verification and forwarding modes to testpmd shell 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="00000000000069c971060e71cca2" 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 --00000000000069c971060e71cca2 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Jan 8, 2024 at 6:35=E2=80=AFAM Juraj Linke=C5=A1 wrote: > On Wed, Jan 3, 2024 at 11:33=E2=80=AFPM wrote: > > > > From: Jeremy Spewock > > > > Added commonly used methods in testpmd such as starting and stopping > > packet forwarding, changing forward modes, and verifying link status of > > ports so that developers can configure testpmd and start forwarding > > through the provided class rather than sending commands to the testpmd > > session directly. > > > > Signed-off-by: Jeremy Spewock > > --- > > dts/framework/exception.py | 7 + > > dts/framework/remote_session/testpmd_shell.py | 149 +++++++++++++++++- > > 2 files changed, 155 insertions(+), 1 deletion(-) > > > > diff --git a/dts/framework/exception.py b/dts/framework/exception.py > > index 658eee2c38..cce1e0231a 100644 > > --- a/dts/framework/exception.py > > +++ b/dts/framework/exception.py > > > @@ -65,9 +108,66 @@ class TestPmdShell(InteractiveShell): > > _command_extra_chars: ClassVar[str] =3D "\n" > > > > def _start_application(self, get_privileged_command: > Callable[[str], str] | None) -> None: > > - self._app_args +=3D " -- -i" > > + """Overrides :meth:`~.interactive_shell._start_application`. > > + > > + Add flags for starting testpmd in interactive mode and > disabling messages for link state > > + change events before starting the application. Link state is > verified before starting > > + packet forwarding and the messages create unexpected newlines > in the terminal which > > + complicates output collection. > > + > > We should adjust the collection so that it can handle the newlines. > Also, can you explain exactly why we are disabling the initial link > state messages? > The problem really comes from the newlines causing the prompt to exist in the buffer before any command is sent. So, what ends up happening is after starting the application these link state change events happen at some point, and they cause an empty "testpmd>" line to exist in the buffer and the next time you send a command it will stop as soon as it encounters that line. An additional issue with this prompt is it is put in the buffer before the link state change event occurs, and there is no prompt that appears after the event messages, just an empty line. This makes it much more difficult to detect when the link state change event occurs and consume it because the event isn't captured the next time you collect output, all that is consumed is a line containing the prompt.. So, this makes you essentially one command's worth of output behind because the next time you send a command you will consume what you were supposed to get from the last command where you stopped early, and this causes false positives for things like the link state detection method and failures in output verification. This puts you in a position where the only way you can really detect that one of these events happened is either assuming that only getting an empty prompt means one of these events happened, or trying to consume output twice and looking ahead to see if one of these events happened. However, because we wouldn't be doing anything with these events and we verify link status before starting anyway, it seemed like the less complex but still functional solution would just be to mask these events. > > > + 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.number_of_ports =3D self._app_args.count("-a ") > > super()._start_application(get_privileged_command) > > > > + def start(self, verify: bool =3D True) -> None: > > + """Start packet forwarding with the current configuration. > > + > > + Args: > > + verify: If :data:`True` , a second start command will be > sent in an attempt to verify > > + packet forwarding started as expected. > > + > > + Raises: > > + InteractiveCommandExecutionError: If `verify` is > :data:`True` and forwarding fails to > > + start or ports fail to come up. > > + """ > > + self.send_command("start") > > + if verify: > > + # If forwarding was already started, sending "start" again > should tell us > > + start_cmd_output =3D self.send_command("start") > > + if "Packet forwarding already started" not in > start_cmd_output: > > + self._logger.debug(f"Failed to start packet forwarding= : > \n{start_cmd_output}") > > + raise InteractiveCommandExecutionError("Testpmd failed > to start packet forwarding.") > > + > > + for port_id in range(self.number_of_ports): > > + if not self.wait_link_status_up(port_id): > > + raise InteractiveCommandExecutionError( > > + "Not all ports came up after starting packet > forwarding in testpmd." > > + ) > > + > > + def stop(self, verify: bool =3D True) -> None: > > + """Stop packet forwarding. > > + > > + Args: > > + verify: If :data:`True` , the output of the stop command i= s > scanned to verify that > > + forwarding was stopped successfully or not started. If > neither is found, it is > > + considered an error. > > + > > + Raises: > > + InteractiveCommandExecutionError: If `verify` is > :data:`True` and the command to stop > > + forwarding results in an error. > > + """ > > + stop_cmd_output =3D self.send_command("stop") > > + if verify: > > + if ( > > + "Done." not in stop_cmd_output > > + and "Packet forwarding not started" not in > stop_cmd_output > > + ): > > I want to make sure I understand this condition. When none of these > appear, it's an error. When just "Done." appears, we successfully > stopped ongoing forwarding and when "Packet forwarding not started" > appears, we're trying to stop forwarding that didn't start (or isn't > ongoing - it could've stopped in the meantime)? > I'm thinking about false failures here (Is there a string that would > indicate a failure even if one of the strings is printed?) - we're > basically looking at "not success" instead of looking for strings > telling us about a failure explicitly. Does the stop command not > produce such output? Or do we not know all of the failure strings or > is looking for the above two strings sufficient to rule out false > failures? > You are correct that essentially what I am looking for here is if we succeeded and else, it's a failure. When I looked through some of the source code for testpmd from the method stop_packet_forwarding, I didn't see any explicit error messages other than displaying that there was an error printing statistics. So this was something where I both didn't know the error messages but it doesn't look like there are any that are explicitly printed. In the case of false failures however, the strings I am detecting are always encountered in our two success cases (forwarding not currently started and successfully stopped). The "Done." message does also get printed in the case of statistics failing to print for a core as well. > > > + self._logger.debug(f"Failed to stop packet forwarding: > \n{stop_cmd_output}") > > + raise InteractiveCommandExecutionError("Testpmd failed > to stop packet forwarding.") > > + > > def get_devices(self) -> list[TestPmdDevice]: > > """Get a list of device names that are known to testpmd. > > > > @@ -82,3 +182,50 @@ def get_devices(self) -> list[TestPmdDevice]: > > if "device name:" in line.lower(): > > dev_list.append(TestPmdDevice(line)) > > return dev_list > > + > > + def wait_link_status_up(self, port_id: int, > timeout=3DSETTINGS.timeout) -> bool: > > + """Wait until the link status on the given port is "up". > > + > > + Arguments: > > + port_id: Port to check the link status on. > > + timeout: Time to wait for the link to come up. The default > value for this > > + argument is set using the :option:`-t, --timeout` > command-line argument > > + or the :envvar:`DTS_TIMEOUT` environment variable. > > + > > This really should be "may be modified", as it is optional. > Good catch, I'll edit this. > > > + Returns: > > + Whether the link came up in time or not. > > + """ > > + time_to_stop =3D time.time() + timeout > > + port_info: str =3D "" > > + while time.time() < time_to_stop: > > + port_info =3D self.send_command(f"show port info {port_id}= ") > > + if "Link status: up" in port_info: > > + break > > + time.sleep(0.5) > > + else: > > + self._logger.error(f"The link for port {port_id} did not > come up in the given timeout.") > > + return "Link status: up" in port_info > > + > > + def set_forward_mode(self, mode: TestPmdForwardingModes, verify: > bool =3D True): > > + """Set packet forwarding mode. > > + > > + Args: > > + mode: The forwarding mode to use. > > + verify: If :data:`True` the output of the command will be > scanned in an attempt to > > + verify that the forwarding mode was set to `mode` > properly. > > + > > + Raises: > > + InteractiveCommandExecutionError: If `verify` is > :data:`True` and forwarding mode fails > > I think there should be a definite article here - the forwarding mode. > Good catch, I'll change this too. > > > + to update. > > + """ > > + set_fwd_output =3D self.send_command(f"set fwd {mode.value}") > > + if f"Set {mode.value} packet forwarding mode" not in > set_fwd_output: > > + self._logger.debug(f"Failed to set fwd mode to > {mode.value}:\n{set_fwd_output}") > > + raise InteractiveCommandExecutionError( > > + f"Test pmd failed to set fwd mode to {mode.value}" > > + ) > > + > > + def close(self) -> None: > > + """Overrides :meth:`~.interactive_shell.close`.""" > > + self.send_command("quit", "") > > + return super().close() > > -- > > 2.43.0 > > > --00000000000069c971060e71cca2 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:35=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>
>
> Added commonly used methods in testpmd such as starting and stopping > packet forwarding, changing forward modes, and verifying link status o= f
> ports so that developers can configure testpmd and start forwarding > through the provided class rather than sending commands to the testpmd=
> session directly.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>=C2=A0 dts/framework/exception.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A07 +
>=C2=A0 dts/framework/remote_session/testpmd_shell.py | 149 ++++++++++++= +++++-
>=C2=A0 2 files changed, 155 insertions(+), 1 deletion(-)
>
> diff --git a/dts/framework/exception.py b/dts/framework/exception.py > index 658eee2c38..cce1e0231a 100644
> --- a/dts/framework/exception.py
> +++ b/dts/framework/exception.py
<snip>
> @@ -65,9 +108,66 @@ class TestPmdShell(InteractiveShell):
>=C2=A0 =C2=A0 =C2=A0 _command_extra_chars: ClassVar[str] =3D "\n&q= uot;
>
>=C2=A0 =C2=A0 =C2=A0 def _start_application(self, get_privileged_comman= d: Callable[[str], str] | None) -> None:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._app_args +=3D " -- -i" > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Overrides :meth:`~.inte= ractive_shell._start_application`.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Add flags for starting testpmd in interac= tive mode and disabling messages for link state
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 change events before starting the applica= tion. Link state is verified before starting
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 packet forwarding and the messages create= unexpected newlines in the terminal which
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 complicates output collection.
> +

We should adjust the collection so that it can handle the newlines.
Also, can you explain exactly why we are disabling the initial link
state messages?

The problem really comes from = the newlines causing the prompt to exist in the buffer before any command i= s sent. So, what ends up happening is after starting the application these = link state change events happen at some point, and they cause an empty &quo= t;testpmd>" line to exist in the buffer and the next time you send = a command it will stop as soon as it encounters that line. An additional is= sue with this prompt is it is put in the buffer before the link state chang= e event occurs, and there is no prompt that appears after the event message= s, just an empty line. This makes it much more difficult to detect when the= link state change event occurs and consume it because the event isn't = captured the next time you collect output, all that is consumed is a line c= ontaining the prompt.. So, this makes you essentially one command's wor= th of output behind because the next time you send a command you will consu= me what you were supposed to get from the last command where you stopped ea= rly, and this causes false positives for things like the link state detecti= on method and failures in output verification.

= This puts you in a position where the only way you can really detect that o= ne of these events happened is either assuming that only getting an empty p= rompt means one of these events happened, or trying to consume output twice= and looking ahead to see if one of these events happened. However, because= we wouldn't be doing anything with these events and we verify link sta= tus before starting anyway, it seemed like the less complex but still funct= ional solution would just be to mask these events.
=C2= =A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Also find the number of pci addresses whi= ch were allowed on the command line when the app
> +=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 self._app_args +=3D " -- -i --mask-e= vent intr_lsc"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.number_of_ports =3D self._app_args.c= ount("-a ")
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 super()._start_application(get_privi= leged_command)
>
> +=C2=A0 =C2=A0 def start(self, verify: bool =3D True) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Start packet forwarding= with the current configuration.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 verify: If :data:`True` , a= second start command will be sent in an attempt to verify
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 packet forwar= ding started as expected.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Raises:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InteractiveCommandExecution= Error: If `verify` is :data:`True` and forwarding fails to
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 start or port= s fail to come up.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.send_command("start")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if verify:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 # If forwarding was already= started, sending "start" again should tell us
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 start_cmd_output =3D self.s= end_command("start")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if "Packet forwarding = already started" not in start_cmd_output:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger.= debug(f"Failed to start packet forwarding: \n{start_cmd_output}")=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise Interac= tiveCommandExecutionError("Testpmd failed to start packet forwarding.&= quot;)
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 for port_id in range(self.n= umber_of_ports):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if not self.w= ait_link_status_up(port_id):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= raise InteractiveCommandExecutionError(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 "Not all ports came up after starting packet forwarding= in testpmd."
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= )
> +
> +=C2=A0 =C2=A0 def stop(self, verify: bool =3D True) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Stop packet forwarding.=
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 verify: If :data:`True` , t= he output of the stop command is scanned to verify that
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 forwarding wa= s stopped successfully or not started. If neither is found, it is
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 considered an= error.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Raises:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InteractiveCommandExecution= Error: If `verify` is :data:`True` and the command to stop
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 forwarding re= sults in an error.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 stop_cmd_output =3D self.send_command(&qu= ot;stop")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if verify:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "Done.&q= uot; not in stop_cmd_output
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 and "Pac= ket forwarding not started" not in stop_cmd_output
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ):

I want to make sure I understand this condition. When none of these
appear, it's an error. When just "Done." appears, we successf= ully
stopped ongoing forwarding and when "Packet forwarding not started&quo= t;
appears, we're trying to stop forwarding that didn't start (or isn&= #39;t
ongoing - it could've stopped in the meantime)?
I'm thinking about false failures here (Is there a string that would indicate a failure even if one of the strings is printed?) - we're
basically looking at "not success" instead of looking for strings=
telling us about a failure explicitly. Does the stop command not
produce such output? Or do we not know all of the failure strings or
is looking for the above two strings sufficient to rule out false
failures?

You are correct that essentially wha= t I am looking for here is if we succeeded and else, it's a failure. Wh= en I looked through some of the source code for testpmd from the method sto= p_packet_forwarding, I didn't see any explicit error messages other tha= n displaying that there was an error printing statistics. So this was somet= hing where I both didn't know the error messages but it doesn't loo= k like there are any that are explicitly printed. In the case of false fail= ures however, the strings I am detecting are always encountered in our two = success cases (forwarding not currently started and successfully stopped). = The "Done." message does also get printed in the case of statisti= cs failing to print for a core as well.
=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger.= debug(f"Failed to stop packet forwarding: \n{stop_cmd_output}") > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise Interac= tiveCommandExecutionError("Testpmd failed to stop packet forwarding.&q= uot;)
> +
>=C2=A0 =C2=A0 =C2=A0 def get_devices(self) -> list[TestPmdDevice]: >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """Get a list of devi= ce names that are known to testpmd.
>
> @@ -82,3 +182,50 @@ def get_devices(self) -> list[TestPmdDevice]: >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if "device name:&= quot; in line.lower():
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 dev_list= .append(TestPmdDevice(line))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return dev_list
> +
> +=C2=A0 =C2=A0 def wait_link_status_up(self, port_id: int, timeout=3DS= ETTINGS.timeout) -> bool:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Wait until the link sta= tus on the given port is "up".
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Arguments:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 port_id: Port to check the = link status on.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timeout: Time to wait for t= he link to come up. The default value for this
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 argument is s= et using the :option:`-t, --timeout` command-line argument
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 or the :envva= r:`DTS_TIMEOUT` environment variable.
> +

This really should be "may be modified", as it is optional.

Good catch, I'll edit this.
=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Returns:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Whether the link came up in= time or not.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 time_to_stop =3D time.time() + timeout > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 port_info: str =3D ""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 while time.time() < time_to_stop:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 port_info =3D self.send_com= mand(f"show port info {port_id}")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if "Link status: up&qu= ot; in port_info:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 break
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 time.sleep(0.5)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 else:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger.error(f"T= he link for port {port_id} did not come up in the given timeout.")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return "Link status: up" in por= t_info
> +
> +=C2=A0 =C2=A0 def set_forward_mode(self, mode: TestPmdForwardingModes= , verify: bool =3D True):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Set packet forwarding m= ode.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 mode: The forwarding mode t= o use.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 verify: If :data:`True` the= output of the command will be scanned in an attempt to
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 verify that t= he forwarding mode was set to `mode` properly.
> +
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Raises:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 InteractiveCommandExecution= Error: If `verify` is :data:`True` and forwarding mode fails

I think there should be a definite article here - the forwarding mode.
<= /blockquote>

Good catch, I'll change this too.
=C2=A0

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 to update. > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 set_fwd_output =3D self.send_command(f&qu= ot;set fwd {mode.value}")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if f"Set {mode.value} packet forward= ing mode" not in set_fwd_output:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger.debug(f"F= ailed to set fwd mode to {mode.value}:\n{set_fwd_output}")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 raise InteractiveCommandExe= cutionError(
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 f"Test p= md failed to set fwd mode to {mode.value}"
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 )
> +
> +=C2=A0 =C2=A0 def close(self) -> None:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Overrides :meth:`~.inte= ractive_shell.close`."""
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.send_command("quit", "= ;")
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return super().close()
> --
> 2.43.0
>
--00000000000069c971060e71cca2--