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 B0F90A0032; Fri, 29 Oct 2021 10:11:12 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 37BE440688; Fri, 29 Oct 2021 10:11:12 +0200 (CEST) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by mails.dpdk.org (Postfix) with ESMTP id F411A40395 for ; Fri, 29 Oct 2021 10:11:10 +0200 (CEST) Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 219AF5C01D3; Fri, 29 Oct 2021 04:11:10 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Fri, 29 Oct 2021 04:11:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=fm2; bh= kFfdvOOXWxaQYXWoSWKQilR1336bmo9tNi3klvTWBWE=; b=WhvjKqM6i6WGy0XM v5EDqdg4397X7T0raR6NeAYHOM684ZHUHfCxx9MuXIdWpKq/TITmvUlkLSITJbjY NlYmsSI5Gl8kKi0WD2iHukQ5W6aAgFmcOhoKhwhYAv1ibhUM178juKwMrqH3sesB I6rEQejlrXAjQ+go0HA+K2JTwhGyHS5MvGC+jZtLLYQvxmPEMFol5sb2Xms4hJ51 RbY3QGEJJINBXvWJFEaQfgz8zak/NVsUqMLtWNnoJeryhbxa1YyQHQwquT1oyVEV eL8lQ5BXKZNoffev+FJRq7LI7MLlcXHLvvU6D79eWZHVv9HnOXxx9inz0nIDsSlq 07owOA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm1; bh=kFfdvOOXWxaQYXWoSWKQilR1336bmo9tNi3klvTWB WE=; b=mWHuQp4VWAwItpQzcQEtv5vnm6Lh60Oh1mzus+hlXESmhaaz17INJBLZS xPla7pcCj8tfyOeJGWRprbO01ShM2nywH2Th2VzSEkoknsR8ncq2cxnWsJkuSIMg cPAbzMk4kiDYKAmNizuF2n5/DhroEqEuJ6tFenM1BaKYGdRI7wanDKnx41ebFQ71 qLRjN516LlTHmtGGQRvfGHsxFr9V/1OvoRsHVhLJLY8npsDma8543JuprCET40kL JDvTPRlkGJPlVBy28IrdKU1AUY82oE626sEx4aFx5UXtiMy+Og1VOMutVYWS4zD2 3xtJ67bsyWTIUVt4jPjR/WuiWfWvw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvdeggedguddvgecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdej ueeiiedvffegheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfh hrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvght X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 29 Oct 2021 04:11:07 -0400 (EDT) From: Thomas Monjalon To: Ivan Malov Cc: dev@dpdk.org, David Marchand , Ferruh Yigit , Ori Kam , Andrew Rybchenko Date: Fri, 29 Oct 2021 10:11:04 +0200 Message-ID: <34074894.qiqEHZsViX@thomas> In-Reply-To: <43fd9b2e-5c2e-d464-5b24-88a2d8da0e1e@oktetlabs.ru> References: <20211027090003.14556-1-ivan.malov@oktetlabs.ru> <5927197.slJBfLvSNU@thomas> <43fd9b2e-5c2e-d464-5b24-88a2d8da0e1e@oktetlabs.ru> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH] ethdev: fine tune error reporting in pick transfer proxy API 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 Sender: "dev" 28/10/2021 18:24, Ivan Malov: > On 27/10/2021 13:57, Thomas Monjalon wrote: > > 27/10/2021 11:55, Ivan Malov: > >> On 27/10/2021 12:46, Thomas Monjalon wrote: > >>> 27/10/2021 11:00, Ivan Malov: > >>>> - if (unlikely(ops == NULL)) > >>>> - return -rte_errno; > >>>> - > >>>> - if (ops->pick_transfer_proxy == NULL) { > >>>> + if (ops == NULL || ops->pick_transfer_proxy == NULL) { > >>>> *proxy_port_id = port_id; > >>>> return 0; > >>>> } > >>> > >>> I prefer this logic. > >> > >> Thank you. > >> > >>> You could add a comment to say that the current port is the default. > >> > >> As far as I remember, the comment ("note") is already in place (rte_flow.h). > > > > I meant adding a comment in the implementation above. > > Technically, I don't object adding it. But isn't the > idea expressed clear enough by the code itself? In general I like having a global idea as comment to make clear it is the intent, but no strong opinion. > >>> There is also this logic in testpmd: > >>> > >>> port->flow_transfer_proxy = port_id; > >>> if (!is_proc_primary()) > >>> return; > >>> > >>> Could we manage secondary process case inside the API? > >> > >> Shouldn't we manage secondary process in *all* flow APIs then? > > > > Hmm, yes logically we should not care about secondary process at all in rte_flow. > > OK to leave it as is. > > Thank you. > > > > >>> One more comment, for testpmd, > >>> we are calling rte_flow_pick_transfer_proxy even if we do not config any transfer flow. > >>> It is called always in init_config_port_offloads(). > >>> It looks wrong. Can we call it only when needed? > >> > >> In which way does it look wrong? Does it inflict error(s), malfunction, > >> performance drops? Please elaborate. > > > > It is testing a function that we don't intend to test in a basic use case. > > Not really. The original idea is to invoke this API only once, on > port (re-)plug and remember the proxy port ID to be used on each > flow create invocation. Theoretically, when the new asynchronous > flow API arrives, this approach will be even more to the point. I understand, but this one-time call could be done only when configuring the first transfer flow. > > A driver can introduce a malfunction with this API while > > we don't use rte_flow at all in the test scenario. > > Fat chance. Even if that happens, it will draw attention. It is > the duty of test-pmd to detect such malfunction after all. If > the current code comes across a bug in some driver, it should > be good, shouldn't it? Test coverage gets extended, right? testpmd duty is to test some precise scenarios. We don't test metering if not requested for example. What others think?