mbox series

[v2,0/2] add abi version testing to app/test

Message ID 20190822160717.13584-1-mdr@ashroe.eu (mailing list archive)
Headers
Series add abi version testing to app/test |

Message

Ray Kinsella Aug. 22, 2019, 4:07 p.m. UTC
  This patchset adds ABI version testing to the app/test unit test framework,
addressing two issues previously raised during ML conversations on ABI
stability;

1. How do we unit test still supported previous ABI versions?
2. How to we unit test inline functions from still supported previous ABI
versions?

Starting with rte_lpm, I did the following:-

* I reproduced mostly unmodified unit tests for the v2.0 ABI, taken from DPDK
  2.2 and 17.02.
* I reproduced the rte_lpm interface header from v2.0, including the inline
  functions and remapping symbols to their appropriate versions.
* I added support for multiple abi versions to the app/test unit test framework
  to allow users to switch between abi versions (set_abi_version), without
  further polluting the already long list of unit tests available in app/test.

The intention here is that in future as developers need to deprecate APIs, the
associated unit tests may move into the ABI version testing mechanism of the
app/test instead of being replaced by the latest set of unit tests as would be
the case today.

v2:

* Added LPM IPv6 test cases for the v2.0 ABI.
* Fixed a number of checkpatch errors, stop short of substantially reworking
  the test code from the v2.0 ABI. 
* Removed duplicating test cases published in the original v1 patch.

Ray Kinsella (2):
  app/test: add abi version testing functionality
  app/test: lpm abi version testing

 app/test/Makefile                          |   12 +-
 app/test/commands.c                        |  131 +-
 app/test/meson.build                       |    6 +
 app/test/test.c                            |    2 +
 app/test/test.h                            |   48 +-
 app/test/test_lpm.c                        |    3 +-
 app/test/test_lpm6.c                       |    2 +-
 app/test/test_lpm_perf.c                   |  293 +---
 app/test/test_lpm_routes.c                 |  287 ++++
 app/test/test_lpm_routes.h                 |   25 +
 app/test/v2.0/dcompat.h                    |   30 +
 app/test/v2.0/rte_lpm.h                    |  451 +++++
 app/test/v2.0/rte_lpm6.h                   |  198 +++
 app/test/v2.0/test_lpm.c                   | 1139 +++++++++++++
 app/test/v2.0/test_lpm6.c                  | 1748 ++++++++++++++++++++
 app/test/v2.0/test_lpm6_perf.c             |  179 ++
 app/test/v2.0/test_lpm_perf.c              |  212 +++
 app/test/v2.0/test_v20.c                   |   14 +
 doc/guides/contributing/versioning.rst     |    4 +
 lib/librte_eal/common/include/rte_compat.h |    7 +
 20 files changed, 4471 insertions(+), 320 deletions(-)
 create mode 100644 app/test/test_lpm_routes.c
 create mode 100644 app/test/test_lpm_routes.h
 create mode 100644 app/test/v2.0/dcompat.h
 create mode 100644 app/test/v2.0/rte_lpm.h
 create mode 100644 app/test/v2.0/rte_lpm6.h
 create mode 100644 app/test/v2.0/test_lpm.c
 create mode 100644 app/test/v2.0/test_lpm6.c
 create mode 100644 app/test/v2.0/test_lpm6_perf.c
 create mode 100644 app/test/v2.0/test_lpm_perf.c
 create mode 100644 app/test/v2.0/test_v20.c
  

Comments

Aaron Conole Aug. 23, 2019, 3:49 p.m. UTC | #1
Ray Kinsella <mdr@ashroe.eu> writes:

> This patchset adds ABI version testing to the app/test unit test framework,
> addressing two issues previously raised during ML conversations on ABI
> stability;
>
> 1. How do we unit test still supported previous ABI versions?
> 2. How to we unit test inline functions from still supported previous ABI
> versions?
>
> Starting with rte_lpm, I did the following:-
>
> * I reproduced mostly unmodified unit tests for the v2.0 ABI, taken from DPDK
>   2.2 and 17.02.
> * I reproduced the rte_lpm interface header from v2.0, including the inline
>   functions and remapping symbols to their appropriate versions.
> * I added support for multiple abi versions to the app/test unit test framework
>   to allow users to switch between abi versions (set_abi_version), without
>   further polluting the already long list of unit tests available in app/test.
>
> The intention here is that in future as developers need to deprecate APIs, the
> associated unit tests may move into the ABI version testing mechanism of the
> app/test instead of being replaced by the latest set of unit tests as would be
> the case today.
>
> v2:
>
> * Added LPM IPv6 test cases for the v2.0 ABI.
> * Fixed a number of checkpatch errors, stop short of substantially reworking
>   the test code from the v2.0 ABI. 
> * Removed duplicating test cases published in the original v1 patch.

