From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <dev@dpdk.org>; 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>
 <CAJFAV8yWO=cLB6Ov2pA5QKqu15n6rUgYHEvt-h0AVdLrFbOtXA@mail.gmail.com>
 <378f35ad-d1cc-4d55-aa1b-7a2aec582cd8@amd.com>
In-Reply-To: <378f35ad-d1cc-4d55-aa1b-7a2aec582cd8@amd.com>
From: David Marchand <david.marchand@redhat.com>
Date: Wed, 13 Mar 2024 12:10:26 +0100
Message-ID: <CAJFAV8xEg-TXutY0yqC7CF9sU_Tcd75LzdU8E8H8MagLNKH-1w@mail.gmail.com>
Subject: Re: [PATCH 3/4] app/testpmd: check queue count for related options
To: Ferruh Yigit <ferruh.yigit@amd.com>
Cc: dev@dpdk.org, Aman Singh <aman.deep.singh@intel.com>, 
 Yuying Zhang <yuying.zhang@intel.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

On Wed, Mar 13, 2024 at 11:52=E2=80=AFAM Ferruh Yigit <ferruh.yigit@amd.com=
> wrote:
>
> On 3/13/2024 7:37 AM, David Marchand wrote:
> > On Tue, Mar 12, 2024 at 5:59=E2=80=AFPM Ferruh Yigit <ferruh.yigit@amd.=
com> 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 <david.marchand@redhat.com>
> >>> ---
> >>>  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 <mb_pool_0>: 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