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 D522043C9F; Wed, 13 Mar 2024 12:10:43 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9559E410E6; Wed, 13 Mar 2024 12:10:43 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id C336040A84 for ; Wed, 13 Mar 2024 12:10:41 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1710328241; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=+Tv06dYK2UAtSTxcqoyYdWmGR81F0/Ipqxwj0nlYQ/4=; b=OxprGXvo3ehbjiyRbU04je5Oqk+jDo2krx1mABguMXlZgdSMicjpBsVz0/iDOCO7RlQwVJ sGcUY+YkTOrFbmUe2o/VI+wfUKYBnkPS8WYUwRMfo88gNzHgRsAsvKTQzofgua76ANB02n 6l6h2df0MdrLmY5q2q04UEpK5KC0tbI= Received: from mail-lf1-f72.google.com (mail-lf1-f72.google.com [209.85.167.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-372-ShiQzEBKODe4COtj86lUag-1; Wed, 13 Mar 2024 07:10:40 -0400 X-MC-Unique: ShiQzEBKODe4COtj86lUag-1 Received: by mail-lf1-f72.google.com with SMTP id 2adb3069b0e04-5131c91bbc3so532001e87.0 for ; Wed, 13 Mar 2024 04:10:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1710328238; x=1710933038; 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=+Tv06dYK2UAtSTxcqoyYdWmGR81F0/Ipqxwj0nlYQ/4=; b=A0XSK8oRgxtdZiHR8RxPcuPCR7Se2dR0JuvbLHuSv/ZODCIrHg4saD9GGLJY+zf7WF 1iM7LL7pOG4q2Dg//HilOS09hhm4qwa1xBSTHnQBxgzwFxMT5zIMYxtHHCoJ57TKqfgF 3a7LW/qv+W8sAm65qD7O4jGul49UY1iDd+ecdnb5BASQWTt8iPfxoAnoX6VL2on09vz4 kktQuumtQQpaymUHT8T/QT62CaFl8idfbekFjLqkMcYJbg54lL/SEne37ypfxAxw7ixX VIVsWv4jTb2se/JrVSowwNyrFfBCplmXBX64PQar3P/6GmYxwwpxzwq4lXsrhr/Zvofl f4zA== X-Gm-Message-State: AOJu0Yyz004ty3yUDPq9P/yoQZc3RD/p0HpfPe0xRMemxXetEWqQ32+Z ahJ5Yrc6bcARUoOjW8nKIc+UzEvsUjSPYQfuzXqemO5JK5hqfNZh+oxDOxvKovdnLnJYhSyxMdF 4vRGX0gfaS3/rI4XsxViGa0zsp9wwzSNgDUokOWb8rtx4XktW932A1sqBaR4u49iGATkeXE8TFF blUWQV2OCN/c2U1wo= X-Received: by 2002:ac2:529a:0:b0:513:c210:8c63 with SMTP id q26-20020ac2529a000000b00513c2108c63mr1818497lfm.64.1710328238483; Wed, 13 Mar 2024 04:10:38 -0700 (PDT) X-Google-Smtp-Source: AGHT+IGKMsRiDeZzbDMRCXXD0naZcOBPC9PSoRJZ4+WMtcPDZcP9RAvtXqNmCVm1PzL4UU+01F92mhAx3cGZMN5S1cU= X-Received: by 2002:ac2:529a:0:b0:513:c210:8c63 with SMTP id q26-20020ac2529a000000b00513c2108c63mr1818481lfm.64.1710328238113; Wed, 13 Mar 2024 04:10:38 -0700 (PDT) MIME-Version: 1.0 References: <20240308144841.3615262-1-david.marchand@redhat.com> <20240308144841.3615262-4-david.marchand@redhat.com> <5d2efba1-ad6e-4e51-86bd-77d299b73697@amd.com> <378f35ad-d1cc-4d55-aa1b-7a2aec582cd8@amd.com> In-Reply-To: <378f35ad-d1cc-4d55-aa1b-7a2aec582cd8@amd.com> From: David Marchand Date: Wed, 13 Mar 2024 12:10:26 +0100 Message-ID: Subject: Re: [PATCH 3/4] app/testpmd: check queue count for related options To: Ferruh Yigit Cc: dev@dpdk.org, Aman Singh , Yuying Zhang X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com 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 Wed, Mar 13, 2024 at 11:52=E2=80=AFAM Ferruh Yigit wrote: > > On 3/13/2024 7:37 AM, David Marchand wrote: > > On Tue, Mar 12, 2024 at 5:59=E2=80=AFPM Ferruh Yigit wrote: > >> > >> On 3/8/2024 2:48 PM, David Marchand wrote: > >>> Checking the number of rxq/txq in the middle of option parsing is > >>> confusing. Move the check where nb_rxq / nb_txq are modified. > >>> > >>> Signed-off-by: David Marchand > >>> --- > >>> app/test-pmd/parameters.c | 8 ++++---- > >>> 1 file changed, 4 insertions(+), 4 deletions(-) > >>> > >>> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c > >>> index 8c21744009..271f0c995a 100644 > >>> --- a/app/test-pmd/parameters.c > >>> +++ b/app/test-pmd/parameters.c > >>> @@ -1063,6 +1063,8 @@ launch_args_parse(int argc, char** argv) > >>> rte_exit(EXIT_FAILURE, "rxq %d = invalid - must be" > >>> " >=3D 0 && <=3D %u\n= ", n, > >>> get_allowed_max_nb_rx= q(&pid)); > >>> + if (!nb_rxq && !nb_txq) > >>> + rte_exit(EXIT_FAILURE, "Either = rx or tx queues should be non-zero\n"); > >>> } > >>> if (!strcmp(lgopts[opt_idx].name, "txq")) { > >>> n =3D atoi(optarg); > >>> @@ -1072,6 +1074,8 @@ launch_args_parse(int argc, char** argv) > >>> rte_exit(EXIT_FAILURE, "txq %d = invalid - must be" > >>> " >=3D 0 && <=3D %u\n= ", n, > >>> get_allowed_max_nb_tx= q(&pid)); > >>> + if (!nb_rxq && !nb_txq) > >>> + rte_exit(EXIT_FAILURE, "Either = rx or tx queues should be non-zero\n"); > >>> } > >>> if (!strcmp(lgopts[opt_idx].name, "hairpinq")) = { > >>> n =3D atoi(optarg); > >>> @@ -1098,10 +1102,6 @@ launch_args_parse(int argc, char** argv) > >>> n + nb_rxq, > >>> get_allowed_max_nb_rx= q(&pid)); > >>> } > >>> - if (!nb_rxq && !nb_txq) { > >>> - rte_exit(EXIT_FAILURE, "Either rx or tx= queues should " > >>> - "be non-zero\n"); > >>> - } > >>> if (!strcmp(lgopts[opt_idx].name, "hairpin-mode= ")) { > >>> char *end =3D NULL; > >>> unsigned int n; > >> > >> There is already a runtime check for queues [1], perhaps we can remove > >> it altogether from arg parse. > > > > Good catch. > > > > This other check comes after parsing args, so I suspect it is just dead= code. > > I guess I'll change it into a rte_exit(EXIT_FAILURE..). > > Is this what you propose? > > > > I think that check is the main check for nb_rxq and nb_txq. > > The one you removed is for the 'hairpinq' parameter, which is not a very > common usecase. This check was present before hairpinq introduction. https://git.dpdk.org/dpdk/commit/app/test-pmd/parameters.c?id=3D1c69df45f8c= 6b727c3b6a78e13f81225c090dde2 This check in parsing args is hit when setting incorrect rxq / txq config. This has nothing to do with hairpinq parsing. $ build-mini/app/dpdk-testpmd -c 3 --no-huge -m 40 -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=3D2048 -ia --rxq 0 --txq 0 EAL: Detected CPU lcores: 16 EAL: Detected NUMA nodes: 1 EAL: Detected static linkage of DPDK EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket EAL: Selected IOVA mode 'VA' TELEMETRY: No legacy callbacks, legacy socket not created Interactive-mode selected Auto-start selected EAL: Error - exiting with code: 1 Cause: Either rx or tx queues should be non-zero Port 0 is closed Port 1 is closed > But nb_rxq and nb_txq requirement is very common and it is protected in > the main after parameter parsing. Sorry, I am not following. > > I am not suggesting adding 'rte_exit()' for that case, probably it will > fail in some other part and error log can provide the required hint. > And I am worried if it breaks some unexpected usecase with exit. If we simply remove this check as you suggest: $ build-mini/app/dpdk-testpmd -c 3 --no-huge -m 40 -a 0:0.0 --vdev net_null1 --vdev net_null2 -- --no-mlockall --total-num-mbufs=3D2048 -ia --rxq 0 --txq 0 EAL: Detected CPU lcores: 16 EAL: Detected NUMA nodes: 1 EAL: Detected static linkage of DPDK EAL: Multi-process socket /run/user/114840/dpdk/rte/mp_socket EAL: Selected IOVA mode 'VA' TELEMETRY: No legacy callbacks, legacy socket not created Interactive-mode selected Auto-start selected Warning: Either rx or tx queues should be non-zero ^^^ Pointless log. testpmd: create a new mbuf pool : n=3D2048, size=3D2176, socket= =3D0 testpmd: preferred mempool ops selected: ring_mp_mc Fail: Cannot allocate fwd streams as number of queues is 0 Segmentation fault (core dumped) ^^^ And crash. --=20 David Marchand