Thanks for this work.  I think it's useful.

I see an error under aarch64 builds because there are some x86_64
specific types being used in the testing.

See:

  https://travis-ci.com/ovsrobot/dpdk/builds/124254699

> Ray Kinsella (2):
>   app/test: add abi version testing functionality
>   app/test: lpm abi version testing
>
>  app/test/Makefile                          |   12 +-
>  app/test/commands.c                        |  131 +-
>  app/test/meson.build                       |    6 +
>  app/test/test.c                            |    2 +
>  app/test/test.h                            |   48 +-
>  app/test/test_lpm.c                        |    3 +-
>  app/test/test_lpm6.c                       |    2 +-
>  app/test/test_lpm_perf.c                   |  293 +---
>  app/test/test_lpm_routes.c                 |  287 ++++
>  app/test/test_lpm_routes.h                 |   25 +
>  app/test/v2.0/dcompat.h                    |   30 +
>  app/test/v2.0/rte_lpm.h                    |  451 +++++
>  app/test/v2.0/rte_lpm6.h                   |  198 +++
>  app/test/v2.0/test_lpm.c                   | 1139 +++++++++++++
>  app/test/v2.0/test_lpm6.c                  | 1748 ++++++++++++++++++++
>  app/test/v2.0/test_lpm6_perf.c             |  179 ++
>  app/test/v2.0/test_lpm_perf.c              |  212 +++
>  app/test/v2.0/test_v20.c                   |   14 +
>  doc/guides/contributing/versioning.rst     |    4 +
>  lib/librte_eal/common/include/rte_compat.h |    7 +
>  20 files changed, 4471 insertions(+), 320 deletions(-)
>  create mode 100644 app/test/test_lpm_routes.c
>  create mode 100644 app/test/test_lpm_routes.h
>  create mode 100644 app/test/v2.0/dcompat.h
>  create mode 100644 app/test/v2.0/rte_lpm.h
>  create mode 100644 app/test/v2.0/rte_lpm6.h
>  create mode 100644 app/test/v2.0/test_lpm.c
>  create mode 100644 app/test/v2.0/test_lpm6.c
>  create mode 100644 app/test/v2.0/test_lpm6_perf.c
>  create mode 100644 app/test/v2.0/test_lpm_perf.c
>  create mode 100644 app/test/v2.0/test_v20.c
  
Ray Kinsella Aug. 26, 2019, 4:45 p.m. UTC | #2
On 23/08/2019 16:49, Aaron Conole wrote:
> Ray Kinsella <mdr@ashroe.eu> writes:
> 
>> This patchset adds ABI version testing to the app/test unit test framework,
>> addressing two issues previously raised during ML conversations on ABI
>> stability;
>>
>> 1. How do we unit test still supported previous ABI versions?
>> 2. How to we unit test inline functions from still supported previous ABI
>> versions?
>>
>> Starting with rte_lpm, I did the following:-
>>
>> * I reproduced mostly unmodified unit tests for the v2.0 ABI, taken from DPDK
>>   2.2 and 17.02.
>> * I reproduced the rte_lpm interface header from v2.0, including the inline
>>   functions and remapping symbols to their appropriate versions.
>> * I added support for multiple abi versions to the app/test unit test framework
>>   to allow users to switch between abi versions (set_abi_version), without
>>   further polluting the already long list of unit tests available in app/test.
>>
>> The intention here is that in future as developers need to deprecate APIs, the
>> associated unit tests may move into the ABI version testing mechanism of the
>> app/test instead of being replaced by the latest set of unit tests as would be
>> the case today.
>>
>> v2:
>>
>> * Added LPM IPv6 test cases for the v2.0 ABI.
>> * Fixed a number of checkpatch errors, stop short of substantially reworking
>>   the test code from the v2.0 ABI. 
>> * Removed duplicating test cases published in the original v1 patch.
> 
> Thanks for this work.  I think it's useful.
> 
> I see an error under aarch64 builds because there are some x86_64
> specific types being used in the testing.

