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 4AB4045966; Thu, 12 Sep 2024 04:17:56 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D8EE74027D; Thu, 12 Sep 2024 04:17:55 +0200 (CEST) Received: from mail-oa1-f52.google.com (mail-oa1-f52.google.com [209.85.160.52]) by mails.dpdk.org (Postfix) with ESMTP id 1810540267 for ; Thu, 12 Sep 2024 04:17:54 +0200 (CEST) Received: by mail-oa1-f52.google.com with SMTP id 586e51a60fabf-277dd761926so280795fac.2 for ; Wed, 11 Sep 2024 19:17:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1726107473; x=1726712273; 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=oYL2YSMqKBpiDIBNLTIIMTtAEawgGuk+ea9CmbGSzS8=; b=HvA571pYDF7FtTATBFKl0fvGIBlj9Fagi5H1Qb34o3DW7XGNuJmmlMjF+roRG54Hbs QTepu8vbzO+eduFJAlaAmp2NJrYKL4x8SyH/juK3NUPYgDTxXKOXO7/C8i4bqNcM0Gzk IZDMED7Alp38AOi2eCa4so5pcj0+gHxo6/TKQ= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1726107473; x=1726712273; 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=oYL2YSMqKBpiDIBNLTIIMTtAEawgGuk+ea9CmbGSzS8=; b=IciOBf9eoHFsKAuMAZ/i4+2N/Xg2bls03kMI5lDfgJAyEN15cxUNGfSEPmrvbcXuA8 KojpiVr2nvXV/sfSGQMy94dzt304xIIpJU1ml4bHcFTHoiBswr533cY4bZ99Rs2vJSGp hIWhUurXn5SPebCud8o5M6WUe1Gfn/Dn3oWrFzoBDDKlZ5uRqPSZcEo6LXnY7CqB7BQH NU2B3MmWlKYol5zx58lBE97jtyhVr99HKL+SAntQB9/NkeNiN+YctYA8B3j7/XKChrZx 4Qu9X+fnOPvO2lzf4NOWS+OBSHPs1IH5riAciF0IfMJwt/MmtrYRP0Wg6hrMj0eHM/or bgDQ== X-Forwarded-Encrypted: i=1; AJvYcCW9Cjyla7zHe4CMg0vxLcqiPWJBfUmqDLArXjneOQ6IIt066tohcGvh6SB3b/oxOUolnEE=@dpdk.org X-Gm-Message-State: AOJu0Yw/sT1R7zO1cTy3+52YqU9qSIJxlyrGEF1lhVShOPra+5kO/5gl VOLbsMvUaydL4UEHZcaaPA2SBqXUa/s/9J31/FEw7O/GoUY6PUHaFhpTuqdOlZ6js6oUz6CqE/4 KB+W7ySohuVCR/kd8axrBm3fQ9b/zA67hKS0xbg== X-Google-Smtp-Source: AGHT+IH5znuB4VR8/tqToS56jtNtOSXtP4GDmAK4uE7qepvbRbYZO6/KRcoQ2ll0/TeJfB6V1qA4eok00Ss5XLG4Jck= X-Received: by 2002:a05:6870:fb94:b0:25e:7a1:ea8f with SMTP id 586e51a60fabf-27c3f6b08efmr782099fac.47.1726107473211; Wed, 11 Sep 2024 19:17:53 -0700 (PDT) MIME-Version: 1.0 References: <20240911173801.16538-1-dmarx@iol.unh.edu> In-Reply-To: <20240911173801.16538-1-dmarx@iol.unh.edu> From: Patrick Robb Date: Wed, 11 Sep 2024 22:17:03 -0400 Message-ID: Subject: Re: [PATCH v1] dts: add VLAN methods to testpmd shell To: Dean Marx Cc: npratte@iol.unh.edu, jspewock@iol.unh.edu, luca.vizzarro@arm.com, yoan.picchi@foss.arm.com, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com, juraj.linkes@pantheon.tech, 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 A new version should be submitted which will apply onto next-dts. Mostly looks good barring 1 or 2 nits and some overlap with Luca's pktgen/testpmd series which was merged to next-dts the other day. On Wed, Sep 11, 2024 at 1:37=E2=80=AFPM Dean Marx wrote= : > > added the following methods to testpmd shell class: > vlan set filter on/off, rx vlan add/rm, > vlan set strip on/off, port stop/start all/port, > tx vlan set/reset, set promisc/verbose > > fixed bug in vlan_offload for > show port info, removed $ at end of regex I think to align with the submission guidelines, commit body should include= : Fixes: 61d5bc9bf974 ("dts: add port info command to testpmd shell") > + def vlan_filter_set(self, port: int, on: bool, verify: bool =3D True= ) -> None: > + """Set vlan filter on. > + > + Args: > + port: The port number to enable VLAN filter on, should be wi= thin 0-32. > + on: Sets filter on if :data:`True`, otherwise turns off. > + verify: If :data:`True`, the output of the command and show = port info > + is scanned to verify that vlan filtering was enabled suc= cessfully. > + If not, it is considered an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and the filter > + fails to update. > + """ > + filter_cmd_output =3D self.send_command(f"vlan set filter {'on' = if on else 'off'} {port}") > + if verify: > + vlan_settings =3D self.show_port_info(port_id=3Dport).vlan_o= ffload > + if on ^ (vlan_settings is not None and VLANOffloadFlag.FILTE= R in vlan_settings): Thank you for teaching me this syntax for python xor :) > + self._logger.debug(f"Failed to set filter on port {port}= : \n{filter_cmd_output}") > + raise InteractiveCommandExecutionError( > + f"Testpmd failed to set VLAN filter on port {port}." > + ) > + > + def rx_vlan(self, vlan: int, port: int, add: bool, verify: bool =3D = True) -> None: > + """Add specified vlan tag to the filter list on a port. > + > + Args: > + vlan: The vlan tag to add, should be within 1-1005, 1-4094 e= xtended. > + port: The port number to add the tag on, should be within 0-= 32. > + add: Adds the tag if :data:`True`, otherwise removes tag. > + verify: If :data:`True`, the output of the command is scanne= d to verify that > + the vlan tag was added to the filter list on the specifi= ed port. If not, it is > + considered an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and the tag > + is not added. > + """ > + rx_output =3D self.send_command(f"rx_vlan {'add' if add else 'rm= '} {vlan} {port}") I guess this variable should be named rx_cmd_output, or rx_vlan_cmd_output to remain consistent with the preceding method, but not a huge deal. > + if verify: > + if ( > + "VLAN-filtering disabled" in rx_output > + or "Invalid vlan_id" in rx_output > + or "Bad arguments" in rx_output > + ): > + self._logger.debug( > + f"Failed to {'add' if add else 'remove'} tag {vlan} = port {port}: \n{rx_output}" > + ) > + raise InteractiveCommandExecutionError( > + f"Testpmd failed to {'add' if add else 'remove'} tag= {vlan} on port {port}." > + ) > + > + def vlan_strip_set(self, port: int, on: bool, verify: bool =3D True)= -> None: > + """Enable vlan stripping on the specified port. > + > + Args: > + port: The port number to use, should be within 0-32. > + on: If :data:`True`, will turn strip on, otherwise will turn= off. > + verify: If :data:`True`, the output of the command and show = port info > + is scanned to verify that vlan stripping was enabled on = the specified port. > + If not, it is considered an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and stripping > + fails to update. > + """ > + strip_output =3D self.send_command(f"vlan set strip {'on' if on = else 'off'} {port}") > + if verify: > + vlan_settings =3D self.show_port_info(port_id=3Dport).vlan_o= ffload > + if on ^ (vlan_settings is not None and VLANOffloadFlag.STRIP= in vlan_settings): > + self._logger.debug( > + f"Failed to set strip {'on' if on else 'off'} port {= port}: \n{strip_output}" > + ) > + raise InteractiveCommandExecutionError( > + f"Testpmd failed to set strip {'on' if on else 'off'= } port {port}." > + ) > + > + def port_stop_all(self, verify: bool =3D True) -> None: > + """Stop all ports. Luca's pktgen and testpmd change series included an equivalent method for stopping all ports, so this can be removed. > + > + Args: > + verify: If :data:`True`, the output of the command is scanne= d > + to ensure all ports are stopped. If not, it is considere= d an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and all ports > + fail to stop. > + """ > + port_output =3D self.send_command("port stop all") > + if verify: > + if "Done" not in port_output: > + self._logger.debug(f"Failed to stop all ports: \n{port_o= utput}") > + raise InteractiveCommandExecutionError("Testpmd failed t= o stop all ports.") > + > + def port_stop(self, port: int, verify: bool =3D True) -> None: > + """Stop specified port. There is a new TestPmdShell attribute, ports_started, which may need to be updated when this is run. You may have to iterate through self._ports and scan testpmd for port info or check otherwise and modify ports_started depending on the result. > + > + Args: > + port: Specifies the port number to use, must be between 0-32= . > + verify: If :data:`True`, the output of the command is scanne= d > + to ensure specified port is stopped. If not, it is consi= dered an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and the port > + is not stopped. > + """ > + port_output =3D self.send_command(f"port stop {port}") > + if verify: > + if "Done" not in port_output: > + self._logger.debug(f"Failed to stop port {port}: \n{port= _output}") > + raise InteractiveCommandExecutionError(f"Testpmd failed = to stop port {port}.") > + > + def port_start_all(self, verify: bool =3D True) -> None: > + """Start all ports. > + > + Args: > + verify: If :data:`True`, the output of the command is scanne= d > + to ensure all ports are started. If not, it is considere= d an error. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`True= ` and all ports > + fail to start. > + """ > + port_output =3D self.send_command("port start all") > + if verify: > + if "Done" not in port_output: > + self._logger.debug(f"Failed to start all ports: \n{port_= output}") > + raise InteractiveCommandExecutionError("Testpmd failed t= o start all ports.") > + > + def port_start(self, port: int, verify: bool =3D True) -> None: > + """Start specified port. Same possible concern as described for port_start()