Hi Ajit

Looks like you agree with this RFC to move meter objects to be managed by PMD pointers.

Thanks for the review!

From: Ajit Khaparde

On Thu, Mar 25, 2021 at 1:21 AM Matan Azrad [off-list ref] wrote:

quoted

Hi Cristian

From: Dumitrescu, Cristian

quoted

Hi Li and Matan,

quoted

-----Original Message-----
From: Li Zhang <redacted>
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 [off-list ref]; Thomas Monjalon
[off-list ref]; Yigit, Ferruh [off-list ref]; Andrew
Rybchenko [off-list ref]; Dumitrescu, Cristian
[off-list ref]
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

quoted

quoted

dk.org%2Farchives%2Fdev%2F2021-

March%2F200710.html&data=04%7C01%7Cmatan%40nvidia.com%7Cab0

quoted

quoted

e3cc77b9e4101344e08d8ee434bbe%7C43083d15727340c1b7db39efd9ccc17a%

quoted

quoted

7C0%7C0%7C637521320105450617%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiM

quoted

quoted

C4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&

quoted

quoted

amp;sdata=94bFRICfGEzk5s53MRUvFMQe5ZlhP2Tmnu82hwUytc4%3D&re

quoted

quoted

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

quoted

quoted

handles at length on this exact email list back in 2017, when we first

introduced

quoted

quoted

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....

quoted

quoted

2. For me, it is more natural and it also helps the application to simplify its

data

quoted

quoted

structures if the user provides its own IDs rather than the user having to

deal

quoted

quoted

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.

quoted

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.

quoted

          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:

quoted

                        1. meter per flow: just save the driver handle in the app flow

context.

quoted

                        2. meter per VM\USER flows\rte_flow group\any other context

grouped multiple flows: just save the driver handle in the app context.

quoted

        If PMD need to map the IDs, it is more complex for sure, requires more

memory and more lookup time.

quoted

        - I'm not sure it is natural for all the use-cases, sometimes generating

unique ID may complex the app.

quoted

quoted

3. It is much easier and portable to pass numeric and string-based IDs

around

quoted

quoted

(e.g. between processes) as opposed to pointer-based IDs, as pointers are

only

quoted

quoted

valid in one address space and not in others. There are several DPDK APIs

that

quoted

quoted

moved away from pointer handles to string IDs.

Pardon my ignorance..
But which DPDK APIs moved to string IDs from pointer handles?

quoted

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.

quoted

quoted

4. The mapping of user IDs to internal pointers within the driver is IMO not

a

quoted

quoted

big issue in terms of memory footprint or API call rate. Matan also

confirmed

quoted

quoted

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.

quoted

quoted

And last but not least, this change obviously propagates in every API

function,

quoted

quoted

so it would result in big churn in API, all drivers and all apps (including

testpmd,

quoted

quoted

etc) implementing it (for IMO no real benefit). Yes, this API is experimental

and

quoted

quoted

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.

quoted

quoted

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