Re: [dpdk-dev] [PATCH v2 0/2] add abi version testing to app/test

[Bruce Richardson <bruce.richardson@intel.com>]

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