So the problem is that LPM didn't fully support ARM until DPDK v16.04.
The ABI versioning code in the LPM library is there to support the 2.0 ABI.

The intention of this unit test is to test backward's compatibility with
an inline LPM function from DPDK v2.2.0, which was essentially x86 only
at that time.

Unless we want to get into the business of backporting ARM support to
DPDK 2.2.0 (from where this test cases came from) - we should probably
restrict these ABI versioning test cases to CONFIG_RTE_ARCH_X86_64 only.

The other option is forget about testing this the LPM ABI versioning
support, which then asks the question should be perhaps excise that code
altogether.

Make sense?

Ray K
  
Bruce Richardson Aug. 27, 2019, 8:17 a.m. UTC | #3
On Mon, Aug 26, 2019 at 05:45:55PM +0100, Ray Kinsella wrote:
> 
> 
> On 23/08/2019 16:49, Aaron Conole wrote:
> > Ray Kinsella <mdr@ashroe.eu> writes:
> > 
> >> This patchset adds ABI version testing to the app/test unit test framework,
> >> addressing two issues previously raised during ML conversations on ABI
> >> stability;
> >>
> >> 1. How do we unit test still supported previous ABI versions?
> >> 2. How to we unit test inline functions from still supported previous ABI
> >> versions?
> >>
> >> Starting with rte_lpm, I did the following:-
> >>
> >> * I reproduced mostly unmodified unit tests for the v2.0 ABI, taken from DPDK
> >>   2.2 and 17.02.
> >> * I reproduced the rte_lpm interface header from v2.0, including the inline
> >>   functions and remapping symbols to their appropriate versions.
> >> * I added support for multiple abi versions to the app/test unit test framework
> >>   to allow users to switch between abi versions (set_abi_version), without
> >>   further polluting the already long list of unit tests available in app/test.
> >>
> >> The intention here is that in future as developers need to deprecate APIs, the
> >> associated unit tests may move into the ABI version testing mechanism of the
> >> app/test instead of being replaced by the latest set of unit tests as would be
> >> the case today.
> >>
> >> v2:
> >>
> >> * Added LPM IPv6 test cases for the v2.0 ABI.
> >> * Fixed a number of checkpatch errors, stop short of substantially reworking
> >>   the test code from the v2.0 ABI. 
> >> * Removed duplicating test cases published in the original v1 patch.
> > 
> > Thanks for this work.  I think it's useful.
> > 
> > I see an error under aarch64 builds because there are some x86_64
> > specific types being used in the testing.
> 
> So the problem is that LPM didn't fully support ARM until DPDK v16.04.
> The ABI versioning code in the LPM library is there to support the 2.0 ABI.
> 
> The intention of this unit test is to test backward's compatibility with
> an inline LPM function from DPDK v2.2.0, which was essentially x86 only
> at that time.
> 
> Unless we want to get into the business of backporting ARM support to
> DPDK 2.2.0 (from where this test cases came from) - we should probably
> restrict these ABI versioning test cases to CONFIG_RTE_ARCH_X86_64 only.
> 
> The other option is forget about testing this the LPM ABI versioning
> support, which then asks the question should be perhaps excise that code
> altogether.
>

I think function versioning is great and should be widely used.
Unfortunately, though, in our case since we break the ABI so consistently,
this old code is pretty useless. Therefore, I think we should remove all
old versionned code from e.g. pre-18.11, since no app is realistically
going to work from that far back anyway.

/Bruce
  
