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 BB8BB41BBC; Fri, 3 Feb 2023 13:05:20 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4D11A4067B; Fri, 3 Feb 2023 13:05:20 +0100 (CET) Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by mails.dpdk.org (Postfix) with ESMTP id 3C5BA4021E for ; Fri, 3 Feb 2023 13:05:18 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1675425918; x=1706961918; h=date:from:to:cc:subject:message-id:references: content-transfer-encoding:in-reply-to:mime-version; bh=GegIhG/2PrL8qw+fFQb0RVGH5XOp9/EnxwgZCZ5Bf7o=; b=FXuXenpZSnrBHe0QOfxqHVruLfMwvL0cEnW//J8lmxLudtPuy85138i8 nnxkGsMO3ECuVkwma3DZTWs8xkIPadpkDhu0YFT2g2+BK/citUxR22jg7 lLliqX3ijgxAkZohARTmfhW1x+HzalzuAHi5a5Vsron+/bzQ7i62JzOZw gRkXgzsJCkhLulUuSV1jRGDlq6li/0IuVMoZR3aDYXDd22WjABWYJCid7 tB9rgQzJ8ZIeUrmXcwJGnj3RygAsAaacbJMSH4dbdCV3kBvRjLDHJz9bk GMpw/VDVWet5uyy6h5N1z0kVOn8CNZ6W2casBC/9b9+XiA4lCSmyP6dE0 w==; X-IronPort-AV: E=McAfee;i="6500,9779,10609"; a="328746886" X-IronPort-AV: E=Sophos;i="5.97,270,1669104000"; d="scan'208";a="328746886" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Feb 2023 04:05:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10609"; a="665698140" X-IronPort-AV: E=Sophos;i="5.97,270,1669104000"; d="scan'208";a="665698140" Received: from fmsmsx602.amr.corp.intel.com ([10.18.126.82]) by orsmga002.jf.intel.com with ESMTP; 03 Feb 2023 04:05:15 -0800 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Fri, 3 Feb 2023 04:05:14 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Fri, 3 Feb 2023 04:05:14 -0800 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.108) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Fri, 3 Feb 2023 04:05:14 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=hQ8R2eZbREUSaliZORPNfCitKedVVuTHcAzIlCtGMae84SZfBT9ceVDj9ZdyXikgVCUcQcWjWqTg1C1HFx8WiK/H74Gk9Pvffq4IsGNGsQWToyejIVzWRdfEKYg4ds1Gz3YA5xJOXabH5ExQN+HKv8mpWH639QlOM1P6NF7oqc3Teq+NtR6O6hPzgxeZMLt0h0l7YbsynwKBmt0EtqeDa5XqYKNYH1y6jlwFGMZgg3ZaQEKZBj7o5e0F5N+NC3/cNfX8sj880ElIWPgMD7zLb2FTMvHkLvwIVcUevyVPuik/z1S7YBfalEe25pE17blz5H1U7Gissx8izRzG/6zTxg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=edPnGYiXVm/3k5/IxOrh5ZgXlvNdv29uacsu3DcX0/A=; b=ihSwKEeJE8Xq/P9UQiRkpIfFjdqfK6d2DvQcbtsmzFmko/2A8bbwU1RozS8x/zbYWP+FnNyV2vcl9LUIGiNHQvidTEvEfgqrPO3BJmXQExZGx7HFvmEVvIx7dHwmR0stJsuqNoy0x/yaGBGEosoPNfC3tk7ixKscSqwyr7BB6lBLd0ebJNibpu4KuyGEyudDi0XH7DQPCCzEE4QZM+LG/dyKAGDGXzG+0vyqeWvhlJow0CbMGbiUGBTr26Bvx9y0aDK3PV5V4I+Mmp0oSDvuzv+PaL4TwdcfFry8fcQgmdJHhdZ1FwKJiJG+h33gH7NFZ95cozu8JXqfUTmXDYJtYQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; Received: from DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) by DM4PR11MB5424.namprd11.prod.outlook.com (2603:10b6:5:39c::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6064.31; Fri, 3 Feb 2023 12:05:12 +0000 Received: from DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::aaba:abb7:8794:def]) by DS0PR11MB7309.namprd11.prod.outlook.com ([fe80::aaba:abb7:8794:def%7]) with mapi id 15.20.6064.024; Fri, 3 Feb 2023 12:05:12 +0000 Date: Fri, 3 Feb 2023 12:05:04 +0000 From: Bruce Richardson To: Morten =?iso-8859-1?Q?Br=F8rup?= CC: Thomas Monjalon , Ben Magistro , Tyler Retzlaff , Olivier Matz , , , , , Stefan Baranoff , , Subject: Re: Sign changes through function signatures Message-ID: References: <20230202202604.GA23284@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <5155790.0VBMTVartN@thomas> <98CBD80474FA8B44BF855DF32C47DC35D876F8@smartserver.smartshare.dk> Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D876F8@smartserver.smartshare.dk> X-ClientProxiedBy: LO3P265CA0013.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:bb::18) To DS0PR11MB7309.namprd11.prod.outlook.com (2603:10b6:8:13e::17) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: DS0PR11MB7309:EE_|DM4PR11MB5424:EE_ X-MS-Office365-Filtering-Correlation-Id: db42fedb-74d1-4216-4de5-08db05dee70a X-LD-Processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: DRu3N5QOsSjj2Tq/Dk4Y3JQHKmC1MMjxN7u4Rf3MGaItv8xLQ4aNYRKKM+TY13j4K3TiA3RWtqwFPY1YvVltC9rasGwg05GqSNX1VjdhtWvM52y9q+fuTKY6lhBbiSwbVNhyUOfckYBKgCLPFg5XlV35TZS0cjn8KPEN1/OdN8CjMCCCv54N94Z2Ivl7hOMqWCd9Z5iLDAS999+/CiFJkaM5p2CUJRSseZgfKaF5S37CFwgTLWVzxtjZPj1fhOtuj4xZ6s+UYq7NtQbQk79NjyGh/Nm887THpv2lzxrAvMaLNpG/KisAAvSVpEGP6IGmbGg4f0ze5j8/J+BG7R8XdScXCNUBL7+v5jMomUd2j926atrDC3N4XDfQJITrsKLtIngDjXgsePmCxzDHtXckMjs+Hg2hZIRzPRbqRmJ/JIn1H1KzD0frcPJCTus9vBOcSPjvmLs4VaB/tJVVR0TYomAjvlBFUlLq7CTSy7wzKtJgwyzY6G9TLgy2eqWfr3YDVK+sWuDJwk9mb1Cs348afIoGaRObM/vzGGQH9gO1wx9DMLe3m6IONQ0QJykEBtY0FuKQlCUU+iXYNS+QapxSEK2Y2NivOjRCWKZAzhAEEdyFHo24eF75c6lMfedWGySw3xOFf948NUZdgxl7ZuqicVXH9JtmDTdD977DfE+DRkri24DWdHCOz82AKLemAIOg98fhiUgNaSx78QzDGY53Px+MCwsigLb+l98jimhOysE= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DS0PR11MB7309.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(366004)(39860400002)(136003)(346002)(376002)(396003)(451199018)(38100700002)(66574015)(83380400001)(26005)(6512007)(6486002)(186003)(478600001)(86362001)(6506007)(82960400001)(966005)(107886003)(6666004)(8676002)(54906003)(316002)(66946007)(6916009)(66476007)(66899018)(66556008)(4326008)(41300700001)(8936002)(5660300002)(7416002)(2906002)(44832011)(67856001); DIR:OUT; SFP:1102; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?iso-8859-1?Q?n9ZUPCrcQBw60RHc9L2PJ74VX6jz+7dYpAIKB6s+XmrDO5Twh8tIeX1/Fv?= =?iso-8859-1?Q?jzpJQUoMIElsX38kziJpUhR3XgftBd7habJ/rw8VMO1QVwcfQcRmvP4j/4?= =?iso-8859-1?Q?s0FJWAcvLey2NYYy0DtXavpQKVht6su5NYh1xMSTcLO9j6SnE6dT8zQ5bW?= =?iso-8859-1?Q?WoQbAfjKt6fopas3h/Cl5k+ZDoGjyHFZaGQ5QGy/EhhTazAjowA9+kDm3e?= =?iso-8859-1?Q?v8y50ZvA4t+dnHKSCYiocExVbtxT/Kmup2f7BI/Xx4zc49qChMN2RXKtbr?= =?iso-8859-1?Q?GNwcwbjRLNbCou3vqXuE8hB/3KpFJUU9GCZpDM/AKDJ/qHWBRXzcwV914t?= =?iso-8859-1?Q?nmju+fZP3WHO06MquReMAKJRd4cMJvIE0lFYUtFrCPHeUSSmNDeFmdj9Qt?= =?iso-8859-1?Q?uK0z6tkVJREJFaDuUQmIYexlLD01c4wGBRUFFXLwYaHltV3k0V4EWLfKuC?= =?iso-8859-1?Q?XNWWb5QC2VR9rMczbj55Y9woxqvrJJ22StWiwPDQ/l/syeQPT8bI2iKixD?= =?iso-8859-1?Q?wh3R9T45a3RL+cu1UhgFBX1QoXzNbZUv6+VICK6+HS0C5mtE39P/YEsgSQ?= =?iso-8859-1?Q?FmsYeXe+lHfGIHi9D4KiBhYb66pu2DCGRgl5DS+jnr7+WFO9PFG9AcOTDv?= =?iso-8859-1?Q?rHuHPlEeevRQO0F6fVSHtEoQraNbiL3E8/8+pb+HmJPJKCXWdviIiPW37l?= =?iso-8859-1?Q?/ixDYqlGCymT3JuGzeELfBNVXbskF23IVInCDowDGcR18HSaC2/GjWle52?= =?iso-8859-1?Q?p2b9WhDgg0xiBLonnQvT3TQ2Yp4T8wqbG6SqDmnGlw7PwOzVnhfACCU8w1?= =?iso-8859-1?Q?BJD6/xIB/4gTN/bX2PoLhygn1QcImlPVz3m9AIQQ1viFIWSZtbgpVJsPQ4?= =?iso-8859-1?Q?wNPbMcD7pgXRgbfClfWe588EkRl53F4KyqaGthNux412UxPs/5Eh9oJOj7?= =?iso-8859-1?Q?zC9I+d+agEGbpwOgpnx1Gz5g4ExySKWQ8sM7EAGZ5amRVGqoVQnkGEfTqn?= =?iso-8859-1?Q?0YO4yWYvA2aoJUIw8DUmBytVaOpDkRFmN9G+4rLvoW7e34U3hSIW77buRs?= =?iso-8859-1?Q?vMr4McXMRZ9EOcnkWTUYuZPzwpiinAhdgenKpVsP9pipbFULkCSd4dJQVo?= =?iso-8859-1?Q?TXCAgc5uPCifKy2QZhheffFAY7cLCD4QNoo67MQgsFp9l/mD5dftt2vKPS?= =?iso-8859-1?Q?O8zccQK+eFYDQ8lbRwvLi7oq7deBH2n4DylY6U9fSzRyi8lUgjAqt4TENh?= =?iso-8859-1?Q?k13F3vLptfiyakmXXdU6oJL87MHq1XoOJO6Ck3cqX5e7lAZG6JvY1NMA0A?= =?iso-8859-1?Q?UaJ24hcvjEbGE0rnBNJJcazCymnqk5kOGAgPE1A33sMIbikizawNsf6TlZ?= =?iso-8859-1?Q?eq47G8RTCeuGfBZkN2ttqszXh8C4ZjXwamMtRKu6iQF26+9wL/2Uqt2GBO?= =?iso-8859-1?Q?oXuE/aRcGqD6qdfuDBbS7NzvFQsoo+JkoyfWAEWCwTMmvS2a+uRlF9FAP6?= =?iso-8859-1?Q?UzcTgHS/7epnWDFvGBeNNtPF1FDOhPjiGebaksRHSY2BLnotkouDv8ZLuT?= =?iso-8859-1?Q?K5ye2ocqI32TwEZ/bXSOdcaO5a8ttnfV1d0xYhbLoIW90HYg6hArA8UEsc?= =?iso-8859-1?Q?3dwYex+c2VP/ApgY2297T+Q0fbVVcQRWoHWt3+8OXkkK7kMZRYuH0zAQ?= =?iso-8859-1?Q?=3D=3D?= X-MS-Exchange-CrossTenant-Network-Message-Id: db42fedb-74d1-4216-4de5-08db05dee70a X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7309.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Feb 2023 12:05:12.2960 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: sCVaTj0OFmelUDVlE6ks6jTCk9jHuvbu3Bsr0iWqyvfBz375pTrNBm1NoIiz2OYAkNz7198JISHB1eYMqbuoSNfijtc9WzdcS9n3EY4mN0Q= X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5424 X-OriginatorOrg: intel.com 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 On Thu, Feb 02, 2023 at 10:26:48PM +0100, Morten Brørup wrote: > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > Sent: Thursday, 2 February 2023 21.45 > > > > 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). > > Why would we use a signed socket ID? We don't use signed port IDs. To me, unsigned seems the way to go. (A minor detail: With unsigned we can use the entire range of values minus one (for the magic "any" value), whereas with signed we can only use the positive range of values. This detail is completely irrelevant when using 32 bit for socket ID, but could be relevant if using fewer bits.) > > Also, we don't need 32 bit for socket ID. 8 or 16 bit should suffice, like port ID. But reducing from 32 bit would probably cause major ABI breakage. > > > > > 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. > > Signedness is not the only problem with the socket ID. The meaning of SOCKET_ID_ANY is excessively overloaded. If we want to clean this up, we should consider the need for another magic value SOCKET_ID_NONE for devices connected to the chipset, as discussed in this other email thread [1]. And as discussed there, there are also size problems, because some device structures use 8 bit to hold the socket ID. > > And functions should always return -1, never SOCKET_ID_ANY, to indicate error. > > [1]: http://inbox.dpdk.org/dev/98CBD80474FA8B44BF855DF32C47DC35D87684@smartserver.smartshare.dk/ > > I only bring warnings and complications to the discussion here, no solutions. Sorry! :-( > Personally, I think if we are going to change things, we should do things properly, especially/even if we are going to have to break ABI or use ABI compatibility. I would suggest rather than a typedef, we should actually wrap the int value in a struct - for two reasons: * it means the compiler will actually error out for us if an int or unsigned int is used instead. This allow easier fixing at compile-time rather than hoping things are correctly specified in existing code. * it allows us to do things like explicitly calling out flags, rather than just using magic values. While still keeping the size 32 bits, we can have the actual socket value as 16-bits and have flags to indicate: - ANY socket, NO socket, INVALID value socket. This could end up being useful in many cases, for example, when allocating memory we could specify a socket number with the ANY flag, indicating that any socket is ok, but we'd ideally prefer the number specified. As for socket id, and numa id, I'm not sure we should have different names/types for the two. For example, for PCI devices, do they need a third type or are they associated with cores or with memory? The socket id for the core only matters in terms of data locality, i.e. what memory or cache location it is in. Therefore, for me, I'd pick one name and stick with it. /Bruce