diff mbox series

[v2] ethdev: mark all traffic manager API as experimental

Message ID 20200910080538.28041-1-ndabilpuram@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers show
Series [v2] ethdev: mark all traffic manager API as experimental | expand

Checks

Context Check Description
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/iol-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/checkpatch success coding style OK

Commit Message

Nithin Dabilpuram Sept. 10, 2020, 8:05 a.m. UTC
This patch marks all traffic manager API as experimental as
per discussion mentioned in below thread.

https://mails.dpdk.org/archives/dev/2020-April/165364.html

Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
---

v2:
- Updated commit message.

 lib/librte_ethdev/rte_ethdev_version.map | 62 ++++++++++++++++----------------
 lib/librte_ethdev/rte_tm.h               | 33 ++++++++++++++++-
 2 files changed, 64 insertions(+), 31 deletions(-)

Comments

Kinsella, Ray Sept. 10, 2020, 8:30 a.m. UTC | #1
On 10/09/2020 09:05, Nithin Dabilpuram wrote:
> This patch marks all traffic manager API as experimental as
> per discussion mentioned in below thread.
> 
> https://mails.dpdk.org/archives/dev/2020-April/165364.html
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> 
> v2:
> - Updated commit message.
> 
>  lib/librte_ethdev/rte_ethdev_version.map | 62 ++++++++++++++++----------------
>  lib/librte_ethdev/rte_tm.h               | 33 ++++++++++++++++-
>  2 files changed, 64 insertions(+), 31 deletions(-)
> 
> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> index 1212a17..3da4512 100644
> --- a/lib/librte_ethdev/rte_ethdev_version.map
> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> @@ -135,36 +135,6 @@ DPDK_20.0 {
>  	rte_flow_isolate;
>  	rte_flow_query;
>  	rte_flow_validate;
> -	rte_tm_capabilities_get;
> -	rte_tm_get_number_of_leaf_nodes;
> -	rte_tm_hierarchy_commit;
> -	rte_tm_level_capabilities_get;
> -	rte_tm_mark_ip_dscp;
> -	rte_tm_mark_ip_ecn;
> -	rte_tm_mark_vlan_dei;
> -	rte_tm_node_add;
> -	rte_tm_node_capabilities_get;
> -	rte_tm_node_cman_update;
> -	rte_tm_node_delete;
> -	rte_tm_node_parent_update;
> -	rte_tm_node_resume;
> -	rte_tm_node_shaper_update;
> -	rte_tm_node_shared_shaper_update;
> -	rte_tm_node_shared_wred_context_update;
> -	rte_tm_node_stats_read;
> -	rte_tm_node_stats_update;
> -	rte_tm_node_suspend;
> -	rte_tm_node_type_get;
> -	rte_tm_node_wfq_weight_mode_update;
> -	rte_tm_node_wred_context_update;
> -	rte_tm_shaper_profile_add;
> -	rte_tm_shaper_profile_delete;
> -	rte_tm_shared_shaper_add_update;
> -	rte_tm_shared_shaper_delete;
> -	rte_tm_shared_wred_context_add_update;
> -	rte_tm_shared_wred_context_delete;
> -	rte_tm_wred_profile_add;
> -	rte_tm_wred_profile_delete;
>  
>  	local: *;
>  };
> @@ -241,6 +211,38 @@ EXPERIMENTAL {
>  	__rte_ethdev_trace_rx_burst;
>  	__rte_ethdev_trace_tx_burst;
>  	rte_flow_get_aged_flows;
> +
> +	# Marked as experimental in 20.11
> +	rte_tm_capabilities_get;
> +	rte_tm_get_number_of_leaf_nodes;
> +	rte_tm_hierarchy_commit;
> +	rte_tm_level_capabilities_get;
> +	rte_tm_mark_ip_dscp;
> +	rte_tm_mark_ip_ecn;
> +	rte_tm_mark_vlan_dei;
> +	rte_tm_node_add;
> +	rte_tm_node_capabilities_get;
> +	rte_tm_node_cman_update;
> +	rte_tm_node_delete;
> +	rte_tm_node_parent_update;
> +	rte_tm_node_resume;
> +	rte_tm_node_shaper_update;
> +	rte_tm_node_shared_shaper_update;
> +	rte_tm_node_shared_wred_context_update;
> +	rte_tm_node_stats_read;
> +	rte_tm_node_stats_update;
> +	rte_tm_node_suspend;
> +	rte_tm_node_type_get;
> +	rte_tm_node_wfq_weight_mode_update;
> +	rte_tm_node_wred_context_update;
> +	rte_tm_shaper_profile_add;
> +	rte_tm_shaper_profile_delete;
> +	rte_tm_shared_shaper_add_update;
> +	rte_tm_shared_shaper_delete;
> +	rte_tm_shared_wred_context_add_update;
> +	rte_tm_shared_wred_context_delete;
> +	rte_tm_wred_profile_add;
> +	rte_tm_wred_profile_delete;
>  };
[SNIP]

So I will re-interate what I said in April, all of these API have existed since DPDK 17.08.
I find it hard to believe they are all at imminent risk of changing to degree they _all_ warrant EXPERIMENTAL status.

