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 3ACBB42C5C; Thu, 8 Jun 2023 12:44:14 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2987B406B5; Thu, 8 Jun 2023 12:44:14 +0200 (CEST) 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 84BA840042 for ; Thu, 8 Jun 2023 12:44:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1686221052; 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=SwrMPG/4SHH6Xpqii8xuiQEkiGxlxFLWOgmAtdgo7ac=; b=bBLCXYZP5MtuukjP7LtbMizZsvuFpAa3qxd9cMLJKuohUq4V71zmba+DIbw6qrx/FhosuB cqAkBH/mHz8vPKgi4b7+26NdgITAx/qWsTsP9VNjkOBdy07VfEiYXt3VK9AffqdVXWs5E6 4duuqDppq5DLG2rTlPagjeIz0OJqCCo= Received: from mail-ed1-f72.google.com (mail-ed1-f72.google.com [209.85.208.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-228-0qGJqCRFMOmW7ITqXwazrg-1; Thu, 08 Jun 2023 06:44:10 -0400 X-MC-Unique: 0qGJqCRFMOmW7ITqXwazrg-1 Received: by mail-ed1-f72.google.com with SMTP id 4fb4d7f45d1cf-514b19ded99so485262a12.0 for ; Thu, 08 Jun 2023 03:44:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686221049; x=1688813049; 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=SwrMPG/4SHH6Xpqii8xuiQEkiGxlxFLWOgmAtdgo7ac=; b=gub2jnLprstTIZYpdrcHpkZyFLac5DVfzXXnfZr04+Ne02xC+b1PjqDK91IZQBOaT6 Ako6xcObt8JSmLfta2hn768kn3BMwE2CASVaA9Mw8EIVHvyz8P/3cZbVly0B4LTL218u TKWx/COeCdVPvntwF+yPUfEZL4Zq6JK6FQYfsnzdDdW0uJIHuMpELx9ST0q792ibDGtV 1LWCOK3iPwv9UC21oanyL0CER53S9SMk1ScOMPEEngfj/hL9Dc+17sWRje7eLUorbWOR aZ5aK93fNqhvmBq91ElWuVTV0ayxCLWL1QYz2U0lUpOPVl2IRsekCkG2336ioyuuDpx/ IGow== X-Gm-Message-State: AC+VfDwD2xUxCODxwoeweG/G2aDGhB4xKqBR0fWby9mPs7eElHVntS+3 MaFSh+i3u1lsTIFYP8zdbCPH/iulk5r2Gc35GlSf3KIcPmnnP2VROYNspCFkf+l3ALF9eXeNH8l cXwKtWcJ7YdcTdgkMCJs= X-Received: by 2002:a17:907:3e0f:b0:96a:ca96:3e49 with SMTP id hp15-20020a1709073e0f00b0096aca963e49mr10291716ejc.13.1686221049639; Thu, 08 Jun 2023 03:44:09 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4jLVhccLKemwj/ZEeJHf3tZu1TcLZuaoef5GI7/rHCbdxLLdwOuozl8Z8X7ZJaywOLqXWIfrtGYspZxuX9p4A= X-Received: by 2002:a17:907:3e0f:b0:96a:ca96:3e49 with SMTP id hp15-20020a1709073e0f00b0096aca963e49mr10291700ejc.13.1686221049221; Thu, 08 Jun 2023 03:44:09 -0700 (PDT) MIME-Version: 1.0 References: <20230606211242.2855795-1-mkp@redhat.com> <20230608095907.557698-1-mkp@redhat.com> <18a33036-d4bc-7500-884f-c9fad506ed3b@amd.com> In-Reply-To: <18a33036-d4bc-7500-884f-c9fad506ed3b@amd.com> From: Mike Pattrick Date: Thu, 8 Jun 2023 06:43:57 -0400 Message-ID: Subject: Re: [PATCH v6] app/testpmd: expand noisy neighbour forward mode support To: Ferruh Yigit Cc: Aman Singh , Yuying Zhang , ktraynor@redhat.com, dev@dpdk.org 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 Thu, Jun 8, 2023 at 6:25=E2=80=AFAM Ferruh Yigit = wrote: > > On 6/8/2023 10:59 AM, Mike Pattrick wrote: > > Previously the noisy neighbour vnf simulation would only operate in io > > mode, forwarding packets as is. However, this limited the usefulness of > > noisy neighbour simulation. > > > > This feature has now been expanded to supporting mac, macswap, and > > 5tswap modes. To facilitate adding this support, some new header files > > were added. > > > > Signed-off-by: Mike Pattrick > > --- > > v2: Reverted changes to random memory lookup > > v3: Refactored entire patch > > v4: Implemented recommended formatting changes > > v5: Corrected copyright statement and formatting changes > > v6: Reordered some variables, preserved noisy subtype for display > > --- > > app/test-pmd/5tswap.c | 118 +---------------------- > > app/test-pmd/5tswap.h | 130 ++++++++++++++++++++++++++ > > app/test-pmd/config.c | 18 +++- > > app/test-pmd/macfwd.c | 33 +------ > > app/test-pmd/macfwd.h | 45 +++++++++ > > app/test-pmd/noisy_vnf.c | 106 +++++++++++++++++---- > > app/test-pmd/parameters.c | 15 +++ > > app/test-pmd/testpmd.c | 14 +++ > > app/test-pmd/testpmd.h | 10 ++ > > doc/guides/testpmd_app_ug/run_app.rst | 9 ++ > > 10 files changed, 334 insertions(+), 164 deletions(-) > > create mode 100644 app/test-pmd/5tswap.h > > create mode 100644 app/test-pmd/macfwd.h > > > > <...> > > > diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c > > index 096c218c12..a3e2c2ac15 100644 > > --- a/app/test-pmd/config.c > > +++ b/app/test-pmd/config.c > > @@ -4052,9 +4052,16 @@ rxtx_config_display(void) > > { > > portid_t pid; > > queueid_t qid; > > + char buf[32]; > > + > > + if (cur_fwd_eng =3D=3D &noisy_vnf_engine) > > + snprintf(buf, sizeof(buf), " (%s)", noisy_fwd_mode_desc[n= oisy_fwd_mode]); > > + else > > + buf[0] =3D '\0'; > > > > - printf(" %s packet forwarding%s packets/burst=3D%d\n", > > + printf(" %s%s packet forwarding%s packets/burst=3D%d\n", > > cur_fwd_eng->fwd_mode_name, > > + buf, > > retry_enabled =3D=3D 0 ? "" : " with retry", > > nb_pkt_per_burst); > > > > @@ -4816,10 +4823,17 @@ pkt_fwd_config_display(struct fwd_config *cfg) > > struct fwd_stream *fs; > > lcoreid_t lc_id; > > streamid_t sm_id; > > + char buf[32]; > > + > > + if (cfg->fwd_eng =3D=3D &noisy_vnf_engine) > > + snprintf(buf, sizeof(buf), " (%s)", noisy_fwd_mode_desc[n= oisy_fwd_mode]); > > + else > > + buf[0] =3D '\0'; > > > > - printf("%s packet forwarding%s - ports=3D%d - cores=3D%d - stream= s=3D%d - " > > + printf("%s%s packet forwarding%s - ports=3D%d - cores=3D%d - stre= ams=3D%d - " > > "NUMA support %s, MP allocation mode: %s\n", > > cfg->fwd_eng->fwd_mode_name, > > + buf, > > retry_enabled =3D=3D 0 ? "" : " with retry", > > cfg->nb_fwd_ports, cfg->nb_fwd_lcores, cfg->nb_fwd_stream= s, > > numa_support =3D=3D 1 ? "enabled" : "disabled", > > > This works but I wonder if we can keep the display functions generic (as > much as possible), without forwarding enginee specific checks. > > What about the idea to update '.fwd_mode_name' in 'noisy_fwd_begin()'? > That way generic display code can stay as it is. I worried that this would interfere with set_pkt_forwarding_mode() among other functions. The .fwd_mode_name seems to play double duty as view and model. I was also thinking about adding another member to struct fwd_engine to display subtype / internal status. But I couldn't think of a compelling use case in the other modules. So I leaned towards this implementation. Which do you prefer? > > <...> > > > +static bool > > +pkt_burst_mac(struct fwd_stream *fs) > > +{ > > + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; > > + uint16_t nb_rx; > > + uint16_t nb_tx; > > + > > + nb_rx =3D common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_bu= rst); > > + if (likely(nb_rx !=3D 0)) > > + do_macfwd(pkts_burst, nb_rx, fs); > > + nb_tx =3D noisy_eth_tx_burst(fs, nb_rx, pkts_burst); > > + > > + return nb_rx > 0 || nb_tx > 0; > > +} > > +static bool > > +pkt_burst_macswap(struct fwd_stream *fs) > > +{ > > + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; > > + uint16_t nb_rx; > > + uint16_t nb_tx; > > + > > + nb_rx =3D common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_bu= rst); > > + if (likely(nb_rx !=3D 0)) > > + do_macswap(pkts_burst, nb_rx, &ports[fs->tx_port]); > > + nb_tx =3D noisy_eth_tx_burst(fs, nb_rx, pkts_burst); > > + > > + return nb_rx > 0 || nb_tx > 0; > > +} > > +static bool > > +pkt_burst_5tswap(struct fwd_stream *fs) > > +{ > > + struct rte_mbuf *pkts_burst[MAX_PKT_BURST]; > > + uint16_t nb_rx; > > + uint16_t nb_tx; > > + > > + nb_rx =3D common_fwd_stream_receive(fs, pkts_burst, nb_pkt_per_bu= rst); > > + if (likely(nb_rx !=3D 0)) > > + do_5tswap(pkts_burst, nb_rx, fs); > > + nb_tx =3D noisy_eth_tx_burst(fs, nb_rx, pkts_burst); > > + > > + return nb_rx > 0 || nb_tx > 0; > > +} > > > > Empty lines are missing between functions. > >