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 9F360A054F; Mon, 15 Mar 2021 09:55:41 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 455642425E5; Mon, 15 Mar 2021 09:55:40 +0100 (CET) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by mails.dpdk.org (Postfix) with ESMTP id 7C1904003C for ; Mon, 15 Mar 2021 09:55:38 +0100 (CET) Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailnew.nyi.internal (Postfix) with ESMTP id B26DF58095D; Mon, 15 Mar 2021 04:55:37 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Mon, 15 Mar 2021 04:55:37 -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=fm3; bh= mAOSa05OmqtcK/rQ0uoDB4d2eV0WrTAzbdWmmcRNdAM=; b=GkEn9BsVaKkdzyBN SLAFR0h0tD2KsvCg7KZG6vv/Jg8Asfn+Tfc9Jal61HSJtRcGcySvri0l9/ILf6h/ L356DmIItVf+hU8tvWNeOauzAMK89ZmfwaWUvY1769uhgbkncsvyxqe12zMA75MB jDANyuzaG6poPicBB7Ppl6IALa+xpy+cITIKAsGOeVWJcKwc1yG0BRAhlo6gavgV fXWQ4Z+v4O/cGdxMSCujFgUuRwtgxarJtwMXsASU9aYwjppdTrAoN29ZpAGS1Gk0 hHW0XXp2VGwCc+cSUwOHej5IQTX/+51bC8K5tFj9VR2KxR5SrWeREgvTe6Gkwxmd oLwMvA== 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=fm2; bh=mAOSa05OmqtcK/rQ0uoDB4d2eV0WrTAzbdWmmcRNd AM=; b=EWgMxyNQ7XovQXyebLOHr0ZutDsBdt3ZWHcKGTkLoKIYHau79JHOn7KPK k3pqJAoM/nJmcgIgmM9GqH7rmMTj9oNmGbJyLCRO/KnjlwhrXCjnjG5wNZ1I8KXu 7cLaD3f3+VGjzmuGb0TVhl0KbFGRCV0zdlP8lx2Utd0fEdHtJVQ+h273VaOD+gc2 gRa+174lDnnJIJ9tn5L3rI4P692XHW+MIwwNPwYWKoz4ynqefr8/rIOzNuavls3j /Or94Ph3Bb/Q7oOFsTA+tMq6ai1I9h2s//N6UcTzqdxEb0Ky0jx5hqoU6ELvuBXT l3rwwpRJSpvdQaLMlte3jfwLIDNJw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduledruddvkedguddvjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhm rghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenuc ggtffrrghtthgvrhhnpedugefgvdefudfftdefgeelgffhueekgfffhfeujedtteeutdej ueeiiedvffegheenucfkphepjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrh fuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgr lhhonhdrnhgvth X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id B6C6C240067; Mon, 15 Mar 2021 04:55:31 -0400 (EDT) From: Thomas Monjalon To: Andrew Rybchenko Cc: dev@dpdk.org, Ori Kam , Ajit Khaparde , Somnath Kotur , Chas Williams , "Min Hu (Connor)" , Rahul Lakkireddy , Hemant Agrawal , Sachin Saxena , Jeff Guo , Haiyue Wang , John Daley , Hyong Youb Kim , Gaetan Rivet , Ziyang Xuan , Xiaoyun Wang , Guoyang Zhou , Yisen Zhuang , Lijun Ou , Beilei Xing , Jingjing Wu , Qiming Yang , Qi Zhang , Rosen Xu , Matan Azrad , Shahaf Shuler , Viacheslav Ovsiienko , Liron Himi , Jerin Jacob , Nithin Dabilpuram , Kiran Kumar K , Rasesh Mody , Shahed Shaikh , Jasvinder Singh , Cristian Dumitrescu , Keith Wiles , Jiawen Wu , Jian Wang , Ferruh Yigit Date: Mon, 15 Mar 2021 09:55:30 +0100 Message-ID: <48964683.5Cnftzig0A@thomas> In-Reply-To: References: <20210311221742.3750589-1-thomas@monjalon.net> <1656619.R5YYQy59bT@thomas> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 1/2] ethdev: replace callback getting filter operations 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" 15/03/2021 09:43, Andrew Rybchenko: > On 3/15/21 10:54 AM, Thomas Monjalon wrote: > > 15/03/2021 08:18, Andrew Rybchenko: > >> On 3/12/21 8:46 PM, Thomas Monjalon wrote: > >>> --- a/lib/librte_ethdev/rte_flow.c > >>> +++ b/lib/librte_ethdev/rte_flow.c > >>> @@ -255,18 +255,19 @@ rte_flow_ops_get(uint16_t port_id, struct rte_flow_error *error) > >>> > >>> if (unlikely(!rte_eth_dev_is_valid_port(port_id))) > >>> code = ENODEV; > >>> - else if (unlikely(!dev->dev_ops->filter_ctrl || > >>> - dev->dev_ops->filter_ctrl(dev, > >>> - RTE_ETH_FILTER_GENERIC, > >>> - RTE_ETH_FILTER_GET, > >>> - &ops) || > >>> - !ops)) > >>> - code = ENOSYS; > >>> + else if (unlikely(dev->dev_ops->flow_ops_get == NULL)) > >>> + code = ENOTSUP; > >>> else > >>> - return ops; > >>> - rte_flow_error_set(error, code, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, > >>> - NULL, rte_strerror(code)); > >>> - return NULL; > >>> + code = dev->dev_ops->flow_ops_get(dev, &ops); > >>> + if (code == 0 && ops == NULL) > >>> + code = EACCES; > >> It looks something new. I think it should be mentioned in flow_ops_get > >> type documentation (similar to eth_promiscuous_enable_t) and > >> rte_flow_validate() etc functions > >> return values description. > > > > It is an internal function used only in rte_flow.c. > > The real consequence is to set rte_errno in a lot of rte_flow API. > > Not sure there is a good way to document the code details. > > Other codes are not documented in rte_flow.h > > First of all it is a behaviour of the flow_ops_get callback and > driver developers should know that it is a legal to return 0 and > ops==NULL and know what it means. The combination code 0 and ops NULL is not new. Previously, it was returning ENOSYS. I've just given a more meaningful error code: EACCES, while replacing ENOSYS with ENOTSUP for the other case. > Second, it is visible as rte_flow_validate() (and other functions > which use rte_flow_ops_get()) return value value which has > special meaning. So, should be documented. Yes, I should update the API doc where ENOSYS was mentioned. Or probably better: I should keep the error code ENOSYS and do not break API. Preference?