Ray K
Ferruh Yigit Sept. 10, 2020, 8:36 a.m. UTC | #2
On 9/10/2020 9:05 AM, Nithin Dabilpuram wrote:
> This patch marks all traffic manager API as experimental as
> per discussion mentioned in below thread.
> 
> https://mails.dpdk.org/archives/dev/2020-April/165364.html
> 
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> 
> v2:
> - Updated commit message.


Hi Nithin Kumar,

Perhaps there was a misunderstanding, the paragraph in the deprecation note
documents about making tm APIs experimental needs to be removed, since it won't
be valid anymore after this patch.
And best place to remove that note is this patch.
Cristian Dumitrescu Sept. 10, 2020, 8:37 a.m. UTC | #3
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, September 10, 2020 9:36 AM
> To: Nithin Dabilpuram <ndabilpuram@marvell.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Dumitrescu, Cristian
> <cristian.dumitrescu@intel.com>; thomas@monjalon.net;
> bluca@debian.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> arybchenko@solarflare.com; Kinsella, Ray <ray.kinsella@intel.com>;
> nhorman@tuxdriver.com; ktraynor@redhat.com;
> david.marchand@redhat.com; Ray Kinsella <mdr@ashroe.eu>
> Cc: dev@dpdk.org; jerinj@marvell.com
> Subject: Re: [PATCH v2] ethdev: mark all traffic manager API as experimental
> 
> On 9/10/2020 9:05 AM, Nithin Dabilpuram wrote:
> > This patch marks all traffic manager API as experimental as
> > per discussion mentioned in below thread.
> >
> > https://mails.dpdk.org/archives/dev/2020-April/165364.html
> >
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> >
> > v2:
> > - Updated commit message.
> 
> 
> Hi Nithin Kumar,
> 
> Perhaps there was a misunderstanding, the paragraph in the deprecation
> note
> documents about making tm APIs experimental needs to be removed, since
> it won't
> be valid anymore after this patch.
> And best place to remove that note is this patch.

And also please include my ack from V1.
Nithin Dabilpuram Sept. 10, 2020, 9:51 a.m. UTC | #4
On Thu, Sep 10, 2020 at 09:36:14AM +0100, Ferruh Yigit wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On 9/10/2020 9:05 AM, Nithin Dabilpuram wrote:
> > This patch marks all traffic manager API as experimental as
> > per discussion mentioned in below thread.
> > 
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__mails.dpdk.org_archives_dev_2020-2DApril_165364.html&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=wqVBW8b3o-0Gamt8jyb4qjfv2NC91yJu72NYGkTiV_Q&s=-2I-O0kTUtPiOhpNsG_7Z06p9jn_h55ZQuO-KW6h-R4&e= 
> > 
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> > 
> > v2:
> > - Updated commit message.
> 
> 
> Hi Nithin Kumar,
> 
> Perhaps there was a misunderstanding, the paragraph in the deprecation note
> documents about making tm APIs experimental needs to be removed, since it won't
> be valid anymore after this patch.
> And best place to remove that note is this patch.

