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 CB7D4A0352; Tue, 25 Jan 2022 02:14:06 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5D57442725; Tue, 25 Jan 2022 02:14:06 +0100 (CET) Received: from NAM04-MW2-obe.outbound.protection.outlook.com (mail-mw2nam08on2056.outbound.protection.outlook.com [40.107.101.56]) by mails.dpdk.org (Postfix) with ESMTP id 93D1441186 for ; Tue, 25 Jan 2022 02:14:04 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=L5hpWo4Zx2JjzOdXt476MuCJ0Jou9SZYgOk3OsLFz9Kn7KNMauwIU/nqnaL4nkN+GNTUPodO/y7ejdIyXyEyZXG5XIXSq577E/EBJfpjRtBHRTKMK6pxBAn5LNPlHrRRyzAnbwgdRV++kb0Pq3699kkY7heR/+5QXESTvNw7fjxDu2uzDrk0GWzHqGlIGu85WXnvuQKXMHImrBXVNbiffHn94oX8BpH+ckYXgEZJoIRE3E+OTqLjPPaZUvvIg/CSgTuhpXRRMY78DzlgEwyrGWPrMjxYow2nh7QGfsBmaQvXSKLBj2jjUQzsObHoPdQyOdYeynvBMNl4vkTiG9lPjw== 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=0lXA+C+rBuDoWCIR9RlVDKl2Uwk0iJqPgmh4JpwRwr0=; b=cfUNYaM6DOCnhPODLfV7ega4ZcC0mB2J3G1RSxDoyHp2CXmWlhdS6HNLHdK0qU1RF8sbc3hBzNTw5RR+no0haBdn43AQQ4Vf1fsGfsWGe09b1ikMSxbkYEbvSQ40hB5R7yRDEH80IvYPTyKAiqCAdsw3n8PEhUy6NEtNTi5rl8XG9nlD4DBwm8Clx62B/CGEMPHqTo+cWwFClLjis5D9X4i9vV79KITprnalwbV7xqtK0ElwRwVAq7rwPYE2BbW7+7dUZRtW5WSDFLIplOa6+NWwDzdTjj7tiXZIiC89DeYonQ6hd+D98LRvaXWUmpFOR6lQePPD0a3UPCrzTiAWiQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=0lXA+C+rBuDoWCIR9RlVDKl2Uwk0iJqPgmh4JpwRwr0=; b=p4lbFKKubQFmyLDldl0i121MOYc7j9ht12wu3CB1+UmOASykmAuM17zfSvxlDH3JeU69zzregI4YOy8g9aJtFvY++pEW7OBdrBQ4h8+Fj8Qf0NJ1Z7TXPxjCnDtlynEUyK2W49H9zblXkSPUIPjytIfJnGR6M1uTlee45MjXxJDP9Pl1R93ZBNjPRonvR7zdy1xxMGA1vkveR05E9Tc/DAFQn2JHVMcFJ5EbLWL11FTXe4J7ccsFwi3rDmqBd6HEawZXWyu0rXFYNZYj2t3Za6lzEXeVP1a0zpYd5Za6hqDK/+YZq4V2uOtzDZs/Na8bvDhE0sUEMHwZrZsrpanVBw== Received: from MN2PR12MB4488.namprd12.prod.outlook.com (2603:10b6:208:24e::19) by CH0PR12MB5172.namprd12.prod.outlook.com (2603:10b6:610:bb::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.10; Tue, 25 Jan 2022 01:14:03 +0000 Received: from DM5PR12MB2405.namprd12.prod.outlook.com (2603:10b6:4:b2::20) by MN2PR12MB4488.namprd12.prod.outlook.com (2603:10b6:208:24e::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.10; Tue, 25 Jan 2022 01:14:02 +0000 Received: from DM5PR12MB2405.namprd12.prod.outlook.com ([fe80::cd5b:cd8d:cd38:8c31]) by DM5PR12MB2405.namprd12.prod.outlook.com ([fe80::cd5b:cd8d:cd38:8c31%6]) with mapi id 15.20.4909.017; Tue, 25 Jan 2022 01:14:02 +0000 From: Alexander Kozyrev To: Bruce Richardson , Jerin Jacob CC: "NBU-Contact-Thomas Monjalon (EXTERNAL)" , dpdk-dev , Ori Kam , Ivan Malov , Andrew Rybchenko , Ferruh Yigit , "mohammad.abdul.awal@intel.com" , Qi Zhang , Jerin Jacob , Ajit Khaparde , David Marchand , Olivier Matz , Stephen Hemminger Subject: RE: [PATCH v2 01/10] ethdev: introduce flow pre-configuration hints Thread-Topic: [PATCH v2 01/10] ethdev: introduce flow pre-configuration hints Thread-Index: AQHYDIB8ez/3PxqCtUa/zNQfuwX8BqxyRq0AgAAx7gCAAAL1gIAABlgAgAB12xA= Date: Tue, 25 Jan 2022 01:14:02 +0000 Message-ID: References: <20211006044835.3936226-1-akozyrev@nvidia.com> <20220118153027.3947448-2-akozyrev@nvidia.com> <4371468.LvFx2qVVIh@thomas> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nvidia.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 795c2feb-d22d-48df-5ddf-08d9df9ff925 x-ms-traffictypediagnostic: MN2PR12MB4488:EE_|CH0PR12MB5172:EE_ x-ld-processed: 43083d15-7273-40c1-b7db-39efd9ccc17a,ExtAddr,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: /hpWwflntwmFMlNXDDLdRUmjbLVCl1GhJ8Bx8erEMsOrFh+HDFOeW2qlopYxVPKQSyWQu6F14u8AuKgAcEVIt3+LtU0jQMBsoomyrTY1WZ6ELGywl3wIIZ3H7OQy7KaMlYEyfsNGwZnWnv+hAJzjKXGUwX6oCeiqQVICQyztcoPjPZuSFy98t9j1bpNkgmJgff4lEgJDfWMv0igA2QRZC6Bk88pvkuCKBve0UQ5JOSDIH862ktJqF54eCTXhdEZzPrDapRR1CpkwLlhdh1IyycQQ8DLQ2zFtw325x/srEjKhYVxp6BKrErQ8UblpsaKRg2t8+C2N7j9qg4HlBc52YJ925UH9rn+PemgiQE/bswY0NcdeGYz+F9EkXxCP+1MsIAFGkxTQKPS41yerRUKTmvYBiGtiP0BPZt2PMQSgDAAsFARqxnZpk1x366cpWBgT8OYnaC1JILPsrVpsNYVgJAigUvP1EJpsXrdFQxdGxYsx48M67Uo80LAyaQ63iKirblNbUKy1PAsFnFxczF3v3zFO7mMC0/G+qgmtPV7wZYntfxFVw0YqkGkdSyDYoz9zD43pg2+o0Tqpk9iRavPplV+Rv5n9GL+wYkpLmWS4YdIZ+YLskeaVWVvvKTVVMgdAN9ElfIiQHHYhXncFhzY/nHObZ4HbPCCTK9ZH92UTvU8ybqYtoU/n5vG1WvLbDQVYOfKcwuDVcACh2STVMPe2R0ZRmUlouxdFRpvNcglE4BK5k4PCG3u6R4pBkYf97qhQkXClpgm1EKMv9OEGwFPjwNS/+RUi3nUNQQkvb/eEJiM= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR12MB4488.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(38070700005)(966005)(53546011)(33656002)(9686003)(6506007)(316002)(7696005)(8676002)(55016003)(8936002)(4326008)(54906003)(38100700002)(110136005)(7416002)(52536014)(83380400001)(5660300002)(66476007)(66556008)(66946007)(76116006)(2906002)(66446008)(122000001)(86362001)(508600001)(71200400001)(186003)(64756008); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?a0vNddHYCKhqa4cYj1Nh8dAFG+sVYAVLxuF9Cxc6RBbmYPKAjXcee1vbF7ha?= =?us-ascii?Q?JCK71D4XWVu3jABFQO0QjIHmwFL6g4Tgu+kj9fE5YP0f487NH7LOrNAP1kFm?= =?us-ascii?Q?NA1RWgGpVqpKw2KOZL2lpwaa4K1LZfZTx1xb1N+CcjB6o8KcsD3WVFx57Iyg?= =?us-ascii?Q?NCBQE4TWFimBv/oXR4SEJz0SHVRIl/0wRTFMFx9kGjMe/SFHN2k3dDRSr7c0?= =?us-ascii?Q?DuxHXfUn/dUQX8DRBNI9mL4zbSrbyXzoBor6by4Hx1o225fabI36XeTxjaqG?= =?us-ascii?Q?ICU1JCeMFK5IFtW0fMNL7usExkywwDQtG9M1PTLstyqaVHt3Gu2oz8OYwUj4?= =?us-ascii?Q?43GTq7y50hmPQspoXQjOytaKCeL5CuDvndb88gDnMgYAKCQtY8yVMbteA0Ra?= =?us-ascii?Q?9o05RewNfBVeYCZCp/6MZpV1yjuW0+faRUzvNTmuc6g3PRfSm4U9ovLXRJrv?= =?us-ascii?Q?OtbJ3pOAGQ0BFCeshXF6iOsVWbAmVsJKXCYFSfnYGp0QKMzwbBp1TFq9320C?= =?us-ascii?Q?Iz1gM4kzH4dDNaR0TMCgSJP5kNd63maf2eF6l0uGWMy1LpOAHduD3zViYiCk?= =?us-ascii?Q?LtXnhdYOB/6crsXOGKvE+rBdloxwhrPP9WIQNjnrsx+Ty3K7SPFpvLD/smNA?= =?us-ascii?Q?RBmQSwQO6O9YFb7NcGIcxmjOMJBNPc8UakY2FNjwtfygsI+VApvyy7cn4HeZ?= =?us-ascii?Q?a9O0ImZHLk7d1nWs3zYvSHILo6Z0od1kH31OLqVYb+ECgsq2Vp68wX4aQcSD?= =?us-ascii?Q?/r+AkslM79g+fdz2P+1uTZe1TRMb33hlBctUsenJ6kptNhN2mEbhBf5ZVJXH?= =?us-ascii?Q?ZVQY4PVcOn0xjNL13Jy7i8uqomV92OpB0VqMPcgbXr4Oy74CYcYijGQUoyfr?= =?us-ascii?Q?MSj14URT9ExjRZ39L5dwGMbEV/GHmjIElc26J2oq66JJhWpcQNjNVorWlyC9?= =?us-ascii?Q?haqJgBQ/7kcRkOyToXzV94mPm1jEMMIhSX+9W6SACvLimRJmdOAAadYZGBlM?= =?us-ascii?Q?WJgMdrLOVLrWaDIuW7fycd1gx7RgLTXJAtLzY4HJt6a8IRqboS7RsPoGO/H0?= =?us-ascii?Q?oCOtB9RxE3DrTCG5ubd/60Fz5ut50BSUlB41UNzv+wxbyZFW8nqYQ1V9Uz4Z?= =?us-ascii?Q?v/Qc1Gne58+TXcVCYQlXauM09doGgCiZiTu5DBsx33VwrxiPSdxqtThNELAn?= =?us-ascii?Q?jRyjj6CFSwxqOg0XXoJ295uBFUwRRwjaZbYF3geTbFgtFfhHzLfJddc1uhH9?= =?us-ascii?Q?+nBJtKbbgLh7MzGuN0mG8dzHWqBXFfUn2q0KAW3LYa6kx/C0qCciJY2H6Q6N?= =?us-ascii?Q?8sJiAWxIJjcVcdilo5XhiNX8vV+NO/R8Au+GyA8K18Fctm1tflhG3o3NMKOc?= =?us-ascii?Q?CTPVifp7Ok/29zPTOEJLQpfPuAH26LUDivnwmBo+kYXya84284LjiV7t+PYI?= =?us-ascii?Q?/hdhje0Jn/9/yEQXg9yq9f5SYKCATOxFEaU2OFFG+fRlySsDxWDWiaENaF5M?= =?us-ascii?Q?sjoOKzuP5BpeflEVCrYCgo4EaEE/IwUuOZyZpMXPYWRb1MM2cmMW/eFeqoa7?= =?us-ascii?Q?CMgG1Yp4/dxOfoAj96DeN3Rpkweq5gPju5q4sSvjNFucDQu2ve8zSYRTXz8r?= =?us-ascii?Q?Pfs3VDxWlTIQLeNVF/0Zya9PNJZ5SEx1ftDhhEekkaRrCF7GnD5o9ywt9XeE?= =?us-ascii?Q?1ZmpCL4lXxAUbEka12Nddf6dxVk=3D?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB2405.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 795c2feb-d22d-48df-5ddf-08d9df9ff925 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jan 2022 01:14:02.1311 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 3WPO8LqEhhs8T8I2sGEify/ec84xunZP3XnDLj+ERwNSE09+opHBy3UPcRB6Z6ifpZ/6YRAyUc3mNRVniWkJYQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5172 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 Monday, January 24, 2022 13:09 Bruce Richardson wrote: > On Mon, Jan 24, 2022 at 11:16:15PM +0530, Jerin Jacob wrote: > > On Mon, Jan 24, 2022 at 11:05 PM Thomas Monjalon > wrote: > > > > > > 24/01/2022 15:36, Jerin Jacob: > > > > On Tue, Jan 18, 2022 at 9:01 PM Alexander Kozyrev > wrote: > > > > > +struct rte_flow_port_attr { > > > > > + /** > > > > > + * Version of the struct layout, should be 0. > > > > > + */ > > > > > + uint32_t version; > > > > > > > > Why version number? Across DPDK, we are using dynamic function > > > > versioning, I think, that would be sufficient for ABI versioning > > > > > > Function versioning is not ideal when the structure is accessed > > > in many places like many drivers and library functions. > > > > > > The idea of this version field (which can be a bitfield) > > > is to update it when some new features are added, > > > so the users of the struct can check if a feature is there > > > before trying to use it. > > > It means a bit more code in the functions, but avoid duplicating func= tions > > > as in function versioning. > > > > > > Another approach was suggested by Bruce, and applied to dmadev. > > > It is assuming we only add new fields at the end (no removal), > > > and focus on the size of the struct. > > > By passing sizeof as an extra parameter, the function knows > > > which fields are OK to use. > > > Example: > http://code.dpdk.org/dpdk/v21.11/source/lib/dmadev/rte_dmadev.c#L476 > > > > + @Richardson, Bruce > > Either approach is fine, No strong opinion. We can have one approach > > and use it across DPDK for consistency. > > >=20 > In general I prefer the size-based approach, mainly because of its > simplicity. However, some other reasons why we may want to choose it: >=20 > * It's completely hidden from the end user, and there is no need for an > extra struct field that needs to be filled in >=20 > * Related to that, for the version-field approach, if the field is presen= t > in a user-allocated struct, then you probably need to start preventing = user > error via: > - having the external struct not have the field and use a separate > internal struct to add in the version info after the fact in the > versioned function. Alternatively, > - provide a separate init function for each structure to fill in the > version field appropriately >=20 > * In general, using the size-based approach like in the linked example is > more resilient since it's compiler-inserted, so there is reduced chance > of error. >=20 > * A sizeof field allows simple-enough handling in the drivers - especiall= y > since it does not allow removed fields. Each driver only needs to check > that the size passed in is greater than that expected, thereby allowing > us to have both updated and non-updated drivers co-existing > simultaneously. > [For a version field, the same scheme could also work if we keep the > no-delete rule, but for a bitmask field, I believe things may get more > complex in terms of checking] >=20 > In terms of the limitations of using sizeof - requiring new fields to > always go on the end, and preventing shrinking the struct - I think that = the > simplicity gains far outweigh the impact of these strictions. >=20 > * Adding fields to struct is far more common than wanting to remove one >=20 > * So long as the added field is at the end, even if the struct size doesn= 't > change the scheme can still work as the versioned function for the old > struct can ensure that the extra field is appropriately zeroed (rather = than > random) on entry into any driver function >=20 > * If we do want to remove a field, the space simply needs to be marked as > reserved in the struct, until the next ABI break release, when it can b= e > compacted. Again, function versioning can take care of appropriately > zeroing this field on return, if necessary. >=20 > My 2c from considering this for the implementation in dmadev. :-) >=20 > /Bruce Thank you for the suggestions. I have no objections in adopting a size-base= d approach. I can keep versions or switch to sizeof as long as we can agree on some uni= form way.