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 3F1D2A0352; Tue, 25 Jan 2022 16:58:52 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 86BE4426F5; Tue, 25 Jan 2022 16:58:51 +0100 (CET) Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2050.outbound.protection.outlook.com [40.107.236.50]) by mails.dpdk.org (Postfix) with ESMTP id 64CDE426E4 for ; Tue, 25 Jan 2022 16:58:49 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=A617v6yq+WTKIkkOTkW/BA6jzlXe5X6BkbUiaZvGnLMQXxStIkVlxw9rerX3djix7hN4v8NHL4rx1cTgC30KvEUmgB4vxPiY9cgMeQ3EcVwLPca6FaLUiWyJuhJepE2xJW6RiltsKuIy/BJB2a/S0itKdDtb4HAhhi+q9SYuUNq3sXHHM3Gfn4uaZai5lkzE5ZmhTd4gG+hCnurPIQZQ1tcgt4oWrI+0sKYzU1ak5z9Zk1QKxK7TiaVYJisdOuCXSe+B2Q4LHE2CbMTERL/m4vHoOY024Mw9Iw1sG5pSFENPWVa7hcuLQ3kgpxyye6vRMIvgKExFbyi8HE0ANleEeg== 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=D+v+cj9lRfswb2ipijmwHbN6aOOBcBoLA6s/ypMQPAI=; b=nPTBsGTOJDZX0kpFBhnVW1F5cIahKdmclMXRVASH77qdqJqhktSzPhfI0jrYmG4Toa55aSz4rUICa3rPadtyV5iYrk2uRWs8VAl1y4mQLEQGlElyTgMep51vmsQQ1TUSjRiCHuweooPsKxg3D7H4akPzM9dee9q1CG3MqL7wcbGSd/jj7muGpejSnXaNXyIKMlOx0XoIitZEWrBQR398Z4eE31ZeYbD6ANpo4tSU/wsK2M0n0Kr+4PYx50rrH24tNY3CD8BkdfU8DwX0x0tkWtIAthY1qIXX57L5ciqVHXJ0EDyMe9IGRE1vYXP5/yJyALzLTr5mi8pj85DYh9qYXw== 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=D+v+cj9lRfswb2ipijmwHbN6aOOBcBoLA6s/ypMQPAI=; b=j4+Buyd4mDNCw+pvQAC3LPAyWsgRuvgi3wFas9yp0PccV1PeTWie7gDVUJf2gPAbsIlsDCF2LiyI7T8Fg5s1nvffKI36r7eG1CWVl+sH+cm6YEqXeSonYwPznmg3ksIHj1rAQm0szGZaV4aoqPJT0rpT6mHb9n63Fi4mFQre/sVNdbLn1MV0wd8TUECGtZrHcdGBHa3P2itmd4EU94zbqnDF9Vgh9gfqaZo4RdT7hluR3fjWE3TIVjPBh3GWlNG2CkotXnrq/1LUz8jkEof4fsj6zxl4f4/p6SXr/eSGQ9D2fxR/Hes/kf7GmLyMwOxxB6QbDBdLWdgIKRy91iKDYg== Received: from MWHPR1201MB2526.namprd12.prod.outlook.com (2603:10b6:300:ec::7) by CH2PR12MB4024.namprd12.prod.outlook.com (2603:10b6:610:2a::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.8; Tue, 25 Jan 2022 15:58:47 +0000 Received: from MW2PR12MB4666.namprd12.prod.outlook.com (2603:10b6:302:13::22) by MWHPR1201MB2526.namprd12.prod.outlook.com (2603:10b6:300:ec::7) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.11; Tue, 25 Jan 2022 15:58:46 +0000 Received: from MW2PR12MB4666.namprd12.prod.outlook.com ([fe80::78:438a:c6b7:1cc1]) by MW2PR12MB4666.namprd12.prod.outlook.com ([fe80::78:438a:c6b7:1cc1%3]) with mapi id 15.20.4909.019; Tue, 25 Jan 2022 15:58:46 +0000 From: Ori Kam To: Bruce Richardson , Jerin Jacob CC: "NBU-Contact-Thomas Monjalon (EXTERNAL)" , Alexander Kozyrev , dpdk-dev , 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: AQHYDIBlr97eIE8EiUKphVVj8UOlEqxyRq0AgAAx7gCAAAL1gIAABlkAgAFK2cA= Date: Tue, 25 Jan 2022 15:58:45 +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: c4bf8c23-74e9-4269-5638-08d9e01b9194 x-ms-traffictypediagnostic: MWHPR1201MB2526:EE_|CH2PR12MB4024: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: VqyIl+qPedbSoVbfZDeoBwvw5sw8bNJy5mVi5buAATaXNGVLM+wwNDzr50UI6rAsC7ccCLqL+6ujiKkoj9s9aDXxX0I01+Y6sG5cQhnX2dSzSzFVFPxrKNfiqEcUt6A5r2oWHTBMyCXiw9KTMgrSRKi/LsrJuRLacH6y+r/MEvbBHFJo9fF9zepzbb/eoiyp9HbHaSF05e3uxUTK+VCO3PQhTu5ih233Tedy4E2kR4yqttsKcpJQnDYB1eu2B4k9GnS2pGnS3IA6PRkTNI12Mzck8lI7+YS2ZtKbxlNZTpDxj/JqH8geAIM4lSMQpljVemXDO11C6eZeymlIiVwM5UFxQwM9TNpMxCJbCpt1x88IVBVpr/6rZsrNlI6c2lRaUgxIltzOOzxq6Dvq+GNkcB8axNEO7CxvaV7STOZerEU0/1+ieU7eLDIXYlldoUy9YGUHyhKZ4VOYmXwNW/0/eLN/wgopI2KCdM7mt5v7JNtvIU0PpPzp17TtvvwQmeVdyVDwabh1k0QoPiqs4YRuPhFbNLqUcy+dS0+dr26ulQQu1NoWdURmqgv5lvkEdljWtYk5rYkPFanmVbjkr2lDNSeaE7uPnyF5/zV0Rzy1KP00bjJdrlomAsXW+/qLgkBCaZHVpzOL5XDmIbdd4owsJkzyrNDJzZk/z+9l8yVz21jCuB2HSJHjFJWD83KXHeeICz3ApM3Rg3+V4jGfIxzVLTMZ8I/nmAHbi2tUOLZeojwlJ/2nOfkMWqxgY9HeK1vEK/d0+1f5CiNu9DKEbqEquPkg0I5XdHahWY1a5cyJKc4= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR1201MB2526.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(8676002)(2906002)(66556008)(76116006)(966005)(8936002)(38100700002)(6506007)(316002)(86362001)(71200400001)(186003)(53546011)(52536014)(122000001)(33656002)(508600001)(5660300002)(9686003)(26005)(66946007)(7416002)(64756008)(66446008)(38070700005)(110136005)(7696005)(83380400001)(54906003)(55016003)(66476007)(4326008); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?sJhpYiVPO5LPn+e9gvQFAerGeojLzhXZ9Fkh4bJrFt36M9NP1fjuWLrOQ0sY?= =?us-ascii?Q?2FnDeicIBpGx23oSLxCeqT2cNO3h4O9V2QyT7IDiE2VeyfXyBelW8isPoTSh?= =?us-ascii?Q?v5Xvh58ksjEI5LAmCqhv/CseT/C33xb/DLyj0YNkbWmPHsCFCw4PbuFjtktR?= =?us-ascii?Q?jKTNTVQLuuS3iZnZj8t+6fUU3W9RoWTOSJhLl78hzShnrpzRbufecMwZI6GI?= =?us-ascii?Q?gONFEn0xWjekljoX2ieb8zm8JC6LYyy8+JmBhT5IVzHX2lFK5A8ELkiaJ0je?= =?us-ascii?Q?NJkkZlpU6q9Y9kDsf3LzHkQYTiGwXPyxSP0WyACOzFcis6dCaYbjbSZnL2mA?= =?us-ascii?Q?ZSQOmSiyl6tR3rs8okVeHfGha99afYWhyaL+iwVOEMSDVwTxFGu5KIaP6epP?= =?us-ascii?Q?fv5xgVRSAJnqSKKT2nz55EIix/d/3LTHAM/z3JUvbq6PlO9Z/xoUeIQberO1?= =?us-ascii?Q?+X4oT2bzu4BArgAiKdoj5rGA6gnQSK9Okq8IJlye3i2wviiAowI0PIcw7ES4?= =?us-ascii?Q?nUzAaoJ+5/d56SsNRwE9V9R1HvVNLtFM0GM5hhW3mBnMQ47hoJes/aVh4vvq?= =?us-ascii?Q?IFQ459+ThAO53lUsiTFayZFx3GrNl1isZUMJRyL2nlLqhbjCTAhi9EkgUQyo?= =?us-ascii?Q?k9sGzSVdkWJZflQ9GAubelrdLtWD7R7jnbbFjRVhXHeVjts+b7SzNZjhrtNX?= =?us-ascii?Q?C83i4HD+GGjdJ8FkM9Y5XcCMPYETwK1r1vaWw0IUj3ZMYHurn75JHwrNO0Fk?= =?us-ascii?Q?UfBCOFNQwsQFH7QXLSGVKe+W0wzjunMTOKUKjQuOs7oLDBj/9opM4gxtTLXu?= =?us-ascii?Q?BuLVqOw6J7n4aRwFpvAQBa0Es/nmUavgis6K5ay5AjfxfdnUO1a25Ovt1wME?= =?us-ascii?Q?Mr3MOYodleMzZEe+JuM9NbC1WDLEzYU+RmTcdx0vOIXJDicNQ8UaR+VNvJx3?= =?us-ascii?Q?ne/1z1kvGPdLYXgsr9rt1WuuqoZwmfcQhga17l0m7OZs2Jo70K7ObqsTPoHY?= =?us-ascii?Q?ShKIGof2jtgPh/Vv35+U1fd97SYShdB1OP7/fucPtQV21gahzMP5mzWOoaHb?= =?us-ascii?Q?RRmwwjPFHKeoXl53PXGw3ETVRwhMSik7g0zG9PudE53kVRfO9CtObElsYb9P?= =?us-ascii?Q?+nZmZGwbEZRa9aA97t2TEkkrm++ytcE2WdCxfjq2/gHxW36njhXLbkuqnnkf?= =?us-ascii?Q?HeJ2y4g1bpj20T+4lU5d4HFk62eqjTORFJHQ9VnXytWvMrdrv8ZF4fuxCSjL?= =?us-ascii?Q?Ev9VNZFCweRIje5y9U6OBGOkEYwW34mj+0Pr2sCvtqiRjNi4hpQQcxy6YmfG?= =?us-ascii?Q?wpfZt/LVGroedns1JiJl/LU3lJzYooZ0RfJ91KXasrbzfR55Zfy++3tqjCmr?= =?us-ascii?Q?HNuhnwJ7QafriyAp42zKZKz0nD9OpoPUJGhuBCegnqRU2sG6PudywebP/e4O?= =?us-ascii?Q?fWNOyjDkL+ejSjUNlPX7SOTSC6y2tW1MykW5GZioOyQyi/CdZFpBM6VIbGQJ?= =?us-ascii?Q?XnlRZsLTbLO5igWzkALU7cbCZ2oG920Yvmj6Ik7th2pdfIWBSUZgXvwgFTs/?= =?us-ascii?Q?JH6oeZBy4mJI6y3ihGANPKGkofxpY6qdp4hpmNhfWHGiESXE7i8H5MDrOQ0q?= =?us-ascii?Q?3wAqgdHB9+LLRVMnAq/9rF0=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: MW2PR12MB4666.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c4bf8c23-74e9-4269-5638-08d9e01b9194 X-MS-Exchange-CrossTenant-originalarrivaltime: 25 Jan 2022 15:58:46.0167 (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: ABD7UWZDvH2pWnYRjpeNI3gfC8eC9zCXgAlh2H5HmBOnCuAo2ORtvG5gA+ikXthNVrfvB9F4/6NcN0FesyuPxQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB4024 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 Hi Bruce, > -----Original Message----- > From: Bruce Richardson > Sent: Monday, January 24, 2022 8:09 PM > Subject: Re: [PATCH v2 01/10] ethdev: introduce flow pre-configuration hi= nts >=20 > 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_dmade= v.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 simultaneou= sly. > [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 Zero can be a valid value so this is may result in an issue. > * 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 This means that PMD will have to change just for removal of a field I would say removal is not allowed. > My 2c from considering this for the implementation in dmadev. :-) Some concerns I have about your suggestion: 1. The size of the struct is dependent on the system, for example Assume this struct=20 { Uint16_t a; Uint32_t b; Uint8_t c; Uint32_t d; } Incase of 32 bit machine the size will be 128 bytes, while in 64 machine it= will be 96 2. ABI breakage, as far as I know changing size of a struct is ABI breakage= , since if=20 the application got the size from previous version and for example created = array or allocated memory then using the new structure will result in memory over= ride. I know that flags/version is not easy since it means creating new=20 Structure for each change. I prefer to declare that size can change between DPDK releases is allowd but as long as we say ABI breakage is forbidden the= n I don't think your solution is valid. And we must go with the version/flags and create new structure for each cha= nge. Best, Ori >=20 > /Bruce