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 18ECE42490; Thu, 26 Jan 2023 12:03:34 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id ACFD940A79; Thu, 26 Jan 2023 12:03:33 +0100 (CET) Received: from wout1-smtp.messagingengine.com (wout1-smtp.messagingengine.com [64.147.123.24]) by mails.dpdk.org (Postfix) with ESMTP id 7F48340697 for ; Thu, 26 Jan 2023 12:03:32 +0100 (CET) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id B8EC03200786; Thu, 26 Jan 2023 06:03:30 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Thu, 26 Jan 2023 06:03:31 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm3; t=1674731010; x= 1674817410; bh=nchz+ObcWcfYMN7koUsHAzI8qs558zXxqu/yxwhT8Nw=; b=m O2h/GN61ZYdatWmQhyJYKWvtMyzRsG69dpZHl68ubHhYZVkNu7etSv6oWA60QFsP Iud4qYKhzD8kG2sxlloOkfPYj6Jak/K1UvKs1Y6rrgv/TMPOaVfY2iKvBse0MRTb A2wt318Bvv+R2ffAY9+QMVlnnpsFNNF4fnD2262VB7m6B8AvXdifDSWyI/yO+y9n X0UnUxDAAjXNLFWYKqclwo4Unx0zBWzX2e2eR0r+9OhoxbtY6fj6CDWNdTHe8AYi 6eiaT0uoSf/vc0yyPSFwzX4vxclMEW3xoQ8eQPxU988W1E59eFv2BM0lD8YLvqCL zmaF0vR0MWc09Wrtl5jyw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1674731010; x= 1674817410; bh=nchz+ObcWcfYMN7koUsHAzI8qs558zXxqu/yxwhT8Nw=; b=o UOonSoTN1xzVEQpiZfpayfANY+wFcv1BWn7ETl3tc+2NpFQCismZRlovD7IWacxr N3G0rmW3w87tf+75bGQt/HVcS0bLVfcaVLJ+1IAzUEE/o7irPLeJeWflQgVxy7CI ea3STzEgqpVN4bsMT8WIJoIlvuXR8yWYc5DpfrbhROL94rTHOMvOmBZEGTISlR55 ICYR6gAlDzav7JO9gayU/UpPI218LK/Hh4ocxo398I7MzUB0QJRS+sYmcawDGYNq IyIMukf+gRjHTC8fQuAsjjNJcV4wdQBh1243HZaDuHscdyo2ckxlywn4xx3DRMKN +wUR1u662Zdp/P8L9lXSA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedruddvgedgvdefucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvvefufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedtjeeiieefhedtfffgvdelteeufeefheeujefgueetfedttdei kefgkeduhedtgfenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 26 Jan 2023 06:03:28 -0500 (EST) From: Thomas Monjalon To: Nithin Kumar Dabilpuram Cc: Aman Singh , Yuying Zhang , Ivan Malov , Andrew Rybchenko , "dev@dpdk.org" , Hanumanth Reddy Pothula , Ferruh Yigit , "viacheslavo@nvidia.com" , Jerin Jacob Kollanukkaran , "david.marchand@redhat.com" Subject: Re: [EXT] Re: [PATCH v5 2/2] app/testpmd: add command to process Rx metadata negotiation Date: Thu, 26 Jan 2023 12:03:27 +0100 Message-ID: <2238537.U75o6juQT5@thomas> In-Reply-To: References: <20221220200250.2413443-1-hpothula@marvell.com> <2252130.NnIJQXNAa5@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" 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 Please reply inline below instead of doing an incomplete copy of the replies on top. 25/01/2023 15:42, Nithin Kumar Dabilpuram: > > >Will it work to enable them all by default and add capability to disable > > >it in testpmd, which helps to run performance tests also to verify the > > > impact of the API? > > The spirit of the negotiating features/Rx/Tx offloads upfront is to have it disabled by default and enable the feature only when needed. Having the features enabled by default is probably against that spirit. > > We understand the concerns with drivers that didn't not implement that API. There is no such concern I think. > Why not disable it by default(like other offloads) and modify rte_flow action creation in testpmd to check for if !ENOSUP and feature disabled and add print to enable. So for the PMD's that won't support rte_eth_rx_metadata_negotiate(), there won't be any difference and for very few PMD's that support this API, they need to enable it before using RTE_FLOW with MARK/FLAG. > Behavior change would be seen only with two PMD's(cnxk, sfc). I think you missed the whole point. Ferruh is proposing to have a command "port config ..." to configure the flags to negotiate. Are you OK with this approach? > > Note: I don't understand why we don't have > > RTE_FLOW_ACTION_TYPE_SET_TAG and RTE_FLOW_ACTION_TYPE_SET_META > > negotiated in this function. Probably something to add. > > The purpose of negotiate is to tell the PMD upfront so that PMD can prepare > HW appropriately. Having these new actions would be very late to inform PMD and > I think won't solve the purpose. I am not talking about your problem here. I am just saying that TAG and META should be negotiated as well in rte_eth_rx_metadata_negotiate(). > From: Thomas Monjalon > > 25/01/2023 14:55, Ferruh Yigit: > > > On 1/25/2023 12:55 PM, Thomas Monjalon wrote: > > > > 25/01/2023 10:30, Hanumanth Reddy Pothula: > > > >> ++ Ivan Malov and Andrew Rybchenko > > > >> > > > >> From: Ferruh Yigit > > > >>> On 12/21/2022 2:07 AM, 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, add new testpmd command, > > > >>>> 'enable port nic_to_pmd_rx_metadata' > > > >>>> > > > >>>> This command helps in sending Rx metadata to PMD and thereby Rx > > > >>>> metadata flow command requests are processed. > > > >>>> > > > >>>> Signed-off-by: Hanumanth Pothula > > > >>> > > > >>> Hi Hanumanth, > > > >>> > > > >>> I agree with Thomas for the patch. > > > >>> > > > >>> 'eth_rx_metadata_negotiate_mp()' requests all Rx metadata offloads to be > > > >>> enabled, but at this stage if there is no flow rule for Rx metadata why it is > > > >>> consuming extra cycles? > > > >>> > > > >>> Can you update driver code to process Rx metadata when it is enabled by > > > >>> application (via 'rte_eth_rx_metadata_negotiate()') AND there is at least > > > >>> one flow rule for it? > > > >> > > > >> #1 What is the purpose of rte_eth_rx_metadata_negotiate() API if it is always called by > > testpmd. > > > >> We thought it was added so that when that metadata is not needed, application need > > not call this > > > >> thereby saving cycles/bandwidth. > > > > > > > > testpmd is for testing all features. That's why all is negotiated. > > > > Cycles should be saved if you don't enable it until a flow rule requires it. > > > > > > > > > > Hi Thomas, > > > > > > Not just for saving cycles, but from testing perspective too, do you > > > think does it work if a way to disable these Rx metadata added by > > > keeping default behavior as it is? > > > > > > And new command can be in a consistent command syntax like: > > > "port config ..." > > > > Yes I agree it would be good to have a way to test different values. > > And it would allow to completely disable metadata I suppose. > > > > Note: I don't understand why we don't have > > RTE_FLOW_ACTION_TYPE_SET_TAG and RTE_FLOW_ACTION_TYPE_SET_META > > negotiated in this function. Probably something to add. > > > > > > > >> #2 We use this API similar to Rx/Tx offload flags so that we can set things up before > > device is > > > >> configured. We thought that is the purpose of having this negotiate API and avoid > > depleting offload flags. > > > > > > > > It is just a configuration negotiation specific to metadata. > > > > > > > >> #3 Generally any new offloads added to DPDK would be in disabled state in testpmd > > and we would have > > > >> an option to enable it. In this case, testpmd is by default calling this negotiation. > > > > > > > > Negotiating is not enabling. > > > > > > > >> We can update the driver if the purpose of this API is clear. > > > > > > > > Please do. > > > > > > Is following understanding correct? > > > > > > API Flow Rule Result > > > ----- ------------ -------- > > > Enable No Rule Feature Disabled > > > Enable Rule exist Feature Enabled > > > Disable X Feature Disabled > > > > In the API column, you should say "negotiated" instead of "Enable". > > > > > >