From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 11B82A32A8 for ; Sat, 26 Oct 2019 00:16:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 453FE1BEF5; Sat, 26 Oct 2019 00:16:33 +0200 (CEST) Received: from out4-smtp.messagingengine.com (out4-smtp.messagingengine.com [66.111.4.28]) by dpdk.org (Postfix) with ESMTP id F0C831BED3 for ; Sat, 26 Oct 2019 00:16:31 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 4AFD321AFB; Fri, 25 Oct 2019 18:16:31 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Fri, 25 Oct 2019 18:16:31 -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=mesmtp; bh=VzmnegcpY1MbFHQZh1I0PXPivWEFKO1Sc/d8rgPx57M=; b=O5ZWvmSN8Ukf WbmX76/8KHvKR0DKoMRPI6mUh7UDI85B7IYc9E8rui6UxObAraBGuGv8ttyAkQ6n Q4fNhANBJRfcNSMBIprs+pWvdlSisT7rYPA9HPEprhuQALJJwzJPFtKFgSTmXvHN tTcHF00QHBLpAzvMyrr8O0SWT1JtNOw= 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=VzmnegcpY1MbFHQZh1I0PXPivWEFKO1Sc/d8rgPx5 7M=; b=uC69XXHrNaBQYck05zFbXBdcVb+X0GjNMkRqcGQhtsCKL2sYd6CJp16n2 R4CSzLvtbM/kt5VGO9PWPVG++l7a0rpfw+e+CTpo5lIqs8T0ecMZWBt1nqHO1e4j I8RiGk9YqTEW3n9JAc/PWFvudhBxyqKeJCSa0fe4Yrn12Zck5bYGY6dlfuD7228N LbDDgUjB0iOqjHKaZu13TB4ZIsiZz6cDMa3tqJTjS5M4vy0BV+4p5jjuXYnFUOAr JVp8xACrr257WRmWEfyTBb9pe5Ram96x3GWtU1JZBXYVrB/7BOaackt2f8i0ndF3 S/by758pfgupvT0N3+NUhMRucNdLw== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedufedrleeggddtlecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkjghfggfgtgesthfuredttddtvdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucffoh hmrghinhepughpughkrdhorhhgpdhouhhtlhhoohhkrdgtohhmnecukfhppeejjedrudef gedrvddtfedrudekgeenucfrrghrrghmpehmrghilhhfrhhomhepthhhohhmrghssehmoh hnjhgrlhhonhdrnhgvthenucevlhhushhtvghrufhiiigvpedt 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 E94CFD6005A; Fri, 25 Oct 2019 18:16:29 -0400 (EDT) From: Thomas Monjalon To: Ori Kam Cc: Andrew Rybchenko , "dev@dpdk.org" , Ferruh Yigit , "jingjing.wu@intel.com" , "stephen@networkplumber.org" Date: Sat, 26 Oct 2019 00:16:28 +0200 Message-ID: <98253428.ADkZfdayxY@xps> In-Reply-To: References: <1569479349-36962-1-git-send-email-orika@mellanox.com> <7486204.JcRLTtQae7@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v5 02/15] ethdev: add support for hairpin queue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" 25/10/2019 21:01, Ori Kam: > From: Thomas Monjalon > > 24/10/2019 17:30, Andrew Rybchenko: > > > On 10/24/19 6:17 PM, Thomas Monjalon wrote: > > > > 24/10/2019 16:47, Andrew Rybchenko: > > > >> On 10/24/19 11:29 AM, Ori Kam wrote: > > > >>> Hi Andrew, > > > >>> > > > >>> When writing the new function I thought about using bool, but > > > >>> I decided against it for the following reasons: > > > >>> 1. There is no use of bool any where in the code, and there is not special > > reason to add it now. > > > >> rte_ethdev.c includes stdbool.h and uses bool > > > >> > > > >>> 2. Other functions of this kind already returns int. for example > > (rte_eth_dev_is_valid_port / rte_eth_is_valid_owner_id) > > > > I agree with Ori here for 2 reasons: > > > > 1. It is better to be consistent in the API > > > > 2. I remember having some issues with some drivers when introducing > > stdbool in the API. > > > > > > > > I think it may be nice to convert all such API to bool in one patch, > > > > and check if there are some remaining issues with bool usage in drivers or > > with PPC. > > > > But I suggest to do such API change in DPDK 20.11. > > > > > > OK, no problem. Does it prevent to avoid comparison == 1? Just to > > > avoid changes in these lines in the future. > > > > Yes probably better to avoid explicit comparison, but prefer boolean operator > > (!). > > > > > > Thomas I understand your comments but from Andrew comment on my V2-01 patch: > " > >>> + ret = (*dev->dev_ops->rx_hairpin_queue_setup)(dev, rx_queue_id, > >>> + nb_rx_desc, conf); > >>> + if (!ret) > >> Please, compare with 0 > >> > > Will do, but again just for my knowledge why? > > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdoc.dpdk.org%2Fguides%2Fcontributing%2Fcoding_style.html%23function-calls&data=02%7C01%7Corika%40mellanox.com%7C022cd953964f4a20d50508d7508a259f%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637066426584029629&sdata=G8ZxEWFFsv1kLWc6L7sKT8O6CSiBcj5ZuwQqmK0Q6nY%3D&reserved=0 > > " > I don't see any relevant info in the link, but maybe I'm missing something . > What are the rules? > Thomas also keep in mind that in most cases the condition that is tested is the positive, meaning it will look something like this: > if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id)) { > rte_errno = EINVAL; > return NULL; > } > > What do you think? I think for normal functions with error codes, we should compare explictly with a value. But for boolean-type functions like "is_hairpin_queue", we should have implicit (natural) comparison. So yes, this is correct: if (rte_eth_dev_is_rx_hairpin_queue(dev, queue_id))