diff mbox series

[v4,1/4] ethdev: add tm support for shaper config in pkt mode

Message ID 20200422172104.23099-1-nithind1988@gmail.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series [v4,1/4] ethdev: add tm support for shaper config in pkt mode |

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues

Commit Message

Nithin Dabilpuram April 22, 2020, 5:21 p.m. UTC 
  From: Nithin Dabilpuram <ndabilpuram@marvell.com>

Some NIC hardware support shaper to work in packet mode i.e
shaping or ratelimiting traffic is in packets per second (PPS) as
opposed to default bytes per second (BPS). Hence this patch
adds support to configure shared or private shaper in packet mode,
provide rate in PPS and add related tm capabilities in port/level/node
capability structures.

This patch also updates tm port/level/node capability structures with
exiting features of scheduler wfq packet mode, scheduler wfq byte mode
and private/shared shaper byte mode.

SoftNIC PMD is also updated with new capabilities.

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---
v3..v4:
- Update text under packet_mode as per Cristian.
- Update rte_eth_softnic_tm.c based on Jasvinder's comments.
- Add error enum RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE 
- Fix shaper_profile_check() with packet mode check
- Fix typo's 

v2..v3:
- Fix typo's
- Add shaper_shared_(packet, byte)_mode_supported in level and node cap
- Fix comment in pkt_length_adjust.
- Move rte_eth_softnic_tm.c capability update to patch 1/4 to 
  avoid compilations issues in node and level cap array in softnicpmd.
  ../drivers/net/softnic/rte_eth_softnic_tm.c:782:3: warning: braces around scalar initializer
   {.nonleaf = {
  ../drivers/net/softnic/rte_eth_softnic_tm.c:782:3: note: (near initialization for ‘tm_node_cap[0].shaper_shared_byte_mode_supported’)
  ../drivers/net/softnic/rte_eth_softnic_tm.c:782:4: error: field name not in record or union initializer
   {.nonleaf = {

v1..v2:
- Add seperate capability for shaper and scheduler pktmode and bytemode.
- Add packet_mode field in struct rte_tm_shaper_params to indicate
packet mode shaper profile.


 drivers/net/softnic/rte_eth_softnic_tm.c |  72 +++++++++++
 lib/librte_ethdev/rte_tm.h               | 197 ++++++++++++++++++++++++++++++-
 2 files changed, 267 insertions(+), 2 deletions(-)

Comments

Cristian Dumitrescu April 24, 2020, 10:28 a.m. UTC | #1 
> -----Original Message-----
> From: Nithin Dabilpuram <nithind1988@gmail.com>
> Sent: Wednesday, April 22, 2020 6:21 PM
> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> Rybchenko <arybchenko@solarflare.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> Dabilpuram <ndabilpuram@marvell.com>
> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> mode
> 
> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> 
> Some NIC hardware support shaper to work in packet mode i.e
> shaping or ratelimiting traffic is in packets per second (PPS) as
> opposed to default bytes per second (BPS). Hence this patch
> adds support to configure shared or private shaper in packet mode,
> provide rate in PPS and add related tm capabilities in port/level/node
> capability structures.
> 
> This patch also updates tm port/level/node capability structures with
> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> and private/shared shaper byte mode.
> 
> SoftNIC PMD is also updated with new capabilities.
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> v3..v4:
> - Update text under packet_mode as per Cristian.
> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> - Add error enum RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> - Fix shaper_profile_check() with packet mode check
> - Fix typo's
> 

Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
Ferruh Yigit April 25, 2020, 8:09 p.m. UTC | #2 
On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> 
> 
>> -----Original Message-----
>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>> Sent: Wednesday, April 22, 2020 6:21 PM
>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>> Rybchenko <arybchenko@solarflare.com>
>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>> Dabilpuram <ndabilpuram@marvell.com>
>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>> mode
>>
>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>
>> Some NIC hardware support shaper to work in packet mode i.e
>> shaping or ratelimiting traffic is in packets per second (PPS) as
>> opposed to default bytes per second (BPS). Hence this patch
>> adds support to configure shared or private shaper in packet mode,
>> provide rate in PPS and add related tm capabilities in port/level/node
>> capability structures.
>>
>> This patch also updates tm port/level/node capability structures with
>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>> and private/shared shaper byte mode.
>>
>> SoftNIC PMD is also updated with new capabilities.
>>
>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>> ---
>> v3..v4:
>> - Update text under packet_mode as per Cristian.
>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
>> - Add error enum RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
>> - Fix shaper_profile_check() with packet mode check
>> - Fix typo's
>>
> 
> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 

Hi Nithin,

It looks like patch is causing ABI break, I am getting following warning [1],
can you please check?

[1]
https://pastebin.com/XYNFg14u
Cristian Dumitrescu April 27, 2020, 9:19 a.m. UTC | #3 
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Saturday, April 25, 2020 9:09 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <arybchenko@solarflare.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> Dabilpuram <ndabilpuram@marvell.com>
> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> mode
> 
> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >
> >
> >> -----Original Message-----
> >> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >> Sent: Wednesday, April 22, 2020 6:21 PM
> >> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> >> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> >> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> >> Rybchenko <arybchenko@solarflare.com>
> >> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >> Dabilpuram <ndabilpuram@marvell.com>
> >> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >> mode
> >>
> >> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>
> >> Some NIC hardware support shaper to work in packet mode i.e
> >> shaping or ratelimiting traffic is in packets per second (PPS) as
> >> opposed to default bytes per second (BPS). Hence this patch
> >> adds support to configure shared or private shaper in packet mode,
> >> provide rate in PPS and add related tm capabilities in port/level/node
> >> capability structures.
> >>
> >> This patch also updates tm port/level/node capability structures with
> >> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> >> and private/shared shaper byte mode.
> >>
> >> SoftNIC PMD is also updated with new capabilities.
> >>
> >> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >> ---
> >> v3..v4:
> >> - Update text under packet_mode as per Cristian.
> >> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> >> - Add error enum
> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> >> - Fix shaper_profile_check() with packet mode check
> >> - Fix typo's
> >>
> >
> > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> >
> 
> Hi Nithin,
> 
> It looks like patch is causing ABI break, I am getting following warning [1],
> can you please check?
> 
> [1]
> https://pastebin.com/XYNFg14u


Hi Ferruh,

The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.

It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.

Regards,
Cristian
Ferruh Yigit April 27, 2020, 4:12 p.m. UTC | #4 
On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> 
> 
>> -----Original Message-----
>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>> Sent: Saturday, April 25, 2020 9:09 PM
>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>> <arybchenko@solarflare.com>
>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>> Dabilpuram <ndabilpuram@marvell.com>
>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>> mode
>>
>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>> Sent: Wednesday, April 22, 2020 6:21 PM
>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>>>> Rybchenko <arybchenko@solarflare.com>
>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>> mode
>>>>
>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>
>>>> Some NIC hardware support shaper to work in packet mode i.e
>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
>>>> opposed to default bytes per second (BPS). Hence this patch
>>>> adds support to configure shared or private shaper in packet mode,
>>>> provide rate in PPS and add related tm capabilities in port/level/node
>>>> capability structures.
>>>>
>>>> This patch also updates tm port/level/node capability structures with
>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>>>> and private/shared shaper byte mode.
>>>>
>>>> SoftNIC PMD is also updated with new capabilities.
>>>>
>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>> ---
>>>> v3..v4:
>>>> - Update text under packet_mode as per Cristian.
>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
>>>> - Add error enum
>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
>>>> - Fix shaper_profile_check() with packet mode check
>>>> - Fix typo's
>>>>
>>>
>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>>>
>>
>> Hi Nithin,
>>
>> It looks like patch is causing ABI break, I am getting following warning [1],
>> can you please check?
>>
>> [1]
>> https://pastebin.com/XYNFg14u
> 
> 
> Hi Ferruh,
> 
> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
> 
> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
> 

:(

Is it time to mature them?

As you said they are not marked as experimental both in header file (function
declarations) and .map file.

The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
so marking them as experimental now will break the ABI. Not sure what to do,
cc'ed a few ABI related names for comment.

For me, we need to proceed as the experimental tag removed and APIs become
mature starting from v19.11, since this is what happened in practice, and remove
a few existing being experimental references in the doxygen comments.

Ray, Neil, David, Luca, Kevin, what do you think?
Cristian Dumitrescu April 27, 2020, 4:28 p.m. UTC | #5 
> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Monday, April 27, 2020 5:13 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> <arybchenko@solarflare.com>
> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> Dabilpuram <ndabilpuram@marvell.com>; Kinsella, Ray
> <ray.kinsella@intel.com>; Neil Horman <nhorman@tuxdriver.com>; Luca
> Boccassi <bluca@debian.org>; Kevin Traynor <ktraynor@redhat.com>; David
> Marchand <david.marchand@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> mode
> 
> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> >
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >> Sent: Saturday, April 25, 2020 9:09 PM
> >> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin
> Dabilpuram
> >> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> >> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> >> <arybchenko@solarflare.com>
> >> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >> Dabilpuram <ndabilpuram@marvell.com>
> >> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in
> pkt
> >> mode
> >>
> >> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >>>> Sent: Wednesday, April 22, 2020 6:21 PM
> >>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> >>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> >>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> Andrew
> >>>> Rybchenko <arybchenko@solarflare.com>
> >>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >>>> Dabilpuram <ndabilpuram@marvell.com>
> >>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >>>> mode
> >>>>
> >>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>>
> >>>> Some NIC hardware support shaper to work in packet mode i.e
> >>>> shaping or ratelimiting traffic is in packets per second (PPS) as
> >>>> opposed to default bytes per second (BPS). Hence this patch
> >>>> adds support to configure shared or private shaper in packet mode,
> >>>> provide rate in PPS and add related tm capabilities in port/level/node
> >>>> capability structures.
> >>>>
> >>>> This patch also updates tm port/level/node capability structures with
> >>>> exiting features of scheduler wfq packet mode, scheduler wfq byte
> mode
> >>>> and private/shared shaper byte mode.
> >>>>
> >>>> SoftNIC PMD is also updated with new capabilities.
> >>>>
> >>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>> ---
> >>>> v3..v4:
> >>>> - Update text under packet_mode as per Cristian.
> >>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> >>>> - Add error enum
> >> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> >>>> - Fix shaper_profile_check() with packet mode check
> >>>> - Fix typo's
> >>>>
> >>>
> >>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> >>>
> >>
> >> Hi Nithin,
> >>
> >> It looks like patch is causing ABI break, I am getting following warning [1],
> >> can you please check?
> >>
> >> [1]
> >> https://pastebin.com/XYNFg14u
> >
> >
> > Hi Ferruh,
> >
> > The RTE_TM API is marked as experimental, but it looks that this was not
> correctly marked when __rte_experimental ABI checker was introduced.
> >
> > It is marked as experimental at the top of the rte_tm.h, similarly to other
> APIs introduced around same time, but it was not correctly picked up by the
> ABI check procedure when later introduced, so __rte_experimental was not
> added to every function.
> >
> 
> :(
> 
> Is it time to mature them?
> 
> As you said they are not marked as experimental both in header file
> (function
> declarations) and .map file.
> 
> The problem is, they are not marked as experimental in DPDK_20.0 ABI
> (v19.11),
> so marking them as experimental now will break the ABI. Not sure what to
> do,
> cc'ed a few ABI related names for comment.
> 
> For me, we need to proceed as the experimental tag removed and APIs
> become
> mature starting from v19.11, since this is what happened in practice, and
> remove
> a few existing being experimental references in the doxygen comments.
> 
> Ray, Neil, David, Luca, Kevin, what do you think?

Hi Ferruh,

IMO your proposed approach is fixing the wrong problem and is probably not the right way of doing things.

This API is correctly marked as experimental in the header file rte_tm.h and in the MAINTAINERS file, therefore it should remain experimental, as more changes are expected from the people like Nithin and others currently upstreaming drivers for it.

For some reason, the __rte_experimental tags were not added to this file when the ABI checker was introduced, and this is the real problem that should be fixed.

Regards,
Cristian
Jerin Jacob April 27, 2020, 4:29 p.m. UTC | #6 
On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> >
> >
> >> -----Original Message-----
> >> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >> Sent: Saturday, April 25, 2020 9:09 PM
> >> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> >> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> >> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> >> <arybchenko@solarflare.com>
> >> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >> Dabilpuram <ndabilpuram@marvell.com>
> >> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >> mode
> >>
> >> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >>>> Sent: Wednesday, April 22, 2020 6:21 PM
> >>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> >>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> >>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> >>>> Rybchenko <arybchenko@solarflare.com>
> >>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >>>> Dabilpuram <ndabilpuram@marvell.com>
> >>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >>>> mode
> >>>>
> >>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>>
> >>>> Some NIC hardware support shaper to work in packet mode i.e
> >>>> shaping or ratelimiting traffic is in packets per second (PPS) as
> >>>> opposed to default bytes per second (BPS). Hence this patch
> >>>> adds support to configure shared or private shaper in packet mode,
> >>>> provide rate in PPS and add related tm capabilities in port/level/node
> >>>> capability structures.
> >>>>
> >>>> This patch also updates tm port/level/node capability structures with
> >>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> >>>> and private/shared shaper byte mode.
> >>>>
> >>>> SoftNIC PMD is also updated with new capabilities.
> >>>>
> >>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>> ---
> >>>> v3..v4:
> >>>> - Update text under packet_mode as per Cristian.
> >>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> >>>> - Add error enum
> >> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> >>>> - Fix shaper_profile_check() with packet mode check
> >>>> - Fix typo's
> >>>>
> >>>
> >>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> >>>
> >>
> >> Hi Nithin,
> >>
> >> It looks like patch is causing ABI break, I am getting following warning [1],
> >> can you please check?
> >>
> >> [1]
> >> https://pastebin.com/XYNFg14u
> >
> >
> > Hi Ferruh,
> >
> > The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
> >
> > It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
> >
>
> :(
>
> Is it time to mature them?
>
> As you said they are not marked as experimental both in header file (function
> declarations) and .map file.
>
> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> so marking them as experimental now will break the ABI. Not sure what to do,
> cc'ed a few ABI related names for comment.
>
> For me, we need to proceed as the experimental tag removed and APIs become
> mature starting from v19.11, since this is what happened in practice, and remove
> a few existing being experimental references in the doxygen comments.

I think, accidentally we can not make a library as NON-experimental.
TM never went through experimental to mature transition(see git log
lib/librte_ethdev/rte_tm.h)
It was a bug to not mark as experimental in each function in the ABI process.
Some of the features like packet marking are not even implemented by any HW.
I think, we can make API stable only all the features are implemented
by one or two HW.

>
> Ray, Neil, David, Luca, Kevin, what do you think?
Ferruh Yigit April 27, 2020, 4:49 p.m. UTC | #7 
On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>> Sent: Saturday, April 25, 2020 9:09 PM
>>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
>>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
>>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>>>> <arybchenko@solarflare.com>
>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>> mode
>>>>
>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
>>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
>>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
>>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>>>>>> Rybchenko <arybchenko@solarflare.com>
>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>> mode
>>>>>>
>>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>
>>>>>> Some NIC hardware support shaper to work in packet mode i.e
>>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
>>>>>> opposed to default bytes per second (BPS). Hence this patch
>>>>>> adds support to configure shared or private shaper in packet mode,
>>>>>> provide rate in PPS and add related tm capabilities in port/level/node
>>>>>> capability structures.
>>>>>>
>>>>>> This patch also updates tm port/level/node capability structures with
>>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>>>>>> and private/shared shaper byte mode.
>>>>>>
>>>>>> SoftNIC PMD is also updated with new capabilities.
>>>>>>
>>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>> ---
>>>>>> v3..v4:
>>>>>> - Update text under packet_mode as per Cristian.
>>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
>>>>>> - Add error enum
>>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
>>>>>> - Fix shaper_profile_check() with packet mode check
>>>>>> - Fix typo's
>>>>>>
>>>>>
>>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>>>>>
>>>>
>>>> Hi Nithin,
>>>>
>>>> It looks like patch is causing ABI break, I am getting following warning [1],
>>>> can you please check?
>>>>
>>>> [1]
>>>> https://pastebin.com/XYNFg14u
>>>
>>>
>>> Hi Ferruh,
>>>
>>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
>>>
>>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
>>>
>>
>> :(
>>
>> Is it time to mature them?
>>
>> As you said they are not marked as experimental both in header file (function
>> declarations) and .map file.
>>
>> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
>> so marking them as experimental now will break the ABI. Not sure what to do,
>> cc'ed a few ABI related names for comment.
>>
>> For me, we need to proceed as the experimental tag removed and APIs become
>> mature starting from v19.11, since this is what happened in practice, and remove
>> a few existing being experimental references in the doxygen comments.
> 
> I think, accidentally we can not make a library as NON-experimental.
> TM never went through experimental to mature transition(see git log
> lib/librte_ethdev/rte_tm.h)
> It was a bug to not mark as experimental in each function in the ABI process.
> Some of the features like packet marking are not even implemented by any HW.
> I think, we can make API stable only all the features are implemented
> by one or two HW.

Fair enough, specially if the API is not ready yet.

But they were part of stable ABI, and marking them as experimental now will
break the old applications using these APIs.

> 
>>
>> Ray, Neil, David, Luca, Kevin, what do you think?
Jerin Jacob April 27, 2020, 4:59 p.m. UTC | #8 
On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >>>> Sent: Saturday, April 25, 2020 9:09 PM
> >>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> >>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> >>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> >>>> <arybchenko@solarflare.com>
> >>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >>>> Dabilpuram <ndabilpuram@marvell.com>
> >>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >>>> mode
> >>>>
> >>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
> >>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> >>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> >>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> >>>>>> Rybchenko <arybchenko@solarflare.com>
> >>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >>>>>> Dabilpuram <ndabilpuram@marvell.com>
> >>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >>>>>> mode
> >>>>>>
> >>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>>>>
> >>>>>> Some NIC hardware support shaper to work in packet mode i.e
> >>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
> >>>>>> opposed to default bytes per second (BPS). Hence this patch
> >>>>>> adds support to configure shared or private shaper in packet mode,
> >>>>>> provide rate in PPS and add related tm capabilities in port/level/node
> >>>>>> capability structures.
> >>>>>>
> >>>>>> This patch also updates tm port/level/node capability structures with
> >>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> >>>>>> and private/shared shaper byte mode.
> >>>>>>
> >>>>>> SoftNIC PMD is also updated with new capabilities.
> >>>>>>
> >>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>>>> ---
> >>>>>> v3..v4:
> >>>>>> - Update text under packet_mode as per Cristian.
> >>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> >>>>>> - Add error enum
> >>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> >>>>>> - Fix shaper_profile_check() with packet mode check
> >>>>>> - Fix typo's
> >>>>>>
> >>>>>
> >>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> >>>>>
> >>>>
> >>>> Hi Nithin,
> >>>>
> >>>> It looks like patch is causing ABI break, I am getting following warning [1],
> >>>> can you please check?
> >>>>
> >>>> [1]
> >>>> https://pastebin.com/XYNFg14u
> >>>
> >>>
> >>> Hi Ferruh,
> >>>
> >>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
> >>>
> >>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
> >>>
> >>
> >> :(
> >>
> >> Is it time to mature them?
> >>
> >> As you said they are not marked as experimental both in header file (function
> >> declarations) and .map file.
> >>
> >> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> >> so marking them as experimental now will break the ABI. Not sure what to do,
> >> cc'ed a few ABI related names for comment.
> >>
> >> For me, we need to proceed as the experimental tag removed and APIs become
> >> mature starting from v19.11, since this is what happened in practice, and remove
> >> a few existing being experimental references in the doxygen comments.
> >
> > I think, accidentally we can not make a library as NON-experimental.
> > TM never went through experimental to mature transition(see git log
> > lib/librte_ethdev/rte_tm.h)
> > It was a bug to not mark as experimental in each function in the ABI process.
> > Some of the features like packet marking are not even implemented by any HW.
> > I think, we can make API stable only all the features are implemented
> > by one or two HW.
>
> Fair enough, specially if the API is not ready yet.
>
> But they were part of stable ABI, and marking them as experimental now will
> break the old applications using these APIs.

it is still marked as EXPERIMENTAL everywhere and API is not ready yet.
Anyway, we need to break the ABI to make it work on various HW.
I am not sure what to do?
IMO, We need to send a patch as Fixes: for the bug of not adding
__rte_experimental in each function.

Traffic Management API - EXPERIMENTAL
M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
T: git://dpdk.org/next/dpdk-next-qos
F: lib/librte_ethdev/rte_tm*
>
> >
> >>
> >> Ray, Neil, David, Luca, Kevin, what do you think?
>
Nithin Dabilpuram April 28, 2020, 11:51 a.m. UTC | #9 
On Mon, Apr 27, 2020 at 10:29:48PM +0530, Jerin Jacob wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >
> > On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > >>
> > >> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > >>>
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > >>>> Sent: Saturday, April 25, 2020 9:09 PM
> > >>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> > >>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> > >>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> > >>>> <arybchenko@solarflare.com>
> > >>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> > >>>> Dabilpuram <ndabilpuram@marvell.com>
> > >>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> > >>>> mode
> > >>>>
> > >>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> > >>>>>
> > >>>>>
> > >>>>>> -----Original Message-----
> > >>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> > >>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
> > >>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> > >>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> > >>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> > >>>>>> Rybchenko <arybchenko@solarflare.com>
> > >>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> > >>>>>> Dabilpuram <ndabilpuram@marvell.com>
> > >>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> > >>>>>> mode
> > >>>>>>
> > >>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > >>>>>>
> > >>>>>> Some NIC hardware support shaper to work in packet mode i.e
> > >>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
> > >>>>>> opposed to default bytes per second (BPS). Hence this patch
> > >>>>>> adds support to configure shared or private shaper in packet mode,
> > >>>>>> provide rate in PPS and add related tm capabilities in port/level/node
> > >>>>>> capability structures.
> > >>>>>>
> > >>>>>> This patch also updates tm port/level/node capability structures with
> > >>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> > >>>>>> and private/shared shaper byte mode.
> > >>>>>>
> > >>>>>> SoftNIC PMD is also updated with new capabilities.
> > >>>>>>
> > >>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > >>>>>> ---
> > >>>>>> v3..v4:
> > >>>>>> - Update text under packet_mode as per Cristian.
> > >>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> > >>>>>> - Add error enum
> > >>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> > >>>>>> - Fix shaper_profile_check() with packet mode check
> > >>>>>> - Fix typo's
> > >>>>>>
> > >>>>>
> > >>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > >>>>>
> > >>>>
> > >>>> Hi Nithin,
> > >>>>
> > >>>> It looks like patch is causing ABI break, I am getting following warning [1],
> > >>>> can you please check?
> > >>>>
> > >>>> [1]
> > >>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=xJB0Qb2Q-1bl0hEDeknUjJqrCDc3z-h0F0e7kj8KvvI&s=R6xtRQRRIIzAilc5z52oYjyHNlvvJB_9SUsKBkpPC6g&e= 
> > >>>
> > >>>
> > >>> Hi Ferruh,
> > >>>
> > >>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
> > >>>
> > >>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
> > >>>
> > >>
> > >> :(
> > >>
> > >> Is it time to mature them?
> > >>
> > >> As you said they are not marked as experimental both in header file (function
> > >> declarations) and .map file.
> > >>
> > >> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> > >> so marking them as experimental now will break the ABI. Not sure what to do,
> > >> cc'ed a few ABI related names for comment.
> > >>
> > >> For me, we need to proceed as the experimental tag removed and APIs become
> > >> mature starting from v19.11, since this is what happened in practice, and remove
> > >> a few existing being experimental references in the doxygen comments.
> > >
> > > I think, accidentally we can not make a library as NON-experimental.
> > > TM never went through experimental to mature transition(see git log
> > > lib/librte_ethdev/rte_tm.h)
> > > It was a bug to not mark as experimental in each function in the ABI process.
> > > Some of the features like packet marking are not even implemented by any HW.
> > > I think, we can make API stable only all the features are implemented
> > > by one or two HW.
> >
> > Fair enough, specially if the API is not ready yet.
> >
> > But they were part of stable ABI, and marking them as experimental now will
> > break the old applications using these APIs.
> 
> it is still marked as EXPERIMENTAL everywhere and API is not ready yet.
> Anyway, we need to break the ABI to make it work on various HW.
> I am not sure what to do?
> IMO, We need to send a patch as Fixes: for the bug of not adding
> __rte_experimental in each function.
> 
> Traffic Management API - EXPERIMENTAL
> M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> T: git://dpdk.org/next/dpdk-next-qos
> F: lib/librte_ethdev/rte_tm*

Ray, Neil, David, Luca, Kevin, Ferruh

Any thoughts on this proposal ?

If it is fine, I can send a "Fixes:" patch to update experimental attribute in rte_tm.h
for all functions so that 20.05 is having the right marking.

> >
> > >
> > >>
> > >> Ray, Neil, David, Luca, Kevin, what do you think?
> >
Ferruh Yigit April 28, 2020, 1:56 p.m. UTC | #10 
On 4/28/2020 12:51 PM, Nithin Dabilpuram wrote:
> On Mon, Apr 27, 2020 at 10:29:48PM +0530, Jerin Jacob wrote:
>> External Email
>>
>> ----------------------------------------------------------------------
>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>
>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>>
>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>>> Sent: Saturday, April 25, 2020 9:09 PM
>>>>>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
>>>>>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
>>>>>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>>>>>>> <arybchenko@solarflare.com>
>>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>>> mode
>>>>>>>
>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
>>>>>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
>>>>>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
>>>>>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>>>>>>>>> Rybchenko <arybchenko@solarflare.com>
>>>>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>>>>> mode
>>>>>>>>>
>>>>>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>>
>>>>>>>>> Some NIC hardware support shaper to work in packet mode i.e
>>>>>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
>>>>>>>>> opposed to default bytes per second (BPS). Hence this patch
>>>>>>>>> adds support to configure shared or private shaper in packet mode,
>>>>>>>>> provide rate in PPS and add related tm capabilities in port/level/node
>>>>>>>>> capability structures.
>>>>>>>>>
>>>>>>>>> This patch also updates tm port/level/node capability structures with
>>>>>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>>
>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>> ---
>>>>>>>>> v3..v4:
>>>>>>>>> - Update text under packet_mode as per Cristian.
>>>>>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
>>>>>>>>> - Add error enum
>>>>>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
>>>>>>>>> - Fix shaper_profile_check() with packet mode check
>>>>>>>>> - Fix typo's
>>>>>>>>>
>>>>>>>>
>>>>>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>>>>>>>>
>>>>>>>
>>>>>>> Hi Nithin,
>>>>>>>
>>>>>>> It looks like patch is causing ABI break, I am getting following warning [1],
>>>>>>> can you please check?
>>>>>>>
>>>>>>> [1]
>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIBaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=xJB0Qb2Q-1bl0hEDeknUjJqrCDc3z-h0F0e7kj8KvvI&s=R6xtRQRRIIzAilc5z52oYjyHNlvvJB_9SUsKBkpPC6g&e= 
>>>>>>
>>>>>>
>>>>>> Hi Ferruh,
>>>>>>
>>>>>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
>>>>>>
>>>>>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
>>>>>>
>>>>>
>>>>> :(
>>>>>
>>>>> Is it time to mature them?
>>>>>
>>>>> As you said they are not marked as experimental both in header file (function
>>>>> declarations) and .map file.
>>>>>
>>>>> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
>>>>> so marking them as experimental now will break the ABI. Not sure what to do,
>>>>> cc'ed a few ABI related names for comment.
>>>>>
>>>>> For me, we need to proceed as the experimental tag removed and APIs become
>>>>> mature starting from v19.11, since this is what happened in practice, and remove
>>>>> a few existing being experimental references in the doxygen comments.
>>>>
>>>> I think, accidentally we can not make a library as NON-experimental.
>>>> TM never went through experimental to mature transition(see git log
>>>> lib/librte_ethdev/rte_tm.h)
>>>> It was a bug to not mark as experimental in each function in the ABI process.
>>>> Some of the features like packet marking are not even implemented by any HW.
>>>> I think, we can make API stable only all the features are implemented
>>>> by one or two HW.
>>>
>>> Fair enough, specially if the API is not ready yet.
>>>
>>> But they were part of stable ABI, and marking them as experimental now will
>>> break the old applications using these APIs.
>>
>> it is still marked as EXPERIMENTAL everywhere and API is not ready yet.
>> Anyway, we need to break the ABI to make it work on various HW.
>> I am not sure what to do?
>> IMO, We need to send a patch as Fixes: for the bug of not adding
>> __rte_experimental in each function.
>>
>> Traffic Management API - EXPERIMENTAL
>> M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>> T: git://dpdk.org/next/dpdk-next-qos
>> F: lib/librte_ethdev/rte_tm*
> 
> Ray, Neil, David, Luca, Kevin, Ferruh
> 
> Any thoughts on this proposal ?
> 
> If it is fine, I can send a "Fixes:" patch to update experimental attribute in rte_tm.h
> for all functions so that 20.05 is having the right marking.

Hi Nithin,

Please send the fix patch, we may continue to discuss in that patch but it would
be good to have it ready.

> 
>>>
>>>>
>>>>>
>>>>> Ray, Neil, David, Luca, Kevin, what do you think?
>>>
Ferruh Yigit April 28, 2020, 2:06 p.m. UTC | #11 
On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>
>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>> Sent: Saturday, April 25, 2020 9:09 PM
>>>>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
>>>>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
>>>>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>>>>>> <arybchenko@solarflare.com>
>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>> mode
>>>>>>
>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
>>>>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
>>>>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
>>>>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>>>>>>>> Rybchenko <arybchenko@solarflare.com>
>>>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>>>> mode
>>>>>>>>
>>>>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>
>>>>>>>> Some NIC hardware support shaper to work in packet mode i.e
>>>>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
>>>>>>>> opposed to default bytes per second (BPS). Hence this patch
>>>>>>>> adds support to configure shared or private shaper in packet mode,
>>>>>>>> provide rate in PPS and add related tm capabilities in port/level/node
>>>>>>>> capability structures.
>>>>>>>>
>>>>>>>> This patch also updates tm port/level/node capability structures with
>>>>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>
>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>>>>>>>>
>>>>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>> ---
>>>>>>>> v3..v4:
>>>>>>>> - Update text under packet_mode as per Cristian.
>>>>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
>>>>>>>> - Add error enum
>>>>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
>>>>>>>> - Fix shaper_profile_check() with packet mode check
>>>>>>>> - Fix typo's
>>>>>>>>
>>>>>>>
>>>>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>>>>>>>
>>>>>>
>>>>>> Hi Nithin,
>>>>>>
>>>>>> It looks like patch is causing ABI break, I am getting following warning [1],
>>>>>> can you please check?
>>>>>>
>>>>>> [1]
>>>>>> https://pastebin.com/XYNFg14u
>>>>>
>>>>>
>>>>> Hi Ferruh,
>>>>>
>>>>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
>>>>>
>>>>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
>>>>>
>>>>
>>>> :(
>>>>
>>>> Is it time to mature them?
>>>>
>>>> As you said they are not marked as experimental both in header file (function
>>>> declarations) and .map file.
>>>>
>>>> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
>>>> so marking them as experimental now will break the ABI. Not sure what to do,
>>>> cc'ed a few ABI related names for comment.
>>>>
>>>> For me, we need to proceed as the experimental tag removed and APIs become
>>>> mature starting from v19.11, since this is what happened in practice, and remove
>>>> a few existing being experimental references in the doxygen comments.
>>>
>>> I think, accidentally we can not make a library as NON-experimental.
>>> TM never went through experimental to mature transition(see git log
>>> lib/librte_ethdev/rte_tm.h)
>>> It was a bug to not mark as experimental in each function in the ABI process.
>>> Some of the features like packet marking are not even implemented by any HW.
>>> I think, we can make API stable only all the features are implemented
>>> by one or two HW.
>>
>> Fair enough, specially if the API is not ready yet.
>>
>> But they were part of stable ABI, and marking them as experimental now will
>> break the old applications using these APIs.
> 
> it is still marked as EXPERIMENTAL everywhere and API is not ready yet.

Existing experimental marks are text only for human parsing.

The compiler attribute and build time checks are missing, and the symbol in the
binary doesn't have experimental tag. Our scripts and automated checks won't
detect it as experimental.

My point is just having experimental comment in header file is not enough to
qualify the APIs as experimental.

> Anyway, we need to break the ABI to make it work on various HW.
> I am not sure what to do?
> IMO, We need to send a patch as Fixes: for the bug of not adding
> __rte_experimental in each function.

Yes, this is where we are, both you and Cristian suggest API is not ready and
should be experimental, but they were part of stable ABI, making them
experimental will break the ABI.
It looks like there is no good option but we should select one of the bad ones.

> 
> Traffic Management API - EXPERIMENTAL
> M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> T: git://dpdk.org/next/dpdk-next-qos
> F: lib/librte_ethdev/rte_tm*
>>
>>>
>>>>
>>>> Ray, Neil, David, Luca, Kevin, what do you think?
>>
Bruce Richardson April 28, 2020, 2:45 p.m. UTC | #12 
On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>
> >> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> >>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> >>>>
> >>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >>>>>> Sent: Saturday, April 25, 2020 9:09 PM
> >>>>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> >>>>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> >>>>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> >>>>>> <arybchenko@solarflare.com>
> >>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >>>>>> Dabilpuram <ndabilpuram@marvell.com>
> >>>>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >>>>>> mode
> >>>>>>
> >>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >>>>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
> >>>>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> >>>>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
> >>>>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> >>>>>>>> Rybchenko <arybchenko@solarflare.com>
> >>>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> >>>>>>>> Dabilpuram <ndabilpuram@marvell.com>
> >>>>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> >>>>>>>> mode
> >>>>>>>>
> >>>>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>>>>>>
> >>>>>>>> Some NIC hardware support shaper to work in packet mode i.e
> >>>>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
> >>>>>>>> opposed to default bytes per second (BPS). Hence this patch
> >>>>>>>> adds support to configure shared or private shaper in packet mode,
> >>>>>>>> provide rate in PPS and add related tm capabilities in port/level/node
> >>>>>>>> capability structures.
> >>>>>>>>
> >>>>>>>> This patch also updates tm port/level/node capability structures with
> >>>>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> >>>>>>>> and private/shared shaper byte mode.
> >>>>>>>>
> >>>>>>>> SoftNIC PMD is also updated with new capabilities.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>>>>>>> ---
> >>>>>>>> v3..v4:
> >>>>>>>> - Update text under packet_mode as per Cristian.
> >>>>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> >>>>>>>> - Add error enum
> >>>>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> >>>>>>>> - Fix shaper_profile_check() with packet mode check
> >>>>>>>> - Fix typo's
> >>>>>>>>
> >>>>>>>
> >>>>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> >>>>>>>
> >>>>>>
> >>>>>> Hi Nithin,
> >>>>>>
> >>>>>> It looks like patch is causing ABI break, I am getting following warning [1],
> >>>>>> can you please check?
> >>>>>>
> >>>>>> [1]
> >>>>>> https://pastebin.com/XYNFg14u
> >>>>>
> >>>>>
> >>>>> Hi Ferruh,
> >>>>>
> >>>>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
> >>>>>
> >>>>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
> >>>>>
> >>>>
> >>>> :(
> >>>>
> >>>> Is it time to mature them?
> >>>>
> >>>> As you said they are not marked as experimental both in header file (function
> >>>> declarations) and .map file.
> >>>>
> >>>> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> >>>> so marking them as experimental now will break the ABI. Not sure what to do,
> >>>> cc'ed a few ABI related names for comment.
> >>>>
> >>>> For me, we need to proceed as the experimental tag removed and APIs become
> >>>> mature starting from v19.11, since this is what happened in practice, and remove
> >>>> a few existing being experimental references in the doxygen comments.
> >>>
> >>> I think, accidentally we can not make a library as NON-experimental.
> >>> TM never went through experimental to mature transition(see git log
> >>> lib/librte_ethdev/rte_tm.h)
> >>> It was a bug to not mark as experimental in each function in the ABI process.
> >>> Some of the features like packet marking are not even implemented by any HW.
> >>> I think, we can make API stable only all the features are implemented
> >>> by one or two HW.
> >>
> >> Fair enough, specially if the API is not ready yet.
> >>
> >> But they were part of stable ABI, and marking them as experimental now will
> >> break the old applications using these APIs.
> > 
> > it is still marked as EXPERIMENTAL everywhere and API is not ready yet.
> 
> Existing experimental marks are text only for human parsing.
> 
> The compiler attribute and build time checks are missing, and the symbol in the
> binary doesn't have experimental tag. Our scripts and automated checks won't
> detect it as experimental.
> 
> My point is just having experimental comment in header file is not enough to
> qualify the APIs as experimental.
> 
> > Anyway, we need to break the ABI to make it work on various HW.
> > I am not sure what to do?
> > IMO, We need to send a patch as Fixes: for the bug of not adding
> > __rte_experimental in each function.
> 
> Yes, this is where we are, both you and Cristian suggest API is not ready and
> should be experimental, but they were part of stable ABI, making them
> experimental will break the ABI.
> It looks like there is no good option but we should select one of the bad ones.
> 
> > 
> > Traffic Management API - EXPERIMENTAL
> > M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > T: git://dpdk.org/next/dpdk-next-qos
> > F: lib/librte_ethdev/rte_tm*
> >>
> >>>
> >>>>
> >>>> Ray, Neil, David, Luca, Kevin, what do you think?
> >>
While I'm not called any of those names, allow me to give my 2c.

Since these are marked in binaries as part of the stable ABI, I think we
need to honour that for the next two releases 20.05 and 20.08 [which means
that we need to put in versioned functions for any changes, not that we
can't change anything]

For 20.11, I think these should then have one of two options taken:
* have these "fixed" and ready to be marked as stable, and officially part
  of v21 ABI or
* mark them as experimental properly, and look to have them as part of the
  v22 or subsequent ABI

Given the comments here, I would tend towards the latter of the above two
options, but that's really a decision for the maintainers.

Remember, this is not the first bug we have encountered where we messed up
some ABI versions in the 19.11 release, and, like the previous one with the
screwed up version number, I think we need to honour the ABI committments
made, especially since in this case it's only for a few more months till
20.11 development starts.

/Bruce
Luca Boccassi April 28, 2020, 3:04 p.m. UTC | #13 
On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> > On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> > > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > > > > > 
> > > > > > > > -----Original Message-----
> > > > > > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > > > > > > Sent: Saturday, April 25, 2020 9:09 PM
> > > > > > > > To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
> > > > > > > > <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> > > > > > > > Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
> > > > > > > > <arybchenko@solarflare.com>
> > > > > > > > Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> > > > > > > > Dabilpuram <ndabilpuram@marvell.com>
> > > > > > > > Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> > > > > > > > mode
> > > > > > > > 
> > > > > > > > On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > 
> > > > > > > > > > -----Original Message-----
> > > > > > > > > > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > > > > > > > > > Sent: Wednesday, April 22, 2020 6:21 PM
> > > > > > > > > > To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
> > > > > > > > > > <cristian.dumitrescu@intel.com>; Thomas Monjalon
> > > > > > > > > > <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
> > > > > > > > > > Rybchenko <arybchenko@solarflare.com>
> > > > > > > > > > Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
> > > > > > > > > > Dabilpuram <ndabilpuram@marvell.com>
> > > > > > > > > > Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> > > > > > > > > > mode
> > > > > > > > > > 
> > > > > > > > > > From: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > > > > > > > > 
> > > > > > > > > > Some NIC hardware support shaper to work in packet mode i.e
> > > > > > > > > > shaping or ratelimiting traffic is in packets per second (PPS) as
> > > > > > > > > > opposed to default bytes per second (BPS). Hence this patch
> > > > > > > > > > adds support to configure shared or private shaper in packet mode,
> > > > > > > > > > provide rate in PPS and add related tm capabilities in port/level/node
> > > > > > > > > > capability structures.
> > > > > > > > > > 
> > > > > > > > > > This patch also updates tm port/level/node capability structures with
> > > > > > > > > > exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> > > > > > > > > > and private/shared shaper byte mode.
> > > > > > > > > > 
> > > > > > > > > > SoftNIC PMD is also updated with new capabilities.
> > > > > > > > > > 
> > > > > > > > > > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > > > > > > > > ---
> > > > > > > > > > v3..v4:
> > > > > > > > > > - Update text under packet_mode as per Cristian.
> > > > > > > > > > - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
> > > > > > > > > > - Add error enum
> > > > > > > > RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
> > > > > > > > > > - Fix shaper_profile_check() with packet mode check
> > > > > > > > > > - Fix typo's
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > > > > > > > 
> > > > > > > > 
> > > > > > > > Hi Nithin,
> > > > > > > > 
> > > > > > > > It looks like patch is causing ABI break, I am getting following warning [1],
> > > > > > > > can you please check?
> > > > > > > > 
> > > > > > > > [1]
> > > > > > > > https://pastebin.com/XYNFg14u
> > > > > > > 
> > > > > > > Hi Ferruh,
> > > > > > > 
> > > > > > > The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
> > > > > > > 
> > > > > > > It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
> > > > > > > 
> > > > > > 
> > > > > > :(
> > > > > > 
> > > > > > Is it time to mature them?
> > > > > > 
> > > > > > As you said they are not marked as experimental both in header file (function
> > > > > > declarations) and .map file.
> > > > > > 
> > > > > > The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> > > > > > so marking them as experimental now will break the ABI. Not sure what to do,
> > > > > > cc'ed a few ABI related names for comment.
> > > > > > 
> > > > > > For me, we need to proceed as the experimental tag removed and APIs become
> > > > > > mature starting from v19.11, since this is what happened in practice, and remove
> > > > > > a few existing being experimental references in the doxygen comments.
> > > > > 
> > > > > I think, accidentally we can not make a library as NON-experimental.
> > > > > TM never went through experimental to mature transition(see git log
> > > > > lib/librte_ethdev/rte_tm.h)
> > > > > It was a bug to not mark as experimental in each function in the ABI process.
> > > > > Some of the features like packet marking are not even implemented by any HW.
> > > > > I think, we can make API stable only all the features are implemented
> > > > > by one or two HW.
> > > > 
> > > > Fair enough, specially if the API is not ready yet.
> > > > 
> > > > But they were part of stable ABI, and marking them as experimental now will
> > > > break the old applications using these APIs.
> > > 
> > > it is still marked as EXPERIMENTAL everywhere and API is not ready yet.
> > 
> > Existing experimental marks are text only for human parsing.
> > 
> > The compiler attribute and build time checks are missing, and the symbol in the
> > binary doesn't have experimental tag. Our scripts and automated checks won't
> > detect it as experimental.
> > 
> > My point is just having experimental comment in header file is not enough to
> > qualify the APIs as experimental.
> > 
> > > Anyway, we need to break the ABI to make it work on various HW.
> > > I am not sure what to do?
> > > IMO, We need to send a patch as Fixes: for the bug of not adding
> > > __rte_experimental in each function.
> > 
> > Yes, this is where we are, both you and Cristian suggest API is not ready and
> > should be experimental, but they were part of stable ABI, making them
> > experimental will break the ABI.
> > It looks like there is no good option but we should select one of the bad ones.
> > 
> > > Traffic Management API - EXPERIMENTAL
> > > M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > T: git://dpdk.org/next/dpdk-next-qos
> > > F: lib/librte_ethdev/rte_tm*
> > > > > > Ray, Neil, David, Luca, Kevin, what do you think?
> While I'm not called any of those names, allow me to give my 2c.
> 
> Since these are marked in binaries as part of the stable ABI, I think we
> need to honour that for the next two releases 20.05 and 20.08 [which means
> that we need to put in versioned functions for any changes, not that we
> can't change anything]
> 
> For 20.11, I think these should then have one of two options taken:
> * have these "fixed" and ready to be marked as stable, and officially part
>   of v21 ABI or
> * mark them as experimental properly, and look to have them as part of the
>   v22 or subsequent ABI
> 
> Given the comments here, I would tend towards the latter of the above two
> options, but that's really a decision for the maintainers.
> 
> Remember, this is not the first bug we have encountered where we messed up
> some ABI versions in the 19.11 release, and, like the previous one with the
> screwed up version number, I think we need to honour the ABI committments
> made, especially since in this case it's only for a few more months till
> 20.11 development starts.
> 
> /Bruce

+1

If they are not ready now, they haven't been ready for the past 6
months either, so staying not ready for 6 more is the lesser evil.
Thomas Monjalon April 28, 2020, 3:30 p.m. UTC | #14 
27/04/2020 18:28, Dumitrescu, Cristian:
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > >> Hi Nithin,
> > >>
> > >> It looks like patch is causing ABI break, I am getting following warning [1],
> > >> can you please check?
> > >>
> > >> [1]
> > >> https://pastebin.com/XYNFg14u
> > >
> > >
> > > Hi Ferruh,
> > >
> > > The RTE_TM API is marked as experimental, but it looks that this was not
> > correctly marked when __rte_experimental ABI checker was introduced.
> > >
> > > It is marked as experimental at the top of the rte_tm.h, similarly to other
> > APIs introduced around same time, but it was not correctly picked up by the
> > ABI check procedure when later introduced, so __rte_experimental was not
> > added to every function.
> > >
> > 
> > :(
> > 
> > Is it time to mature them?
> > 
> > As you said they are not marked as experimental both in header file
> > (function
> > declarations) and .map file.
> > 
> > The problem is, they are not marked as experimental in DPDK_20.0 ABI
> > (v19.11),
> > so marking them as experimental now will break the ABI. Not sure what to
> > do,
> > cc'ed a few ABI related names for comment.
> > 
> > For me, we need to proceed as the experimental tag removed and APIs
> > become
> > mature starting from v19.11, since this is what happened in practice, and
> > remove
> > a few existing being experimental references in the doxygen comments.
> > 
> > Ray, Neil, David, Luca, Kevin, what do you think?
> 
> Hi Ferruh,
> 
> IMO your proposed approach is fixing the wrong problem and is
> probably not the right way of doing things.
> 
> This API is correctly marked as experimental in the header
> file rte_tm.h and in the MAINTAINERS file,

in rte_tm.h:
	* @warning
	* @b EXPERIMENTAL: this API may change without prior notice

in MAINTAINERS:
	Traffic Management API - EXPERIMENTAL
	M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
	T: git://dpdk.org/next/dpdk-next-qos
	F: lib/librte_ethdev/rte_tm*


> therefore it should remain experimental,

in rte_ethdev_version.map:
	before 19.11: DPDK_17.08 {
	since 19.11: DPDK_20.0 {

When adding rte_tm in 17.08:
http://git.dpdk.org/dpdk/diff/lib/librte_ether/rte_ether_version.map?id=5d109deffa

In 17.08, early July 2017, the first EXPERIMENTAL section was declared in EAL:
http://git.dpdk.org/dpdk/diff/lib/librte_eal/linuxapp/eal/rte_eal_version.map?id=a3ee360f444

When adding rte_mtr in 17.11, all functions are made experimental:
http://git.dpdk.org/dpdk/diff/lib/librte_ether/rte_ethdev_version.map?id=6613ffe1

In 18.02, the tag __rte_experimental was introduced:
	http://git.dpdk.org/dpdk/commit/?id=7d540a3e735
and functions are marked (including rte_mtr but not rte_tm):
	http://git.dpdk.org/dpdk/commit/?id=77b7b81e32e


> as more changes are expected from the people like Nithin and others
> currently upstreaming drivers for it.

They are doing changes in the API introduced 3 years ago.


> For some reason, the __rte_experimental tags were not added to
> this file when the ABI checker was introduced,
> and this is the real problem that should be fixed.

The mistake was done 2 years ago.
As maintainer of rte_tm code, you are expected to notice the issue.
As maintainer of rte_mtr code, you were expected to review the change
on the mtr functions being marked as experimental.
I am sorry that it took 2 years to discover the gap.

We can fix the ABI in the ABI-breakage window: in 20.11.
Ray Kinsella April 28, 2020, 3:42 p.m. UTC | #15 
On 28/04/2020 15:45, Bruce Richardson wrote:
> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>
>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>>>
>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>>>> Sent: Saturday, April 25, 2020 9:09 PM
>>>>>>>> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>; Nithin Dabilpuram
>>>>>>>> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
>>>>>>>> Thomas Monjalon <thomas@monjalon.net>; Andrew Rybchenko
>>>>>>>> <arybchenko@solarflare.com>
>>>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>>>> mode
>>>>>>>>
>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>>>> Sent: Wednesday, April 22, 2020 6:21 PM
>>>>>>>>>> To: Singh, Jasvinder <jasvinder.singh@intel.com>; Dumitrescu, Cristian
>>>>>>>>>> <cristian.dumitrescu@intel.com>; Thomas Monjalon
>>>>>>>>>> <thomas@monjalon.net>; Yigit, Ferruh <ferruh.yigit@intel.com>; Andrew
>>>>>>>>>> Rybchenko <arybchenko@solarflare.com>
>>>>>>>>>> Cc: dev@dpdk.org; jerinj@marvell.com; kkanas@marvell.com; Nithin
>>>>>>>>>> Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>>> Subject: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
>>>>>>>>>> mode
>>>>>>>>>>
>>>>>>>>>> From: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>>>
>>>>>>>>>> Some NIC hardware support shaper to work in packet mode i.e
>>>>>>>>>> shaping or ratelimiting traffic is in packets per second (PPS) as
>>>>>>>>>> opposed to default bytes per second (BPS). Hence this patch
>>>>>>>>>> adds support to configure shared or private shaper in packet mode,
>>>>>>>>>> provide rate in PPS and add related tm capabilities in port/level/node
>>>>>>>>>> capability structures.
>>>>>>>>>>
>>>>>>>>>> This patch also updates tm port/level/node capability structures with
>>>>>>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>>>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>>>
>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>>>>>>>>> ---
>>>>>>>>>> v3..v4:
>>>>>>>>>> - Update text under packet_mode as per Cristian.
>>>>>>>>>> - Update rte_eth_softnic_tm.c based on Jasvinder's comments.
>>>>>>>>>> - Add error enum
>>>>>>>> RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE
>>>>>>>>>> - Fix shaper_profile_check() with packet mode check
>>>>>>>>>> - Fix typo's
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Acked-by: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>>>>>>>>>
>>>>>>>>
>>>>>>>> Hi Nithin,
>>>>>>>>
>>>>>>>> It looks like patch is causing ABI break, I am getting following warning [1],
>>>>>>>> can you please check?
>>>>>>>>
>>>>>>>> [1]
>>>>>>>> https://pastebin.com/XYNFg14u
>>>>>>>
>>>>>>>
>>>>>>> Hi Ferruh,
>>>>>>>
>>>>>>> The RTE_TM API is marked as experimental, but it looks that this was not correctly marked when __rte_experimental ABI checker was introduced.
>>>>>>>
>>>>>>> It is marked as experimental at the top of the rte_tm.h, similarly to other APIs introduced around same time, but it was not correctly picked up by the ABI check procedure when later introduced, so __rte_experimental was not added to every function.
>>>>>>>
>>>>>>
>>>>>> :(
>>>>>>
>>>>>> Is it time to mature them?
>>>>>>
>>>>>> As you said they are not marked as experimental both in header file (function
>>>>>> declarations) and .map file.
>>>>>>
>>>>>> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
>>>>>> so marking them as experimental now will break the ABI. Not sure what to do,
>>>>>> cc'ed a few ABI related names for comment.
>>>>>>
>>>>>> For me, we need to proceed as the experimental tag removed and APIs become
>>>>>> mature starting from v19.11, since this is what happened in practice, and remove
>>>>>> a few existing being experimental references in the doxygen comments.
>>>>>
>>>>> I think, accidentally we can not make a library as NON-experimental.
>>>>> TM never went through experimental to mature transition(see git log
>>>>> lib/librte_ethdev/rte_tm.h)
>>>>> It was a bug to not mark as experimental in each function in the ABI process.
>>>>> Some of the features like packet marking are not even implemented by any HW.
>>>>> I think, we can make API stable only all the features are implemented
>>>>> by one or two HW.
>>>>
>>>> Fair enough, specially if the API is not ready yet.
>>>>
>>>> But they were part of stable ABI, and marking them as experimental now will
>>>> break the old applications using these APIs.
>>>
>>> it is still marked as EXPERIMENTAL everywhere and API is not ready yet.
>>
>> Existing experimental marks are text only for human parsing.
>>
>> The compiler attribute and build time checks are missing, and the symbol in the
>> binary doesn't have experimental tag. Our scripts and automated checks won't
>> detect it as experimental.
>>
>> My point is just having experimental comment in header file is not enough to
>> qualify the APIs as experimental.
>>
>>> Anyway, we need to break the ABI to make it work on various HW.
>>> I am not sure what to do?
>>> IMO, We need to send a patch as Fixes: for the bug of not adding
>>> __rte_experimental in each function.
>>
>> Yes, this is where we are, both you and Cristian suggest API is not ready and
>> should be experimental, but they were part of stable ABI, making them
>> experimental will break the ABI.
>> It looks like there is no good option but we should select one of the bad ones.
>>
>>>
>>> Traffic Management API - EXPERIMENTAL
>>> M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
>>> T: git://dpdk.org/next/dpdk-next-qos
>>> F: lib/librte_ethdev/rte_tm*
>>>>
>>>>>
>>>>>>
>>>>>> Ray, Neil, David, Luca, Kevin, what do you think?
>>>>
> While I'm not called any of those names, allow me to give my 2c.
> 
> Since these are marked in binaries as part of the stable ABI, I think we
> need to honour that for the next two releases 20.05 and 20.08 [which means
> that we need to put in versioned functions for any changes, not that we
> can't change anything]
> 
> For 20.11, I think these should then have one of two options taken:
> * have these "fixed" and ready to be marked as stable, and officially part
>   of v21 ABI or
> * mark them as experimental properly, and look to have them as part of the
>   v22 or subsequent ABI
> 
> Given the comments here, I would tend towards the latter of the above two
> options, but that's really a decision for the maintainers.
> 
> Remember, this is not the first bug we have encountered where we messed up
> some ABI versions in the 19.11 release, and, like the previous one with the
> screwed up version number, I think we need to honour the ABI committments
> made, especially since in this case it's only for a few more months till
> 20.11 development starts.
> 
> /Bruce
> 


So the rte_tm API has been "EXPERIMENTAL" for quiet a long time, as far back as v17.11.
To the extent it predates the experimental infrastructure the community developed 
since then. TM added in 17.08, EXPERIMENTAL appears to have been added in 17.11.

That said, some form of stable TM API should have emerged by this point. 
So I am not sure that EXPERIMENTAL status was 100% warranted. 

As Bruce points out.
It is simpler to wait for the next ABI breakage window in August. 
And mark them EXPERIMENTAL at that point. 

Thanks,

Ray K
Thomas Monjalon April 28, 2020, 3:54 p.m. UTC | #16 
28/04/2020 17:04, Luca Boccassi:
> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> > On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> > > On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> > > > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > > > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > > > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > > > > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > > > > > > > On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > > > > > > > > > > This patch also updates tm port/level/node capability structures with
> > > > > > > > > > > exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> > > > > > > > > > > and private/shared shaper byte mode.
> > > > > > > > > > > 
> > > > > > > > > > > SoftNIC PMD is also updated with new capabilities.
[...]
> > > > > > > > > Hi Nithin,
> > > > > > > > > 
> > > > > > > > > It looks like patch is causing ABI break, I am getting following warning [1],
> > > > > > > > > can you please check?
> > > > > > > > > 
> > > > > > > > > [1]
> > > > > > > > > https://pastebin.com/XYNFg14u
> > > > > > > > 
> > > > > > > > Hi Ferruh,
> > > > > > > > 
> > > > > > > > The RTE_TM API is marked as experimental,
> > > > > > > > but it looks that this was not correctly marked
> > > > > > > > when __rte_experimental ABI checker was introduced.
> > > > > > > > 
> > > > > > > > It is marked as experimental at the top of the rte_tm.h,
> > > > > > > > similarly to other APIs introduced around same time,
> > > > > > > > but it was not correctly picked up by the ABI check procedure
> > > > > > > > when later introduced, so __rte_experimental was not added to every function.
> > > > > > > > 
> > > > > > > 
> > > > > > > :(
> > > > > > > 
> > > > > > > Is it time to mature them?
> > > > > > > 
> > > > > > > As you said they are not marked as experimental both in header file (function
> > > > > > > declarations) and .map file.
> > > > > > > 
> > > > > > > The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> > > > > > > so marking them as experimental now will break the ABI. Not sure what to do,
> > > > > > > cc'ed a few ABI related names for comment.
> > > > > > > 
> > > > > > > For me, we need to proceed as the experimental tag removed and APIs become
> > > > > > > mature starting from v19.11, since this is what happened in practice, and remove
> > > > > > > a few existing being experimental references in the doxygen comments.
> > > > > > 
> > > > > > I think, accidentally we can not make a library as NON-experimental.
> > > > > > TM never went through experimental to mature transition(see git log
> > > > > > lib/librte_ethdev/rte_tm.h)
> > > > > > It was a bug to not mark as experimental in each function in the ABI process.
> > > > > > Some of the features like packet marking are not even implemented by any HW.
> > > > > > I think, we can make API stable only all the features are implemented
> > > > > > by one or two HW.

Yes this is what was decided one or two years ago I think.
But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.



> > > > > Fair enough, specially if the API is not ready yet.
> > > > > 
> > > > > But they were part of stable ABI, and marking them as experimental now will
> > > > > break the old applications using these APIs.
> > > > 
> > > > it is still marked as EXPERIMENTAL everywhere and API is not ready yet.

rte_tm is implemented in 6 PMDs.


> > > Existing experimental marks are text only for human parsing.
> > > 
> > > The compiler attribute and build time checks are missing, and the symbol in the
> > > binary doesn't have experimental tag. Our scripts and automated checks won't
> > > detect it as experimental.
> > > 
> > > My point is just having experimental comment in header file is not enough to
> > > qualify the APIs as experimental.
> > > 
> > > > Anyway, we need to break the ABI to make it work on various HW.

Yes this is why I was asking in 19.11 to check our API,
in order to avoid such situation.


> > > > I am not sure what to do?

Either manage ABI versioning, or wait 20.11.


> > > > IMO, We need to send a patch as Fixes: for the bug of not adding
> > > > __rte_experimental in each function.

No, this is wrong.


> > > Yes, this is where we are, both you and Cristian suggest API is not ready and
> > > should be experimental, but they were part of stable ABI, making them
> > > experimental will break the ABI.
> > > It looks like there is no good option but we should select one of the bad ones.
> > > 
> > > > Traffic Management API - EXPERIMENTAL
> > > > M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > > T: git://dpdk.org/next/dpdk-next-qos
> > > > F: lib/librte_ethdev/rte_tm*
> > > > > > > Ray, Neil, David, Luca, Kevin, what do you think?
> > While I'm not called any of those names, allow me to give my 2c.
> > 
> > Since these are marked in binaries as part of the stable ABI, I think we
> > need to honour that for the next two releases 20.05 and 20.08 [which means
> > that we need to put in versioned functions for any changes, not that we
> > can't change anything]
> > 
> > For 20.11, I think these should then have one of two options taken:
> > * have these "fixed" and ready to be marked as stable, and officially part
> >   of v21 ABI or
> > * mark them as experimental properly, and look to have them as part of the
> >   v22 or subsequent ABI
> > 
> > Given the comments here, I would tend towards the latter of the above two
> > options, but that's really a decision for the maintainers.
> > 
> > Remember, this is not the first bug we have encountered where we messed up
> > some ABI versions in the 19.11 release, and, like the previous one with the
> > screwed up version number, I think we need to honour the ABI committments
> > made, especially since in this case it's only for a few more months till
> > 20.11 development starts.
> > 
> > /Bruce
> 
> +1
> 
> If they are not ready now, they haven't been ready for the past 6
> months either, so staying not ready for 6 more is the lesser evil.

This API is almost 3 years old (release 17.08).
That's good to improve it but we must respect the ABI contract that
we all agreed.


Summary:
17.08: rte_tm is introduced.
17.11: rte_mtr is introduced as experimental, but rte_tm remains stable.
18.02: __rte_experimental tag is introduced (including for rte_mtr),
but rte_tm remains untouched as it is in stable ABI.
19.11: stable ABI is frozen until 20.11
20.05: rte_tm improvement is blocked because of ABI breakage.


It should remind everybody of reviewing the new API and policies,
and maintaining the existing code appropriately.
Cristian Dumitrescu April 28, 2020, 5:35 p.m. UTC | #17 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, April 28, 2020 4:30 PM
> To: Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; Nithin Dabilpuram
> <nithind1988@gmail.com>; Singh, Jasvinder <jasvinder.singh@intel.com>;
> Andrew Rybchenko <arybchenko@solarflare.com>; dev@dpdk.org;
> jerinj@marvell.com; kkanas@marvell.com; Nithin Dabilpuram
> <ndabilpuram@marvell.com>; Kinsella, Ray <ray.kinsella@intel.com>; Neil
> Horman <nhorman@tuxdriver.com>; Luca Boccassi <bluca@debian.org>;
> Kevin Traynor <ktraynor@redhat.com>; David Marchand
> <david.marchand@redhat.com>; Richardson, Bruce
> <bruce.richardson@intel.com>
> Subject: Re: [PATCH v4 1/4] ethdev: add tm support for shaper config in pkt
> mode
> 
> 27/04/2020 18:28, Dumitrescu, Cristian:
> > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > >> Hi Nithin,
> > > >>
> > > >> It looks like patch is causing ABI break, I am getting following warning
> [1],
> > > >> can you please check?
> > > >>
> > > >> [1]
> > > >> https://pastebin.com/XYNFg14u
> > > >
> > > >
> > > > Hi Ferruh,
> > > >
> > > > The RTE_TM API is marked as experimental, but it looks that this was
> not
> > > correctly marked when __rte_experimental ABI checker was introduced.
> > > >
> > > > It is marked as experimental at the top of the rte_tm.h, similarly to
> other
> > > APIs introduced around same time, but it was not correctly picked up by
> the
> > > ABI check procedure when later introduced, so __rte_experimental was
> not
> > > added to every function.
> > > >
> > >
> > > :(
> > >
> > > Is it time to mature them?
> > >
> > > As you said they are not marked as experimental both in header file
> > > (function
> > > declarations) and .map file.
> > >
> > > The problem is, they are not marked as experimental in DPDK_20.0 ABI
> > > (v19.11),
> > > so marking them as experimental now will break the ABI. Not sure what
> to
> > > do,
> > > cc'ed a few ABI related names for comment.
> > >
> > > For me, we need to proceed as the experimental tag removed and APIs
> > > become
> > > mature starting from v19.11, since this is what happened in practice, and
> > > remove
> > > a few existing being experimental references in the doxygen comments.
> > >
> > > Ray, Neil, David, Luca, Kevin, what do you think?
> >
> > Hi Ferruh,
> >
> > IMO your proposed approach is fixing the wrong problem and is
> > probably not the right way of doing things.
> >
> > This API is correctly marked as experimental in the header
> > file rte_tm.h and in the MAINTAINERS file,
> 
> in rte_tm.h:
> 	* @warning
> 	* @b EXPERIMENTAL: this API may change without prior notice
> 
> in MAINTAINERS:
> 	Traffic Management API - EXPERIMENTAL
> 	M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> 	T: git://dpdk.org/next/dpdk-next-qos
> 	F: lib/librte_ethdev/rte_tm*
> 
> 
> > therefore it should remain experimental,
> 
> in rte_ethdev_version.map:
> 	before 19.11: DPDK_17.08 {
> 	since 19.11: DPDK_20.0 {
> 
> When adding rte_tm in 17.08:
> http://git.dpdk.org/dpdk/diff/lib/librte_ether/rte_ether_version.map?id=5
> d109deffa
> 
> In 17.08, early July 2017, the first EXPERIMENTAL section was declared in EAL:
> http://git.dpdk.org/dpdk/diff/lib/librte_eal/linuxapp/eal/rte_eal_version.m
> ap?id=a3ee360f444
> 
> When adding rte_mtr in 17.11, all functions are made experimental:
> http://git.dpdk.org/dpdk/diff/lib/librte_ether/rte_ethdev_version.map?id=
> 6613ffe1
> 
> In 18.02, the tag __rte_experimental was introduced:
> 	http://git.dpdk.org/dpdk/commit/?id=7d540a3e735
> and functions are marked (including rte_mtr but not rte_tm):
> 	http://git.dpdk.org/dpdk/commit/?id=77b7b81e32e
> 
> 

Thanks, Thomas, for taking the time to put all the historic events in their proper sequence.

> > as more changes are expected from the people like Nithin and others
> > currently upstreaming drivers for it.
> 
> They are doing changes in the API introduced 3 years ago.
> 
> 
> > For some reason, the __rte_experimental tags were not added to
> > this file when the ABI checker was introduced,
> > and this is the real problem that should be fixed.
> 
> The mistake was done 2 years ago.
> As maintainer of rte_tm code, you are expected to notice the issue.

Apologies for not noticing this gap at the time. As Thomas shows above, the __rte_experimental tags were introduced several DPDK releases after the rte_tm API.

> As maintainer of rte_mtr code, you were expected to review the change
> on the mtr functions being marked as experimental.

I did.

> I am sorry that it took 2 years to discover the gap.
> 
> We can fix the ABI in the ABI-breakage window: in 20.11.
>
Cristian Dumitrescu April 29, 2020, 8:45 a.m. UTC | #18 
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, April 28, 2020 4:54 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>
> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
> Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>; Andrew Rybchenko
> <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
> kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
> <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
> Marchand <david.marchand@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
> config in pkt mode
> 
> 28/04/2020 17:04, Luca Boccassi:
> > On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> > > On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> > > > On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> > > > > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
> <ferruh.yigit@intel.com> wrote:
> > > > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > > > > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
> <ferruh.yigit@intel.com> wrote:
> > > > > > > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > > > > > > > > On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > > > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > > > > > > > > > > > This patch also updates tm port/level/node capability
> structures with
> > > > > > > > > > > > exiting features of scheduler wfq packet mode,
> scheduler wfq byte mode
> > > > > > > > > > > > and private/shared shaper byte mode.
> > > > > > > > > > > >
> > > > > > > > > > > > SoftNIC PMD is also updated with new capabilities.
> [...]
> > > > > > > > > > Hi Nithin,
> > > > > > > > > >
> > > > > > > > > > It looks like patch is causing ABI break, I am getting following
> warning [1],
> > > > > > > > > > can you please check?
> > > > > > > > > >
> > > > > > > > > > [1]
> > > > > > > > > > https://pastebin.com/XYNFg14u
> > > > > > > > >
> > > > > > > > > Hi Ferruh,
> > > > > > > > >
> > > > > > > > > The RTE_TM API is marked as experimental,
> > > > > > > > > but it looks that this was not correctly marked
> > > > > > > > > when __rte_experimental ABI checker was introduced.
> > > > > > > > >
> > > > > > > > > It is marked as experimental at the top of the rte_tm.h,
> > > > > > > > > similarly to other APIs introduced around same time,
> > > > > > > > > but it was not correctly picked up by the ABI check procedure
> > > > > > > > > when later introduced, so __rte_experimental was not added
> to every function.
> > > > > > > > >
> > > > > > > >
> > > > > > > > :(
> > > > > > > >
> > > > > > > > Is it time to mature them?
> > > > > > > >
> > > > > > > > As you said they are not marked as experimental both in header
> file (function
> > > > > > > > declarations) and .map file.
> > > > > > > >
> > > > > > > > The problem is, they are not marked as experimental in
> DPDK_20.0 ABI (v19.11),
> > > > > > > > so marking them as experimental now will break the ABI. Not
> sure what to do,
> > > > > > > > cc'ed a few ABI related names for comment.
> > > > > > > >
> > > > > > > > For me, we need to proceed as the experimental tag removed
> and APIs become
> > > > > > > > mature starting from v19.11, since this is what happened in
> practice, and remove
> > > > > > > > a few existing being experimental references in the doxygen
> comments.
> > > > > > >
> > > > > > > I think, accidentally we can not make a library as NON-
> experimental.
> > > > > > > TM never went through experimental to mature transition(see git
> log
> > > > > > > lib/librte_ethdev/rte_tm.h)
> > > > > > > It was a bug to not mark as experimental in each function in the
> ABI process.
> > > > > > > Some of the features like packet marking are not even
> implemented by any HW.
> > > > > > > I think, we can make API stable only all the features are
> implemented
> > > > > > > by one or two HW.
> 
> Yes this is what was decided one or two years ago I think.
> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
> 
> 
> 
> > > > > > Fair enough, specially if the API is not ready yet.
> > > > > >
> > > > > > But they were part of stable ABI, and marking them as experimental
> now will
> > > > > > break the old applications using these APIs.
> > > > >
> > > > > it is still marked as EXPERIMENTAL everywhere and API is not ready
> yet.
> 
> rte_tm is implemented in 6 PMDs.
> 
> 
> > > > Existing experimental marks are text only for human parsing.
> > > >
> > > > The compiler attribute and build time checks are missing, and the
> symbol in the
> > > > binary doesn't have experimental tag. Our scripts and automated
> checks won't
> > > > detect it as experimental.
> > > >
> > > > My point is just having experimental comment in header file is not
> enough to
> > > > qualify the APIs as experimental.
> > > >
> > > > > Anyway, we need to break the ABI to make it work on various HW.
> 
> Yes this is why I was asking in 19.11 to check our API,
> in order to avoid such situation.
> 
> 
> > > > > I am not sure what to do?
> 
> Either manage ABI versioning, or wait 20.11.
> 
> 
> > > > > IMO, We need to send a patch as Fixes: for the bug of not adding
> > > > > __rte_experimental in each function.
> 
> No, this is wrong.
> 

Why exactly is this wrong? This is the gap that caused the current discussion, right?

> 
> > > > Yes, this is where we are, both you and Cristian suggest API is not ready
> and
> > > > should be experimental, but they were part of stable ABI, making them
> > > > experimental will break the ABI.
> > > > It looks like there is no good option but we should select one of the bad
> ones.
> > > >
> > > > > Traffic Management API - EXPERIMENTAL
> > > > > M: Cristian Dumitrescu <cristian.dumitrescu@intel.com>
> > > > > T: git://dpdk.org/next/dpdk-next-qos
> > > > > F: lib/librte_ethdev/rte_tm*
> > > > > > > > Ray, Neil, David, Luca, Kevin, what do you think?
> > > While I'm not called any of those names, allow me to give my 2c.
> > >
> > > Since these are marked in binaries as part of the stable ABI, I think we
> > > need to honour that for the next two releases 20.05 and 20.08 [which
> means
> > > that we need to put in versioned functions for any changes, not that we
> > > can't change anything]
> > >
> > > For 20.11, I think these should then have one of two options taken:
> > > * have these "fixed" and ready to be marked as stable, and officially part
> > >   of v21 ABI or
> > > * mark them as experimental properly, and look to have them as part of
> the
> > >   v22 or subsequent ABI
> > >
> > > Given the comments here, I would tend towards the latter of the above
> two
> > > options, but that's really a decision for the maintainers.
> > >
> > > Remember, this is not the first bug we have encountered where we
> messed up
> > > some ABI versions in the 19.11 release, and, like the previous one with
> the
> > > screwed up version number, I think we need to honour the ABI
> committments
> > > made, especially since in this case it's only for a few more months till
> > > 20.11 development starts.
> > >
> > > /Bruce
> >
> > +1
> >
> > If they are not ready now, they haven't been ready for the past 6
> > months either, so staying not ready for 6 more is the lesser evil.
> 
> This API is almost 3 years old (release 17.08).
> That's good to improve it but we must respect the ABI contract that
> we all agreed.
> 
> 
> Summary:
> 17.08: rte_tm is introduced.
> 17.11: rte_mtr is introduced as experimental, but rte_tm remains stable.
> 18.02: __rte_experimental tag is introduced (including for rte_mtr),
> but rte_tm remains untouched as it is in stable ABI.
> 19.11: stable ABI is frozen until 20.11
> 20.05: rte_tm improvement is blocked because of ABI breakage.
> 
> 
> It should remind everybody of reviewing the new API and policies,
> and maintaining the existing code appropriately.
>
Bruce Richardson April 29, 2020, 9:03 a.m. UTC | #19 
On Wed, Apr 29, 2020 at 09:45:44AM +0100, Dumitrescu, Cristian wrote:
> 
> 
> > -----Original Message-----
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, April 28, 2020 4:54 PM
> > To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>
> > Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> > <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
> > Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
> > <jasvinder.singh@intel.com>; Andrew Rybchenko
> > <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
> > kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> > Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
> > <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
> > Marchand <david.marchand@redhat.com>
> > Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
> > config in pkt mode
> >
> > 28/04/2020 17:04, Luca Boccassi:
> > > On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> > > > On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> > > > > On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> > > > > > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
> > <ferruh.yigit@intel.com> wrote:
> > > > > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > > > > > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
> > <ferruh.yigit@intel.com> wrote:
> > > > > > > > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > > > > > > > > > On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > > > > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > > > > > > > > > > > > This patch also updates tm port/level/node capability
> > structures with
> > > > > > > > > > > > > exiting features of scheduler wfq packet mode,
> > scheduler wfq byte mode
> > > > > > > > > > > > > and private/shared shaper byte mode.
> > > > > > > > > > > > >
> > > > > > > > > > > > > SoftNIC PMD is also updated with new capabilities.
> > [...]
> > > > > > > > > > > Hi Nithin,
> > > > > > > > > > >
> > > > > > > > > > > It looks like patch is causing ABI break, I am getting following
> > warning [1],
> > > > > > > > > > > can you please check?
> > > > > > > > > > >
> > > > > > > > > > > [1]
> > > > > > > > > > > https://pastebin.com/XYNFg14u
> > > > > > > > > >
> > > > > > > > > > Hi Ferruh,
> > > > > > > > > >
> > > > > > > > > > The RTE_TM API is marked as experimental,
> > > > > > > > > > but it looks that this was not correctly marked
> > > > > > > > > > when __rte_experimental ABI checker was introduced.
> > > > > > > > > >
> > > > > > > > > > It is marked as experimental at the top of the rte_tm.h,
> > > > > > > > > > similarly to other APIs introduced around same time,
> > > > > > > > > > but it was not correctly picked up by the ABI check procedure
> > > > > > > > > > when later introduced, so __rte_experimental was not added
> > to every function.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > :(
> > > > > > > > >
> > > > > > > > > Is it time to mature them?
> > > > > > > > >
> > > > > > > > > As you said they are not marked as experimental both in header
> > file (function
> > > > > > > > > declarations) and .map file.
> > > > > > > > >
> > > > > > > > > The problem is, they are not marked as experimental in
> > DPDK_20.0 ABI (v19.11),
> > > > > > > > > so marking them as experimental now will break the ABI. Not
> > sure what to do,
> > > > > > > > > cc'ed a few ABI related names for comment.
> > > > > > > > >
> > > > > > > > > For me, we need to proceed as the experimental tag removed
> > and APIs become
> > > > > > > > > mature starting from v19.11, since this is what happened in
> > practice, and remove
> > > > > > > > > a few existing being experimental references in the doxygen
> > comments.
> > > > > > > >
> > > > > > > > I think, accidentally we can not make a library as NON-
> > experimental.
> > > > > > > > TM never went through experimental to mature transition(see git
> > log
> > > > > > > > lib/librte_ethdev/rte_tm.h)
> > > > > > > > It was a bug to not mark as experimental in each function in the
> > ABI process.
> > > > > > > > Some of the features like packet marking are not even
> > implemented by any HW.
> > > > > > > > I think, we can make API stable only all the features are
> > implemented
> > > > > > > > by one or two HW.
> >
> > Yes this is what was decided one or two years ago I think.
> > But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
> >
> >
> >
> > > > > > > Fair enough, specially if the API is not ready yet.
> > > > > > >
> > > > > > > But they were part of stable ABI, and marking them as experimental
> > now will
> > > > > > > break the old applications using these APIs.
> > > > > >
> > > > > > it is still marked as EXPERIMENTAL everywhere and API is not ready
> > yet.
> >
> > rte_tm is implemented in 6 PMDs.
> >
> >
> > > > > Existing experimental marks are text only for human parsing.
> > > > >
> > > > > The compiler attribute and build time checks are missing, and the
> > symbol in the
> > > > > binary doesn't have experimental tag. Our scripts and automated
> > checks won't
> > > > > detect it as experimental.
> > > > >
> > > > > My point is just having experimental comment in header file is not
> > enough to
> > > > > qualify the APIs as experimental.
> > > > >
> > > > > > Anyway, we need to break the ABI to make it work on various HW.
> >
> > Yes this is why I was asking in 19.11 to check our API,
> > in order to avoid such situation.
> >
> >
> > > > > > I am not sure what to do?
> >
> > Either manage ABI versioning, or wait 20.11.
> >
> >
> > > > > > IMO, We need to send a patch as Fixes: for the bug of not adding
> > > > > > __rte_experimental in each function.
> >
> > No, this is wrong.
> >
> 
> Why exactly is this wrong? This is the gap that caused the current discussion, right?
> 
It's wrong for this release, since we can't change things from stable back
to experimental. Any such patch will have to wait for 20.11, as agreed in
the discussion.

/Bruce
Ferruh Yigit May 1, 2020, 10:27 a.m. UTC | #20 
On 4/29/2020 10:03 AM, Bruce Richardson wrote:
> On Wed, Apr 29, 2020 at 09:45:44AM +0100, Dumitrescu, Cristian wrote:
>>
>>
>>> -----Original Message-----
>>> From: Thomas Monjalon <thomas@monjalon.net>
>>> Sent: Tuesday, April 28, 2020 4:54 PM
>>> To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
>>> <cristian.dumitrescu@intel.com>
>>> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>>> <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
>>> Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
>>> <jasvinder.singh@intel.com>; Andrew Rybchenko
>>> <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
>>> kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
>>> Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
>>> <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
>>> Marchand <david.marchand@redhat.com>
>>> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
>>> config in pkt mode
>>>
>>> 28/04/2020 17:04, Luca Boccassi:
>>>> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
>>>>> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
>>>>>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
>>>>>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
>>> <ferruh.yigit@intel.com> wrote:
>>>>>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>>>>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
>>> <ferruh.yigit@intel.com> wrote:
>>>>>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>>>>>>>> This patch also updates tm port/level/node capability
>>> structures with
>>>>>>>>>>>>>> exiting features of scheduler wfq packet mode,
>>> scheduler wfq byte mode
>>>>>>>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>>> [...]
>>>>>>>>>>>> Hi Nithin,
>>>>>>>>>>>>
>>>>>>>>>>>> It looks like patch is causing ABI break, I am getting following
>>> warning [1],
>>>>>>>>>>>> can you please check?
>>>>>>>>>>>>
>>>>>>>>>>>> [1]
>>>>>>>>>>>> https://pastebin.com/XYNFg14u
>>>>>>>>>>>
>>>>>>>>>>> Hi Ferruh,
>>>>>>>>>>>
>>>>>>>>>>> The RTE_TM API is marked as experimental,
>>>>>>>>>>> but it looks that this was not correctly marked
>>>>>>>>>>> when __rte_experimental ABI checker was introduced.
>>>>>>>>>>>
>>>>>>>>>>> It is marked as experimental at the top of the rte_tm.h,
>>>>>>>>>>> similarly to other APIs introduced around same time,
>>>>>>>>>>> but it was not correctly picked up by the ABI check procedure
>>>>>>>>>>> when later introduced, so __rte_experimental was not added
>>> to every function.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> :(
>>>>>>>>>>
>>>>>>>>>> Is it time to mature them?
>>>>>>>>>>
>>>>>>>>>> As you said they are not marked as experimental both in header
>>> file (function
>>>>>>>>>> declarations) and .map file.
>>>>>>>>>>
>>>>>>>>>> The problem is, they are not marked as experimental in
>>> DPDK_20.0 ABI (v19.11),
>>>>>>>>>> so marking them as experimental now will break the ABI. Not
>>> sure what to do,
>>>>>>>>>> cc'ed a few ABI related names for comment.
>>>>>>>>>>
>>>>>>>>>> For me, we need to proceed as the experimental tag removed
>>> and APIs become
>>>>>>>>>> mature starting from v19.11, since this is what happened in
>>> practice, and remove
>>>>>>>>>> a few existing being experimental references in the doxygen
>>> comments.
>>>>>>>>>
>>>>>>>>> I think, accidentally we can not make a library as NON-
>>> experimental.
>>>>>>>>> TM never went through experimental to mature transition(see git
>>> log
>>>>>>>>> lib/librte_ethdev/rte_tm.h)
>>>>>>>>> It was a bug to not mark as experimental in each function in the
>>> ABI process.
>>>>>>>>> Some of the features like packet marking are not even
>>> implemented by any HW.
>>>>>>>>> I think, we can make API stable only all the features are
>>> implemented
>>>>>>>>> by one or two HW.
>>>
>>> Yes this is what was decided one or two years ago I think.
>>> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
>>>
>>>
>>>
>>>>>>>> Fair enough, specially if the API is not ready yet.
>>>>>>>>
>>>>>>>> But they were part of stable ABI, and marking them as experimental
>>> now will
>>>>>>>> break the old applications using these APIs.
>>>>>>>
>>>>>>> it is still marked as EXPERIMENTAL everywhere and API is not ready
>>> yet.
>>>
>>> rte_tm is implemented in 6 PMDs.
>>>
>>>
>>>>>> Existing experimental marks are text only for human parsing.
>>>>>>
>>>>>> The compiler attribute and build time checks are missing, and the
>>> symbol in the
>>>>>> binary doesn't have experimental tag. Our scripts and automated
>>> checks won't
>>>>>> detect it as experimental.
>>>>>>
>>>>>> My point is just having experimental comment in header file is not
>>> enough to
>>>>>> qualify the APIs as experimental.
>>>>>>
>>>>>>> Anyway, we need to break the ABI to make it work on various HW.
>>>
>>> Yes this is why I was asking in 19.11 to check our API,
>>> in order to avoid such situation.
>>>
>>>
>>>>>>> I am not sure what to do?
>>>
>>> Either manage ABI versioning, or wait 20.11.
>>>
>>>
>>>>>>> IMO, We need to send a patch as Fixes: for the bug of not adding
>>>>>>> __rte_experimental in each function.
>>>
>>> No, this is wrong.
>>>
>>
>> Why exactly is this wrong? This is the gap that caused the current discussion, right?
>>
> It's wrong for this release, since we can't change things from stable back
> to experimental. Any such patch will have to wait for 20.11, as agreed in
> the discussion.
> 

Deferring the patchet for this release.

Reminder that if the option "to mark rte_tm_* as experimental in v20.11"
selected, requires deprecation notice before v20.11.
Nithin Dabilpuram May 1, 2020, 1:16 p.m. UTC | #21 
On Fri, May 01, 2020 at 11:27:02AM +0100, Ferruh Yigit wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On 4/29/2020 10:03 AM, Bruce Richardson wrote:
> > On Wed, Apr 29, 2020 at 09:45:44AM +0100, Dumitrescu, Cristian wrote:
> >>
> >>
> >>> -----Original Message-----
> >>> From: Thomas Monjalon <thomas@monjalon.net>
> >>> Sent: Tuesday, April 28, 2020 4:54 PM
> >>> To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
> >>> <cristian.dumitrescu@intel.com>
> >>> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> >>> <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
> >>> Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
> >>> <jasvinder.singh@intel.com>; Andrew Rybchenko
> >>> <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
> >>> kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> >>> Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
> >>> <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
> >>> Marchand <david.marchand@redhat.com>
> >>> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
> >>> config in pkt mode
> >>>
> >>> 28/04/2020 17:04, Luca Boccassi:
> >>>> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> >>>>> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> >>>>>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> >>>>>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
> >>> <ferruh.yigit@intel.com> wrote:
> >>>>>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> >>>>>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
> >>> <ferruh.yigit@intel.com> wrote:
> >>>>>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> >>>>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >>>>>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >>>>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >>>>>>>>>>>>>> This patch also updates tm port/level/node capability
> >>> structures with
> >>>>>>>>>>>>>> exiting features of scheduler wfq packet mode,
> >>> scheduler wfq byte mode
> >>>>>>>>>>>>>> and private/shared shaper byte mode.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
> >>> [...]
> >>>>>>>>>>>> Hi Nithin,
> >>>>>>>>>>>>
> >>>>>>>>>>>> It looks like patch is causing ABI break, I am getting following
> >>> warning [1],
> >>>>>>>>>>>> can you please check?
> >>>>>>>>>>>>
> >>>>>>>>>>>> [1]
> >>>>>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=ej5sP3-cEhEoCTZOia-QivXqgljtzBcMLtZGs-5c-Uc&s=B8z_5mQ2xO3C1izjmRe2zBApMrCUcW6KcAN-adglhJQ&e= 
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Ferruh,
> >>>>>>>>>>>
> >>>>>>>>>>> The RTE_TM API is marked as experimental,
> >>>>>>>>>>> but it looks that this was not correctly marked
> >>>>>>>>>>> when __rte_experimental ABI checker was introduced.
> >>>>>>>>>>>
> >>>>>>>>>>> It is marked as experimental at the top of the rte_tm.h,
> >>>>>>>>>>> similarly to other APIs introduced around same time,
> >>>>>>>>>>> but it was not correctly picked up by the ABI check procedure
> >>>>>>>>>>> when later introduced, so __rte_experimental was not added
> >>> to every function.
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> :(
> >>>>>>>>>>
> >>>>>>>>>> Is it time to mature them?
> >>>>>>>>>>
> >>>>>>>>>> As you said they are not marked as experimental both in header
> >>> file (function
> >>>>>>>>>> declarations) and .map file.
> >>>>>>>>>>
> >>>>>>>>>> The problem is, they are not marked as experimental in
> >>> DPDK_20.0 ABI (v19.11),
> >>>>>>>>>> so marking them as experimental now will break the ABI. Not
> >>> sure what to do,
> >>>>>>>>>> cc'ed a few ABI related names for comment.
> >>>>>>>>>>
> >>>>>>>>>> For me, we need to proceed as the experimental tag removed
> >>> and APIs become
> >>>>>>>>>> mature starting from v19.11, since this is what happened in
> >>> practice, and remove
> >>>>>>>>>> a few existing being experimental references in the doxygen
> >>> comments.
> >>>>>>>>>
> >>>>>>>>> I think, accidentally we can not make a library as NON-
> >>> experimental.
> >>>>>>>>> TM never went through experimental to mature transition(see git
> >>> log
> >>>>>>>>> lib/librte_ethdev/rte_tm.h)
> >>>>>>>>> It was a bug to not mark as experimental in each function in the
> >>> ABI process.
> >>>>>>>>> Some of the features like packet marking are not even
> >>> implemented by any HW.
> >>>>>>>>> I think, we can make API stable only all the features are
> >>> implemented
> >>>>>>>>> by one or two HW.
> >>>
> >>> Yes this is what was decided one or two years ago I think.
> >>> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
> >>>
> >>>
> >>>
> >>>>>>>> Fair enough, specially if the API is not ready yet.
> >>>>>>>>
> >>>>>>>> But they were part of stable ABI, and marking them as experimental
> >>> now will
> >>>>>>>> break the old applications using these APIs.
> >>>>>>>
> >>>>>>> it is still marked as EXPERIMENTAL everywhere and API is not ready
> >>> yet.
> >>>
> >>> rte_tm is implemented in 6 PMDs.
> >>>
> >>>
> >>>>>> Existing experimental marks are text only for human parsing.
> >>>>>>
> >>>>>> The compiler attribute and build time checks are missing, and the
> >>> symbol in the
> >>>>>> binary doesn't have experimental tag. Our scripts and automated
> >>> checks won't
> >>>>>> detect it as experimental.
> >>>>>>
> >>>>>> My point is just having experimental comment in header file is not
> >>> enough to
> >>>>>> qualify the APIs as experimental.
> >>>>>>
> >>>>>>> Anyway, we need to break the ABI to make it work on various HW.
> >>>
> >>> Yes this is why I was asking in 19.11 to check our API,
> >>> in order to avoid such situation.
> >>>
> >>>
> >>>>>>> I am not sure what to do?
> >>>
> >>> Either manage ABI versioning, or wait 20.11.
> >>>
> >>>
> >>>>>>> IMO, We need to send a patch as Fixes: for the bug of not adding
> >>>>>>> __rte_experimental in each function.
> >>>
> >>> No, this is wrong.
> >>>
> >>
> >> Why exactly is this wrong? This is the gap that caused the current discussion, right?
> >>
> > It's wrong for this release, since we can't change things from stable back
> > to experimental. Any such patch will have to wait for 20.11, as agreed in
> > the discussion.
> > 
> 
> Deferring the patchet for this release.
> 
> Reminder that if the option "to mark rte_tm_* as experimental in v20.11"
> selected, requires deprecation notice before v20.11.

Thanks Ferruh for reminder. I'll send a deprecation notice patch for the same.
Jerin Jacob May 1, 2020, 1:18 p.m. UTC | #22 
On Tue, Apr 28, 2020 at 9:24 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 28/04/2020 17:04, Luca Boccassi:
> > On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> > > On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> > > > On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> > > > > On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > > > On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> > > > > > > On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > > > > > > > On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > From: Yigit, Ferruh <ferruh.yigit@intel.com>
> > > > > > > > > > On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> > > > > > > > > > > From: Nithin Dabilpuram <nithind1988@gmail.com>
> > > > > > > > > > > > This patch also updates tm port/level/node capability structures with
> > > > > > > > > > > > exiting features of scheduler wfq packet mode, scheduler wfq byte mode
> > > > > > > > > > > > and private/shared shaper byte mode.
> > > > > > > > > > > >
> > > > > > > > > > > > SoftNIC PMD is also updated with new capabilities.
> [...]
> > > > > > > > > > Hi Nithin,
> > > > > > > > > >
> > > > > > > > > > It looks like patch is causing ABI break, I am getting following warning [1],
> > > > > > > > > > can you please check?
> > > > > > > > > >
> > > > > > > > > > [1]
> > > > > > > > > > https://pastebin.com/XYNFg14u
> > > > > > > > >
> > > > > > > > > Hi Ferruh,
> > > > > > > > >
> > > > > > > > > The RTE_TM API is marked as experimental,
> > > > > > > > > but it looks that this was not correctly marked
> > > > > > > > > when __rte_experimental ABI checker was introduced.
> > > > > > > > >
> > > > > > > > > It is marked as experimental at the top of the rte_tm.h,
> > > > > > > > > similarly to other APIs introduced around same time,
> > > > > > > > > but it was not correctly picked up by the ABI check procedure
> > > > > > > > > when later introduced, so __rte_experimental was not added to every function.
> > > > > > > > >
> > > > > > > >
> > > > > > > > :(
> > > > > > > >
> > > > > > > > Is it time to mature them?
> > > > > > > >
> > > > > > > > As you said they are not marked as experimental both in header file (function
> > > > > > > > declarations) and .map file.
> > > > > > > >
> > > > > > > > The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
> > > > > > > > so marking them as experimental now will break the ABI. Not sure what to do,
> > > > > > > > cc'ed a few ABI related names for comment.
> > > > > > > >
> > > > > > > > For me, we need to proceed as the experimental tag removed and APIs become
> > > > > > > > mature starting from v19.11, since this is what happened in practice, and remove
> > > > > > > > a few existing being experimental references in the doxygen comments.
> > > > > > >
> > > > > > > I think, accidentally we can not make a library as NON-experimental.
> > > > > > > TM never went through experimental to mature transition(see git log
> > > > > > > lib/librte_ethdev/rte_tm.h)
> > > > > > > It was a bug to not mark as experimental in each function in the ABI process.
> > > > > > > Some of the features like packet marking are not even implemented by any HW.
> > > > > > > I think, we can make API stable only all the features are implemented
> > > > > > > by one or two HW.
>
> Yes this is what was decided one or two years ago I think.
> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.

None of the 6 PMDS covers all the features.

>
> > > > Existing experimental marks are text only for human parsing.
> > > >
> > > > The compiler attribute and build time checks are missing, and the symbol in the
> > > > binary doesn't have experimental tag. Our scripts and automated checks won't
> > > > detect it as experimental.
> > > >
> > > > My point is just having experimental comment in header file is not enough to
> > > > qualify the APIs as experimental.
> > > >
> > > > > Anyway, we need to break the ABI to make it work on various HW.
>
> Yes this is why I was asking in 19.11 to check our API,
> in order to avoid such situation.
>
>
> > > > > I am not sure what to do?
>
> Either manage ABI versioning, or wait 20.11.

ABI change are in structures. So the function versioning does not
help. So we will wait for 20.11 then :-(
Ray Kinsella May 5, 2020, 8:01 a.m. UTC | #23 
On 01/05/2020 14:18, Jerin Jacob wrote:
> On Tue, Apr 28, 2020 at 9:24 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>> 28/04/2020 17:04, Luca Boccassi:
>>> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
>>>> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
>>>>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
>>>>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>>>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>>>>>>> This patch also updates tm port/level/node capability structures with
>>>>>>>>>>>>> exiting features of scheduler wfq packet mode, scheduler wfq byte mode
>>>>>>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>>>>>>
>>>>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>> [...]
>>>>>>>>>>> Hi Nithin,
>>>>>>>>>>>
>>>>>>>>>>> It looks like patch is causing ABI break, I am getting following warning [1],
>>>>>>>>>>> can you please check?
>>>>>>>>>>>
>>>>>>>>>>> [1]
>>>>>>>>>>> https://pastebin.com/XYNFg14u
>>>>>>>>>>
>>>>>>>>>> Hi Ferruh,
>>>>>>>>>>
>>>>>>>>>> The RTE_TM API is marked as experimental,
>>>>>>>>>> but it looks that this was not correctly marked
>>>>>>>>>> when __rte_experimental ABI checker was introduced.
>>>>>>>>>>
>>>>>>>>>> It is marked as experimental at the top of the rte_tm.h,
>>>>>>>>>> similarly to other APIs introduced around same time,
>>>>>>>>>> but it was not correctly picked up by the ABI check procedure
>>>>>>>>>> when later introduced, so __rte_experimental was not added to every function.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> :(
>>>>>>>>>
>>>>>>>>> Is it time to mature them?
>>>>>>>>>
>>>>>>>>> As you said they are not marked as experimental both in header file (function
>>>>>>>>> declarations) and .map file.
>>>>>>>>>
>>>>>>>>> The problem is, they are not marked as experimental in DPDK_20.0 ABI (v19.11),
>>>>>>>>> so marking them as experimental now will break the ABI. Not sure what to do,
>>>>>>>>> cc'ed a few ABI related names for comment.
>>>>>>>>>
>>>>>>>>> For me, we need to proceed as the experimental tag removed and APIs become
>>>>>>>>> mature starting from v19.11, since this is what happened in practice, and remove
>>>>>>>>> a few existing being experimental references in the doxygen comments.
>>>>>>>>
>>>>>>>> I think, accidentally we can not make a library as NON-experimental.
>>>>>>>> TM never went through experimental to mature transition(see git log
>>>>>>>> lib/librte_ethdev/rte_tm.h)
>>>>>>>> It was a bug to not mark as experimental in each function in the ABI process.
>>>>>>>> Some of the features like packet marking are not even implemented by any HW.
>>>>>>>> I think, we can make API stable only all the features are implemented
>>>>>>>> by one or two HW.
>>
>> Yes this is what was decided one or two years ago I think.
>> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
> 
> None of the 6 PMDS covers all the features.
> 
>>
>>>>> Existing experimental marks are text only for human parsing.
>>>>>
>>>>> The compiler attribute and build time checks are missing, and the symbol in the
>>>>> binary doesn't have experimental tag. Our scripts and automated checks won't
>>>>> detect it as experimental.
>>>>>
>>>>> My point is just having experimental comment in header file is not enough to
>>>>> qualify the APIs as experimental.
>>>>>
>>>>>> Anyway, we need to break the ABI to make it work on various HW.
>>
>> Yes this is why I was asking in 19.11 to check our API,
>> in order to avoid such situation.
>>
>>
>>>>>> I am not sure what to do?
>>
>> Either manage ABI versioning, or wait 20.11.
> 
> ABI change are in structures. So the function versioning does not
> help. So we will wait for 20.11 then :-(
> 

yes - I spent some time looking at this also, and came to same conclusion.

Ray K
Ferruh Yigit Aug. 25, 2020, 4:59 p.m. UTC | #24 
On 5/1/2020 2:16 PM, Nithin Dabilpuram wrote:
> On Fri, May 01, 2020 at 11:27:02AM +0100, Ferruh Yigit wrote:
>> External Email
>>
>> ----------------------------------------------------------------------
>> On 4/29/2020 10:03 AM, Bruce Richardson wrote:
>>> On Wed, Apr 29, 2020 at 09:45:44AM +0100, Dumitrescu, Cristian wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>> Sent: Tuesday, April 28, 2020 4:54 PM
>>>>> To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
>>>>> <cristian.dumitrescu@intel.com>
>>>>> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>>>>> <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
>>>>> Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
>>>>> <jasvinder.singh@intel.com>; Andrew Rybchenko
>>>>> <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
>>>>> kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
>>>>> Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
>>>>> <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
>>>>> Marchand <david.marchand@redhat.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
>>>>> config in pkt mode
>>>>>
>>>>> 28/04/2020 17:04, Luca Boccassi:
>>>>>> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
>>>>>>> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
>>>>>>>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
>>>>>>>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
>>>>> <ferruh.yigit@intel.com> wrote:
>>>>>>>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>>>>>>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
>>>>> <ferruh.yigit@intel.com> wrote:
>>>>>>>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>>>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>>>>>>>>>> This patch also updates tm port/level/node capability
>>>>> structures with
>>>>>>>>>>>>>>>> exiting features of scheduler wfq packet mode,
>>>>> scheduler wfq byte mode
>>>>>>>>>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>>>>> [...]
>>>>>>>>>>>>>> Hi Nithin,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> It looks like patch is causing ABI break, I am getting following
>>>>> warning [1],
>>>>>>>>>>>>>> can you please check?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=ej5sP3-cEhEoCTZOia-QivXqgljtzBcMLtZGs-5c-Uc&s=B8z_5mQ2xO3C1izjmRe2zBApMrCUcW6KcAN-adglhJQ&e= 
>>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Ferruh,
>>>>>>>>>>>>>
>>>>>>>>>>>>> The RTE_TM API is marked as experimental,
>>>>>>>>>>>>> but it looks that this was not correctly marked
>>>>>>>>>>>>> when __rte_experimental ABI checker was introduced.
>>>>>>>>>>>>>
>>>>>>>>>>>>> It is marked as experimental at the top of the rte_tm.h,
>>>>>>>>>>>>> similarly to other APIs introduced around same time,
>>>>>>>>>>>>> but it was not correctly picked up by the ABI check procedure
>>>>>>>>>>>>> when later introduced, so __rte_experimental was not added
>>>>> to every function.
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> :(
>>>>>>>>>>>>
>>>>>>>>>>>> Is it time to mature them?
>>>>>>>>>>>>
>>>>>>>>>>>> As you said they are not marked as experimental both in header
>>>>> file (function
>>>>>>>>>>>> declarations) and .map file.
>>>>>>>>>>>>
>>>>>>>>>>>> The problem is, they are not marked as experimental in
>>>>> DPDK_20.0 ABI (v19.11),
>>>>>>>>>>>> so marking them as experimental now will break the ABI. Not
>>>>> sure what to do,
>>>>>>>>>>>> cc'ed a few ABI related names for comment.
>>>>>>>>>>>>
>>>>>>>>>>>> For me, we need to proceed as the experimental tag removed
>>>>> and APIs become
>>>>>>>>>>>> mature starting from v19.11, since this is what happened in
>>>>> practice, and remove
>>>>>>>>>>>> a few existing being experimental references in the doxygen
>>>>> comments.
>>>>>>>>>>>
>>>>>>>>>>> I think, accidentally we can not make a library as NON-
>>>>> experimental.
>>>>>>>>>>> TM never went through experimental to mature transition(see git
>>>>> log
>>>>>>>>>>> lib/librte_ethdev/rte_tm.h)
>>>>>>>>>>> It was a bug to not mark as experimental in each function in the
>>>>> ABI process.
>>>>>>>>>>> Some of the features like packet marking are not even
>>>>> implemented by any HW.
>>>>>>>>>>> I think, we can make API stable only all the features are
>>>>> implemented
>>>>>>>>>>> by one or two HW.
>>>>>
>>>>> Yes this is what was decided one or two years ago I think.
>>>>> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
>>>>>
>>>>>
>>>>>
>>>>>>>>>> Fair enough, specially if the API is not ready yet.
>>>>>>>>>>
>>>>>>>>>> But they were part of stable ABI, and marking them as experimental
>>>>> now will
>>>>>>>>>> break the old applications using these APIs.
>>>>>>>>>
>>>>>>>>> it is still marked as EXPERIMENTAL everywhere and API is not ready
>>>>> yet.
>>>>>
>>>>> rte_tm is implemented in 6 PMDs.
>>>>>
>>>>>
>>>>>>>> Existing experimental marks are text only for human parsing.
>>>>>>>>
>>>>>>>> The compiler attribute and build time checks are missing, and the
>>>>> symbol in the
>>>>>>>> binary doesn't have experimental tag. Our scripts and automated
>>>>> checks won't
>>>>>>>> detect it as experimental.
>>>>>>>>
>>>>>>>> My point is just having experimental comment in header file is not
>>>>> enough to
>>>>>>>> qualify the APIs as experimental.
>>>>>>>>
>>>>>>>>> Anyway, we need to break the ABI to make it work on various HW.
>>>>>
>>>>> Yes this is why I was asking in 19.11 to check our API,
>>>>> in order to avoid such situation.
>>>>>
>>>>>
>>>>>>>>> I am not sure what to do?
>>>>>
>>>>> Either manage ABI versioning, or wait 20.11.
>>>>>
>>>>>
>>>>>>>>> IMO, We need to send a patch as Fixes: for the bug of not adding
>>>>>>>>> __rte_experimental in each function.
>>>>>
>>>>> No, this is wrong.
>>>>>
>>>>
>>>> Why exactly is this wrong? This is the gap that caused the current discussion, right?
>>>>
>>> It's wrong for this release, since we can't change things from stable back
>>> to experimental. Any such patch will have to wait for 20.11, as agreed in
>>> the discussion.
>>>
>>
>> Deferring the patchet for this release.
>>
>> Reminder that if the option "to mark rte_tm_* as experimental in v20.11"
>> selected, requires deprecation notice before v20.11.
> 
> Thanks Ferruh for reminder. I'll send a deprecation notice patch for the same.
> 

Hi Nithin Kumar, Cristian, Jerin,

Who is working on updating APIs as experimental? We need that patch to proceed
with this one.
Nithin Dabilpuram Sept. 7, 2020, 11:12 a.m. UTC | #25 
On Tue, Aug 25, 2020 at 05:59:10PM +0100, Ferruh Yigit wrote:
> On 5/1/2020 2:16 PM, Nithin Dabilpuram wrote:
> > On Fri, May 01, 2020 at 11:27:02AM +0100, Ferruh Yigit wrote:
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> On 4/29/2020 10:03 AM, Bruce Richardson wrote:
> >>> On Wed, Apr 29, 2020 at 09:45:44AM +0100, Dumitrescu, Cristian wrote:
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Thomas Monjalon <thomas@monjalon.net>
> >>>>> Sent: Tuesday, April 28, 2020 4:54 PM
> >>>>> To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
> >>>>> <cristian.dumitrescu@intel.com>
> >>>>> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
> >>>>> <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
> >>>>> Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
> >>>>> <jasvinder.singh@intel.com>; Andrew Rybchenko
> >>>>> <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
> >>>>> kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> >>>>> Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
> >>>>> <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
> >>>>> Marchand <david.marchand@redhat.com>
> >>>>> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
> >>>>> config in pkt mode
> >>>>>
> >>>>> 28/04/2020 17:04, Luca Boccassi:
> >>>>>> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
> >>>>>>> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
> >>>>>>>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
> >>>>>>>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
> >>>>> <ferruh.yigit@intel.com> wrote:
> >>>>>>>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
> >>>>>>>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
> >>>>> <ferruh.yigit@intel.com> wrote:
> >>>>>>>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
> >>>>>>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> >>>>>>>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
> >>>>>>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
> >>>>>>>>>>>>>>>> This patch also updates tm port/level/node capability
> >>>>> structures with
> >>>>>>>>>>>>>>>> exiting features of scheduler wfq packet mode,
> >>>>> scheduler wfq byte mode
> >>>>>>>>>>>>>>>> and private/shared shaper byte mode.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
> >>>>> [...]
> >>>>>>>>>>>>>> Hi Nithin,
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> It looks like patch is causing ABI break, I am getting following
> >>>>> warning [1],
> >>>>>>>>>>>>>> can you please check?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> [1]
> >>>>>>>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=ej5sP3-cEhEoCTZOia-QivXqgljtzBcMLtZGs-5c-Uc&s=B8z_5mQ2xO3C1izjmRe2zBApMrCUcW6KcAN-adglhJQ&e= 
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Hi Ferruh,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> The RTE_TM API is marked as experimental,
> >>>>>>>>>>>>> but it looks that this was not correctly marked
> >>>>>>>>>>>>> when __rte_experimental ABI checker was introduced.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> It is marked as experimental at the top of the rte_tm.h,
> >>>>>>>>>>>>> similarly to other APIs introduced around same time,
> >>>>>>>>>>>>> but it was not correctly picked up by the ABI check procedure
> >>>>>>>>>>>>> when later introduced, so __rte_experimental was not added
> >>>>> to every function.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> :(
> >>>>>>>>>>>>
> >>>>>>>>>>>> Is it time to mature them?
> >>>>>>>>>>>>
> >>>>>>>>>>>> As you said they are not marked as experimental both in header
> >>>>> file (function
> >>>>>>>>>>>> declarations) and .map file.
> >>>>>>>>>>>>
> >>>>>>>>>>>> The problem is, they are not marked as experimental in
> >>>>> DPDK_20.0 ABI (v19.11),
> >>>>>>>>>>>> so marking them as experimental now will break the ABI. Not
> >>>>> sure what to do,
> >>>>>>>>>>>> cc'ed a few ABI related names for comment.
> >>>>>>>>>>>>
> >>>>>>>>>>>> For me, we need to proceed as the experimental tag removed
> >>>>> and APIs become
> >>>>>>>>>>>> mature starting from v19.11, since this is what happened in
> >>>>> practice, and remove
> >>>>>>>>>>>> a few existing being experimental references in the doxygen
> >>>>> comments.
> >>>>>>>>>>>
> >>>>>>>>>>> I think, accidentally we can not make a library as NON-
> >>>>> experimental.
> >>>>>>>>>>> TM never went through experimental to mature transition(see git
> >>>>> log
> >>>>>>>>>>> lib/librte_ethdev/rte_tm.h)
> >>>>>>>>>>> It was a bug to not mark as experimental in each function in the
> >>>>> ABI process.
> >>>>>>>>>>> Some of the features like packet marking are not even
> >>>>> implemented by any HW.
> >>>>>>>>>>> I think, we can make API stable only all the features are
> >>>>> implemented
> >>>>>>>>>>> by one or two HW.
> >>>>>
> >>>>> Yes this is what was decided one or two years ago I think.
> >>>>> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
> >>>>>
> >>>>>
> >>>>>
> >>>>>>>>>> Fair enough, specially if the API is not ready yet.
> >>>>>>>>>>
> >>>>>>>>>> But they were part of stable ABI, and marking them as experimental
> >>>>> now will
> >>>>>>>>>> break the old applications using these APIs.
> >>>>>>>>>
> >>>>>>>>> it is still marked as EXPERIMENTAL everywhere and API is not ready
> >>>>> yet.
> >>>>>
> >>>>> rte_tm is implemented in 6 PMDs.
> >>>>>
> >>>>>
> >>>>>>>> Existing experimental marks are text only for human parsing.
> >>>>>>>>
> >>>>>>>> The compiler attribute and build time checks are missing, and the
> >>>>> symbol in the
> >>>>>>>> binary doesn't have experimental tag. Our scripts and automated
> >>>>> checks won't
> >>>>>>>> detect it as experimental.
> >>>>>>>>
> >>>>>>>> My point is just having experimental comment in header file is not
> >>>>> enough to
> >>>>>>>> qualify the APIs as experimental.
> >>>>>>>>
> >>>>>>>>> Anyway, we need to break the ABI to make it work on various HW.
> >>>>>
> >>>>> Yes this is why I was asking in 19.11 to check our API,
> >>>>> in order to avoid such situation.
> >>>>>
> >>>>>
> >>>>>>>>> I am not sure what to do?
> >>>>>
> >>>>> Either manage ABI versioning, or wait 20.11.
> >>>>>
> >>>>>
> >>>>>>>>> IMO, We need to send a patch as Fixes: for the bug of not adding
> >>>>>>>>> __rte_experimental in each function.
> >>>>>
> >>>>> No, this is wrong.
> >>>>>
> >>>>
> >>>> Why exactly is this wrong? This is the gap that caused the current discussion, right?
> >>>>
> >>> It's wrong for this release, since we can't change things from stable back
> >>> to experimental. Any such patch will have to wait for 20.11, as agreed in
> >>> the discussion.
> >>>
> >>
> >> Deferring the patchet for this release.
> >>
> >> Reminder that if the option "to mark rte_tm_* as experimental in v20.11"
> >> selected, requires deprecation notice before v20.11.
> > 
> > Thanks Ferruh for reminder. I'll send a deprecation notice patch for the same.
> > 
> 
> Hi Nithin Kumar, Cristian, Jerin,
> 
> Who is working on updating APIs as experimental? We need that patch to proceed
> with this one.

Hi Ferruh,

I'll send out a patch marking all TM API's experimental.
Ferruh Yigit Sept. 14, 2020, 1:01 p.m. UTC | #26 
On 9/7/2020 12:12 PM, Nithin Dabilpuram wrote:
> On Tue, Aug 25, 2020 at 05:59:10PM +0100, Ferruh Yigit wrote:
>> On 5/1/2020 2:16 PM, Nithin Dabilpuram wrote:
>>> On Fri, May 01, 2020 at 11:27:02AM +0100, Ferruh Yigit wrote:
>>>> External Email
>>>>
>>>> ----------------------------------------------------------------------
>>>> On 4/29/2020 10:03 AM, Bruce Richardson wrote:
>>>>> On Wed, Apr 29, 2020 at 09:45:44AM +0100, Dumitrescu, Cristian wrote:
>>>>>>
>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>>>>> Sent: Tuesday, April 28, 2020 4:54 PM
>>>>>>> To: Jerin Jacob <jerinjacobk@gmail.com>; Dumitrescu, Cristian
>>>>>>> <cristian.dumitrescu@intel.com>
>>>>>>> Cc: Richardson, Bruce <bruce.richardson@intel.com>; Yigit, Ferruh
>>>>>>> <ferruh.yigit@intel.com>; Luca Boccassi <bluca@debian.org>; Nithin
>>>>>>> Dabilpuram <nithind1988@gmail.com>; Singh, Jasvinder
>>>>>>> <jasvinder.singh@intel.com>; Andrew Rybchenko
>>>>>>> <arybchenko@solarflare.com>; dev@dpdk.org; jerinj@marvell.com;
>>>>>>> kkanas@marvell.com; Nithin Dabilpuram <ndabilpuram@marvell.com>;
>>>>>>> Kinsella, Ray <ray.kinsella@intel.com>; Neil Horman
>>>>>>> <nhorman@tuxdriver.com>; Kevin Traynor <ktraynor@redhat.com>; David
>>>>>>> Marchand <david.marchand@redhat.com>
>>>>>>> Subject: Re: [dpdk-dev] [PATCH v4 1/4] ethdev: add tm support for shaper
>>>>>>> config in pkt mode
>>>>>>>
>>>>>>> 28/04/2020 17:04, Luca Boccassi:
>>>>>>>> On Tue, 2020-04-28 at 15:45 +0100, Bruce Richardson wrote:
>>>>>>>>> On Tue, Apr 28, 2020 at 03:06:20PM +0100, Ferruh Yigit wrote:
>>>>>>>>>> On 4/27/2020 5:59 PM, Jerin Jacob wrote:
>>>>>>>>>>> On Mon, Apr 27, 2020 at 10:19 PM Ferruh Yigit
>>>>>>> <ferruh.yigit@intel.com> wrote:
>>>>>>>>>>>> On 4/27/2020 5:29 PM, Jerin Jacob wrote:
>>>>>>>>>>>>> On Mon, Apr 27, 2020 at 9:42 PM Ferruh Yigit
>>>>>>> <ferruh.yigit@intel.com> wrote:
>>>>>>>>>>>>>> On 4/27/2020 10:19 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>>>>>> From: Yigit, Ferruh <ferruh.yigit@intel.com>
>>>>>>>>>>>>>>>> On 4/24/2020 11:28 AM, Dumitrescu, Cristian wrote:
>>>>>>>>>>>>>>>>> From: Nithin Dabilpuram <nithind1988@gmail.com>
>>>>>>>>>>>>>>>>>> This patch also updates tm port/level/node capability
>>>>>>> structures with
>>>>>>>>>>>>>>>>>> exiting features of scheduler wfq packet mode,
>>>>>>> scheduler wfq byte mode
>>>>>>>>>>>>>>>>>> and private/shared shaper byte mode.
>>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>> SoftNIC PMD is also updated with new capabilities.
>>>>>>> [...]
>>>>>>>>>>>>>>>> Hi Nithin,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> It looks like patch is causing ABI break, I am getting following
>>>>>>> warning [1],
>>>>>>>>>>>>>>>> can you please check?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> [1]
>>>>>>>>>>>>>>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__pastebin.com_XYNFg14u&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=ej5sP3-cEhEoCTZOia-QivXqgljtzBcMLtZGs-5c-Uc&s=B8z_5mQ2xO3C1izjmRe2zBApMrCUcW6KcAN-adglhJQ&e= 
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hi Ferruh,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The RTE_TM API is marked as experimental,
>>>>>>>>>>>>>>> but it looks that this was not correctly marked
>>>>>>>>>>>>>>> when __rte_experimental ABI checker was introduced.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> It is marked as experimental at the top of the rte_tm.h,
>>>>>>>>>>>>>>> similarly to other APIs introduced around same time,
>>>>>>>>>>>>>>> but it was not correctly picked up by the ABI check procedure
>>>>>>>>>>>>>>> when later introduced, so __rte_experimental was not added
>>>>>>> to every function.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> :(
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is it time to mature them?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> As you said they are not marked as experimental both in header
>>>>>>> file (function
>>>>>>>>>>>>>> declarations) and .map file.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> The problem is, they are not marked as experimental in
>>>>>>> DPDK_20.0 ABI (v19.11),
>>>>>>>>>>>>>> so marking them as experimental now will break the ABI. Not
>>>>>>> sure what to do,
>>>>>>>>>>>>>> cc'ed a few ABI related names for comment.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> For me, we need to proceed as the experimental tag removed
>>>>>>> and APIs become
>>>>>>>>>>>>>> mature starting from v19.11, since this is what happened in
>>>>>>> practice, and remove
>>>>>>>>>>>>>> a few existing being experimental references in the doxygen
>>>>>>> comments.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think, accidentally we can not make a library as NON-
>>>>>>> experimental.
>>>>>>>>>>>>> TM never went through experimental to mature transition(see git
>>>>>>> log
>>>>>>>>>>>>> lib/librte_ethdev/rte_tm.h)
>>>>>>>>>>>>> It was a bug to not mark as experimental in each function in the
>>>>>>> ABI process.
>>>>>>>>>>>>> Some of the features like packet marking are not even
>>>>>>> implemented by any HW.
>>>>>>>>>>>>> I think, we can make API stable only all the features are
>>>>>>> implemented
>>>>>>>>>>>>> by one or two HW.
>>>>>>>
>>>>>>> Yes this is what was decided one or two years ago I think.
>>>>>>> But rte_tm API was introduced 3 years ago and is implemented by 6 PMDs.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>>>>>> Fair enough, specially if the API is not ready yet.
>>>>>>>>>>>>
>>>>>>>>>>>> But they were part of stable ABI, and marking them as experimental
>>>>>>> now will
>>>>>>>>>>>> break the old applications using these APIs.
>>>>>>>>>>>
>>>>>>>>>>> it is still marked as EXPERIMENTAL everywhere and API is not ready
>>>>>>> yet.
>>>>>>>
>>>>>>> rte_tm is implemented in 6 PMDs.
>>>>>>>
>>>>>>>
>>>>>>>>>> Existing experimental marks are text only for human parsing.
>>>>>>>>>>
>>>>>>>>>> The compiler attribute and build time checks are missing, and the
>>>>>>> symbol in the
>>>>>>>>>> binary doesn't have experimental tag. Our scripts and automated
>>>>>>> checks won't
>>>>>>>>>> detect it as experimental.
>>>>>>>>>>
>>>>>>>>>> My point is just having experimental comment in header file is not
>>>>>>> enough to
>>>>>>>>>> qualify the APIs as experimental.
>>>>>>>>>>
>>>>>>>>>>> Anyway, we need to break the ABI to make it work on various HW.
>>>>>>>
>>>>>>> Yes this is why I was asking in 19.11 to check our API,
>>>>>>> in order to avoid such situation.
>>>>>>>
>>>>>>>
>>>>>>>>>>> I am not sure what to do?
>>>>>>>
>>>>>>> Either manage ABI versioning, or wait 20.11.
>>>>>>>
>>>>>>>
>>>>>>>>>>> IMO, We need to send a patch as Fixes: for the bug of not adding
>>>>>>>>>>> __rte_experimental in each function.
>>>>>>>
>>>>>>> No, this is wrong.
>>>>>>>
>>>>>>
>>>>>> Why exactly is this wrong? This is the gap that caused the current discussion, right?
>>>>>>
>>>>> It's wrong for this release, since we can't change things from stable back
>>>>> to experimental. Any such patch will have to wait for 20.11, as agreed in
>>>>> the discussion.
>>>>>
>>>>
>>>> Deferring the patchet for this release.
>>>>
>>>> Reminder that if the option "to mark rte_tm_* as experimental in v20.11"
>>>> selected, requires deprecation notice before v20.11.
>>>
>>> Thanks Ferruh for reminder. I'll send a deprecation notice patch for the same.
>>>
>>
>> Hi Nithin Kumar, Cristian, Jerin,
>>
>> Who is working on updating APIs as experimental? We need that patch to proceed
>> with this one.
> 
> Hi Ferruh,
> 
> I'll send out a patch marking all TM API's experimental.
> 

The experimental API change is done & applied,
hence for this series:

Series applied to dpdk-next-net/main, thanks.
diff mbox series

Patch

diff --git a/drivers/net/softnic/rte_eth_softnic_tm.c b/drivers/net/softnic/rte_eth_softnic_tm.c
index 80a470c..d309763 100644
--- a/drivers/net/softnic/rte_eth_softnic_tm.c
+++ b/drivers/net/softnic/rte_eth_softnic_tm.c
@@ -447,6 +447,8 @@  static const struct rte_tm_capabilities tm_cap = {
 	.shaper_private_dual_rate_n_max = 0,
 	.shaper_private_rate_min = 1,
 	.shaper_private_rate_max = UINT32_MAX,
+	.shaper_private_packet_mode_supported = 0,
+	.shaper_private_byte_mode_supported = 1,
 
 	.shaper_shared_n_max = UINT32_MAX,
 	.shaper_shared_n_nodes_per_shaper_max = UINT32_MAX,
@@ -454,6 +456,8 @@  static const struct rte_tm_capabilities tm_cap = {
 	.shaper_shared_dual_rate_n_max = 0,
 	.shaper_shared_rate_min = 1,
 	.shaper_shared_rate_max = UINT32_MAX,
+	.shaper_shared_packet_mode_supported = 0,
+	.shaper_shared_byte_mode_supported = 1,
 
 	.shaper_pkt_length_adjust_min = RTE_TM_ETH_FRAMING_OVERHEAD_FCS,
 	.shaper_pkt_length_adjust_max = RTE_TM_ETH_FRAMING_OVERHEAD_FCS,
@@ -463,6 +467,8 @@  static const struct rte_tm_capabilities tm_cap = {
 	.sched_wfq_n_children_per_group_max = UINT32_MAX,
 	.sched_wfq_n_groups_max = 1,
 	.sched_wfq_weight_max = UINT32_MAX,
+	.sched_wfq_packet_mode_supported = 0,
+	.sched_wfq_byte_mode_supported = 1,
 
 	.cman_wred_packet_mode_supported = WRED_SUPPORTED,
 	.cman_wred_byte_mode_supported = 0,
@@ -548,13 +554,19 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.shaper_private_dual_rate_supported = 0,
 			.shaper_private_rate_min = 1,
 			.shaper_private_rate_max = UINT32_MAX,
+			.shaper_private_packet_mode_supported = 0,
+			.shaper_private_byte_mode_supported = 1,
 			.shaper_shared_n_max = 0,
+			.shaper_shared_packet_mode_supported = 0,
+			.shaper_shared_byte_mode_supported = 0,
 
 			.sched_n_children_max = UINT32_MAX,
 			.sched_sp_n_priorities_max = 1,
 			.sched_wfq_n_children_per_group_max = UINT32_MAX,
 			.sched_wfq_n_groups_max = 1,
 			.sched_wfq_weight_max = 1,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 
 			.stats_mask = STATS_MASK_DEFAULT,
 		} },
@@ -572,7 +584,11 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.shaper_private_dual_rate_supported = 0,
 			.shaper_private_rate_min = 1,
 			.shaper_private_rate_max = UINT32_MAX,
+			.shaper_private_packet_mode_supported = 0,
+			.shaper_private_byte_mode_supported = 1,
 			.shaper_shared_n_max = 0,
+			.shaper_shared_packet_mode_supported = 0,
+			.shaper_shared_byte_mode_supported = 0,
 
 			.sched_n_children_max = UINT32_MAX,
 			.sched_sp_n_priorities_max = 1,
@@ -580,9 +596,14 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.sched_wfq_n_groups_max = 1,
 #ifdef RTE_SCHED_SUBPORT_TC_OV
 			.sched_wfq_weight_max = UINT32_MAX,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 #else
 			.sched_wfq_weight_max = 1,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 #endif
+
 			.stats_mask = STATS_MASK_DEFAULT,
 		} },
 	},
@@ -599,7 +620,11 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.shaper_private_dual_rate_supported = 0,
 			.shaper_private_rate_min = 1,
 			.shaper_private_rate_max = UINT32_MAX,
+			.shaper_private_packet_mode_supported = 0,
+			.shaper_private_byte_mode_supported = 1,
 			.shaper_shared_n_max = 0,
+			.shaper_shared_packet_mode_supported = 0,
+			.shaper_shared_byte_mode_supported = 0,
 
 			.sched_n_children_max =
 				RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE,
@@ -608,6 +633,8 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.sched_wfq_n_children_per_group_max = 1,
 			.sched_wfq_n_groups_max = 0,
 			.sched_wfq_weight_max = 1,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 0,
 
 			.stats_mask = STATS_MASK_DEFAULT,
 		} },
@@ -625,7 +652,11 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.shaper_private_dual_rate_supported = 0,
 			.shaper_private_rate_min = 1,
 			.shaper_private_rate_max = UINT32_MAX,
+			.shaper_private_packet_mode_supported = 0,
+			.shaper_private_byte_mode_supported = 1,
 			.shaper_shared_n_max = 1,
+			.shaper_shared_packet_mode_supported = 0,
+			.shaper_shared_byte_mode_supported = 1,
 
 			.sched_n_children_max =
 				RTE_SCHED_BE_QUEUES_PER_PIPE,
@@ -634,6 +665,8 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 				RTE_SCHED_BE_QUEUES_PER_PIPE,
 			.sched_wfq_n_groups_max = 1,
 			.sched_wfq_weight_max = UINT32_MAX,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 
 			.stats_mask = STATS_MASK_DEFAULT,
 		} },
@@ -651,7 +684,11 @@  static const struct rte_tm_level_capabilities tm_level_cap[] = {
 			.shaper_private_dual_rate_supported = 0,
 			.shaper_private_rate_min = 0,
 			.shaper_private_rate_max = 0,
+			.shaper_private_packet_mode_supported = 0,
+			.shaper_private_byte_mode_supported = 0,
 			.shaper_shared_n_max = 0,
+			.shaper_shared_packet_mode_supported = 0,
+			.shaper_shared_byte_mode_supported = 0,
 
 			.cman_head_drop_supported = 0,
 			.cman_wred_packet_mode_supported = WRED_SUPPORTED,
@@ -736,7 +773,11 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 		.shaper_private_dual_rate_supported = 0,
 		.shaper_private_rate_min = 1,
 		.shaper_private_rate_max = UINT32_MAX,
+		.shaper_private_packet_mode_supported = 0,
+		.shaper_private_byte_mode_supported = 1,
 		.shaper_shared_n_max = 0,
+		.shaper_shared_packet_mode_supported = 0,
+		.shaper_shared_byte_mode_supported = 0,
 
 		{.nonleaf = {
 			.sched_n_children_max = UINT32_MAX,
@@ -744,6 +785,8 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 			.sched_wfq_n_children_per_group_max = UINT32_MAX,
 			.sched_wfq_n_groups_max = 1,
 			.sched_wfq_weight_max = 1,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 		} },
 
 		.stats_mask = STATS_MASK_DEFAULT,
@@ -754,7 +797,11 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 		.shaper_private_dual_rate_supported = 0,
 		.shaper_private_rate_min = 1,
 		.shaper_private_rate_max = UINT32_MAX,
+		.shaper_private_packet_mode_supported = 0,
+		.shaper_private_byte_mode_supported = 1,
 		.shaper_shared_n_max = 0,
+		.shaper_shared_packet_mode_supported = 0,
+		.shaper_shared_byte_mode_supported = 0,
 
 		{.nonleaf = {
 			.sched_n_children_max = UINT32_MAX,
@@ -762,6 +809,8 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 			.sched_wfq_n_children_per_group_max = UINT32_MAX,
 			.sched_wfq_n_groups_max = 1,
 			.sched_wfq_weight_max = UINT32_MAX,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 		} },
 
 		.stats_mask = STATS_MASK_DEFAULT,
@@ -772,7 +821,11 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 		.shaper_private_dual_rate_supported = 0,
 		.shaper_private_rate_min = 1,
 		.shaper_private_rate_max = UINT32_MAX,
+		.shaper_private_packet_mode_supported = 0,
+		.shaper_private_byte_mode_supported = 1,
 		.shaper_shared_n_max = 0,
+		.shaper_shared_packet_mode_supported = 0,
+		.shaper_shared_byte_mode_supported = 0,
 
 		{.nonleaf = {
 			.sched_n_children_max =
@@ -782,6 +835,8 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 			.sched_wfq_n_children_per_group_max = 1,
 			.sched_wfq_n_groups_max = 0,
 			.sched_wfq_weight_max = 1,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 0,
 		} },
 
 		.stats_mask = STATS_MASK_DEFAULT,
@@ -792,7 +847,11 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 		.shaper_private_dual_rate_supported = 0,
 		.shaper_private_rate_min = 1,
 		.shaper_private_rate_max = UINT32_MAX,
+		.shaper_private_packet_mode_supported = 0,
+		.shaper_private_byte_mode_supported = 1,
 		.shaper_shared_n_max = 1,
+		.shaper_shared_packet_mode_supported = 0,
+		.shaper_shared_byte_mode_supported = 1,
 
 		{.nonleaf = {
 			.sched_n_children_max =
@@ -802,6 +861,8 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 				RTE_SCHED_BE_QUEUES_PER_PIPE,
 			.sched_wfq_n_groups_max = 1,
 			.sched_wfq_weight_max = UINT32_MAX,
+			.sched_wfq_packet_mode_supported = 0,
+			.sched_wfq_byte_mode_supported = 1,
 		} },
 
 		.stats_mask = STATS_MASK_DEFAULT,
@@ -812,7 +873,11 @@  static const struct rte_tm_node_capabilities tm_node_cap[] = {
 		.shaper_private_dual_rate_supported = 0,
 		.shaper_private_rate_min = 0,
 		.shaper_private_rate_max = 0,
+		.shaper_private_packet_mode_supported = 0,
+		.shaper_private_byte_mode_supported = 0,
 		.shaper_shared_n_max = 0,
+		.shaper_shared_packet_mode_supported = 0,
+		.shaper_shared_byte_mode_supported = 0,
 
 
 		{.leaf = {
@@ -947,6 +1012,13 @@  shaper_profile_check(struct rte_eth_dev *dev,
 			NULL,
 			rte_strerror(EINVAL));
 
+	/* Packet mode is not supported. */
+	if (profile->packet_mode != 0)
+		return -rte_tm_error_set(error,
+			EINVAL,
+			RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE,
+			NULL,
+			rte_strerror(EINVAL));
 	return 0;
 }
 
diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h
index f9c0cf3..110c263 100644
--- a/lib/librte_ethdev/rte_tm.h
+++ b/lib/librte_ethdev/rte_tm.h
@@ -250,6 +250,23 @@  struct rte_tm_capabilities {
 	 */
 	uint64_t shaper_private_rate_max;
 
+	/** Shaper private packet mode supported. When non-zero, this parameter
+	 * indicates that there is at least one node that can be configured
+	 * with packet mode in its private shaper. When shaper is configured
+	 * in packet mode, committed/peak rate provided is interpreted
+	 * in packets per second.
+	 */
+	int shaper_private_packet_mode_supported;
+
+	/** Shaper private byte mode supported. When non-zero, this parameter
+	 * indicates that there is at least one node that can be configured
+	 * with byte mode in its private shaper. When shaper is configured
+	 * in byte mode, committed/peak rate provided is interpreted in
+	 * bytes per second.
+	 */
+	int shaper_private_byte_mode_supported;
+
+
 	/** Maximum number of shared shapers. The value of zero indicates that
 	 * shared shapers are not supported.
 	 */
@@ -284,6 +301,21 @@  struct rte_tm_capabilities {
 	 */
 	uint64_t shaper_shared_rate_max;
 
+	/** Shaper shared packet mode supported. When non-zero, this parameter
+	 * indicates a shared shaper can be configured with packet mode.
+	 * When shared shaper is configured in packet mode, committed/peak rate
+	 * provided is interpreted in packets per second.
+	 */
+	int shaper_shared_packet_mode_supported;
+
+	/** Shaper shared byte mode supported. When non-zero, this parameter
+	 * indicates that a shared shaper can be configured with byte mode.
+	 * When shared shaper is configured in byte mode, committed/peak rate
+	 * provided is interpreted in bytes per second.
+	 */
+	int shaper_shared_byte_mode_supported;
+
+
 	/** Minimum value allowed for packet length adjustment for any private
 	 * or shared shaper.
 	 */
@@ -339,6 +371,22 @@  struct rte_tm_capabilities {
 	 */
 	uint32_t sched_wfq_weight_max;
 
+	/** WFQ packet mode supported. When non-zero, this parameter indicates
+	 * that there is at least one non-leaf node that supports packet mode
+	 * for WFQ among its children. WFQ weights will be applied against
+	 * packet count for scheduling children when a non-leaf node
+	 * is configured appropriately.
+	 */
+	int sched_wfq_packet_mode_supported;
+
+	/** WFQ byte mode supported. When non-zero, this parameter indicates
+	 * that there is at least one non-leaf node that supports byte mode
+	 * for WFQ among its children. WFQ weights will be applied against
+	 * bytes for scheduling children when a non-leaf node is configured
+	 * appropriately.
+	 */
+	int sched_wfq_byte_mode_supported;
+
 	/** WRED packet mode support. When non-zero, this parameter indicates
 	 * that there is at least one leaf node that supports the WRED packet
 	 * mode, which might not be true for all the leaf nodes. In packet
@@ -485,6 +533,24 @@  struct rte_tm_level_capabilities {
 			 */
 			uint64_t shaper_private_rate_max;
 
+			/** Shaper private packet mode supported. When non-zero,
+			 * this parameter indicates there is at least one
+			 * non-leaf node at this level that can be configured
+			 * with packet mode in its private shaper. When private
+			 * shaper is configured in packet mode, committed/peak
+			 * rate provided is interpreted in packets per second.
+			 */
+			int shaper_private_packet_mode_supported;
+
+			/** Shaper private byte mode supported. When non-zero,
+			 * this parameter indicates there is at least one
+			 * non-leaf node at this level that can be configured
+			 * with byte mode in its private shaper. When private
+			 * shaper is configured in byte mode, committed/peak
+			 * rate provided is interpreted in bytes per second.
+			 */
+			int shaper_private_byte_mode_supported;
+
 			/** Maximum number of shared shapers that any non-leaf
 			 * node on this level can be part of. The value of zero
 			 * indicates that shared shapers are not supported by
@@ -495,6 +561,20 @@  struct rte_tm_level_capabilities {
 			 */
 			uint32_t shaper_shared_n_max;
 
+			/** Shaper shared packet mode supported. When non-zero,
+			 * this parameter indicates that there is at least one
+			 * non-leaf node on this level that can be part of
+			 * shared shapers which work in packet mode.
+			 */
+			int shaper_shared_packet_mode_supported;
+
+			/** Shaper shared byte mode supported. When non-zero,
+			 * this parameter indicates that there is at least one
+			 * non-leaf node on this level that can be part of
+			 * shared shapers which work in byte mode.
+			 */
+			int shaper_shared_byte_mode_supported;
+
 			/** Maximum number of children nodes. This parameter
 			 * indicates that there is at least one non-leaf node on
 			 * this level that can be configured with this many
@@ -554,6 +634,25 @@  struct rte_tm_level_capabilities {
 			 */
 			uint32_t sched_wfq_weight_max;
 
+			/** WFQ packet mode supported. When non-zero, this
+			 * parameter indicates that there is at least one
+			 * non-leaf node at this level that supports packet
+			 * mode for WFQ among its children. WFQ weights will
+			 * be applied against packet count for scheduling
+			 * children when a non-leaf node is configured
+			 * appropriately.
+			 */
+			int sched_wfq_packet_mode_supported;
+
+			/** WFQ byte mode supported. When non-zero, this
+			 * parameter indicates that there is at least one
+			 * non-leaf node at this level that supports byte
+			 * mode for WFQ among its children. WFQ weights will
+			 * be applied against bytes for scheduling children
+			 * when a non-leaf node is configured appropriately.
+			 */
+			int sched_wfq_byte_mode_supported;
+
 			/** Mask of statistics counter types supported by the
 			 * non-leaf nodes on this level. Every supported
 			 * statistics counter type is supported by at least one
@@ -596,6 +695,24 @@  struct rte_tm_level_capabilities {
 			 */
 			uint64_t shaper_private_rate_max;
 
+			/** Shaper private packet mode supported. When non-zero,
+			 * this parameter indicates there is at least one leaf
+			 * node at this level that can be configured with
+			 * packet mode in its private shaper. When private
+			 * shaper is configured in packet mode, committed/peak
+			 * rate provided is interpreted in packets per second.
+			 */
+			int shaper_private_packet_mode_supported;
+
+			/** Shaper private byte mode supported. When non-zero,
+			 * this parameter indicates there is at least one leaf
+			 * node at this level that can be configured with
+			 * byte mode in its private shaper. When private shaper
+			 * is configured in byte mode, committed/peak rate
+			 * provided is interpreted in bytes per second.
+			 */
+			int shaper_private_byte_mode_supported;
+
 			/** Maximum number of shared shapers that any leaf node
 			 * on this level can be part of. The value of zero
 			 * indicates that shared shapers are not supported by
@@ -606,6 +723,20 @@  struct rte_tm_level_capabilities {
 			 */
 			uint32_t shaper_shared_n_max;
 
+			/** Shaper shared packet mode supported. When non-zero,
+			 * this parameter indicates that there is at least one
+			 * leaf node on this level that can be part of
+			 * shared shapers which work in packet mode.
+			 */
+			int shaper_shared_packet_mode_supported;
+
+			/** Shaper shared byte mode supported. When non-zero,
+			 * this parameter indicates that there is at least one
+			 * leaf node on this level that can be part of
+			 * shared shapers which work in byte mode.
+			 */
+			int shaper_shared_byte_mode_supported;
+
 			/** WRED packet mode support. When non-zero, this
 			 * parameter indicates that there is at least one leaf
 			 * node on this level that supports the WRED packet
@@ -686,12 +817,38 @@  struct rte_tm_node_capabilities {
 	 */
 	uint64_t shaper_private_rate_max;
 
+	/** Shaper private packet mode supported. When non-zero, this parameter
+	 * indicates private shaper of current node can be configured with
+	 * packet mode. When configured in packet mode, committed/peak rate
+	 * provided is interpreted in packets per second.
+	 */
+	int shaper_private_packet_mode_supported;
+
+	/** Shaper private byte mode supported. When non-zero, this parameter
+	 * indicates private shaper of current node can be configured with
+	 * byte mode. When configured in byte mode, committed/peak rate
+	 * provided is interpreted in bytes per second.
+	 */
+	int shaper_private_byte_mode_supported;
+
 	/** Maximum number of shared shapers the current node can be part of.
 	 * The value of zero indicates that shared shapers are not supported by
 	 * the current node.
 	 */
 	uint32_t shaper_shared_n_max;
 
+	/** Shaper shared packet mode supported. When non-zero,
+	 * this parameter indicates that current node can be part of
+	 * shared shapers which work in packet mode.
+	 */
+	int shaper_shared_packet_mode_supported;
+
+	/** Shaper shared byte mode supported. When non-zero,
+	 * this parameter indicates that current node can be part of
+	 * shared shapers which work in byte mode.
+	 */
+	int shaper_shared_byte_mode_supported;
+
 	RTE_STD_C11
 	union {
 		/** Items valid only for non-leaf nodes. */
@@ -735,6 +892,23 @@  struct rte_tm_node_capabilities {
 			 * WFQ weight, so WFQ is reduced to FQ.
 			 */
 			uint32_t sched_wfq_weight_max;
+
+			/** WFQ packet mode supported. When non-zero, this
+			 * parameter indicates that current node supports packet
+			 * mode for WFQ among its children. WFQ weights will be
+			 * applied against packet count for scheduling children
+			 * when configured appropriately.
+			 */
+			int sched_wfq_packet_mode_supported;
+
+			/** WFQ byte mode supported. When non-zero, this
+			 * parameter indicates that current node supports byte
+			 * mode for WFQ among its children. WFQ weights will be
+			 * applied against  bytes for scheduling children when
+			 * configured appropriately.
+			 */
+			int sched_wfq_byte_mode_supported;
+
 		} nonleaf;
 
 		/** Items valid only for leaf nodes. */
@@ -836,10 +1010,10 @@  struct rte_tm_wred_params {
  * Token bucket
  */
 struct rte_tm_token_bucket {
-	/** Token bucket rate (bytes per second) */
+	/** Token bucket rate (bytes per second or packets per second) */
 	uint64_t rate;
 
-	/** Token bucket size (bytes), a.k.a. max burst size */
+	/** Token bucket size (bytes or packets), a.k.a. max burst size */
 	uint64_t size;
 };
 
@@ -860,6 +1034,11 @@  struct rte_tm_token_bucket {
  * Dual rate shapers use both the committed and the peak token buckets. The
  * rate of the peak bucket has to be bigger than zero, as well as greater than
  * or equal to the rate of the committed bucket.
+ *
+ * @see struct rte_tm_capabilities::shaper_private_packet_mode_supported
+ * @see struct rte_tm_capabilities::shaper_private_byte_mode_supported
+ * @see struct rte_tm_capabilities::shaper_shared_packet_mode_supported
+ * @see struct rte_tm_capabilities::shaper_shared_byte_mode_supported
  */
 struct rte_tm_shaper_params {
 	/** Committed token bucket */
@@ -872,8 +1051,19 @@  struct rte_tm_shaper_params {
 	 * purpose of shaping. Can be used to correct the packet length with
 	 * the framing overhead bytes that are also consumed on the wire (e.g.
 	 * RTE_TM_ETH_FRAMING_OVERHEAD_FCS).
+	 * This field is ignored when the profile enables packet mode.
 	 */
 	int32_t pkt_length_adjust;
+
+	/** When zero, the byte mode is enabled for the current profile, so the
+	 * *rate* and *size* fields in both the committed and peak token buckets
+	 * are specified in bytes per second and bytes, respectively.
+	 * When non-zero, the packet mode is enabled for the current profile,
+	 * so the *rate* and *size* fields in both the committed and peak token
+	 * buckets are specified in packets per second and packets,
+	 * respectively.
+	 */
+	int packet_mode;
 };
 
 /**
@@ -925,6 +1115,8 @@  struct rte_tm_node_params {
 			 * When non-NULL, it points to a pre-allocated array of
 			 * *n_sp_priorities* values, with non-zero value for
 			 * byte-mode and zero for packet-mode.
+			 * @see struct rte_tm_node_capabilities::sched_wfq_packet_mode_supported
+			 * @see struct rte_tm_node_capabilities::sched_wfq_byte_mode_supported
 			 */
 			int *wfq_weight_mode;
 
@@ -997,6 +1189,7 @@  enum rte_tm_error_type {
 	RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PEAK_RATE,
 	RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PEAK_SIZE,
 	RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PKT_ADJUST_LEN,
+	RTE_TM_ERROR_TYPE_SHAPER_PROFILE_PACKET_MODE,
 	RTE_TM_ERROR_TYPE_SHAPER_PROFILE_ID,
 	RTE_TM_ERROR_TYPE_SHARED_SHAPER_ID,
 	RTE_TM_ERROR_TYPE_NODE_PARENT_NODE_ID,