Apologies for misunderstanding your statement. Will send a V3 with the same.
Nithin Dabilpuram Sept. 10, 2020, 9:54 a.m. UTC | #5
On Thu, Sep 10, 2020 at 08:37:36AM +0000, Dumitrescu, Cristian wrote:
> External Email
> 
> ----------------------------------------------------------------------
> 
> 
> > -----Original Message-----
> > From: Ferruh Yigit <ferruh.yigit@intel.com>
> > Sent: Thursday, September 10, 2020 9:36 AM
> > To: Nithin Dabilpuram <ndabilpuram@marvell.com>; Richardson, Bruce
> > <bruce.richardson@intel.com>; Dumitrescu, Cristian
> > <cristian.dumitrescu@intel.com>; thomas@monjalon.net;
> > bluca@debian.org; Singh, Jasvinder <jasvinder.singh@intel.com>;
> > arybchenko@solarflare.com; Kinsella, Ray <ray.kinsella@intel.com>;
> > nhorman@tuxdriver.com; ktraynor@redhat.com;
> > david.marchand@redhat.com; Ray Kinsella <mdr@ashroe.eu>
> > Cc: dev@dpdk.org; jerinj@marvell.com
> > Subject: Re: [PATCH v2] ethdev: mark all traffic manager API as experimental
> > 
> > On 9/10/2020 9:05 AM, Nithin Dabilpuram wrote:
> > > This patch marks all traffic manager API as experimental as
> > > per discussion mentioned in below thread.
> > >
> > > https://urldefense.proofpoint.com/v2/url?u=https-3A__mails.dpdk.org_archives_dev_2020-2DApril_165364.html&d=DwIGaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=1aU10CHItu21NpV_gU3dlmy4PBwlNcfkHEqviKGFU3U&s=U0XtjGvqaDr1fYaF06g9__xeFexlEHybu8kgMopYrG8&e= 
> > >
> > > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > > ---
> > >
> > > v2:
> > > - Updated commit message.
> > 
> > 
> > Hi Nithin Kumar,
> > 
> > Perhaps there was a misunderstanding, the paragraph in the deprecation
> > note
> > documents about making tm APIs experimental needs to be removed, since
> > it won't
> > be valid anymore after this patch.
> > And best place to remove that note is this patch.
> 
> And also please include my ack from V1.
Ack, sorry for missing that.
Nithin Dabilpuram Sept. 10, 2020, 10:56 a.m. UTC | #6
On Thu, Sep 10, 2020 at 09:30:15AM +0100, Kinsella, Ray wrote:
> External Email
> 
> ----------------------------------------------------------------------
> On 10/09/2020 09:05, Nithin Dabilpuram wrote:
> > This patch marks all traffic manager API as experimental as
> > per discussion mentioned in below thread.
> > 
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__mails.dpdk.org_archives_dev_2020-2DApril_165364.html&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=NlIUXSsugCPTKBv9qxsaMHOhTuC90TUqLcGg962VVyA&s=DZL_m0a73uef5Qgal1iFqc9dTdAuUd8J6eC0mZOQlFs&e= 
> > 
> > Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> > ---
> > 
> > v2:
> > - Updated commit message.
> > 
> >  lib/librte_ethdev/rte_ethdev_version.map | 62 ++++++++++++++++----------------
> >  lib/librte_ethdev/rte_tm.h               | 33 ++++++++++++++++-
> >  2 files changed, 64 insertions(+), 31 deletions(-)
> > 
> > diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> > index 1212a17..3da4512 100644
> > --- a/lib/librte_ethdev/rte_ethdev_version.map
> > +++ b/lib/librte_ethdev/rte_ethdev_version.map
> > @@ -135,36 +135,6 @@ DPDK_20.0 {
> >  	rte_flow_isolate;
> >  	rte_flow_query;
> >  	rte_flow_validate;
> > -	rte_tm_capabilities_get;
> > -	rte_tm_get_number_of_leaf_nodes;
> > -	rte_tm_hierarchy_commit;
> > -	rte_tm_level_capabilities_get;
> > -	rte_tm_mark_ip_dscp;
> > -	rte_tm_mark_ip_ecn;
> > -	rte_tm_mark_vlan_dei;
> > -	rte_tm_node_add;
> > -	rte_tm_node_capabilities_get;
> > -	rte_tm_node_cman_update;
> > -	rte_tm_node_delete;
> > -	rte_tm_node_parent_update;
> > -	rte_tm_node_resume;
> > -	rte_tm_node_shaper_update;
> > -	rte_tm_node_shared_shaper_update;
> > -	rte_tm_node_shared_wred_context_update;
> > -	rte_tm_node_stats_read;
> > -	rte_tm_node_stats_update;
> > -	rte_tm_node_suspend;
> > -	rte_tm_node_type_get;
> > -	rte_tm_node_wfq_weight_mode_update;
> > -	rte_tm_node_wred_context_update;
> > -	rte_tm_shaper_profile_add;
> > -	rte_tm_shaper_profile_delete;
> > -	rte_tm_shared_shaper_add_update;
> > -	rte_tm_shared_shaper_delete;
> > -	rte_tm_shared_wred_context_add_update;
> > -	rte_tm_shared_wred_context_delete;
> > -	rte_tm_wred_profile_add;
> > -	rte_tm_wred_profile_delete;
> >  
> >  	local: *;
> >  };
> > @@ -241,6 +211,38 @@ EXPERIMENTAL {
> >  	__rte_ethdev_trace_rx_burst;
> >  	__rte_ethdev_trace_tx_burst;
> >  	rte_flow_get_aged_flows;
> > +
> > +	# Marked as experimental in 20.11
> > +	rte_tm_capabilities_get;
> > +	rte_tm_get_number_of_leaf_nodes;
> > +	rte_tm_hierarchy_commit;
> > +	rte_tm_level_capabilities_get;
> > +	rte_tm_mark_ip_dscp;
> > +	rte_tm_mark_ip_ecn;
> > +	rte_tm_mark_vlan_dei;
> > +	rte_tm_node_add;
> > +	rte_tm_node_capabilities_get;
> > +	rte_tm_node_cman_update;
> > +	rte_tm_node_delete;
> > +	rte_tm_node_parent_update;
> > +	rte_tm_node_resume;
> > +	rte_tm_node_shaper_update;
> > +	rte_tm_node_shared_shaper_update;
> > +	rte_tm_node_shared_wred_context_update;
> > +	rte_tm_node_stats_read;
> > +	rte_tm_node_stats_update;
> > +	rte_tm_node_suspend;
> > +	rte_tm_node_type_get;
> > +	rte_tm_node_wfq_weight_mode_update;
> > +	rte_tm_node_wred_context_update;
> > +	rte_tm_shaper_profile_add;
> > +	rte_tm_shaper_profile_delete;
> > +	rte_tm_shared_shaper_add_update;
> > +	rte_tm_shared_shaper_delete;
> > +	rte_tm_shared_wred_context_add_update;
> > +	rte_tm_shared_wred_context_delete;
> > +	rte_tm_wred_profile_add;
> > +	rte_tm_wred_profile_delete;
> >  };
> [SNIP]
> 
> So I will re-interate what I said in April, all of these API have existed since DPDK 17.08.
> I find it hard to believe they are all at imminent risk of changing to degree they _all_ warrant EXPERIMENTAL status.

As discussed earlier and agreed by TM and Ethdev maintainers, __rte_experimental tag was introduced several
releases after TM spec was added and TM spec remained untouched ending up making it stable unintentionally. 

