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 5F58941BB2; Thu, 2 Feb 2023 21:45:14 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5190942D32; Thu, 2 Feb 2023 21:45:14 +0100 (CET) Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by mails.dpdk.org (Postfix) with ESMTP id 1A7CE42D33 for ; Thu, 2 Feb 2023 21:45:13 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.west.internal (Postfix) with ESMTP id 446863200065; Thu, 2 Feb 2023 15:45:11 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 02 Feb 2023 15:45:12 -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=1675370710; x= 1675457110; bh=T4gov1AMH3zgLyq6glWnCSNGeeB3jcKecoV4M1gEdns=; b=s Dl37t7kCwcbUZpvN0JX5m8WrWfE0Cz7yFmqDBAtiqLWdMc8YNSmwy08eyF2GPC39 mNWqI7MCOq5DD2SohjT3mmNr7OHGWFMNksMp21gq1GYfe55R5E5FrckM74q3ESPN tCshUqYlH9o5jo6Ivn1cO4Y+PP8gDNdwZhPY/Mi0Z81ps9du8+Pjcg81HJmdvaUW EYTA0Y7jaF8EYLaA4z//dQnwraF4NSr1D7TKY8EUJPtgshsb0e3SnfV5GPiCmxVv BJEs2aG9b7zCJ+PK6xJTGNlzmx/Oq5knTyf3dT5O/KInLKaKOYH7RH8eBxveeheA Cu6wcaibsqQvBBku3eJuw== 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=1675370710; x= 1675457110; bh=T4gov1AMH3zgLyq6glWnCSNGeeB3jcKecoV4M1gEdns=; b=P DTGWzb+ofXmH2vtQl+29f+WJ3ZyZ7jBSLpIwi1gt1UEkGF505ar4x4nw5Ow6GrNm QaqCUsyv2w0lBjWnIlNJoJVv+6eaEHLnZ3i64ZY5D7XIwMdnvaKdChEo74M/3rkA J+jufKGKK0uvCuR9ZocHq4FDmb8jPrIMSVlg7QokvumLTbl/OXgpjxKMMbM02qaz CRR/4vkupciFhQdN+8fh5UIpoj2IsmIc6WUjKqVIVSoUk373jmRatt+lrt07DxC5 PUkLim1BjwJe+P4zBALCNxghF7Jz1zYJF1+3pg7DdNb+cQm8Fab+AEmqawZ09LHS dgZ5AaGlZ8RIwee0w/PSA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrudefkedgudefjecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddt ieekgfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 2 Feb 2023 15:45:09 -0500 (EST) From: Thomas Monjalon To: Ben Magistro , Tyler Retzlaff Cc: Olivier Matz , ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru, ben.magistro@trinitycyber.com, dev@dpdk.org, Stefan Baranoff , david.marchand@redhat.com, bruce.richardson@intel.com, anatoly.burakov@intel.com Subject: Re: Sign changes through function signatures Date: Thu, 02 Feb 2023 21:45:07 +0100 Message-ID: <5155790.0VBMTVartN@thomas> In-Reply-To: <20230202202604.GA23284@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> References: <20230202202604.GA23284@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> 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 02/02/2023 21:26, Tyler Retzlaff: > On Thu, Feb 02, 2023 at 02:23:39PM -0500, Ben Magistro wrote: > > Hello, > > > > While making some updates to our code base for 22.11.1 that were missed in > > our first pass through, we hit the numa node change[1]. In the process of > > updating our code, we noticed that a couple functions (rx/tx_queue_setup, > > maybe more that we aren't using) state they accept `SOCKET_ID_ANY` but the > > function signature then asks for an unsigned integer while `SOCKET_ID_ANY` > > is `-1`. Following it through the redirect to the "real" function it also > > asks for an unsigned integer which is then passed on to one or more > > functions asking for an integer. As an example using the the i40e driver > > -- we would call `rte_eth_tx_queue_setup` [2] which ultimately calls > > `i40e_dev_tx_queue_setup`[3] which finally calls `rte_zmalloc_socket`[4] > > and `rte_eth_dma_zone_reserve`[5]. > > > > I guess what I am looking for is clarification on if this is intentional or > > if this is additional cleanup that may need to be completed/be desirable so > > that signs are maintained through the call paths and avoid potentially > > producing sign-conversion warnings. From the very quick glance I took at > > the i40e driver, it seems these are just passed through to other functions > > and no direct use/manipulation occurs (at least in the mentioned functions). > > i believe this is just sloppyness with sign in our api surface. i too > find it frustrating that use of these api force either explicit > casts or suffer having to suppress warnings. > > in the past examples of this have been cleaned up without full deprecation > notices but there are a lot of instances. i also feel (unpopular opinion) > that for some integer types like this that have constrained range / number > spaces it would be of value to introduce a typedef that can be used > consistently. > > for now you'll just have to add the casts and hopefully in the future we > will fix the api making them unnecessary. of course feel free to submit > patches too, it would be great to have these cleaned up. I agree it should be cleaned up. Those IDs should accept negative values. Not sure which type we should choose (int, int32_t, or a typedef). Another thing to check is the name of the variable. It should be a socket ID when talking about CPU, and a NUMA node ID when talking about memory. And last but not the least, how can we keep ABI compatibility? I hope we can use function versioning to avoid deprecation and breaking. Trials and suggestions are welcome.