Re: [dpdk-dev] [PATCH v4 0/4] Support PPS(packet per second) on meter

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 4/13/2021 12:02 PM, Li Zhang wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Tuesday, April 13, 2021 6:25 PM
>> To: Li Zhang <lizh@nvidia.com>; dekelp@nvidia.com; Ori Kam
>> <orika@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Matan
>> Azrad <matan@nvidia.com>; Shahaf Shuler <shahafs@nvidia.com>;
>> cristian.dumitrescu@intel.com; lironh@marvell.com; jerinj@marvell.com
>> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon <thomas@monjalon.net>;
>> Raslan Darawsheh <rasland@nvidia.com>; Roni Bar Yanai
>> <roniba@nvidia.com>
>> Subject: Re: [PATCH v4 0/4] Support PPS(packet per second) on meter
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 4/13/2021 4:50 AM, Li Zhang wrote:
>>> Currently meter algorithms only supports rate is bytes per second(BPS).
>>> Add packet_mode flag in meter profile parameters data structure.
>>> So that it can meter traffic by packet per second.
>>>
>>> When packet_mode is 0, the profile rates and bucket sizes are
>>> specified in bytes per second and bytes when packet_mode is not 0, the
>>> profile rates and bucket sizes are specified in packets and packets
>>> per second.
>>>
>>> Add the necessary checks to the existing drivers implementing the
>>> rte_mtr API to makes sure that profiles with packet_mode set to TRUE
>>> are rejected.
>>>
>>> RFC ("adds support PPS(packet per second) on meter")
>>> https://patchwork.dpdk.org/project/dpdk/patch/20210125012023.1769769-
>> 2
>>> -lizh@nvidia.com/
>>>
>>> Depends-on: series=16301  ("Support meter policy API ")
>>> https://patchwork.dpdk.org/project/dpdk/list/?series=16301
>>>
>>
>> Hi Li,
>>
>> I am not clear with the dependency chain, can you please clarify,
>>
>> 1) Is this set depends to series-16301? Because it compiles fine after conflict
>> resolved, I can see in your repo there is an order, but if there is no
>> functional/logical dependency you can set this patch exactly on top of HEAD
>> (removing the series-16301 in between), so the CI will be enabled.
> 
> I will delete series-16301.
> But it will merge conflict when series-16301 merged after it.
> 

Please send both this patch, and series-16301 on top of latest head, this 
enables CI for both.

When merging them we can handle the conflict, based on which one merged first, 
or can ask you to rebase the second one but for this case it does not look too 
complex to resolve ourselves.

>> 2) According its cover letter series-16301 depends on mlx ASO patch, this
>> makes all ethdev patches dependent to mlx5 set, I guess that is wrong, can you
>> please confirm?
>>
>> Above (1) is more important, since series-16301 not fully acked, it is blocking
>> me to proceed.
>>
>>> V2: create a unified patch that contains both the series with
>>>        the API changes and the series with the necessary error checks in the
>> drivers.
>>> V3: Fix comments about commit-log.
>>> V4: Fix comments about Depends-on and rebase.
>>>
>>> Li Zhang (4):
>>>     ethdev: add packet mode in meter profile structure
>>>     app/testpmd: add meter profile packet mode option
>>>     net/softnic: check meter packet mode
>>>     net/mvpp2: check meter packet mode
>>>
>>>    app/test-pmd/cmdline_mtr.c                  | 40 ++++++++-
>>>    doc/guides/rel_notes/release_21_05.rst      | 12 +++
>>>    doc/guides/testpmd_app_ug/testpmd_funcs.rst | 31 +++----
>>>    drivers/net/mvpp2/mrvl_mtr.c                |  6 ++
>>>    drivers/net/softnic/rte_eth_softnic_meter.c |  8 ++
>>>    lib/librte_ethdev/rte_mtr.h                 | 90 ++++++++++++++++++---
>>>    6 files changed, 159 insertions(+), 28 deletions(-)
>>>
>