Ray Kinsella Aug. 27, 2019, 8:28 a.m. UTC | #4
On 27/08/2019 09:17, Bruce Richardson wrote:
> On Mon, Aug 26, 2019 at 05:45:55PM +0100, Ray Kinsella wrote:
>>
>>
>> On 23/08/2019 16:49, Aaron Conole wrote:
>>> Ray Kinsella <mdr@ashroe.eu> writes:
>>>
>>>> This patchset adds ABI version testing to the app/test unit test framework,
>>>> addressing two issues previously raised during ML conversations on ABI
>>>> stability;
>>>>
>>>> 1. How do we unit test still supported previous ABI versions?
>>>> 2. How to we unit test inline functions from still supported previous ABI
>>>> versions?
>>>>
>>>> Starting with rte_lpm, I did the following:-
>>>>
>>>> * I reproduced mostly unmodified unit tests for the v2.0 ABI, taken from DPDK
>>>>   2.2 and 17.02.
>>>> * I reproduced the rte_lpm interface header from v2.0, including the inline
>>>>   functions and remapping symbols to their appropriate versions.
>>>> * I added support for multiple abi versions to the app/test unit test framework
>>>>   to allow users to switch between abi versions (set_abi_version), without
>>>>   further polluting the already long list of unit tests available in app/test.
>>>>
>>>> The intention here is that in future as developers need to deprecate APIs, the
>>>> associated unit tests may move into the ABI version testing mechanism of the
>>>> app/test instead of being replaced by the latest set of unit tests as would be
>>>> the case today.
>>>>
>>>> v2:
>>>>
>>>> * Added LPM IPv6 test cases for the v2.0 ABI.
>>>> * Fixed a number of checkpatch errors, stop short of substantially reworking
>>>>   the test code from the v2.0 ABI. 
>>>> * Removed duplicating test cases published in the original v1 patch.
>>>
>>> Thanks for this work.  I think it's useful.
>>>
>>> I see an error under aarch64 builds because there are some x86_64
>>> specific types being used in the testing.
>>
>> So the problem is that LPM didn't fully support ARM until DPDK v16.04.
>> The ABI versioning code in the LPM library is there to support the 2.0 ABI.
>>
>> The intention of this unit test is to test backward's compatibility with
>> an inline LPM function from DPDK v2.2.0, which was essentially x86 only
>> at that time.
>>
>> Unless we want to get into the business of backporting ARM support to
>> DPDK 2.2.0 (from where this test cases came from) - we should probably
>> restrict these ABI versioning test cases to CONFIG_RTE_ARCH_X86_64 only.
>>
>> The other option is forget about testing this the LPM ABI versioning
>> support, which then asks the question should be perhaps excise that code
>> altogether.
>>
> 
> I think function versioning is great and should be widely used.
> Unfortunately, though, in our case since we break the ABI so consistently,
> this old code is pretty useless. Therefore, I think we should remove all
> old versionned code from e.g. pre-18.11, since no app is realistically
> going to work from that far back anyway.
> 
> /Bruce 
> 

I had come to a similar conclusion, that we likely need to deprecate
much or all of the existing ABI Compatibility code, it needs a wider
review.

BIND_VERSION_SYMBOL and friends, are still needed to unit test ABI
Versioning, the general idea is sound. And I liked LPM as an example,
because it is well understood and contained, but I will look for
something more recent we could use instead.
  
