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

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

[Andy Green]

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.

-Andy

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

[Andy Green]

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

-Andy

> -Andy

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

[Singh, Jasvinder]

<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.

   

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

[Andy Green]

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

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

[Kevin Traynor]

On 05/28/2018 04: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,
> 

Hi Cristian,

Are these API still to be considered experimental in 18.08, or the tags
can be removed?

Kevin.

> 
> 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.
> 
> -Andy

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

[Andy Green]

On 08/01/2018 06:47 PM, Kevin Traynor wrote:
> On 05/28/2018 04: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,
>>
> 
> Hi Cristian,
> 
> Are these API still to be considered experimental in 18.08, or the tags
> can be removed?

... to be clear that these apis claimed to be 'experimental' in 18.05 at 
all, when they aren't, is already broken in 18.05.

The only question is whether they want to continue ignoring the breakage 
into 18.08+ so future generations can enjoy it.

-Andy

> Kevin.
> 
>>
>> 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.
>>
>> -Andy
> 

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

[Dumitrescu, Cristian]

> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Wednesday, August 1, 2018 11:48 AM
> To: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
> Cc: Andy Green <andy@warmcat.com>; Singh, Jasvinder
> <jasvinder.singh@intel.com>
> Subject: Re: [dpdk-dev] Stable ABI status of
> rte_meter_[t|s]rtcm_profile_config
> 
> On 05/28/2018 04: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,
> >
> 
> Hi Cristian,
> 
> Are these API still to be considered experimental in 18.08, or the tags
> can be removed?
> 
> Kevin.

No, we should remove the experimental tag on these functions.

> 
> >
> > 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.
> >
> > -Andy

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

[Kevin Traynor]

On 08/01/2018 03:30 PM, Dumitrescu, Cristian wrote:
> 
> 
>> -----Original Message-----
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Wednesday, August 1, 2018 11:48 AM
>> To: dev@dpdk.org; Dumitrescu, Cristian <cristian.dumitrescu@intel.com>
>> Cc: Andy Green <andy@warmcat.com>; Singh, Jasvinder
>> <jasvinder.singh@intel.com>
>> Subject: Re: [dpdk-dev] Stable ABI status of
>> rte_meter_[t|s]rtcm_profile_config
>>
>> On 05/28/2018 04: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,
>>>
>>
>> Hi Cristian,
>>
>> Are these API still to be considered experimental in 18.08, or the tags
>> can be removed?
>>
>> Kevin.
> 
> No, we should remove the experimental tag on these functions.
> 

ok, I just did a quick compile tested patch. Will send.

>>
>>>
>>> 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.
>>>
>>> -Andy
>