Re: [dpdk-dev] Stable ABI status of rte_meter_[t|s]rtcm_profile_config

[Andy Green <andy@warmcat.com>]

On 05/30/2018 05:55 PM, Singh, Jasvinder wrote:
> <snip>
> 
>> On 05/28/2018 11:31 AM, Andy Green wrote:
>>> Hi -
>>>
>>> Between 18.02 and the putative 18.05 there were changes in the way the
>>> meter stuff deals with its config.
>>>
>>> I updated the related code in lagopus, but I get warnings about using
>>> the new APIs (it's the same for rte_meter_trtcm_profile_config())
>>>
>>> ./dpdk/meter.c: In function 'dpdk_register_meter':
>>> ./dpdk/meter.c:119:7: warning: 'rte_meter_srtcm_profile_config' is
>>> deprecated: Symbol is not yet part of stable ABI
>>> [-Wdeprecated-declarations]
>>>          rte_meter_srtcm_profile_config(&lband->sp, &param);
>>>          ^
>>> In file included from ./dpdk/meter.c:27:0:
>>> /home/agreen/lagopus/src/dpdk/build/include/rte_meter.h:86:1: note:
>>> declared here
>>>    rte_meter_srtcm_profile_config(struct rte_meter_srtcm_profile *p,
>>>    ^
>>> ./dpdk/meter.c:132:7: warning: 'rte_meter_srtcm_profile_config' is
>>> deprecated: Symbol is not yet part of stable ABI
>>> [-Wdeprecated-declarations]
>>>          rte_meter_srtcm_profile_config(&lband->sp, &param);
>>>          ^
>>> In file included from ./dpdk/meter.c:27:0:
>>> /home/agreen/lagopus/src/dpdk/build/include/rte_meter.h:86:1: note:
>>> declared here
>>>    rte_meter_srtcm_profile_config(struct rte_meter_srtcm_profile *p,
>>>
>>>
>>> As far as I can see this api change is not optional, it changes the
>>> parameters for related apis to require a struct prepared with these
>>> new apis.
>>
>> IOW should these exports still be "experimental"
>>
>> EXPERIMENTAL {
>>           global:
>>
>>           rte_meter_srtcm_profile_config;
>>           rte_meter_trtcm_profile_config; };
>>
>> ...when the changes also introduced in
>> c06ddf9698e0c2a9653cfa971f9ddc205065662c unconditionally modify the
>> existing APIs to require the new stuff?
>>
>> @@ -138,6 +187,7 @@ rte_meter_srtcm_color_aware_check(struct
>> rte_meter_srtcm *m,
>>     */
>>    static inline enum rte_meter_color
>>    rte_meter_trtcm_color_blind_check(struct rte_meter_trtcm *m,
>> +       struct rte_meter_trtcm_profile *p,
>>           uint64_t time,
>>           uint32_t pkt_len);
>>
>> etc
> 
> The above meter APIs change has followed the deprecation procedure, therefore, IMO, should not be treated as experimental. In general, this is open question as nothing is specified in the docs.

Thanks.

Another way to ask the same question is: "after we changed the related, 
pre-existing api to require this, why is it useful to want it to blow 
warnings unless we enable EXPERIMENTAL"?

Before the patch, meter api itself wasn't experimental evidently. 
Lagopus uses the old api without EXPERIMENTAL enabled for 18.02; then on 
18.05, after the api it was previously using without EXPERIMENTAL 
changed, it blows warnings after it was actually unconditionally forced 
to adapt to use the new stuff.

Unless I misunderstood it, you can't meaningfully, retrospectively, mark 
a public API EXPERIMENTAL after you changed it, and this is marking 
EXPERIMENTAL a new unconditional dependency on a new type for an API 
that was a previously available via a non-EXPERIMENTAL public api.

Effectively I think you just decided to change the public api (which is 
normal enough for living projects...) and that's the end of it. 
EXPERIMENTAL is only meaningful for stuff you can cleanly enable or 
disable at build-time, this is not such a situation.

If so it should get un-experimentalized for export...

-Andy