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 5FD51A0547; Wed, 27 Oct 2021 12:57:47 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 2EE304068C; Wed, 27 Oct 2021 12:57:47 +0200 (CEST) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by mails.dpdk.org (Postfix) with ESMTP id 9FD4E4003F for ; Wed, 27 Oct 2021 12:57:46 +0200 (CEST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id 23F3B5C00B5; Wed, 27 Oct 2021 06:57:46 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Wed, 27 Oct 2021 06:57:46 -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= YjCDsGYRdlsOxz1/HDsqxL3pmDRqBlhAera/+vt07Bk=; b=YxTqWYeYbo9IScej 8m2uVFHmBm7vklDAK3gDaUPFXajSma3KAphfsQ1ZOqY66aOFeqxZKf6NJ85/Ox0e 1NEzIfgfIADFwfq5NWM+79HEAEqXflrqMu1TWM9DK57rTSRC4xYXa7Tc9yBOYjG3 yZDJHb8tgm/R9npU68jV2DJiCHXQ0tuBgzAOG/nh0CPUh98yRRWAOQIoKM8l5Tp8 8DMVh2V50ZPJxyEEvNrKv0bEPk3vQeo87VsaX+/vRTt+HXzc/YcfqVTaYl/z15p3 aTuEUJqCU//NdeHASwNF40kcL9zPFUR6M3dvIxtf8O3w1OPT9kQ47QWSSJKlIO94 Ric3Ug== 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=YjCDsGYRdlsOxz1/HDsqxL3pmDRqBlhAera/+vt07 Bk=; b=WlnFMdLO0nhUA9mHMS57m4Mv8DZd/GeNR9EKzmM2ilijVsLYOfOKERVye 7K2OIhab3oCPzXDdoIkifXL3NO+HyB2uCN6p6ioC0ImgDyiLYv2fxCXVMaNiDDRJ luPkhcdK9tLCG3eTioT14dXESQOmdLgZkXZuMpIGnCmTnV3/F8T31e5UJKPvFb+o u/OAgIUZHBwdVSBnlV49at1AiEIUkTz4+S233t2VKPxKlzoi2+20AXh4k3fdvVJX pqs2b9nK/mDEQArWafn5L4cYrc9Wa1l0ZRqJxhQSywjCbzlxxBaAy/mPgqq+SQHr GpecflFSagdoewNLnSpj7c/oXnaLA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrvdegtddgfeduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecugg ftrfgrthhtvghrnhepudeggfdvfeduffdtfeeglefghfeukefgfffhueejtdetuedtjeeu ieeivdffgeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrh homhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 27 Oct 2021 06:57:44 -0400 (EDT) From: Thomas Monjalon To: Ivan Malov Cc: dev@dpdk.org, David Marchand , Ferruh Yigit , Ori Kam , Andrew Rybchenko Date: Wed, 27 Oct 2021 12:57:42 +0200 Message-ID: <5927197.slJBfLvSNU@thomas> In-Reply-To: References: <20211027090003.14556-1-ivan.malov@oktetlabs.ru> <2080007.0klm6OBeOA@thomas> 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" 27/10/2021 11:55, Ivan Malov: > Hi Thomas, > > On 27/10/2021 12:46, Thomas Monjalon wrote: > > 27/10/2021 11:00, Ivan Malov: > >> There are PMDs which do not support flow offloads at all. > >> In such cases, the API in question returns ENOTSUP. This > >> is too loud. Restructure the code to avoid spamming logs. > >> > >> Fixes: 1179f05cc9a0 ("ethdev: query proxy port to manage transfer flows") > >> > >> Signed-off-by: Ivan Malov > >> --- > >> --- a/lib/ethdev/rte_flow.c > >> +++ b/lib/ethdev/rte_flow.c > >> @@ -1335,10 +1335,7 @@ rte_flow_pick_transfer_proxy(uint16_t port_id, uint16_t *proxy_port_id, > >> const struct rte_flow_ops *ops = rte_flow_ops_get(port_id, error); > >> struct rte_eth_dev *dev; > >> > >> - 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. > > 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. > > 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. A driver can introduce a malfunction with this API while we don't use rte_flow at all in the test scenario.