Bruce and other maintainers pointed out that we cannot fix it as a "Fixes:" patch in order
to honor ABI commitments, and suggestion was to change the complete spec back to
experimental in next ABI change window and proceed with improvements that make
it stable API in subsequent releases.
> 
> Ray K
Kinsella, Ray Sept. 10, 2020, 11:01 a.m. UTC | #7
On 10/09/2020 11:56, Nithin Dabilpuram wrote:
> On Thu, Sep 10, 2020 at 09:30:15AM +0100, Kinsella, Ray wrote:
>> External Email
>>
>> ----------------------------------------------------------------------
>> On 10/09/2020 09:05, Nithin Dabilpuram wrote:
>>> This patch marks all traffic manager API as experimental as
>>> per discussion mentioned in below thread.
>>>
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__mails.dpdk.org_archives_dev_2020-2DApril_165364.html&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=NlIUXSsugCPTKBv9qxsaMHOhTuC90TUqLcGg962VVyA&s=DZL_m0a73uef5Qgal1iFqc9dTdAuUd8J6eC0mZOQlFs&e= 
>>>
>>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
>>> ---
>>>
>>> v2:
>>> - Updated commit message.
>>>
>>>  lib/librte_ethdev/rte_ethdev_version.map | 62 ++++++++++++++++----------------
>>>  lib/librte_ethdev/rte_tm.h               | 33 ++++++++++++++++-
>>>  2 files changed, 64 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
>>> index 1212a17..3da4512 100644
>>> --- a/lib/librte_ethdev/rte_ethdev_version.map
>>> +++ b/lib/librte_ethdev/rte_ethdev_version.map
>>> @@ -135,36 +135,6 @@ DPDK_20.0 {
>>>  	rte_flow_isolate;
>>>  	rte_flow_query;
>>>  	rte_flow_validate;
>>> -	rte_tm_capabilities_get;
>>> -	rte_tm_get_number_of_leaf_nodes;
>>> -	rte_tm_hierarchy_commit;
>>> -	rte_tm_level_capabilities_get;
>>> -	rte_tm_mark_ip_dscp;
>>> -	rte_tm_mark_ip_ecn;
>>> -	rte_tm_mark_vlan_dei;
>>> -	rte_tm_node_add;
>>> -	rte_tm_node_capabilities_get;
>>> -	rte_tm_node_cman_update;
>>> -	rte_tm_node_delete;
>>> -	rte_tm_node_parent_update;
>>> -	rte_tm_node_resume;
>>> -	rte_tm_node_shaper_update;
>>> -	rte_tm_node_shared_shaper_update;
>>> -	rte_tm_node_shared_wred_context_update;
>>> -	rte_tm_node_stats_read;
>>> -	rte_tm_node_stats_update;
>>> -	rte_tm_node_suspend;
>>> -	rte_tm_node_type_get;
>>> -	rte_tm_node_wfq_weight_mode_update;
>>> -	rte_tm_node_wred_context_update;
>>> -	rte_tm_shaper_profile_add;
>>> -	rte_tm_shaper_profile_delete;
>>> -	rte_tm_shared_shaper_add_update;
>>> -	rte_tm_shared_shaper_delete;
>>> -	rte_tm_shared_wred_context_add_update;
>>> -	rte_tm_shared_wred_context_delete;
>>> -	rte_tm_wred_profile_add;
>>> -	rte_tm_wred_profile_delete;
>>>  
>>>  	local: *;
>>>  };
>>> @@ -241,6 +211,38 @@ EXPERIMENTAL {
>>>  	__rte_ethdev_trace_rx_burst;
>>>  	__rte_ethdev_trace_tx_burst;
>>>  	rte_flow_get_aged_flows;
>>> +
>>> +	# Marked as experimental in 20.11
>>> +	rte_tm_capabilities_get;
>>> +	rte_tm_get_number_of_leaf_nodes;
>>> +	rte_tm_hierarchy_commit;
>>> +	rte_tm_level_capabilities_get;
>>> +	rte_tm_mark_ip_dscp;
>>> +	rte_tm_mark_ip_ecn;
>>> +	rte_tm_mark_vlan_dei;
>>> +	rte_tm_node_add;
>>> +	rte_tm_node_capabilities_get;
>>> +	rte_tm_node_cman_update;
>>> +	rte_tm_node_delete;
>>> +	rte_tm_node_parent_update;
>>> +	rte_tm_node_resume;
>>> +	rte_tm_node_shaper_update;
>>> +	rte_tm_node_shared_shaper_update;
>>> +	rte_tm_node_shared_wred_context_update;
>>> +	rte_tm_node_stats_read;
>>> +	rte_tm_node_stats_update;
>>> +	rte_tm_node_suspend;
>>> +	rte_tm_node_type_get;
>>> +	rte_tm_node_wfq_weight_mode_update;
>>> +	rte_tm_node_wred_context_update;
>>> +	rte_tm_shaper_profile_add;
>>> +	rte_tm_shaper_profile_delete;
>>> +	rte_tm_shared_shaper_add_update;
>>> +	rte_tm_shared_shaper_delete;
>>> +	rte_tm_shared_wred_context_add_update;
>>> +	rte_tm_shared_wred_context_delete;
>>> +	rte_tm_wred_profile_add;
>>> +	rte_tm_wred_profile_delete;
>>>  };
>> [SNIP]
>>
>> So I will re-interate what I said in April, all of these API have existed since DPDK 17.08.
>> I find it hard to believe they are all at imminent risk of changing to degree they _all_ warrant EXPERIMENTAL status.
> 
> As discussed earlier and agreed by TM and Ethdev maintainers, __rte_experimental tag was introduced several
> releases after TM spec was added and TM spec remained untouched ending up making it stable unintentionally. 

