Re: [PATCH 2/3] ethdev: add new rte_mtr API for traffic metering and policing

From: "Dumitrescu, Cristian" <cristian.dumitrescu@intel.com>
To: Hemant Agrawal <hemant.agrawal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
	"adrien.mazarguil@6wind.com" <adrien.mazarguil@6wind.com>,
	"jerin.jacob@caviumnetworks.com" <jerin.jacob@caviumnetworks.com>
Subject: Re: [PATCH 2/3] ethdev: add new rte_mtr API for traffic metering and policing
Date: Tue, 19 Sep 2017 16:14:14 +0000	[thread overview]
Message-ID: <3EB4FA525960D640B5BDFFD6A3D891267BABC059@IRSMSX108.ger.corp.intel.com> (raw)
In-Reply-To: <5610d7e1-701b-71ea-42fc-4ac427b87eb6@nxp.com>

Hi Hemant,

<snip>...

> >>> +/**
> >>> + * Meter profile delete
> >>> + *
> >>> + * Delete an existing meter profile. This operation fails when there is
> >>> + * currently at least one user (i.e. MTR object) of this profile.
> >>> + *
> >>
> >> Do you want to do this checking at library level or at driver level?
> >> Same profile can be used by two distinct drivers!
> >>
> >> This is applicable for other APIs as well. Your current API
> >> implementation is mostly pass-through. A stateful will be better.
> >>
> >
> > Yes, the same meter profiles can potentially be shared for a group of
> ethdev ports, but unfortunately we cannot do this with the current ethdev
> API, which is per port with no sharing ability between groups of ports.
> >
> > Here we are lucky to have just a read-only configuration template that
> needs to be shared amongst ports, so the quick workaround is to define the
> same profile for each port at the expense of some (small) work duplication.
> Hopefully this is OK with you?
> >
> Is that mean you will create the meter profile template first and then
> apply/remove that on a port?
> 

Meter profile is just the configuration params (configuration template) that can be applied to many meter objects.

Approach A (my proposal): Keep all API functions per port, including meter profile create, including meter create. So in the likely case that meters on different ports use the same configuration, the same profile needs to be registered once for each port.

Approach B (your suggestion, if I got it right): Have a global list of profiles maintained internally by the library. This allows using the same profile on different ports, so no need to register the same profile explicitly for each port.

The reason I don't favour approach B  is it requires maintaining static global data in the library, but I don't see any tricky implications with either A or B.

Do you have strong preference towards B?

> how about adding a use-count to the profile?
> 

Yes, this is already part of the current proposal: the number of meter objects that share the same profile (i.e. configuration template) is counted, so the profile can be deleted only when it does not have any users (i.e. meter objects).

> On a different lines, can we do following
> 1. Metering profile is not applied to a port.  Metering object maintain
> the use-count.

Not sure I understand your statement on: "metering object maintain the use-count". Maybe we have a terminology misalignment on meter profile and meter object and I am not fully getting your point?

In my mind this is approach B from above. Also the above statement should be corrected to: metering _profile_ maintain the use-count (of how many metering objects are currently using it, i.e. use the same configuration template)?

> 2. metering object is applied to the port, which in turn contains
> metering profile.
> 

Yes, this is approach A from above :)

> Regards,
> Hemant

Regards,
Cristian