On Thu, Mar 25, 2021 at 1:21 AM Matan Azrad wrote: > > Hi Cristian > > From: Dumitrescu, Cristian > > Hi Li and Matan, > > > > > -----Original Message----- > > > From: Li Zhang > > > Sent: Thursday, March 18, 2021 8:58 AM > > > To: dekelp@nvidia.com; orika@nvidia.com; viacheslavo@nvidia.com; > > > matan@nvidia.com; shahafs@nvidia.com; lironh@marvell.com; Singh, > > > Jasvinder ; Thomas Monjalon > > > ; Yigit, Ferruh ; Andrew > > > Rybchenko ; Dumitrescu, Cristian > > > > > > Cc: dev@dpdk.org; rasland@nvidia.com; roniba@nvidia.com > > > Subject: [PATCH 2/2] [RFC]: ethdev: manage meter API object handles by > > > the drivers > > > > > > Currently, all the meter objects are managed by the user IDs: > > > meter, profile and policy. > > > Hence, each PMD should manage data-structure in order to map each API > > > ID to the private PMD management structure. > > > > > > From the application side, it has all the picture how meter is going > > > to be assigned to flows and can easily use direct mapping even when > > > the meter handler is provided by the PMDs. > > > > > > Also, this is the approach of the rte_flow API handles: > > > the flow handle and the shared action handle is provided by the PMDs. > > > > > > Use drivers handlers in order to manage all the meter API objects. > > > > > > > This seems to be take 2 of the discussion that we already had in this thread: > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmails.dp > > dk.org%2Farchives%2Fdev%2F2021- > > March%2F200710.html&data=04%7C01%7Cmatan%40nvidia.com%7Cab0 > > e3cc77b9e4101344e08d8ee434bbe%7C43083d15727340c1b7db39efd9ccc17a% > > 7C0%7C0%7C637521320105450617%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM > > C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000& > > amp;sdata=94bFRICfGEzk5s53MRUvFMQe5ZlhP2Tmnu82hwUytc4%3D&re > > served=0, so apologies for mostly summarizing my previous feedback here. > > > > I am against this proposal because: > > 1. We already discussed this topic of user-provided handles vs. driver-provided > > handles at length on this exact email list back in 2017, when we first introduced > > this API, and I don't see any real reason to revisit the decision we took then. > > Why not? > There is more experiences\usages now. > New drivers added the support and also now scalability is growing and growing.... > > > > 2. For me, it is more natural and it also helps the application to simplify its data > > structures if the user provides its own IDs rather than the user having to deal > > with the IDs provided by the driver. > > Generally I don't think other flow DPDK APIs align with your feelings here, see rte_flow object and rte_flow_shared_action. > > Specifically for meter: > - here, meter is HW\driver offload where performance\rate either for meter creation\deletion or for the actual data-path is very important especially when we talk on very big numbers, so "natural" has less importance here. > We need to think on the global solution for application ->API->driver. in meter feature, the user has the ability to manage the IDs better than the PMDs for the most of the use-cases: > 1. meter per flow: just save the driver handle in the app flow context. > 2. meter per VM\USER flows\rte_flow group\any other context grouped multiple flows: just save the driver handle in the app context. > If PMD need to map the IDs, it is more complex for sure, requires more memory and more lookup time. > > - I'm not sure it is natural for all the use-cases, sometimes generating unique ID may complex the app. > > > > 3. It is much easier and portable to pass numeric and string-based IDs around > > (e.g. between processes) as opposed to pointer-based IDs, as pointers are only > > valid in one address space and not in others. There are several DPDK APIs that > > moved away from pointer handles to string IDs. Pardon my ignorance.. But which DPDK APIs moved to string IDs from pointer handles? > > Yes, I agree here generally. > But again, since meter is used only by rte_flow, it is better to align the same handle mechanism. I don't want to say - do this because rte_flow uses a pointer. I don't have a strong opinion for one over the other. In the end the logic can be adapted one way or the other. But having implemented rte_flow support in the PMD, I think it is a good idea to avoid the duplication of meter_id to pointer based handle conversion and bookkeeping logic in the application and the PMD. > > > 4. The mapping of user IDs to internal pointers within the driver is IMO not a > > big issue in terms of memory footprint or API call rate. Matan also confirmed > > this in the above thread when saying tis is not about either driver memory > > footprint or API call speed, as this mapping is easy to optimize. > > Yes, it is not very big deal, but still costs more than the new suggestion, especially in big scale. > > > And last but not least, this change obviously propagates in every API function, > > so it would result in big churn in API, all drivers and all apps (including testpmd, > > etc) implementing it (for IMO no real benefit). Yes, this API is experimental and > > therefore we can operate changes in it, but I'd rather see incremental and > > converging improvements rather than this. > > Yes, it changes all API, but very small part in each, will be very easy to align all the current dpdk components to use this concept. > > > If you guys insist with this proposal, I would like to get more opinions from > > other vendors and contributors from within our DPDK community. > > > Yes, more opinions are very welcomed. > > Thanks