We discussed it I agree, but to identify how this happened.
I am not sure we all agreed it was ok to give a blanket EXPERIMENTAL. 

This API has been largely unchanged for 3 years.
How can we justify keeping EXPERIMENTAL without a specific reason?
Can you be more specific about planned changes?

> 
> Bruce and other maintainers pointed out that we cannot fix it as a "Fixes:" patch in order
> to honor ABI commitments, and suggestion was to change the complete spec back to
> experimental in next ABI change window and proceed with improvements that make
> it stable API in subsequent releases.
>>
>> Ray K
Nithin Dabilpuram Sept. 10, 2020, 11:22 a.m. UTC | #8
On Thu, Sep 10, 2020 at 12:01:49PM +0100, Kinsella, Ray wrote:
> 
> 
> On 10/09/2020 11:56, Nithin Dabilpuram wrote:
> > On Thu, Sep 10, 2020 at 09:30:15AM +0100, Kinsella, Ray wrote:
> >> External Email
> >>
> >> ----------------------------------------------------------------------
> >> On 10/09/2020 09:05, Nithin Dabilpuram wrote:
> >>> This patch marks all traffic manager API as experimental as
> >>> per discussion mentioned in below thread.
> >>>
> >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__mails.dpdk.org_archives_dev_2020-2DApril_165364.html&d=DwIDaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=FZ_tPCbgFOh18zwRPO9H0yDx8VW38vuapifdDfc8SFQ&m=NlIUXSsugCPTKBv9qxsaMHOhTuC90TUqLcGg962VVyA&s=DZL_m0a73uef5Qgal1iFqc9dTdAuUd8J6eC0mZOQlFs&e= 
> >>>
> >>> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> >>> ---
> >>>
> >>> v2:
> >>> - Updated commit message.
> >>>
> >>>  lib/librte_ethdev/rte_ethdev_version.map | 62 ++++++++++++++++----------------
> >>>  lib/librte_ethdev/rte_tm.h               | 33 ++++++++++++++++-
> >>>  2 files changed, 64 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
> >>> index 1212a17..3da4512 100644
> >>> --- a/lib/librte_ethdev/rte_ethdev_version.map
> >>> +++ b/lib/librte_ethdev/rte_ethdev_version.map
> >>> @@ -135,36 +135,6 @@ DPDK_20.0 {
> >>>  	rte_flow_isolate;
> >>>  	rte_flow_query;
> >>>  	rte_flow_validate;
> >>> -	rte_tm_capabilities_get;
> >>> -	rte_tm_get_number_of_leaf_nodes;
> >>> -	rte_tm_hierarchy_commit;
> >>> -	rte_tm_level_capabilities_get;
> >>> -	rte_tm_mark_ip_dscp;
> >>> -	rte_tm_mark_ip_ecn;
> >>> -	rte_tm_mark_vlan_dei;
> >>> -	rte_tm_node_add;
> >>> -	rte_tm_node_capabilities_get;
> >>> -	rte_tm_node_cman_update;
> >>> -	rte_tm_node_delete;
> >>> -	rte_tm_node_parent_update;
> >>> -	rte_tm_node_resume;
> >>> -	rte_tm_node_shaper_update;
> >>> -	rte_tm_node_shared_shaper_update;
> >>> -	rte_tm_node_shared_wred_context_update;
> >>> -	rte_tm_node_stats_read;
> >>> -	rte_tm_node_stats_update;
> >>> -	rte_tm_node_suspend;
> >>> -	rte_tm_node_type_get;
> >>> -	rte_tm_node_wfq_weight_mode_update;
> >>> -	rte_tm_node_wred_context_update;
> >>> -	rte_tm_shaper_profile_add;
> >>> -	rte_tm_shaper_profile_delete;
> >>> -	rte_tm_shared_shaper_add_update;
> >>> -	rte_tm_shared_shaper_delete;
> >>> -	rte_tm_shared_wred_context_add_update;
> >>> -	rte_tm_shared_wred_context_delete;
> >>> -	rte_tm_wred_profile_add;
> >>> -	rte_tm_wred_profile_delete;
> >>>  
> >>>  	local: *;
> >>>  };
> >>> @@ -241,6 +211,38 @@ EXPERIMENTAL {
> >>>  	__rte_ethdev_trace_rx_burst;
> >>>  	__rte_ethdev_trace_tx_burst;
> >>>  	rte_flow_get_aged_flows;
> >>> +
> >>> +	# Marked as experimental in 20.11
> >>> +	rte_tm_capabilities_get;
> >>> +	rte_tm_get_number_of_leaf_nodes;
> >>> +	rte_tm_hierarchy_commit;
> >>> +	rte_tm_level_capabilities_get;
> >>> +	rte_tm_mark_ip_dscp;
> >>> +	rte_tm_mark_ip_ecn;
> >>> +	rte_tm_mark_vlan_dei;
> >>> +	rte_tm_node_add;
> >>> +	rte_tm_node_capabilities_get;
> >>> +	rte_tm_node_cman_update;
> >>> +	rte_tm_node_delete;
> >>> +	rte_tm_node_parent_update;
> >>> +	rte_tm_node_resume;
> >>> +	rte_tm_node_shaper_update;
> >>> +	rte_tm_node_shared_shaper_update;
> >>> +	rte_tm_node_shared_wred_context_update;
> >>> +	rte_tm_node_stats_read;
> >>> +	rte_tm_node_stats_update;
> >>> +	rte_tm_node_suspend;
> >>> +	rte_tm_node_type_get;
> >>> +	rte_tm_node_wfq_weight_mode_update;
> >>> +	rte_tm_node_wred_context_update;
> >>> +	rte_tm_shaper_profile_add;
> >>> +	rte_tm_shaper_profile_delete;
> >>> +	rte_tm_shared_shaper_add_update;
> >>> +	rte_tm_shared_shaper_delete;
> >>> +	rte_tm_shared_wred_context_add_update;
> >>> +	rte_tm_shared_wred_context_delete;
> >>> +	rte_tm_wred_profile_add;
> >>> +	rte_tm_wred_profile_delete;
> >>>  };
> >> [SNIP]
> >>
> >> So I will re-interate what I said in April, all of these API have existed since DPDK 17.08.
> >> I find it hard to believe they are all at imminent risk of changing to degree they _all_ warrant EXPERIMENTAL status.
> > 
> > As discussed earlier and agreed by TM and Ethdev maintainers, __rte_experimental tag was introduced several
> > releases after TM spec was added and TM spec remained untouched ending up making it stable unintentionally. 
> 
> We discussed it I agree, but to identify how this happened.
> I am not sure we all agreed it was ok to give a blanket EXPERIMENTAL. 
> 
> This API has been largely unchanged for 3 years.
> How can we justify keeping EXPERIMENTAL without a specific reason?
> Can you be more specific about planned changes?

