From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 34C5558CD for ; Mon, 10 Jul 2017 12:55:55 +0200 (CEST) Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Jul 2017 03:55:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.40,339,1496127600"; d="scan'208";a="123225329" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by orsmga005.jf.intel.com with ESMTP; 10 Jul 2017 03:55:53 -0700 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.133]) by IRSMSX106.ger.corp.intel.com ([169.254.8.236]) with mapi id 14.03.0319.002; Mon, 10 Jul 2017 11:55:52 +0100 From: "Dumitrescu, Cristian" To: Thomas Monjalon CC: "dev@dpdk.org" , "jerin.jacob@caviumnetworks.com" , "hemant.agrawal@nxp.com" , "Singh, Jasvinder" , "Lu, Wenzhuo" , "O'Driscoll, Tim" , "Glynn, Michael J" , Adrien Mazarguil Thread-Topic: [dpdk-dev] [pull-request] next-tm 17.08 pre-rc1 Thread-Index: AQHS9NvD1n6mRK62C0yv42q5/ePxaaJL4dWAgAD3fpA= Date: Mon, 10 Jul 2017 10:55:51 +0000 Message-ID: <3EB4FA525960D640B5BDFFD6A3D891267BA7D62D@IRSMSX108.ger.corp.intel.com> References: <1499182731-86830-1-git-send-email-cristian.dumitrescu@intel.com> <1838852.sCttUoyffy@xps> In-Reply-To: <1838852.sCttUoyffy@xps> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMDJkMGZlYjctYmE3ZC00ZWNjLWFmZDEtMDNkNWNiM2YyYTdhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjJLY2doTk4rQjNaWnZUN1FaR3hrRWJ0c3hcL1VJQWtjNml0M3dnNCtHU0g4PSJ9 x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [pull-request] next-tm 17.08 pre-rc1 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: , X-List-Received-Date: Mon, 10 Jul 2017 10:55:56 -0000 Hi Thomas, > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Sunday, July 9, 2017 9:02 PM > To: Dumitrescu, Cristian > Cc: dev@dpdk.org; jerin.jacob@caviumnetworks.com; > hemant.agrawal@nxp.com; Singh, Jasvinder ; > Lu, Wenzhuo > Subject: Re: [dpdk-dev] [pull-request] next-tm 17.08 pre-rc1 >=20 > Hi, >=20 > 04/07/2017 17:38, Cristian Dumitrescu: > > http://dpdk.org/git/next/dpdk-next-tm >=20 > I'm sorry to not have considered this tree as a high priority. > I think it may be integrated in RC2 because it is a totally new area > and should not break any existing code. >=20 > I prefer to wait because I have seen some things to fix: >=20 > 1/ There is a compilation error with clang (notified in related thread). Thanks for sending the error log, looks simple to fix, we will fix this ASA= P. >=20 > 2/ Some functions are exposed in the API to query the ops. > It seems dangerous and useless: > - rte_eth_dev_tm_ops_get > - rte_tm_ops_get >=20 Thomas, hopefully this is a misunderstanding on your side :(((. This is a critical point that we debated ad nauseam on this email list (RFC= , V1 -V6) and privately as well. You were included in the conversation, you= also provided feed-back that we incorporated in the code, as documented in= the patchset history log. This is simply the mechanism that we (including you) agreed to use for modu= larizing the DPDK ethdev by adding new functionality in a modular plug-in w= ay using separate namespace. This is the exact clone of the same mechanism = that rte_flow is using and was merged in DPDK release 17.02. Why this chang= e on the fundamentals now? Hopefully, it is just misunderstanding. > 3/ The PMD interface file is referenced in the doxygen index: > + [rte_tm_driver] (@ref rte_tm_driver.h), > I see that rte_flow_driver.h is also referenced but it seems a mistake. We simply followed the rte_flow precedent. We will remove this line. >=20 > 4/ As it is a totally new API, it should be declared as EXPERIMENTAL > in the MAINTAINERS file and in the doxygen. We can add the EXPERIMENTAL tag for this API in the MANTAINERS file and at = the top of the API file for DPDK release 17.08. Is this OK with you? But, as Jerin also asked at the time when eventdev API was introduced, what= is the process to remove the EXPERIMENTAL label? Do you agree that we can = remove the experimental label in the next release 17.11? IMO it makes sense to mark new APIs as experimental for some time, it shoul= d be very clear when this label can be removed. We had examples of customer= s being confused by this label and questioning us whether they should use s= uch APIs or not, therefore the process or applying & removing this label sh= ould be clearly documented, which right now it is not at all. To this moment, this was not followed consistently in DPDK either. Recent e= xamples: 1. rte_flow API, introduced in DPDK release 17.02 was never marked as EXPER= IMENTAL in either MANTAINERS or API file 2. eventdev API, introduced in DPDK release 17.05, was marked as EXPERIMENT= AL in the MAINTAINERS file, but not in the API file >=20 > 5/ There is no doc in the programmer's guide. We are definitely going to add comprehensive documentation for this new API= before the 17.08 documentation deadline. Is this OK with you? As a precedent, eventdev API was introduced in 17.05 with test application = just added now (one release later). >=20 > 6/ There is no application to test it. >=20 Yes, we will either extend test-pmd or add a new example application to exe= rcise this API for next DPDK release 17.11. CC-ing Tim and Mike to confirm. > I know that the points 5/ and 6/ are long to complete. > However I would like to know what is the plan? > And should we integrate TM in 17.08 without neither doc nor app? As stated above, we are going to add documentation for this new API on this= release and test application in the next release. Regards, Cristian