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 530DBA00BE; Wed, 29 Apr 2020 10:46:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2C12B1D98D; Wed, 29 Apr 2020 10:46:01 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id BE7D91D61D for ; Wed, 29 Apr 2020 10:45:58 +0200 (CEST) IronPort-SDR: 4PZIS9NueBRlivUmmRQKAQH8nbefD4f0QtL9v638ctUZIYq1CLXqAu4bwcMXLyPt8Dd3tBH9ZH X3SmXh4E/X8w== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 29 Apr 2020 01:45:57 -0700 IronPort-SDR: /Xl6FWKWQrcFrh6e+inTX4Ukp2kcGGjeiutozQw1hOGcNGCxQ/Wv6jUex7A/YrV2ns6DjnDNe3 Iuy2laynaXtA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,330,1583222400"; d="scan'208";a="293140372" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga002.fm.intel.com with ESMTP; 29 Apr 2020 01:45:57 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 29 Apr 2020 01:45:57 -0700 Received: from fmsmsx606.amr.corp.intel.com (10.18.126.86) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Wed, 29 Apr 2020 01:45:57 -0700 Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by fmsmsx606.amr.corp.intel.com (10.18.126.86) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Wed, 29 Apr 2020 01:45:57 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.170) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 29 Apr 2020 01:45:56 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=QfB3ADCQ1lZsP9UQvFjCBRcYX32MbGsXPViEe4qKlFOTNMglb2Wm5CuEtk3t26NnnFj2GRApv0rXbZwuKJyU6AreByV7pJS/TXAxa2CX9/wm1PEru5RgrgmeMoT1LSOK/7OxHD1wJDJKIBe2uc15vrxHolbbOrzIBJt/5Yg5EeMR5/99xr24Eze5GePNj3VYYyPgZKeRAMBu4B7kLnmZDxkT0IBBgEF/T95/9pQsJsC3vz11vBnVjEs1NeSabSEzg9oUFNqDZzeuWdz7kyQUSY8iwDNhkBX7NsMtFTyIbsZajiTTdCj+DqXMdhLEaKm4eVJOJcRnBCs0kU/NprD40g== 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-SenderADCheck; bh=UnKOHbWz9DP2vxnPrOXXtpqkkx3iP8XsRP87utW36yg=; b=LwE4/PcKfy3vx7jNFvEz+6/hOQ8BBpvXyVrZQuoxgH0FdkJjw6eFw7Wd0UawsDi4diaLqlXUDo7MZ0BGNnv8dTQwdzbEhRo0DlGwnz91DzMn0zjeVIixqFK/BLEVxjo4HZOpu7Wccpm6n2zPj3Vq3x8FrkXdTEnMcXe48G4EFUwN4OoHIZR5KM177uplDk779XdLJOcWT9mDm8A4PzqEWEoM+I8ha8uU7kfBdapgfaQSOsIX3JrEpW5PkRRo/2OG+H/idreGjRp1t6M9BPyvO51iYBteXDsTVRJiTL/RsaI+VlgWRapItiFMchUciwwr1nATSMJnOtPWTatAoCC8iA== 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 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=UnKOHbWz9DP2vxnPrOXXtpqkkx3iP8XsRP87utW36yg=; b=zdfLQTpeB4LW1RVpPORcLNz4L43EVbhzZgHaXKhnGZWq2tkqLr7VQEnTITQ+uqm5WZLeHH0r5pAiIlJvmIoMg2TqmuTwrJHzzavqpt2qzZ2IWWhw3vNuAcLKnxpEAt86qoKxysnEYgtFcd9uyoFlbt0EtSXEUse0zVU8VRihIYo= Received: from BYAPR11MB2935.namprd11.prod.outlook.com (2603:10b6:a03:82::24) by BYAPR11MB2710.namprd11.prod.outlook.com (2603:10b6:a02:c7::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2937.22; Wed, 29 Apr 2020 08:45:45 +0000 Received: from BYAPR11MB2935.namprd11.prod.outlook.com ([fe80::786e:a42b:df03:a829]) by BYAPR11MB2935.namprd11.prod.outlook.com ([fe80::786e:a42b:df03:a829%5]) with mapi id 15.20.2937.023; Wed, 29 Apr 2020 08:45:45 +0000 From: "Dumitrescu, Cristian" To: Thomas Monjalon , Jerin Jacob CC: "Richardson, Bruce" , "Yigit, Ferruh" , Luca Boccassi , Nithin Dabilpuram , "Singh, Jasvinder" , Andrew Rybchenko , "dev@dpdk.org" , "jerinj@marvell.com" , "kkanas@marvell.com" , Nithin Dabilpuram , "Kinsella, Ray" , Neil Horman , "Kevin Traynor" , David Marchand Thread-Topic: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt mode Thread-Index: AQHWGMqR3VqxJG8NS0ipNyOJfkoYrKiIFDWggAI0/4CAAmzroIAAdZkAgAAEp4CAAAXJAIAAAscAgAFh3QCAAAr4gIAABVUAgAAN4wCAABy1AA== Date: Wed, 29 Apr 2020 08:45:44 +0000 Message-ID: References: <20200330160019.29674-1-ndabilpuram@marvell.com> <20200428144535.GC1897@bricha3-MOBL.ger.corp.intel.com> <19c8b69f68bcdb7ac23126e63456223f7aff0465.camel@debian.org> <1923738.gORTcIGjah@thomas> In-Reply-To: <1923738.gORTcIGjah@thomas> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.2.0.6 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: monjalon.net; dkim=none (message not signed) header.d=none;monjalon.net; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.151.176] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: b74d162d-cfef-427b-dc02-08d7ec19b501 x-ms-traffictypediagnostic: BYAPR11MB2710: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:9508; x-forefront-prvs: 03883BD916 x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB2935.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(396003)(39860400002)(346002)(136003)(376002)(366004)(66446008)(316002)(64756008)(7416002)(5660300002)(7696005)(55016002)(110136005)(4326008)(66556008)(8676002)(54906003)(52536014)(86362001)(76116006)(66946007)(66476007)(26005)(2906002)(478600001)(8936002)(33656002)(186003)(966005)(53546011)(6506007)(9686003)(71200400001)(10126625002); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: /hehoItl8QrKZComH3/fmLYQ9IIHKVQSsR1bWWZlENXd06yyKbSLD8o3RlpScZgkOR9jGVGuwXeKOvJqLNxcBqFAkO2BwyW/760UapdLZmlxk4SdtnRHFevTCESt5b4nMmElAaY3V8hGJE3sLFktYgl9aCXu0hpV0wyOABINuaj52FUU331WADuBGRBIsbv/vlNHmOIY99e8TpeSB2mppwa2vCtpye7BHXBalZfm0gECx4QoE5jLM/V4tni9rYKwsA/JMBD/yshOwRp0aAt9UJLRGzeYscBuA/IuiS95DjLsnJ/FGNB97DS73Wb0IsHrerTvdEd1G5HpJZ7Y0OOIp2PIfP8DebT2EsOk9kTifFuLBBGKAte+ifj7MCB0zwMzZZ+O85yuvV4GBVk+Nf5y1+8eRBzYGETM24TuBThKQSRM8L3KMZ+RwqwD1JosJWonC5CQcPHSYLOo9hPCqgiAeboep7dnOo1IrUs9hjSZFTIlxwjNQcV4tUDMlaOZKna6/UfKi0XiVfqOtQLW6o6BZkvNgBUgP7TLKNgdzx8EbMkStg2jKPVvm5NWBZxYBQoBwOfTG63Rv+qMleSjGZqJuQ== x-ms-exchange-antispam-messagedata: tc+WGiiNNZlPhZ9L5/Ys6MVKaqpuQnyxWvlzl4WqguK8Yow9yRsiT3mpNo7mGJ7xkpkOa9ZIuvzwZZ/01ynB+o65Kxe8WZVgKBZqsPdNmArwxEWoQzBpEduX1d7UJucdvv7sx+OWisopZn4p1CnyNyu3KdFdnaDIOF7LKySp66Hr7EPVIFoAUvOzKMD/5Djovr0w2jY/zWjVaMpdS1K4DXvcIiAw1/0JF1JJMTvrQn3y+WlOp+/69RMiUag4il7Kl5y5G5vgQtThS43A/d6jLp9Q9d1pHbJNXvNAILEZIOhlUMvMuotYM/UGWKiJeupyakO0I8n+RC4dWNcpXrzJM+gmCGeWh9s7dqYTbN2cJgksMULGzqBqiwuYs0VQouaxjTovv5AFsuvSQA1C+zmVEeLZ+TVUf1TDQZSsMpSAoK5PGrF4oOHbEngYHoWqm4UC2RIof+yhsR81lLMZI/zUjj6GA+XcE5gbERyUdUEdNfuFjCZjwAqz/seIeQVvpYhoNVdQVlE5SFQvnjhYrVZxzGD/EJbQaiWq6J7fyR3YYDiGmNtyKjwdsIeXsiERqDCNVINd02KZRMyaUd7OCMC2GW4xSkoXmJ4khOmvQLx7tjPN3ptcddCughKG/V7f+GRMriUZDOlbkvYH6hXcIEmOnl4Wambu0a1yVcfIM2+Zuqze0ilU9Md5ccc+rFdkoG1yPlpZXxAX3FO1c6ztfpIZ/+KLd6pp0BizXncjkm/SxlprhD19f3s9QLocqrnG5zPOa1g2qjsBG3M6qmYN3Ghy9jd9x0ELPffT+zRoAGuMLLCCq8HDHdGoK5/UQ/0R7Ika Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: b74d162d-cfef-427b-dc02-08d7ec19b501 X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Apr 2020 08:45:45.0460 (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: 8/zAZhAUpZhtBn/M0kGLzjkFxmJgJeeuS1moUlLr4/2XODP4+6QSnh2aa1ywP6PziMc4yRScvq0LZ0a8kKgcK4kCEgnzy7h7rMuhvf1yHSY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2710 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt mode 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" > -----Original Message----- > From: Thomas Monjalon > Sent: Tuesday, April 28, 2020 4:54 PM > To: Jerin Jacob ; Dumitrescu, Cristian > > Cc: Richardson, Bruce ; Yigit, Ferruh > ; Luca Boccassi ; Nithin > Dabilpuram ; Singh, Jasvinder > ; Andrew Rybchenko > ; dev@dpdk.org; jerinj@marvell.com; > kkanas@marvell.com; Nithin Dabilpuram ; > Kinsella, Ray ; Neil Horman > ; Kevin Traynor ; David > Marchand > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper > config in pkt mode >=20 > 28/04/2020 17:04, Luca Boccassi: > > On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote: > > > On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote: > > > > On 4/27/2020 5:59 PM, Jerin Jacob wrote: > > > > > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit > wrote: > > > > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote: > > > > > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit > wrote: > > > > > > > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote: > > > > > > > > > From: Yigit, Ferruh > > > > > > > > > > On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote: > > > > > > > > > > > From: Nithin Dabilpuram > > > > > > > > > > > > This patch also updates tm port/level/node capabili= ty > structures with > > > > > > > > > > > > exiting features of scheduler wfq packet mode, > scheduler wfq byte mode > > > > > > > > > > > > and private/shared shaper byte mode. > > > > > > > > > > > > > > > > > > > > > > > > SoftNIC PMD is also updated with new capabilities. > [...] > > > > > > > > > > Hi Nithin, > > > > > > > > > > > > > > > > > > > > It looks like patch is causing ABI break, I am getting = following > warning [1], > > > > > > > > > > can you please check? > > > > > > > > > > > > > > > > > > > > [1] > > > > > > > > > > https://pastebin.com/XYNFg14u > > > > > > > > > > > > > > > > > > Hi Ferruh, > > > > > > > > > > > > > > > > > > The RTE_TM API is marked as experimental, > > > > > > > > > but it looks that this was not correctly marked > > > > > > > > > when __rte_experimental ABI checker was introduced. > > > > > > > > > > > > > > > > > > It is marked as experimental at the top of the rte_tm.h, > > > > > > > > > similarly to other APIs introduced around same time, > > > > > > > > > but it was not correctly picked up by the ABI check proce= dure > > > > > > > > > when later introduced, so __rte_experimental was not adde= d > to every function. > > > > > > > > > > > > > > > > > > > > > > > > > :( > > > > > > > > > > > > > > > > Is it time to mature them? > > > > > > > > > > > > > > > > As you said they are not marked as experimental both in hea= der > file (function > > > > > > > > declarations) and .map file. > > > > > > > > > > > > > > > > The problem is, they are not marked as experimental in > DPDK_20.0 ABI (v19.11), > > > > > > > > so marking them as experimental now will break the ABI. Not > sure what to do, > > > > > > > > cc'ed a few ABI related names for comment. > > > > > > > > > > > > > > > > For me, we need to proceed as the experimental tag removed > and APIs become > > > > > > > > mature starting from v19.11, since this is what happened in > practice, and remove > > > > > > > > a few existing being experimental references in the doxygen > comments. > > > > > > > > > > > > > > I think, accidentally we can not make a library as NON- > experimental. > > > > > > > TM never went through experimental to mature transition(see g= it > log > > > > > > > lib/librte_ethdev/rte_tm.h) > > > > > > > It was a bug to not mark as experimental in each function in = the > ABI process. > > > > > > > Some of the features like packet marking are not even > implemented by any HW. > > > > > > > I think, we can make API stable only all the features are > implemented > > > > > > > by one or two HW. >=20 > Yes this is what was decided one or two years ago I think. > But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs. >=20 >=20 >=20 > > > > > > Fair enough, specially if the API is not ready yet. > > > > > > > > > > > > But they were part of stable ABI, and marking them as experimen= tal > now will > > > > > > break the old applications using these APIs. > > > > > > > > > > it is still marked as EXPERIMENTAL everywhere and API is not read= y > yet. >=20 > rte_tm is implemented in 6 PMDs. >=20 >=20 > > > > Existing experimental marks are text only for human parsing. > > > > > > > > The compiler attribute and build time checks are missing, and the > symbol in the > > > > binary doesn't have experimental tag. Our scripts and automated > checks won't > > > > detect it as experimental. > > > > > > > > My point is just having experimental comment in header file is not > enough to > > > > qualify the APIs as experimental. > > > > > > > > > Anyway, we need to break the ABI to make it work on various HW. >=20 > Yes this is why I was asking in 19.11 to check our API, > in order to avoid such situation. >=20 >=20 > > > > > I am not sure what to do? >=20 > Either manage ABI versioning, or wait 20.11. >=20 >=20 > > > > > IMO, We need to send a patch as Fixes: for the bug of not adding > > > > > __rte_experimental in each function. >=20 > No, this is wrong. >=20 Why exactly is this wrong? This is the gap that caused the current discussi= on, right? >=20 > > > > Yes, this is where we are, both you and Cristian suggest API is not= ready > and > > > > should be experimental, but they were part of stable ABI, making th= em > > > > experimental will break the ABI. > > > > It looks like there is no good option but we should select one of t= he bad > ones. > > > > > > > > > Traffic Management API - EXPERIMENTAL > > > > > M: Cristian Dumitrescu > > > > > T: git://dpdk.org/next/dpdk-next-qos > > > > > F: lib/librte_ethdev/rte_tm* > > > > > > > > Ray, Neil, David, Luca, Kevin, what do you think? > > > While I'm not called any of those names, allow me to give my 2c. > > > > > > Since these are marked in binaries as part of the stable ABI, I think= we > > > need to honour that for the next two releases 20.05 and 20.08 [which > means > > > that we need to put in versioned functions for any changes, not that = we > > > can't change anything] > > > > > > For 20.11, I think these should then have one of two options taken: > > > * have these "fixed" and ready to be marked as stable, and officially= part > > > of v21 ABI or > > > * mark them as experimental properly, and look to have them as part o= f > the > > > v22 or subsequent ABI > > > > > > Given the comments here, I would tend towards the latter of the above > two > > > options, but that's really a decision for the maintainers. > > > > > > Remember, this is not the first bug we have encountered where we > messed up > > > some ABI versions in the 19.11 release, and, like the previous one wi= th > the > > > screwed up version number, I think we need to honour the ABI > committments > > > made, especially since in this case it's only for a few more months t= ill > > > 20.11 development starts. > > > > > > /Bruce > > > > +1 > > > > If they are not ready now, they haven't been ready for the past 6 > > months either, so staying not ready for 6 more is the lesser evil. >=20 > This API is almost 3 years old (release 17.08). > That's good to improve it but we must respect the ABI contract that > we all agreed. >=20 >=20 > Summary: > 17.08: rte_tm is introduced. > 17.11: rte_mtr is introduced as experimental, but rte_tm remains stable. > 18.02: __rte_experimental tag is introduced (including for rte_mtr), > but rte_tm remains untouched as it is in stable ABI. > 19.11: stable ABI is frozen until 20.11 > 20.05: rte_tm improvement is blocked because of ABI breakage. >=20 >=20 > It should remind everybody of reviewing the new API and policies, > and maintaining the existing code appropriately. >=20