Basically the changes atleast which initiated this discussion were

#1 Support to configure shaper in pkt mode that helps ratelimiting in
PPS(Packets per Sec) as opposed to BPS(Bits per Sec).
Needs change to a lot of existing tm param and capability structs.

#2 Support dynamic offload flags based Tx offload for tm packet marking.

#3 Support TM node color stats that report per tm color(red, green, yellow) pkt stats.

All the above three changes though add significant features and doesn't break existing
PMD's. 

> 
> > 
> > Bruce and other maintainers pointed out that we cannot fix it as a "Fixes:" patch in order
> > to honor ABI commitments, and suggestion was to change the complete spec back to
> > experimental in next ABI change window and proceed with improvements that make
> > it stable API in subsequent releases.
> >>
> >> Ray K
diff mbox series

Patch

diff --git a/lib/librte_ethdev/rte_ethdev_version.map b/lib/librte_ethdev/rte_ethdev_version.map
index 1212a17..3da4512 100644
--- a/lib/librte_ethdev/rte_ethdev_version.map
+++ b/lib/librte_ethdev/rte_ethdev_version.map
@@ -135,36 +135,6 @@  DPDK_20.0 {
 	rte_flow_isolate;
 	rte_flow_query;
 	rte_flow_validate;
-	rte_tm_capabilities_get;
-	rte_tm_get_number_of_leaf_nodes;
-	rte_tm_hierarchy_commit;
-	rte_tm_level_capabilities_get;
-	rte_tm_mark_ip_dscp;
-	rte_tm_mark_ip_ecn;
-	rte_tm_mark_vlan_dei;
-	rte_tm_node_add;
-	rte_tm_node_capabilities_get;
-	rte_tm_node_cman_update;
-	rte_tm_node_delete;
-	rte_tm_node_parent_update;
-	rte_tm_node_resume;
-	rte_tm_node_shaper_update;
-	rte_tm_node_shared_shaper_update;
-	rte_tm_node_shared_wred_context_update;
-	rte_tm_node_stats_read;
-	rte_tm_node_stats_update;
-	rte_tm_node_suspend;
-	rte_tm_node_type_get;
-	rte_tm_node_wfq_weight_mode_update;
-	rte_tm_node_wred_context_update;
-	rte_tm_shaper_profile_add;
-	rte_tm_shaper_profile_delete;
-	rte_tm_shared_shaper_add_update;
-	rte_tm_shared_shaper_delete;
-	rte_tm_shared_wred_context_add_update;
-	rte_tm_shared_wred_context_delete;
-	rte_tm_wred_profile_add;
-	rte_tm_wred_profile_delete;
 
 	local: *;
 };
