Re: [dpdk-dev] [RFC v4 1/4] ethdev: add meter PPS profile

[Matan Azrad <matan@nvidia.com>]

Hi Cristian

Thank you for review, please see inline.

From: Dumitrescu, Cristian
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Li Zhang
<snip>
> We had this same problem earlier for the rte_tm.h API, where people asked to
> add support for WRED and shaper rates specified in packets to the existing byte
> rate support. I am more than happy to support adding the same here, but
> please let's adopt the same solution here rather than invent a different
> approach.
> 
> Please refer to struct rte_tm_wred_params and struct rte_tm_shaper_params
> from rte_tm.h: the packets vs. bytes mode is explicitly specified through the use
> of a flag called packet_mode that is added to the WRED and shaper profile.
> When packet_mode is 0, the profile rates and bucket sizes are specified in
> bytes per second and bytes, respectively; when packet_mode is not 0, the
> profile rates and bucket sizes are specified in packets and packets per second,
> respectively. The same profile parameters are used, no need to invent
> additional algorithms (such as srTCM - packet mode) or profile data structures.
> Can we do the same here, please?

This flag approach is very intuitive suggestion and it has advantages.

The main problem with the flag approach is that it breaks ABI and API.
The profile structure size is changed due to a new field - ABI breakage.
The user must initialize the flag with zero to get old behavior - API breakage. 
 
I don't see issues with Li suggestion, Do you think Li suggestion has critical issues?

> This is a quick summary of the required API changes to add support for the
> packet mode, they are minimal:
> a) Introduce the packet_mode flag in the profile parameters data structure.
> b) Change the description (comment) of the rate and bucket size parameters in
> the meter profile parameters data structures to reflect that their values
> represents either bytes or packets, depending on the value of the new flag
> packet_mode from the same structure.
> c) Add the relevant capabilities: just search for "packet" in the rte_tm.h
> capabilities data structures and apply the same to the rte_mtr.h capabilities,
> when applicable.
 
> Regards,
> Cristian