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 156C4A0545; Mon, 1 Aug 2022 21:41:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E50FC4014F; Mon, 1 Aug 2022 21:41:44 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id A5B43400D7 for ; Mon, 1 Aug 2022 21:41:43 +0200 (CEST) Received: by shelob.oktetlabs.ru (Postfix, from userid 115) id 0CB92F3; Mon, 1 Aug 2022 22:41:42 +0300 (MSK) X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on mail1.oktetlabs.ru X-Spam-Level: X-Spam-Status: No, score=0.8 required=5.0 tests=ALL_TRUSTED, DKIM_ADSP_DISCARD autolearn=no autolearn_force=no version=3.4.6 Received: from bree.oktetlabs.ru (bree.oktetlabs.ru [192.168.34.5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPS id 70B05F1; Mon, 1 Aug 2022 22:41:42 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 70B05F1 Authentication-Results: shelob.oktetlabs.ru/70B05F1; dkim=none; dkim-atps=neutral Date: Mon, 1 Aug 2022 22:41:42 +0300 (MSK) From: Ivan Malov To: Hanumanth Pothula cc: Aman Singh , Yuying Zhang , dev@dpdk.org, jerinj@marvell.com Subject: Re: [PATCH] app/testpmd: add command line argument 'rx-metadata' In-Reply-To: <20220801131338.1710737-1-hpothula@marvell.com> Message-ID: References: <20220801044111.1706871-1-hpothula@marvell.com> <20220801131338.1710737-1-hpothula@marvell.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="-1463810782-1653239758-1659382902=:4092782" 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 This message is in MIME format. The first part should be readable text, while the remaining parts are likely unreadable without MIME-aware tools. ---1463810782-1653239758-1659382902=:4092782 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8BIT Hi Hanumanth, Thanks for expanding support for the NIC-to-PMD Rx metadata feature. I do not object the idea of the patch, it looks aceeptable. However, please find my comments below. On Mon, 1 Aug 2022, Hanumanth Pothula wrote: > Presently, rx metadata is sent to PMD by default, leading > to a performance drop as processing for the same in rx path > takes extra cycles. > > Hence, introducing command line argument, 'rx-metadata' to > control passing rx metadata to PMD. By default it’s disabled. > > Signed-off-by: Hanumanth Pothula > --- > app/test-pmd/parameters.c | 4 ++++ > app/test-pmd/testpmd.c | 6 +++++- > app/test-pmd/testpmd.h | 2 ++ > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c > index e3c9757f3f..daf1218977 100644 > --- a/app/test-pmd/parameters.c > +++ b/app/test-pmd/parameters.c > @@ -213,6 +213,7 @@ usage(char* progname) > printf(" --hairpin-mode=0xXX: bitmask set the hairpin port mode.\n" > " 0x10 - explicit Tx rule, 0x02 - hairpin ports paired\n" > " 0x01 - hairpin ports loop, 0x00 - hairpin port self\n"); > + printf(" --rx-metadata: send rx metadata to driver \n"); In accordance with the above "printf", one should have 2 space characters between the opening double quote character and "--". Also, it is quite common to capitalise "R" in "Rx". The space character before "\n" seems redundant. Regarding the option name, I would appreciate if it gets more precise and clear. Consider: "--nic-to-pmd-rx-metadata". I do not insist on this particular name. Perhaps it pays to have a bit more verbose description as well. Consider: "let the NIC deliver per-packet Rx metadata to PMD". > } > #ifdef RTE_LIB_CMDLINE > @@ -710,6 +711,7 @@ launch_args_parse(int argc, char** argv) > { "record-burst-stats", 0, 0, 0 }, > { PARAM_NUM_PROCS, 1, 0, 0 }, > { PARAM_PROC_ID, 1, 0, 0 }, > + { "rx-metadata", 0, 0, 0 }, > { 0, 0, 0, 0 }, > }; > @@ -1510,6 +1512,8 @@ launch_args_parse(int argc, char** argv) > num_procs = atoi(optarg); > if (!strcmp(lgopts[opt_idx].name, PARAM_PROC_ID)) > proc_id = atoi(optarg); > + if (!strcmp(lgopts[opt_idx].name, "rx-metadata")) > + rx_metadata_negotiate = 1; > break; > case 'h': > usage(argv[0]); > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c > index addcbcac85..ebbde5dfc9 100644 > --- a/app/test-pmd/testpmd.c > +++ b/app/test-pmd/testpmd.c > @@ -411,6 +411,9 @@ uint8_t clear_ptypes = true; > /* Hairpin ports configuration mode. */ > uint16_t hairpin_mode; > +/* send rx metadata */ Consider: "/* Send Rx metadata */". > +uint8_t rx_metadata_negotiate; > + > /* Pretty printing of ethdev events */ > static const char * const eth_event_desc[] = { > [RTE_ETH_EVENT_UNKNOWN] = "unknown", > @@ -1628,7 +1631,8 @@ init_config_port_offloads(portid_t pid, uint32_t socket_id) > int ret; > int i; > - eth_rx_metadata_negotiate_mp(pid); > + if (rx_metadata_negotiate) > + eth_rx_metadata_negotiate_mp(pid); > port->dev_conf.txmode = tx_mode; > port->dev_conf.rxmode = rx_mode; > diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h > index fb2f5195d3..8a9168c51e 100644 > --- a/app/test-pmd/testpmd.h > +++ b/app/test-pmd/testpmd.h > @@ -621,6 +621,8 @@ extern struct rte_ether_addr peer_eth_addrs[RTE_MAX_ETHPORTS]; > extern uint32_t burst_tx_delay_time; /**< Burst tx delay time(us) for mac-retry. */ > extern uint32_t burst_tx_retry_num; /**< Burst tx retry number for mac-retry. */ > +extern uint8_t rx_metadata_negotiate; > + > #ifdef RTE_LIB_GRO > #define GRO_DEFAULT_ITEM_NUM_PER_FLOW 32 > #define GRO_DEFAULT_FLOW_NUM (RTE_GRO_MAX_BURST_ITEM_NUM / \ > > -- > 2.25.1 > > -- Best regards, Ivan M ---1463810782-1653239758-1659382902=:4092782--