@@ -241,6 +211,38 @@  EXPERIMENTAL {
 	__rte_ethdev_trace_rx_burst;
 	__rte_ethdev_trace_tx_burst;
 	rte_flow_get_aged_flows;
+
+	# Marked as experimental in 20.11
+	rte_tm_capabilities_get;
+	rte_tm_get_number_of_leaf_nodes;
+	rte_tm_hierarchy_commit;
+	rte_tm_level_capabilities_get;
+	rte_tm_mark_ip_dscp;
+	rte_tm_mark_ip_ecn;
+	rte_tm_mark_vlan_dei;
+	rte_tm_node_add;
+	rte_tm_node_capabilities_get;
+	rte_tm_node_cman_update;
+	rte_tm_node_delete;
+	rte_tm_node_parent_update;
+	rte_tm_node_resume;
+	rte_tm_node_shaper_update;
+	rte_tm_node_shared_shaper_update;
+	rte_tm_node_shared_wred_context_update;
+	rte_tm_node_stats_read;
+	rte_tm_node_stats_update;
+	rte_tm_node_suspend;
+	rte_tm_node_type_get;
+	rte_tm_node_wfq_weight_mode_update;
+	rte_tm_node_wred_context_update;
+	rte_tm_shaper_profile_add;
+	rte_tm_shaper_profile_delete;
+	rte_tm_shared_shaper_add_update;
+	rte_tm_shared_shaper_delete;
+	rte_tm_shared_wred_context_add_update;
+	rte_tm_shared_wred_context_delete;
+	rte_tm_wred_profile_add;
+	rte_tm_wred_profile_delete;
 };
 
 INTERNAL {
diff --git a/lib/librte_ethdev/rte_tm.h b/lib/librte_ethdev/rte_tm.h
index f9c0cf3..c80c0bc 100644
--- a/lib/librte_ethdev/rte_tm.h
+++ b/lib/librte_ethdev/rte_tm.h
@@ -16,7 +16,8 @@ 
  * traffic shaping, congestion management, packet marking, etc.
  *
  * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
+ * @b EXPERIMENTAL:
+ * All functions in this file may be changed or removed without prior notice.
  */
 
 #include <stdint.h>
@@ -1050,6 +1051,7 @@  struct rte_tm_error {
  * @return
  *   0 on success, non-zero error code otherwise.
  */
+__rte_experimental
 int
 rte_tm_get_number_of_leaf_nodes(uint16_t port_id,
 	uint32_t *n_leaf_nodes,
@@ -1074,6 +1076,7 @@  rte_tm_get_number_of_leaf_nodes(uint16_t port_id,
  * @return
  *   0 on success, non-zero error code otherwise.
  */
+__rte_experimental
 int
 rte_tm_node_type_get(uint16_t port_id,
 	uint32_t node_id,
@@ -1092,6 +1095,7 @@  rte_tm_node_type_get(uint16_t port_id,
  * @return
  *   0 on success, non-zero error code otherwise.
  */
+__rte_experimental
 int
 rte_tm_capabilities_get(uint16_t port_id,
 	struct rte_tm_capabilities *cap,
@@ -1112,6 +1116,7 @@  rte_tm_capabilities_get(uint16_t port_id,
  * @return
  *   0 on success, non-zero error code otherwise.
  */
+__rte_experimental
 int
 rte_tm_level_capabilities_get(uint16_t port_id,
 	uint32_t level_id,
@@ -1132,6 +1137,7 @@  rte_tm_level_capabilities_get(uint16_t port_id,
  * @return
  *   0 on success, non-zero error code otherwise.
  */
+__rte_experimental
 int
 rte_tm_node_capabilities_get(uint16_t port_id,
 	uint32_t node_id,
@@ -1157,6 +1163,7 @@  rte_tm_node_capabilities_get(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::cman_wred_context_n_max
  */
+__rte_experimental
 int
 rte_tm_wred_profile_add(uint16_t port_id,
 	uint32_t wred_profile_id,
@@ -1180,6 +1187,7 @@  rte_tm_wred_profile_add(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::cman_wred_context_n_max
  */
+__rte_experimental
 int
 rte_tm_wred_profile_delete(uint16_t port_id,
 	uint32_t wred_profile_id,
@@ -1211,6 +1219,7 @@  rte_tm_wred_profile_delete(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::cman_wred_context_shared_n_max
  */
+__rte_experimental
 int
 rte_tm_shared_wred_context_add_update(uint16_t port_id,
 	uint32_t shared_wred_context_id,
@@ -1235,6 +1244,7 @@  rte_tm_shared_wred_context_add_update(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::cman_wred_context_shared_n_max
  */
+__rte_experimental
 int
 rte_tm_shared_wred_context_delete(uint16_t port_id,
 	uint32_t shared_wred_context_id,
@@ -1259,6 +1269,7 @@  rte_tm_shared_wred_context_delete(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::shaper_n_max
  */
+__rte_experimental
 int
 rte_tm_shaper_profile_add(uint16_t port_id,
 	uint32_t shaper_profile_id,
@@ -1282,6 +1293,7 @@  rte_tm_shaper_profile_add(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::shaper_n_max
  */
+__rte_experimental
 int
 rte_tm_shaper_profile_delete(uint16_t port_id,
 	uint32_t shaper_profile_id,
@@ -1311,6 +1323,7 @@  rte_tm_shaper_profile_delete(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::shaper_shared_n_max
  */
+__rte_experimental
 int
 rte_tm_shared_shaper_add_update(uint16_t port_id,
 	uint32_t shared_shaper_id,
@@ -1334,6 +1347,7 @@  rte_tm_shared_shaper_add_update(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::shaper_shared_n_max
  */
+__rte_experimental
 int
 rte_tm_shared_shaper_delete(uint16_t port_id,
 	uint32_t shared_shaper_id,
@@ -1402,6 +1416,7 @@  rte_tm_shared_shaper_delete(uint16_t port_id,
  * @see RTE_TM_NODE_LEVEL_ID_ANY
  * @see struct rte_tm_capabilities
  */
+__rte_experimental
 int
 rte_tm_node_add(uint16_t port_id,
 	uint32_t node_id,
@@ -1435,6 +1450,7 @@  rte_tm_node_add(uint16_t port_id,
  *
  * @see RTE_TM_UPDATE_NODE_ADD_DELETE
  */
+__rte_experimental
 int
 rte_tm_node_delete(uint16_t port_id,
 	uint32_t node_id,
@@ -1459,6 +1475,7 @@  rte_tm_node_delete(uint16_t port_id,
  * @see rte_tm_node_resume()
  * @see RTE_TM_UPDATE_NODE_SUSPEND_RESUME
  */
+__rte_experimental
 int
 rte_tm_node_suspend(uint16_t port_id,
 	uint32_t node_id,
@@ -1482,6 +1499,7 @@  rte_tm_node_suspend(uint16_t port_id,
  * @see rte_tm_node_suspend()
  * @see RTE_TM_UPDATE_NODE_SUSPEND_RESUME
  */
+__rte_experimental
 int
 rte_tm_node_resume(uint16_t port_id,
 	uint32_t node_id,
@@ -1523,6 +1541,7 @@  rte_tm_node_resume(uint16_t port_id,
  * @see rte_tm_node_add()
  * @see rte_tm_node_delete()
  */
+__rte_experimental
 int
 rte_tm_hierarchy_commit(uint16_t port_id,
 	int clear_on_fail,
@@ -1563,6 +1582,7 @@  rte_tm_hierarchy_commit(uint16_t port_id,
  * @see RTE_TM_UPDATE_NODE_PARENT_KEEP_LEVEL
  * @see RTE_TM_UPDATE_NODE_PARENT_CHANGE_LEVEL
  */
+__rte_experimental
 int
 rte_tm_node_parent_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1592,6 +1612,7 @@  rte_tm_node_parent_update(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::shaper_private_n_max
  */
+__rte_experimental
 int
 rte_tm_node_shaper_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1619,6 +1640,7 @@  rte_tm_node_shaper_update(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::shaper_shared_n_max
  */
+__rte_experimental
 int
 rte_tm_node_shared_shaper_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1646,6 +1668,7 @@  rte_tm_node_shared_shaper_update(uint16_t port_id,
  * @see enum rte_tm_stats_type
  * @see RTE_TM_UPDATE_NODE_STATS
  */
+__rte_experimental
 int
 rte_tm_node_stats_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1674,6 +1697,7 @@  rte_tm_node_stats_update(uint16_t port_id,
  * @see RTE_TM_UPDATE_NODE_WFQ_WEIGHT_MODE
  * @see RTE_TM_UPDATE_NODE_N_SP_PRIORITIES
  */
+__rte_experimental
 int
 rte_tm_node_wfq_weight_mode_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1697,6 +1721,7 @@  rte_tm_node_wfq_weight_mode_update(uint16_t port_id,
  *
  * @see RTE_TM_UPDATE_NODE_CMAN
  */
+__rte_experimental
 int
 rte_tm_node_cman_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1721,6 +1746,7 @@  rte_tm_node_cman_update(uint16_t port_id,
   *
  * @see struct rte_tm_capabilities::cman_wred_context_private_n_max
 */
+__rte_experimental
 int
 rte_tm_node_wred_context_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1746,6 +1772,7 @@  rte_tm_node_wred_context_update(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::cman_wred_context_shared_n_max
  */
+__rte_experimental
 int
 rte_tm_node_shared_wred_context_update(uint16_t port_id,
 	uint32_t node_id,
@@ -1778,6 +1805,7 @@  rte_tm_node_shared_wred_context_update(uint16_t port_id,
  *
  * @see enum rte_tm_stats_type
  */
+__rte_experimental
 int
 rte_tm_node_stats_read(uint16_t port_id,
 	uint32_t node_id,
@@ -1815,6 +1843,7 @@  rte_tm_node_stats_read(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::mark_vlan_dei_supported
  */
+__rte_experimental
 int
 rte_tm_mark_vlan_dei(uint16_t port_id,
 	int mark_green,
@@ -1865,6 +1894,7 @@  rte_tm_mark_vlan_dei(uint16_t port_id,
  * @see struct rte_tm_capabilities::mark_ip_ecn_tcp_supported
  * @see struct rte_tm_capabilities::mark_ip_ecn_sctp_supported
  */
+__rte_experimental
 int
 rte_tm_mark_ip_ecn(uint16_t port_id,
 	int mark_green,
@@ -1913,6 +1943,7 @@  rte_tm_mark_ip_ecn(uint16_t port_id,
  *
  * @see struct rte_tm_capabilities::mark_ip_dscp_supported
  */
+__rte_experimental
 int
 rte_tm_mark_ip_dscp(uint16_t port_id,
 	int mark_green,