Hi, 10/07/2017 12:55, Dumitrescu, Cristian: > Hi Thomas, > > From: Thomas Monjalon [mailto:tho...@monjalon.net] > > > > Hi, > > > > 04/07/2017 17:38, Cristian Dumitrescu: > > > http://dpdk.org/git/next/dpdk-next-tm > > > > 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. > > > > I prefer to wait because I have seen some things to fix: > > > > 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 ASAP. > > > 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 > > Thomas, hopefully this is a misunderstanding on your side :(((.
Don't worry :) > 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 > modularizing the DPDK ethdev by adding new functionality in a modular plug-in > way 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 change > on the fundamentals now? > > Hopefully, it is just misunderstanding. I mean that only the drivers need to get the ops. The applications are using some dedicated functions rte_tm_* , right? So the applications does not need direct ops access with rte_eth_dev_tm_ops_get()? Sorry if it is my misunderstanding. About rte_tm_ops_get, I don't remember why I talked about it. It seems exposed only to drivers. My mistake. No issue there. > > 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. > > > 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? Yes, perfect. > 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? Yes I think it is reasonnable. > IMO it makes sense to mark new APIs as experimental for some time, it should > be very clear when this label can be removed. We had examples of customers > being confused by this label and questioning us whether they should use such > APIs or not, therefore the process or applying & removing this label should > be clearly documented, which right now it is not at all. The idea is to highlight that it is a new API and may be not stable enough. The question is when do we know it is mature enough? I don't know and I guess it is up to the maintainer of the API. Please remind that after removing the EXPERIMENTAL status, you must follow the API deprecation status. > To this moment, this was not followed consistently in DPDK either. Recent > examples: > 1. rte_flow API, introduced in DPDK release 17.02 was never marked as > EXPERIMENTAL in either MANTAINERS or API file > 2. eventdev API, introduced in DPDK release 17.05, was marked as EXPERIMENTAL > in the MAINTAINERS file, but not in the API file Yes, not perfect ;) > > 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? Yes, good to know. > As a precedent, eventdev API was introduced in 17.05 with test application > just added now (one release later). > > > 6/ There is no application to test it. > > Yes, we will either extend test-pmd or add a new example application to > exercise this API for next DPDK release 17.11. CC-ing Tim and Mike to confirm. Yes I think it would be important to have it in 17.11. > > 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. OK looks a good plan. Please be aware that the documentation must be submitted in two weeks to let us few days for reviewing.
