From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
by inbox.dpdk.org (Postfix) with ESMTP id DAA28455D5;
Wed, 10 Jul 2024 18:44:37 +0200 (CEST)
Received: from mails.dpdk.org (localhost [127.0.0.1])
by mails.dpdk.org (Postfix) with ESMTP id C882B40E0A;
Wed, 10 Jul 2024 18:44:37 +0200 (CEST)
Received: from mail-pf1-f170.google.com (mail-pf1-f170.google.com
[209.85.210.170])
by mails.dpdk.org (Postfix) with ESMTP id 97151402C9
for <dev@dpdk.org>; Wed, 10 Jul 2024 18:44:36 +0200 (CEST)
Received: by mail-pf1-f170.google.com with SMTP id
d2e1a72fcca58-70af22a9c19so4789958b3a.2
for <dev@dpdk.org>; Wed, 10 Jul 2024 09:44:36 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=iol.unh.edu; s=unh-iol; t=1720629875; x=1721234675; 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=UvnqtdbTJigtUKw15aGOGrRJ1FJvi7WoTe1/XamMEUo=;
b=SzkM6iF/zVG5lj8giBiOm9cgluADsw/4I70G4hRzhhmGp288uALkkreJCr6Kquzyoh
7ps2t3WRY92cKYEvDvrr5tihbIDyYb68c7oZ6CLcOBsQtMeKlC1iEYwvH41WqYJhJl/9
ZXj0rB2zmqngipXZmQAFoMJK7hNEn+x1Px5dc=
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20230601; t=1720629875; x=1721234675;
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=UvnqtdbTJigtUKw15aGOGrRJ1FJvi7WoTe1/XamMEUo=;
b=pI7GMQBbXaqvtWMlFPXIdgfgxHjHesPJbwhnLE3anrj3je+HjvPTSmvg11AsuMxI1z
+k9fwth42qWEZYX/995wSIPpd5xeDbNiPEgCB9kAbXvM0iXKmD6sj1J2NXgtpCGLVHHv
1y3aS6FgopV3IKoziBEdpzry0vLhoFCrRKiTLaGZl0awg9IZJ5egiddFC1OJUwcCPuFY
cocV+fd2whmDVPyxRbJ5gWrvsMg8dyByg2W5f4Ai3wQyVOO9UMkF5u23yCCwqkJHdvJm
JgkRh6Et1iM5gSeNsUrHZdA+7wdC1vsAzlaiVl5oocSWihZxgba7wBvsqIc2DnuLU3ee
pk1g==
X-Forwarded-Encrypted: i=1;
AJvYcCUm1OtlsMt2X+QJo0Vn0gUs5r+Un2x06w3I2IdPG4QP+WUj2xYGsANw7LQe6yVlTvd5xfVQDr2KK4di2XQ=
X-Gm-Message-State: AOJu0Ywk1F4e9KQ3uaVCJW5KAJwtTcukH5c7anGcBctxRq6F8jIlR06V
UQa4oK3mICkVUFbEkBxJXm64ACpk0s7a9lxvxujemlexBi3IdC4ggmHh4kIo6xWGxdyaIsgQZgy
zQZxE7fZ23Cfk8c+XqCY0NLdCEpfr4feQhHftDQ==
X-Google-Smtp-Source: AGHT+IFg/nDjR0z+/OCXSOSnvuVb1YIcZPDP3uYPQ5kTBdaCIgC3TEF+EejXooIJnUWzm4JG3j34mJi0LYWjH/7PmWI=
X-Received: by 2002:a05:6a00:1ca2:b0:70b:17df:cbc1 with SMTP id
d2e1a72fcca58-70b4361b82amr6572944b3a.30.1720629875529; Wed, 10 Jul 2024
09:44:35 -0700 (PDT)
MIME-Version: 1.0
References: <20240708191938.32132-1-dmarx@iol.unh.edu>
<20240708191938.32132-2-dmarx@iol.unh.edu>
In-Reply-To: <20240708191938.32132-2-dmarx@iol.unh.edu>
From: Jeremy Spewock <jspewock@iol.unh.edu>
Date: Wed, 10 Jul 2024 12:44:23 -0400
Message-ID: <CAAA20UQgrMC79J49unvwy8iGUM+UQpuSdd0m36rrUen-tDju9g@mail.gmail.com>
Subject: Re: [PATCH v2 1/4] dts: add multicast set function to shell
To: Dean Marx <dmarx@iol.unh.edu>
Cc: Honnappa.Nagarahalli@arm.com, juraj.linkes@pantheon.tech,
probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com,
bruce.richardson@intel.com, luca.vizzarro@arm.com, 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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
On Mon, Jul 8, 2024 at 3:19=E2=80=AFPM Dean Marx <dmarx@iol.unh.edu> wrote:
>
> added set multicast function for changing allmulticast mode within testpm=
d.
>
> Signed-off-by: Dean Marx <dmarx@iol.unh.edu>
> ---
> dts/framework/remote_session/testpmd_shell.py | 46 +++++++++++++++++++
> 1 file changed, 46 insertions(+)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor=
k/remote_session/testpmd_shell.py
> index ec22f72221..a0be0bd09d 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -806,6 +806,52 @@ def show_port_stats(self, port_id: int) -> TestPmdPo=
rtStats:
>
> return TestPmdPortStats.parse(output)
>
> + def set_promisc(self, port: int, on: bool, verify: bool =3D True):
This public method seems to be missing the return type, it should
probably specify that it doesn't return anything with a None-type.
> + """Turns promiscuous mode on/off for the specified port.
> +
> + Args:
> + port: Port number to use, should be within 0-32.
> + on: If :data:`True`, turn promisc mode on, otherwise turn of=
f.
> + verify: If :data:`True` an additional command will be sent t=
o verify that promisc mode
> + is properly set. Defaults to :data:`True`.
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True=
` and promisc mode
> + is not correctly set.
> + """
> + promisc_output =3D self.send_command(f"set promisc {port} {'on' =
if on else 'off'}")
> + if verify:
> + stats =3D self.show_port_info(port_id=3Dport)
> + if on ^ stats.is_promiscuous_mode_enabled:
> + self._logger.debug(f"Failed to set promisc mode on port =
{port}: \n{promisc_output}")
> + raise InteractiveCommandExecutionError(
> + f"Testpmd failed to set promisc mode on port {port}.=
"
> + )
> +
> + def set_multicast_all(self, on: bool, verify: bool =3D True):
This method also should probably have a typehint for the return type.
> + """Turns multicast mode on/off for the specified port.
> +
> + Args:
> + on: If :data:`True`, turns multicast mode on, otherwise turn=
s off.
> + verify: If :data:`True` an additional command will be sent t=
o verify
> + that multicast mode is properly set. Defaults to :data:`True=
`.
I'm surprised the formatting script doesn't complain about a
mismatched indent here, I thought it normally did for doc-strings. The
second line of this verify argument should probably be indented just
to better show that it is a continuation of the previous line.
> +
> + Raises:
> + InteractiveCommandExecutionError: If `verify` is :data:`True=
` and multicast
> + mode is not properly set.
> + """
> + multicast_output =3D self.send_command(f"set allmulti all {'on' =
if on else 'off'}")
> + if verify:
> + stats0 =3D self.show_port_info(port_id=3D0)
> + stats1 =3D self.show_port_info(port_id=3D1)
Getting the stats for port 0 and port 1 limit this method to only
working when there are exactly 2 ports. Right now in DTS this is a
restriction, but we should be able to make this robust enough to work
with any number of ports using self.show_port_info_all().
> + if on ^ (stats0.is_allmulticast_mode_enabled and stats1.is_a=
llmulticast_mode_enabled):
If you used the show port info all command, you could then just
replace this `and` statement with something like this:
all(info.is_allmulticast_mode_enabled for info in all_stats)
if `all_stats` was the name of the list you stored the port stats in.
> + self._logger.debug(
> + f"Failed to set multicast mode on all ports.: \n{mul=
ticast_output}"
> + )
> + raise InteractiveCommandExecutionError(
> + "Testpmd failed to set multicast mode on all ports."
> + )
> +
> def close(self) -> None:
> """Overrides :meth:`~.interactive_shell.close`."""
> self.send_command("quit", "")
> --
> 2.44.0
>