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 0817E454BF; Fri, 21 Jun 2024 23:28:19 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ED59542EBE; Fri, 21 Jun 2024 23:28:18 +0200 (CEST) Received: from mail-pj1-f47.google.com (mail-pj1-f47.google.com [209.85.216.47]) by mails.dpdk.org (Postfix) with ESMTP id A402E40041 for ; Fri, 21 Jun 2024 23:28:17 +0200 (CEST) Received: by mail-pj1-f47.google.com with SMTP id 98e67ed59e1d1-2c8062f9097so1426337a91.3 for ; Fri, 21 Jun 2024 14:28:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1719005297; x=1719610097; 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=Mw3kestaltvzn5sqZSmhfxrExD7Dhc31YeRpmoCozKk=; b=bTnihsHnYmFfGZnCwO5cz/Vk6ybb2Od3m6poEReG5f7eAN+ymudWyNHzxt9C2ZsyfL 3/js4W+0ffMqxoZYVoPAhlcX+gMJA24ZddkcibZn2URlgnMrRd3fPn6LrzL1Dc/KitbP 26uDCUKBs+i3qldHaWKPEX3b9ygTKwTFfiCcM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719005297; x=1719610097; 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=Mw3kestaltvzn5sqZSmhfxrExD7Dhc31YeRpmoCozKk=; b=Lm+wFFqFl5E2h4LGP+uVcOuvDhuSfM4rVNN3qoDtFLBMJNV87/rYiLGKRL64YnfTvu 94uLsEKppyRRA+7F9Pz0oWn5DcOUB1C+DPhLKmYlHBGLq7vcnhSu3+RgrweX+7Ny3Df8 bdoBg01yDRkGb+2UTlzR7iTLUwxlH+/zT1C7xq3+g8FHAOsRvHJVyimneVdyf4oJgDUw h/LvtT192H0pain7Bu24EioSygIR8Bvv3p9lchz5S4rm5AV9g/ojjeBft+ixqg5EfuaW WL3mUNbOfPU4VMq5r24yNxB+S9BC1vOZVJ3yXfQMbOV2/HhIn0JgH4pMmemHBLybtERi IptA== X-Forwarded-Encrypted: i=1; AJvYcCX8HEZPcFZeAlKf86byGg5uLjyuFcjX4cwQIJQAVJCE2iB7X2E3Zsj8njEEqYvHy6jIHm2mtskjYKh2lA8= X-Gm-Message-State: AOJu0YyFDR/ckzTBoTZs+tSXss+rwP5I9nXiPFTaFik7moBGzbufTFgQ k0tGbu7GsUwXts7iHN+oBh0uRt3f9Qop8lwFb4QIV958ozlDYI8DeLHjMcxV1g/z16DDSO5xlQh Q7T8FqTl2FBa4CqLcUzRXYwk+n2ksx7VBqucceQ== X-Google-Smtp-Source: AGHT+IFsipuxi6YQYwYopH2OdktROJW1In8hYTwNMwdyy5QHirjCsDjTa8D1xJ+uwph2yBKOXh7VE8CCDIPsxx9JKaE= X-Received: by 2002:a17:90a:e643:b0:2c7:b194:6881 with SMTP id 98e67ed59e1d1-2c7b5d56bc9mr10340247a91.31.1719005296726; Fri, 21 Jun 2024 14:28:16 -0700 (PDT) MIME-Version: 1.0 References: <20240617194638.12926-2-dmarx@iol.unh.edu> <20240617194638.12926-6-dmarx@iol.unh.edu> In-Reply-To: <20240617194638.12926-6-dmarx@iol.unh.edu> From: Jeremy Spewock Date: Fri, 21 Jun 2024 17:28:05 -0400 Message-ID: Subject: Re: [PATCH v2 2/3] dts: added promisc/verbose func to testpmd 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, Jun 17, 2024 at 3:48=E2=80=AFPM Dean Marx wrote= : > > Setting the verbose mode and promiscuous mode is a common > command in test suites, these additions will allow the dev > to set both modes from the testpmd shell module. It looks like you're also adding the ability to start and stop ports here as well, we should add that to the commit body and subject. > > Signed-off-by: Dean Marx > --- > dts/framework/remote_session/testpmd_shell.py | 179 +++++++++++++++++- > 1 file changed, 178 insertions(+), 1 deletion(-) > > diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framewor= k/remote_session/testpmd_shell.py > index cb2ab6bd00..425f3ec220 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -147,7 +147,7 @@ def start(self, verify: bool =3D True) -> None: > "Not all ports came up after starting packet for= warding in testpmd." > ) > > - def stop(self, verify: bool =3D True) -> None: > + def stop(self, verify: bool =3D True) -> str: > """Stop packet forwarding. > > Args: > @@ -155,6 +155,9 @@ def stop(self, verify: bool =3D True) -> None: > forwarding was stopped successfully or not started. If n= either is found, it is > considered an error. > > + Returns: > + Output gathered from sending the stop command. > + > Raises: > InteractiveCommandExecutionError: If `verify` is :data:`True= ` and the command to stop > forwarding results in an error. > @@ -167,6 +170,7 @@ def stop(self, verify: bool =3D True) -> None: > ): > self._logger.debug(f"Failed to stop packet forwarding: \= n{stop_cmd_output}") > raise InteractiveCommandExecutionError("Testpmd failed t= o stop packet forwarding.") > + return stop_cmd_output I know these are changes coming from us both using the same port start and stop methods, but you don't seem to use the changes to the stop method so we should probably omit them. > > def get_devices(self) -> list[TestPmdDevice]: > """Get a list of device names that are known to testpmd. > + > + def setup_port_queue(self, port_id: int, queue_id: int, is_rx_queue:= bool) -> None: > + """Setup a given queue on a port. > + > + This functionality cannot be verified because the setup action o= nly takes effect when the > + queue is started. > + > + Args: > + port_id: ID of the port where the queue resides. > + queue_id: ID of the queue to setup. > + is_rx_queue: Type of queue to setup. If :data:`True` an RX q= ueue will be setup, > + otherwise a TX queue will be setup. > + """ > + self.send_command(f"port {port_id} {'rxq' if is_rx_queue else 't= xq'} {queue_id} setup") > + > + def change_queue_ring_size( > + self, > + port_id: int, > + queue_id: int, > + size: int, > + is_rx_queue: bool, > + verify: bool =3D True, > + ) -> None: > + """Update the ring size of an RX/TX queue on a given port. > + > + Args: > + port_id: The port that the queue resides on. > + queue_id: The ID of the queue on the port. > + size: The size to update the ring size to. > + is_rx_queue: Whether to modify an RX or TX queue. If :da= ta:`True` an RX queue will be > + updated, otherwise a TX queue will be updated. > + verify: If :data:`True` an additional command will be se= nt to check the ring size of > + the queue in an attempt to validate that the size wa= s changes properly. > + > + Raises: > + InteractiveCommandExecutionError: If `verify` is :data:`= True` and there is a failure > + when updating ring size. > + """ > + queue_type =3D "rxq" if is_rx_queue else "txq" > + self.send_command(f"port config {port_id} {queue_type} {queu= e_id} ring_size {size}") > + self.setup_port_queue(port_id, queue_id, is_rx_queue) > + if verify: > + queue_info =3D self.send_command(f"show {queue_type} inf= o {port_id} {queue_id}") > + if f"Number of RXDs: {size}" not in queue_info: > + self._logger.debug( > + f"Failed up update ring size of queue {queue_id}= on port {port_id}:" > + f"\n{queue_info}" > + ) > + raise InteractiveCommandExecutionError( > + f"Failed to update ring size of queue {queue_id}= on port {port_id}" > + ) Same thing here, it doesn't seem like you use this method for changing the ring size or the previous for setting up ports, it's probably better to omit them. > + > def close(self) -> None: > """Overrides :meth:`~.interactive_shell.close`.""" > self.send_command("quit", "") > -- > 2.44.0 >