Ray Kinsella Aug. 27, 2019, 2:19 p.m. UTC | #5
On 27/08/2019 09:28, Ray Kinsella wrote:
> 
> 
> On 27/08/2019 09:17, Bruce Richardson wrote:
>> On Mon, Aug 26, 2019 at 05:45:55PM +0100, Ray Kinsella wrote:
>>>
>>>
>>> On 23/08/2019 16:49, Aaron Conole wrote:
>>>> Ray Kinsella <mdr@ashroe.eu> writes:
>>>>
>>>>> This patchset adds ABI version testing to the app/test unit test framework,
>>>>> addressing two issues previously raised during ML conversations on ABI
>>>>> stability;
>>>>>
>>>>> 1. How do we unit test still supported previous ABI versions?
>>>>> 2. How to we unit test inline functions from still supported previous ABI
>>>>> versions?
>>>>>
>>>>> Starting with rte_lpm, I did the following:-
>>>>>
>>>>> * I reproduced mostly unmodified unit tests for the v2.0 ABI, taken from DPDK
>>>>>   2.2 and 17.02.
>>>>> * I reproduced the rte_lpm interface header from v2.0, including the inline
>>>>>   functions and remapping symbols to their appropriate versions.
>>>>> * I added support for multiple abi versions to the app/test unit test framework
>>>>>   to allow users to switch between abi versions (set_abi_version), without
>>>>>   further polluting the already long list of unit tests available in app/test.
>>>>>
>>>>> The intention here is that in future as developers need to deprecate APIs, the
>>>>> associated unit tests may move into the ABI version testing mechanism of the
>>>>> app/test instead of being replaced by the latest set of unit tests as would be
>>>>> the case today.
>>>>>
>>>>> v2:
>>>>>
>>>>> * Added LPM IPv6 test cases for the v2.0 ABI.
>>>>> * Fixed a number of checkpatch errors, stop short of substantially reworking
>>>>>   the test code from the v2.0 ABI. 
>>>>> * Removed duplicating test cases published in the original v1 patch.
>>>>
>>>> Thanks for this work.  I think it's useful.
>>>>
>>>> I see an error under aarch64 builds because there are some x86_64
>>>> specific types being used in the testing.
>>>
>>> So the problem is that LPM didn't fully support ARM until DPDK v16.04.
>>> The ABI versioning code in the LPM library is there to support the 2.0 ABI.
>>>
>>> The intention of this unit test is to test backward's compatibility with
>>> an inline LPM function from DPDK v2.2.0, which was essentially x86 only
>>> at that time.
>>>
>>> Unless we want to get into the business of backporting ARM support to
>>> DPDK 2.2.0 (from where this test cases came from) - we should probably
>>> restrict these ABI versioning test cases to CONFIG_RTE_ARCH_X86_64 only.
>>>
>>> The other option is forget about testing this the LPM ABI versioning
>>> support, which then asks the question should be perhaps excise that code
>>> altogether.
>>>
>>
>> I think function versioning is great and should be widely used.
>> Unfortunately, though, in our case since we break the ABI so consistently,
>> this old code is pretty useless. Therefore, I think we should remove all
>> old versionned code from e.g. pre-18.11, since no app is realistically
>> going to work from that far back anyway.
>>
>> /Bruce 
>>
> 
> I had come to a similar conclusion, that we likely need to deprecate
> much or all of the existing ABI Compatibility code, it needs a wider
> review.
> 
> BIND_VERSION_SYMBOL and friends, are still needed to unit test ABI
> Versioning, the general idea is sound. And I liked LPM as an example,
> because it is well understood and contained, but I will look for
> something more recent we could use instead.
> 

Only recent example I can find of ABI versioning is the Timer Library,
changed in April 2019. After that are the distributor and the lpm
library both changes in 2017, does this seem right?

Ray K

root@xxx:/build/dpdk# find lib -name *.map | xargs -I{} -- git log -1
--format="%ai {}" {} | sort -k 1,2 | tail -n 10
2019-04-03 18:20:13 -0500 lib/librte_stack/rte_stack_version.map
2019-04-15 16:41:28 -0500 lib/librte_timer/rte_timer_version.map
2019-04-30 22:54:16 -0500 lib/librte_rcu/rte_rcu_version.map
2019-06-25 04:46:02 +0530 lib/librte_eventdev/rte_eventdev_version.map
2019-07-05 10:16:17 -0700 lib/librte_net/rte_net_version.map
2019-07-11 09:26:05 +0000 lib/librte_metrics/rte_metrics_version.map
2019-07-17 03:23:55 +0800 lib/librte_ring/rte_ring_version.map
2019-07-26 15:10:19 +0100 lib/librte_security/rte_security_version.map
2019-07-27 09:21:33 +0200 lib/librte_eal/rte_eal_version.map
2019-07-31 14:27:16 +0200 lib/librte_ethdev/rte_ethdev_version.map


root@xxx:/build/dpdk# find lib -name *.map | xargs -I{} -- git log -1
--format="%ai {}" {} | sort -k 1,2 | tail -n 50 | awk '{print $4}' |
xargs sort | uniq -c | sort -k 1


...
      2         rte_distributor_clear_returns;
      2         rte_distributor_create;
      2         rte_distributor_flush;
      2         rte_distributor_get_pkt;
      2         rte_distributor_poll_pkt;
      2         rte_distributor_process;
      2         rte_distributor_request_pkt;
      2         rte_distributor_returned_pkts;
      2         rte_distributor_return_pkt;
      2         rte_lpm6_add;
      2         rte_lpm6_is_rule_present;
      2         rte_lpm6_lookup;
      2         rte_lpm6_lookup_bulk_func;
      2         rte_lpm_add;
      2         rte_lpm_create;
      2         rte_lpm_delete;
      2         rte_lpm_delete_all;
      2         rte_lpm_find_existing;
      2         rte_lpm_free;
      2         rte_lpm_is_rule_present;
      2         rte_timer_dump_stats;
      2         rte_timer_manage;
      2         rte_timer_reset;
      2         rte_timer_stop;
      2         rte_timer_subsystem_init;
...