[dpdk-dev] [PATCH] doc: API change notice for librte_meter

[dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Cristian Dumitrescu]

Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
Acked-by: John McNamara <John.McNamara@intel.com>
Acked-by: Jasvinder Singh <Jasvinder.Singh@intel.com>
Acked-by: Radu Nicolau <Radu.Nicolau@intel.com>
Acked-by: David Hunt <David.Hunt@intel.com>
---
 doc/guides/rel_notes/deprecation.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index 72aa404..d0236af 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -64,3 +64,6 @@ Deprecation Notices
   be removed in 17.11:
 
   - ``rte_cryptodev_create_vdev``
+
+* librte_meter: The API will change to accommodate configuration profiles.
+  Most of the API functions will have an additional opaque parameter.
-- 
2.7.4

Re: [dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Thomas Monjalon]

04/08/2017 15:19, Cristian Dumitrescu:
> +* librte_meter: The API will change to accommodate configuration profiles.
> +  Most of the API functions will have an additional opaque parameter.

Why?
Why opaque parameter?
If you want to use it with a configuration file, you just have to
implement a configuration file in your application.

Moreover, I already explained my fear of adding this library in DPDK
which is really an application-level statistics lib.

Without more explanations, my vote is a nack.

However I remember there was a promise to merge every metrics libs in one.

Re: [dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, August 4, 2017 3:29 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Horton, Remy
> <remy.horton@intel.com>
> Cc: dev@dpdk.org; Mcnamara, John <john.mcnamara@intel.com>; Singh,
> Jasvinder <jasvinder.singh@intel.com>; Nicolau, Radu
> <radu.nicolau@intel.com>; Hunt, David <david.hunt@intel.com>
> Subject: Re: [PATCH] doc: API change notice for librte_meter
> 
> 04/08/2017 15:19, Cristian Dumitrescu:
> > +* librte_meter: The API will change to accommodate configuration
> profiles.
> > +  Most of the API functions will have an additional opaque parameter.
> 
> Why?
> Why opaque parameter?
> If you want to use it with a configuration file, you just have to
> implement a configuration file in your application.
> 
> Moreover, I already explained my fear of adding this library in DPDK
> which is really an application-level statistics lib.
> 
> Without more explanations, my vote is a nack.
> 
> However I remember there was a promise to merge every metrics libs in
> one.

Thomas,

Confusion with librte_metrics, which is a totally different library? This is about librte_meter, nothing to do with stats/metrics.

This librte_meter is doing traffic metering, essentially the computing the packet color according to the IETF RFCs 2697 (srTCM = Single Rate Three Color Marker) and 2698 (trTCM = Two Rate Three Color Marker). This is a fundamental block for pretty much every edge router upstream path.

You asked me on numerous occasions to be concise, so here is a concise deprecation notice. I have to say initially I wrote a more laborious one, then I remembered your advice and cut it down to this version. Do you need more details on the motivation?

Regards,
Cristian

Re: [dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Thomas Monjalon]

04/08/2017 16:38, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 04/08/2017 15:19, Cristian Dumitrescu:
> > > +* librte_meter: The API will change to accommodate configuration
> > profiles.
> > > +  Most of the API functions will have an additional opaque parameter.
> > 
> > Why?
> > Why opaque parameter?
> > If you want to use it with a configuration file, you just have to
> > implement a configuration file in your application.
> > 
> > Moreover, I already explained my fear of adding this library in DPDK
> > which is really an application-level statistics lib.
> > 
> > Without more explanations, my vote is a nack.
> > 
> > However I remember there was a promise to merge every metrics libs in
> > one.
> 
> Thomas,
> 
> Confusion with librte_metrics, which is a totally different library? This is about librte_meter, nothing to do with stats/metrics.

Yes you're right! Sorry for the confusion.

> This librte_meter is doing traffic metering, essentially the computing the packet color according to the IETF RFCs 2697 (srTCM = Single Rate Three Color Marker) and 2698 (trTCM = Two Rate Three Color Marker). This is a fundamental block for pretty much every edge router upstream path.
> 
> You asked me on numerous occasions to be concise, so here is a concise deprecation notice. I have to say initially I wrote a more laborious one, then I remembered your advice and cut it down to this version.

Thanks for being concise :)

> Do you need more details on the motivation?

Yes we need to understand why the configuration profiles must be
managed by the API instead of separately.

Re: [dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Friday, August 4, 2017 3:49 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: Horton, Remy <remy.horton@intel.com>; dev@dpdk.org; Mcnamara,
> John <john.mcnamara@intel.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> Hunt, David <david.hunt@intel.com>
> Subject: Re: [PATCH] doc: API change notice for librte_meter
> 
> 04/08/2017 16:38, Dumitrescu, Cristian:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > 04/08/2017 15:19, Cristian Dumitrescu:
> > > > +* librte_meter: The API will change to accommodate configuration
> > > profiles.
> > > > +  Most of the API functions will have an additional opaque parameter.
> > >
> > > Why?
> > > Why opaque parameter?
> > > If you want to use it with a configuration file, you just have to
> > > implement a configuration file in your application.
> > >
> > > Moreover, I already explained my fear of adding this library in DPDK
> > > which is really an application-level statistics lib.
> > >
> > > Without more explanations, my vote is a nack.
> > >
> > > However I remember there was a promise to merge every metrics libs in
> > > one.
> >
> > Thomas,
> >
> > Confusion with librte_metrics, which is a totally different library? This is
> about librte_meter, nothing to do with stats/metrics.
> 
> Yes you're right! Sorry for the confusion.
> 

Never occurred to me before, but yes, I have to say it is easy to confuse these two names METeR and METRics.

> > This librte_meter is doing traffic metering, essentially the computing the
> packet color according to the IETF RFCs 2697 (srTCM = Single Rate Three Color
> Marker) and 2698 (trTCM = Two Rate Three Color Marker). This is a
> fundamental block for pretty much every edge router upstream path.
> >
> > You asked me on numerous occasions to be concise, so here is a concise
> deprecation notice. I have to say initially I wrote a more laborious one, then I
> remembered your advice and cut it down to this version.
> 
> Thanks for being concise :)
> 
> > Do you need more details on the motivation?
> 
> Yes we need to understand why the configuration profiles must be
> managed by the API instead of separately.

