[1/2] tests: Fix unit tests for shared builds
Checks
Commit Message
From: Michael Santana <msantana@redhat.com>
Currently many unit tests fail when running tests under shared builds.
This happens because of missing driver dependencies. This is fixed by
explicitly linking in missing drivers for the test application.
before and after (clang):
https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
Suggested-by: David Marchand <david.marchand@redhat.com>
Signed-off-by: Michael Santana <msantana@redhat.com>
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
app/test/meson.build | 9 +++++++++
1 file changed, 9 insertions(+)
Comments
On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
> From: Michael Santana <msantana@redhat.com>
>
> Currently many unit tests fail when running tests under shared builds.
> This happens because of missing driver dependencies. This is fixed by
> explicitly linking in missing drivers for the test application.
>
> before and after (clang):
> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
>
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Suggested-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Michael Santana <msantana@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
Rather than linking in the libraries explicitly, can you have the build do
a "ninja install" at the end to place the libraries and drivers in their
correct paths. That should mean that the test app (via eal) auto-loads all
drivers from EAL_PMD_PATH (/usr/local/...). It would save having to make
further changes to this file to link in any additional drivers.
EAL_PMD_PATH is based off of $prefix for the build or install, so you can
adjust that using meson options, if putting the drivers in /usr/local is
not desirable for your test environments.
/Bruce
Bruce Richardson <bruce.richardson@intel.com> writes:
> On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
>> From: Michael Santana <msantana@redhat.com>
>>
>> Currently many unit tests fail when running tests under shared builds.
>> This happens because of missing driver dependencies. This is fixed by
>> explicitly linking in missing drivers for the test application.
>>
>> before and after (clang):
>> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
>> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
>>
>> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
>> Suggested-by: David Marchand <david.marchand@redhat.com>
>> Signed-off-by: Michael Santana <msantana@redhat.com>
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
> Rather than linking in the libraries explicitly, can you have the build do
> a "ninja install" at the end to place the libraries and drivers in their
> correct paths. That should mean that the test app (via eal) auto-loads all
> drivers from EAL_PMD_PATH (/usr/local/...). It would save having to make
> further changes to this file to link in any additional drivers.
>
> EAL_PMD_PATH is based off of $prefix for the build or install, so you can
> adjust that using meson options, if putting the drivers in /usr/local is
> not desirable for your test environments.
The downside of not explicitly linking this way is that a developer
won't be able to do something like:
meson build
ninja -C build
... development work
ninja -C build test
Rather, they will notice broken tests and not care about testing any
more (since that's what we're dealing with now).
Requiring that a developer do the install (ninja -C build install) then
presents a different problem: before doing the 'test' I *must* remember
to do the install. Otherwise I won't be testing with the correct
version of the libraries.
Is there a way to adjust the library search path? Maybe we can solve
this by making that search path adjusted during the unit tests to look
in all the various build directories?
> /Bruce
Aaron Conole <aconole@redhat.com> writes:
> Bruce Richardson <bruce.richardson@intel.com> writes:
>
>> On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
>>> From: Michael Santana <msantana@redhat.com>
>>>
>>> Currently many unit tests fail when running tests under shared builds.
>>> This happens because of missing driver dependencies. This is fixed by
>>> explicitly linking in missing drivers for the test application.
>>>
>>> before and after (clang):
>>> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
>>> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
>>>
>>> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
>>> Suggested-by: David Marchand <david.marchand@redhat.com>
>>> Signed-off-by: Michael Santana <msantana@redhat.com>
>>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>>> ---
>> Rather than linking in the libraries explicitly, can you have the build do
>> a "ninja install" at the end to place the libraries and drivers in their
>> correct paths. That should mean that the test app (via eal) auto-loads all
>> drivers from EAL_PMD_PATH (/usr/local/...). It would save having to make
>> further changes to this file to link in any additional drivers.
Oops, forgot to include a link to a build where I did this. It's
failing (you can see the corresponding commit at
https://github.com/orgcandman/dpdk/commit/ccba975bdfe851b4c8ec3f208451bb105317b76d):
https://travis-ci.org/orgcandman/dpdk/jobs/566044409
>> EAL_PMD_PATH is based off of $prefix for the build or install, so you can
>> adjust that using meson options, if putting the drivers in /usr/local is
>> not desirable for your test environments.
>
> The downside of not explicitly linking this way is that a developer
> won't be able to do something like:
>
> meson build
> ninja -C build
>
> ... development work
>
> ninja -C build test
>
> Rather, they will notice broken tests and not care about testing any
> more (since that's what we're dealing with now).
>
> Requiring that a developer do the install (ninja -C build install) then
> presents a different problem: before doing the 'test' I *must* remember
> to do the install. Otherwise I won't be testing with the correct
> version of the libraries.
>
> Is there a way to adjust the library search path? Maybe we can solve
> this by making that search path adjusted during the unit tests to look
> in all the various build directories?
>
>> /Bruce
On Wed, Jul 31, 2019 at 12:10:55PM -0400, Aaron Conole wrote:
> Aaron Conole <aconole@redhat.com> writes:
>
> > Bruce Richardson <bruce.richardson@intel.com> writes:
> >
> >> On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
> >>> From: Michael Santana <msantana@redhat.com>
> >>>
> >>> Currently many unit tests fail when running tests under shared builds.
> >>> This happens because of missing driver dependencies. This is fixed by
> >>> explicitly linking in missing drivers for the test application.
> >>>
> >>> before and after (clang):
> >>> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
> >>> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
> >>>
> >>> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> >>> Suggested-by: David Marchand <david.marchand@redhat.com>
> >>> Signed-off-by: Michael Santana <msantana@redhat.com>
> >>> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >>> ---
> >> Rather than linking in the libraries explicitly, can you have the build do
> >> a "ninja install" at the end to place the libraries and drivers in their
> >> correct paths. That should mean that the test app (via eal) auto-loads all
> >> drivers from EAL_PMD_PATH (/usr/local/...). It would save having to make
> >> further changes to this file to link in any additional drivers.
>
> Oops, forgot to include a link to a build where I did this. It's
> failing (you can see the corresponding commit at
> https://github.com/orgcandman/dpdk/commit/ccba975bdfe851b4c8ec3f208451bb105317b76d):
>
> https://travis-ci.org/orgcandman/dpdk/jobs/566044409
>
I think the error may be due to having some drivers already linked in while
trying to reload all drivers dynamically. The dynamic loading of drivers I
actually think we could make more robust. For example:
* only load .so files. If you pass in the drivers path from the build, it
errors out trying to load .a files
* when loading directories, maybe skip any files which don't have a "librte"
prefix - anyone who has their own drivers they want loaded can add the
prefix or load it explicitly via -d
* alternatively, skip files which don't have a pmdinfo section in them
* allow skipping of drivers which are already linked in
* don't fail the whole process if one driver fails to load from a
directory.
Regards,
/Bruce
Bruce Richardson <bruce.richardson@intel.com> writes:
> On Wed, Jul 31, 2019 at 12:10:55PM -0400, Aaron Conole wrote:
>> Aaron Conole <aconole@redhat.com> writes:
>>
>> > Bruce Richardson <bruce.richardson@intel.com> writes:
>> >
>> >> On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
>> >>> From: Michael Santana <msantana@redhat.com>
>> >>>
>> >>> Currently many unit tests fail when running tests under shared builds.
>> >>> This happens because of missing driver dependencies. This is fixed by
>> >>> explicitly linking in missing drivers for the test application.
>> >>>
>> >>> before and after (clang):
>> >>> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
>> >>> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
>> >>>
>> >>> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
>> >>> Suggested-by: David Marchand <david.marchand@redhat.com>
>> >>> Signed-off-by: Michael Santana <msantana@redhat.com>
>> >>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> >>> ---
>> >> Rather than linking in the libraries explicitly, can you have the build do
>> >> a "ninja install" at the end to place the libraries and drivers in their
>> >> correct paths. That should mean that the test app (via eal) auto-loads all
>> >> drivers from EAL_PMD_PATH (/usr/local/...). It would save having to make
>> >> further changes to this file to link in any additional drivers.
>>
>> Oops, forgot to include a link to a build where I did this. It's
>> failing (you can see the corresponding commit at
>> https://github.com/orgcandman/dpdk/commit/ccba975bdfe851b4c8ec3f208451bb105317b76d):
>>
>> https://travis-ci.org/orgcandman/dpdk/jobs/566044409
>>
> I think the error may be due to having some drivers already linked in while
> trying to reload all drivers dynamically. The dynamic loading of drivers I
> actually think we could make more robust. For example:
>
> * only load .so files. If you pass in the drivers path from the build, it
> errors out trying to load .a files
> * when loading directories, maybe skip any files which don't have a "librte"
> prefix - anyone who has their own drivers they want loaded can add the
> prefix or load it explicitly via -d
> * alternatively, skip files which don't have a pmdinfo section in them
> * allow skipping of drivers which are already linked in
> * don't fail the whole process if one driver fails to load from a
> directory.
What change do you suggest I make to the steps I outlined? I think it
seems to be more complex than a simple 'ninja install'.
Maybe it makes sense to merge these patches now and fix the library
loading code as a separate action? Or is there a different procedure I
(and other developers) should follow? Does it really need a 'ninja
install' even for a drive-by developer to make a small patch and test
before submitting?
> Regards,
> /Bruce
On Thu, Aug 01, 2019 at 11:40:33AM -0400, Aaron Conole wrote:
> Bruce Richardson <bruce.richardson@intel.com> writes:
>
> > On Wed, Jul 31, 2019 at 12:10:55PM -0400, Aaron Conole wrote:
> >> Aaron Conole <aconole@redhat.com> writes:
> >>
> >> > Bruce Richardson <bruce.richardson@intel.com> writes:
> >> >
> >> >> On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
> >> >>> From: Michael Santana <msantana@redhat.com>
> >> >>>
> >> >>> Currently many unit tests fail when running tests under shared builds.
> >> >>> This happens because of missing driver dependencies. This is fixed by
> >> >>> explicitly linking in missing drivers for the test application.
> >> >>>
> >> >>> before and after (clang):
> >> >>> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
> >> >>> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
> >> >>>
> >> >>> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> >> >>> Suggested-by: David Marchand <david.marchand@redhat.com>
> >> >>> Signed-off-by: Michael Santana <msantana@redhat.com>
> >> >>> Signed-off-by: Aaron Conole <aconole@redhat.com>
> >> >>> ---
> >> >> Rather than linking in the libraries explicitly, can you have the build do
> >> >> a "ninja install" at the end to place the libraries and drivers in their
> >> >> correct paths. That should mean that the test app (via eal) auto-loads all
> >> >> drivers from EAL_PMD_PATH (/usr/local/...). It would save having to make
> >> >> further changes to this file to link in any additional drivers.
> >>
> >> Oops, forgot to include a link to a build where I did this. It's
> >> failing (you can see the corresponding commit at
> >> https://github.com/orgcandman/dpdk/commit/ccba975bdfe851b4c8ec3f208451bb105317b76d):
> >>
> >> https://travis-ci.org/orgcandman/dpdk/jobs/566044409
> >>
> > I think the error may be due to having some drivers already linked in while
> > trying to reload all drivers dynamically. The dynamic loading of drivers I
> > actually think we could make more robust. For example:
> >
> > * only load .so files. If you pass in the drivers path from the build, it
> > errors out trying to load .a files
> > * when loading directories, maybe skip any files which don't have a "librte"
> > prefix - anyone who has their own drivers they want loaded can add the
> > prefix or load it explicitly via -d
> > * alternatively, skip files which don't have a pmdinfo section in them
> > * allow skipping of drivers which are already linked in
> > * don't fail the whole process if one driver fails to load from a
> > directory.
>
> What change do you suggest I make to the steps I outlined? I think it
> seems to be more complex than a simple 'ninja install'.
>
> Maybe it makes sense to merge these patches now and fix the library
> loading code as a separate action? Or is there a different procedure I
> (and other developers) should follow? Does it really need a 'ninja
> install' even for a drive-by developer to make a small patch and test
> before submitting?
>
I think your patch is the best solution for 19.08. Fixing the library
loading for developers is a larger job beyond that.
Another item for future consideration: since meson/ninja automatically sets
the rpath for binaries in build directory - and then strips that out on
install - it would be really nice if we could leverage that somehow for
finding drivers.
/Bruce
On Wed, Jul 31, 2019 at 10:50:29AM -0400, Aaron Conole wrote:
> From: Michael Santana <msantana@redhat.com>
>
> Currently many unit tests fail when running tests under shared builds.
> This happens because of missing driver dependencies. This is fixed by
> explicitly linking in missing drivers for the test application.
>
> before and after (clang):
> https://travis-ci.com/Maickii/dpdk-2/jobs/212329160#L623
> https://travis-ci.com/Maickii/dpdk-2/jobs/212335912#L620
>
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Suggested-by: David Marchand <david.marchand@redhat.com>
> Signed-off-by: Michael Santana <msantana@redhat.com>
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> app/test/meson.build | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
31/07/2019 16:50, Aaron Conole:
> From: Michael Santana <msantana@redhat.com>
> --- a/app/test/meson.build
> +++ b/app/test/meson.build
> +if dpdk_conf.has('RTE_LIBRTE_RING_MEMPOOL')
> + test_deps += 'mempool_ring'
> +endif
> +if dpdk_conf.has('RTE_LIBRTE_STACK_MEMPOOL')
> + test_deps += 'mempool_stack'
> +endif
> +if dpdk_conf.has('RTE_LIBRTE_SKELETON_EVENTDEV_PMD')
> + test_deps += 'pmd_skeleton_event'
> +endif
> if dpdk_conf.has('RTE_LIBRTE_PDUMP')
> test_deps += 'pdump'
> endif
As these libraries are dynamically loadable as plugins,
it should be explained in comments that this explicit linking
is an exception to allow the developer run the unit tests
without intalling the libraries.
Otherwise, other applications will copy it.
Thomas Monjalon <thomas@monjalon.net> writes:
> 31/07/2019 16:50, Aaron Conole:
>> From: Michael Santana <msantana@redhat.com>
>> --- a/app/test/meson.build
>> +++ b/app/test/meson.build
>> +if dpdk_conf.has('RTE_LIBRTE_RING_MEMPOOL')
>> + test_deps += 'mempool_ring'
>> +endif
>> +if dpdk_conf.has('RTE_LIBRTE_STACK_MEMPOOL')
>> + test_deps += 'mempool_stack'
>> +endif
>> +if dpdk_conf.has('RTE_LIBRTE_SKELETON_EVENTDEV_PMD')
>> + test_deps += 'pmd_skeleton_event'
>> +endif
>> if dpdk_conf.has('RTE_LIBRTE_PDUMP')
>> test_deps += 'pdump'
>> endif
>
> As these libraries are dynamically loadable as plugins,
> it should be explained in comments that this explicit linking
> is an exception to allow the developer run the unit tests
> without intalling the libraries.
> Otherwise, other applications will copy it.
Okay. I'll add it.
@@ -297,6 +297,15 @@ dump_test_names = [
'dump_memzone',
]
+if dpdk_conf.has('RTE_LIBRTE_RING_MEMPOOL')
+ test_deps += 'mempool_ring'
+endif
+if dpdk_conf.has('RTE_LIBRTE_STACK_MEMPOOL')
+ test_deps += 'mempool_stack'
+endif
+if dpdk_conf.has('RTE_LIBRTE_SKELETON_EVENTDEV_PMD')
+ test_deps += 'pmd_skeleton_event'
+endif
if dpdk_conf.has('RTE_LIBRTE_PDUMP')
test_deps += 'pdump'
endif