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 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 ; Wed, 10 Jul 2024 18:44:36 +0200 (CEST) Received: by mail-pf1-f170.google.com with SMTP id d2e1a72fcca58-70af22a9c19so4789958b3a.2 for ; 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 Date: Wed, 10 Jul 2024 12:44:23 -0400 Message-ID: Subject: Re: [PATCH v2 1/4] dts: add multicast set function to shell To: Dean Marx 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Mon, Jul 8, 2024 at 3:19=E2=80=AFPM Dean Marx wrote: > > added set multicast function for changing allmulticast mode within testpm= d. > > Signed-off-by: Dean Marx > --- > 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 >