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 E1CB545920; Fri, 6 Sep 2024 18:51:42 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 79CEE42EA7; Fri, 6 Sep 2024 18:51:42 +0200 (CEST) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 5962D42E82 for ; Fri, 6 Sep 2024 18:51:41 +0200 (CEST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 02A51FEC; Fri, 6 Sep 2024 09:52:08 -0700 (PDT) Received: from [10.57.17.223] (unknown [10.57.17.223]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E09D83F73B; Fri, 6 Sep 2024 09:51:39 -0700 (PDT) Message-ID: <9fd8af3d-7cc0-49d7-afb9-1017e6542fcf@arm.com> Date: Fri, 6 Sep 2024 17:51:38 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 5/5] dts: add testpmd set ports queues Content-Language: en-GB To: Jeremy Spewock Cc: dev@dpdk.org, =?UTF-8?Q?Juraj_Linke=C5=A1?= , Honnappa Nagarahalli , Paul Szczepanek References: <20240806121417.2567708-1-Luca.Vizzarro@arm.com> <20240806124642.2580828-1-luca.vizzarro@arm.com> <20240806124642.2580828-6-luca.vizzarro@arm.com> From: Luca Vizzarro In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 09/08/2024 16:14, Jeremy Spewock wrote: > On Tue, Aug 6, 2024 at 8:49 AM Luca Vizzarro wrote: >> >> Add a facility to update the number of TX/RX queues during the runtime >> of testpmd. >> >> Signed-off-by: Luca Vizzarro >> Reviewed-by: Paul Szczepanek >> --- >> dts/framework/remote_session/testpmd_shell.py | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py >> index ca24b28070..85fbc42696 100644 >> --- a/dts/framework/remote_session/testpmd_shell.py >> +++ b/dts/framework/remote_session/testpmd_shell.py >> @@ -805,6 +805,22 @@ def start_all_ports(self, verify: bool = True) -> None: >> >> self.ports_started = True >> >> + @requires_stopped_ports >> + def set_ports_queues(self, number_of: int) -> None: > > Was there a reason to not add a verify option to this method? It seems > like it could be useful, and the general pattern with testpmd methods > so far is verifying the outcome wherever we can. Yes, I've noticed that the value (in show port info) doesn't get updated until actually restarting the ports. So it wouldn't work here unfortunately. Internally it looks like it's only updating a variable, the actual operation can only be verified upon ports starting. >> + """Sets the number of queues per port. >> + >> + Args: >> + number_of: The number of RX/TX queues to create per port. > > Is there any value in allowing users to set either Rx or Tx rather > than enforcing they change both? I guess I can't think of a specific > reason to do one and not the other off the top of my head, but it > seems like the option could be useful. My initial code was only updating the RX queues for my l2fwd test suite, and I was having lots of unpredictable and flaky behaviour. Sometimes it would work other times it wouldn't. When setting different values for RX and TX queues you actually get a warning from the driver to expect unexpected behaviour by doing this. And indeed I did get it. From this I gathered that we don't want to have different values, unless we explicitly want to make the drivers unhappy. I had unexpected and different behaviour on both ice and mlx5_core. > I might be more in favor of this being an > InteractiveCommandExecutionError or some other DTSError just to have a > little more control over the error codes and to make it more clear > that the error came specifically from the framework. I noticed you already gave yourself an answer :D