The configuration profile is simply a cool name for the meter configuration parameter set, which includes committed/peak rates, etc.

The profile notion makes sense when many meter objects share the same set of configuration parameters, which is pretty much the de-facto behaviour.

Right now, a metering object is handled through an opaque parameter, which is really a data structure storing the low level data required to run the meter. This structure contains two types of data:
A) variable data: running counters per meter
B) fixed data: low level translation of configuration parameters; this should not be duplicated per every object, as it is shared by many objects

So basically, will break the meter handle into two handles: one for the A) data (the one we already have), and one for the B) data, which can be shared by many objects.

Why? For significant performance improvements, as the per object context memory footprint cuts in half or even more by moving the fixed data B) elsewhere. Right now, this context is 2-3 cache lines, it will eventually fit in about half of cache line. This helps a lot when you have thousands of flows, each with one or two meters.

Makes sense?

Regards,
Cristian

PS: Mot sure I managed to be concise, but I tried :)

Re: [dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Thomas Monjalon]

04/08/2017 17:04, Dumitrescu, Cristian:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > 04/08/2017 16:38, Dumitrescu, Cristian:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > > 04/08/2017 15:19, Cristian Dumitrescu:
> > > > > +* librte_meter: The API will change to accommodate configuration
> > > > profiles.
> > > > > +  Most of the API functions will have an additional opaque parameter.
> > > >
> > > > Why?
> > > > Why opaque parameter?
> > > > If you want to use it with a configuration file, you just have to
> > > > implement a configuration file in your application.
> > > >
> > > > Moreover, I already explained my fear of adding this library in DPDK
> > > > which is really an application-level statistics lib.
> > > >
> > > > Without more explanations, my vote is a nack.
> > > >
> > > > However I remember there was a promise to merge every metrics libs in
> > > > one.
> > >
> > > Thomas,
> > >
> > > Confusion with librte_metrics, which is a totally different library? This is
> > about librte_meter, nothing to do with stats/metrics.
> > 
> > Yes you're right! Sorry for the confusion.
> > 
> 
> Never occurred to me before, but yes, I have to say it is easy to confuse these two names METeR and METRics.
> 
> > > This librte_meter is doing traffic metering, essentially the computing the
> > packet color according to the IETF RFCs 2697 (srTCM = Single Rate Three Color
> > Marker) and 2698 (trTCM = Two Rate Three Color Marker). This is a
> > fundamental block for pretty much every edge router upstream path.
> > >
> > > You asked me on numerous occasions to be concise, so here is a concise
> > deprecation notice. I have to say initially I wrote a more laborious one, then I
> > remembered your advice and cut it down to this version.
> > 
> > Thanks for being concise :)
> > 
> > > Do you need more details on the motivation?
> > 
> > Yes we need to understand why the configuration profiles must be
> > managed by the API instead of separately.
> 
> The configuration profile is simply a cool name for the meter configuration parameter set, which includes committed/peak rates, etc.
> 
> The profile notion makes sense when many meter objects share the same set of configuration parameters, which is pretty much the de-facto behaviour.
> 
> Right now, a metering object is handled through an opaque parameter, which is really a data structure storing the low level data required to run the meter. This structure contains two types of data:
> A) variable data: running counters per meter
> B) fixed data: low level translation of configuration parameters; this should not be duplicated per every object, as it is shared by many objects
> 
> So basically, will break the meter handle into two handles: one for the A) data (the one we already have), and one for the B) data, which can be shared by many objects.
> 
> Why? For significant performance improvements, as the per object context memory footprint cuts in half or even more by moving the fixed data B) elsewhere. Right now, this context is 2-3 cache lines, it will eventually fit in about half of cache line. This helps a lot when you have thousands of flows, each with one or two meters.
> 
> Makes sense?

It proves that you are an expert of this stuff and you seem to know what
you do, so I trust you :)
(I remove my wrong nack)

> PS: Mot sure I managed to be concise, but I tried :)

It was less concise but probably necessary, thanks!

Re: [dpdk-dev] [PATCH] doc: API change notice for librte_meter

[Thomas Monjalon]

04/08/2017 15:19, Cristian Dumitrescu:
> Signed-off-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> Acked-by: John McNamara <John.McNamara@intel.com>
> Acked-by: Jasvinder Singh <Jasvinder.Singh@intel.com>
> Acked-by: Radu Nicolau <Radu.Nicolau@intel.com>
> Acked-by: David Hunt <David.Hunt@intel.com>

Applied, thanks