mbox series

[v4,0/4] Support PPS(packet per second) on meter

Message ID 20210413035046.28578-1-lizh@nvidia.com (mailing list archive)
Headers
Series Support PPS(packet per second) on meter |

Message

Li Zhang April 13, 2021, 3:50 a.m. UTC
  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

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(-)
  

Comments

Ferruh Yigit April 13, 2021, 10:24 a.m. UTC | #1
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.

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(-)
>
  
Li Zhang April 13, 2021, 11:02 a.m. UTC | #2
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.

> 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(-)
> >
  
Ferruh Yigit April 13, 2021, 11:05 a.m. UTC | #3
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(-)
>>>
>
  
Li Zhang April 13, 2021, 3:46 p.m. UTC | #4
Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Tuesday, April 13, 2021 7:06 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 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.
> 

Got it and will sent it on V5 patch.

> >> 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(-)
> >>>
> >