[3/3] lib/librte_meter: update abi to include new rfc4115 function
Checks
Commit Message
Update the ABI to include the new RFC4115 meter functions
---
lib/librte_meter/Makefile | 2 +-
lib/librte_meter/meson.build | 2 +-
lib/librte_meter/rte_meter_version.map | 9 +++++++++
3 files changed, 11 insertions(+), 2 deletions(-)
Comments
On 28 Nov 2018, at 9:38, David Marchand wrote:
> Hello Eelco,
>
> On Tue, Nov 27, 2018 at 4:22 PM Eelco Chaudron <echaudro@redhat.com>
> wrote:
>
>> Update the ABI to include the new RFC4115 meter functions
>> ---
>> lib/librte_meter/Makefile | 2 +-
>> lib/librte_meter/meson.build | 2 +-
>> lib/librte_meter/rte_meter_version.map | 9 +++++++++
>> 3 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/librte_meter/Makefile b/lib/librte_meter/Makefile
>> index 2dc071e8e..79ad79744 100644
>> --- a/lib/librte_meter/Makefile
>> +++ b/lib/librte_meter/Makefile
>> @@ -16,7 +16,7 @@ LDLIBS += -lrte_eal
>>
>> EXPORT_MAP := rte_meter_version.map
>>
>> -LIBABIVER := 2
>> +LIBABIVER := 3
>>
>> #
>> # all source are stored in SRCS-y
>>
>
> As far as I understood the policy around the EXPERIMENTAL section, you
> don't need to bump the library version.
Thought I would add the new function as none experimental, i.e. next
version, but checkpatch did not allow me to do this.
Tried to find info on what the right process was, as these functions are
just another meter implementation using similar existing APIs. If anyone
has any background on this please point me to it.
I changed the library version as an existing data structure changed
(which in theory should not change the location of the data), but the
ABI check popped warnings so I decided to increase the version.
>
> + you should squash this into the first patch.
I’ll do this on the next version.
Thanks,
Eelco
28/11/2018 10:27, Eelco Chaudron:
> On 28 Nov 2018, at 9:38, David Marchand wrote:
> > On Tue, Nov 27, 2018 at 4:22 PM Eelco Chaudron <echaudro@redhat.com>
> > wrote:
> >> --- a/lib/librte_meter/Makefile
> >> +++ b/lib/librte_meter/Makefile
> >> -LIBABIVER := 2
> >> +LIBABIVER := 3
> >
> > As far as I understood the policy around the EXPERIMENTAL section, you
> > don't need to bump the library version.
>
> Thought I would add the new function as none experimental, i.e. next
> version, but checkpatch did not allow me to do this.
>
> Tried to find info on what the right process was, as these functions are
> just another meter implementation using similar existing APIs. If anyone
> has any background on this please point me to it.
It is documented here:
http://doc.dpdk.org/guides/contributing/versioning.html
The case for "similar API" is not handled specifically so far.
So you need to introduce it as experimental.
> I changed the library version as an existing data structure changed
> (which in theory should not change the location of the data), but the
> ABI check popped warnings so I decided to increase the version.
It deserves to analyze why the ABI check raises a warning.
If it really needs to bump the ABI version, you should justify it
in the commit message, and explain what changed in the ABI section
of the release notes, plus update the version in the release notes.
On 28 Nov 2018, at 11:09, Thomas Monjalon wrote:
> 28/11/2018 10:27, Eelco Chaudron:
>> On 28 Nov 2018, at 9:38, David Marchand wrote:
>>> On Tue, Nov 27, 2018 at 4:22 PM Eelco Chaudron <echaudro@redhat.com>
>>> wrote:
>>>> --- a/lib/librte_meter/Makefile
>>>> +++ b/lib/librte_meter/Makefile
>>>> -LIBABIVER := 2
>>>> +LIBABIVER := 3
>>>
>>> As far as I understood the policy around the EXPERIMENTAL section,
>>> you
>>> don't need to bump the library version.
>>
>> Thought I would add the new function as none experimental, i.e. next
>> version, but checkpatch did not allow me to do this.
>>
>> Tried to find info on what the right process was, as these functions
>> are
>> just another meter implementation using similar existing APIs. If
>> anyone
>> has any background on this please point me to it.
>
> It is documented here:
> http://doc.dpdk.org/guides/contributing/versioning.html
>
> The case for "similar API" is not handled specifically so far.
> So you need to introduce it as experimental.
Thanks for the clarification, will update the APIs with
__rte_experimental in the next iteration.
>> I changed the library version as an existing data structure changed
>> (which in theory should not change the location of the data), but the
>> ABI check popped warnings so I decided to increase the version.
>
> It deserves to analyze why the ABI check raises a warning.
> If it really needs to bump the ABI version, you should justify it
> in the commit message, and explain what changed in the ABI section
> of the release notes, plus update the version in the release notes.
Will look at it more closely and update it for the next iteration.
28/11/2018 13:40, Eelco Chaudron:
>
> On 28 Nov 2018, at 11:09, Thomas Monjalon wrote:
>
> > 28/11/2018 10:27, Eelco Chaudron:
> >> On 28 Nov 2018, at 9:38, David Marchand wrote:
> >>> On Tue, Nov 27, 2018 at 4:22 PM Eelco Chaudron <echaudro@redhat.com>
> >>> wrote:
> >>>> --- a/lib/librte_meter/Makefile
> >>>> +++ b/lib/librte_meter/Makefile
> >>>> -LIBABIVER := 2
> >>>> +LIBABIVER := 3
> >>>
> >>> As far as I understood the policy around the EXPERIMENTAL section,
> >>> you
> >>> don't need to bump the library version.
> >>
> >> Thought I would add the new function as none experimental, i.e. next
> >> version, but checkpatch did not allow me to do this.
> >>
> >> Tried to find info on what the right process was, as these functions
> >> are
> >> just another meter implementation using similar existing APIs. If
> >> anyone
> >> has any background on this please point me to it.
> >
> > It is documented here:
> > http://doc.dpdk.org/guides/contributing/versioning.html
> >
> > The case for "similar API" is not handled specifically so far.
> > So you need to introduce it as experimental.
>
> Thanks for the clarification, will update the APIs with
> __rte_experimental in the next iteration.
You should also add this on top of the doxygen comment:
* @warning
* @b EXPERIMENTAL: this API may change without prior notice
On 28 Nov 2018, at 13:51, Thomas Monjalon wrote:
> 28/11/2018 13:40, Eelco Chaudron:
>>
>> On 28 Nov 2018, at 11:09, Thomas Monjalon wrote:
>>
>>> 28/11/2018 10:27, Eelco Chaudron:
>>>> On 28 Nov 2018, at 9:38, David Marchand wrote:
>>>>> On Tue, Nov 27, 2018 at 4:22 PM Eelco Chaudron <echaudro@redhat.com>
>>>>> wrote:
>>>>>> --- a/lib/librte_meter/Makefile
>>>>>> +++ b/lib/librte_meter/Makefile
>>>>>> -LIBABIVER := 2
>>>>>> +LIBABIVER := 3
>>>>>
>>>>> As far as I understood the policy around the EXPERIMENTAL section,
>>>>> you
>>>>> don't need to bump the library version.
>>>>
>>>> Thought I would add the new function as none experimental, i.e. next
>>>> version, but checkpatch did not allow me to do this.
>>>>
>>>> Tried to find info on what the right process was, as these functions
>>>> are
>>>> just another meter implementation using similar existing APIs. If
>>>> anyone
>>>> has any background on this please point me to it.
>>>
>>> It is documented here:
>>> http://doc.dpdk.org/guides/contributing/versioning.html
>>>
>>> The case for "similar API" is not handled specifically so far.
>>> So you need to introduce it as experimental.
>>
>> Thanks for the clarification, will update the APIs with
>> __rte_experimental in the next iteration.
>
> You should also add this on top of the doxygen comment:
>
> * @warning
> * @b EXPERIMENTAL: this API may change without prior notice
Thanks, done!
@@ -16,7 +16,7 @@ LDLIBS += -lrte_eal
EXPORT_MAP := rte_meter_version.map
-LIBABIVER := 2
+LIBABIVER := 3
#
# all source are stored in SRCS-y
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: BSD-3-Clause
# Copyright(c) 2017 Intel Corporation
-version = 2
+version = 3
sources = files('rte_meter.c')
headers = files('rte_meter.h')
@@ -17,3 +17,12 @@ DPDK_18.08 {
rte_meter_srtcm_profile_config;
rte_meter_trtcm_profile_config;
};
+
+EXPERIMENTAL {
+ global:
+
+ rte_meter_trtcm_rfc4115_color_aware_check;
+ rte_meter_trtcm_rfc4115_color_blind_check;
+ rte_meter_trtcm_rfc4115_config;
+ rte_meter_trtcm_rfc4115_profile_config;
+};