Re: [dpdk-dev] [PATCH 2/2] [RFC]: ethdev: manage meter API object handles by the drivers

[Matan Azrad <matan@nvidia.com>]

Hi Cristian

From: Dumitrescu, Cristian
> Hi Li and Matan,
> 
> > -----Original Message-----
> > From: Li Zhang <lizh@nvidia.com>
> > 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 <jasvinder.singh@intel.com>; Thomas Monjalon
> > <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> > Rybchenko <andrew.rybchenko@oktetlabs.ru>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>
> > 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&amp;data=04%7C01%7Cmatan%40nvidia.com%7Cab0
> e3cc77b9e4101344e08d8ee434bbe%7C43083d15727340c1b7db39efd9ccc17a%
> 7C0%7C0%7C637521320105450617%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM
> C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&
> amp;sdata=94bFRICfGEzk5s53MRUvFMQe5ZlhP2Tmnu82hwUytc4%3D&amp;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.

Yes, I agree here generally.
But again, since meter is used only by rte_flow, it is better to align the same handle mechanism.

> 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