Hello Thomas,
On Tue, 14 Jun 2016 10:59:49 +0200
Thomas Monjalon <thomas.monjalon at 6wind.com> wrote:
> The commit 66819e6 has introduced a dependency on libarchive to be able
> to use some tar resources in the unit tests.
> It is now an optional dependency because some systems do not have it
> installed.
I am surprised how big deal is this. So, let's live with this fact.
Thank you, Thomas, for proposing a solution.
>
> If CONFIG_RTE_APP_TEST_RESOURCE_TAR is disabled, the PCI test will not
> be run. When a "configure" script will be integrated, the libarchive
> availability could be checked to automatically enable the option.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon at 6wind.com>
> ---
> app/test/Makefile | 11 +++++++----
> app/test/resource.c | 8 ++++++--
> app/test/test_mp_secondary.c | 4 ++++
> app/test/test_resource.c | 5 +++++
> config/common_base | 1 +
> doc/guides/linux_gsg/sys_reqs.rst | 3 +++
> scripts/test-build.sh | 4 ++++
> 7 files changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/app/test/Makefile b/app/test/Makefile
> index 7e4d484..5ca5c0b 100644
> --- a/app/test/Makefile
> +++ b/app/test/Makefile
> @@ -71,9 +71,6 @@ SRCS-y += test.c
> SRCS-y += resource.c
> SRCS-y += test_resource.c
> $(eval $(call linked_resource,test_resource_c,resource.c))
> -$(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> -SRCS-y += test_pci.c
> -$(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
> SRCS-y += test_prefetch.c
> SRCS-y += test_byteorder.c
> SRCS-y += test_per_lcore.c
> @@ -88,6 +85,13 @@ SRCS-y += test_ring.c
> SRCS-y += test_ring_perf.c
> SRCS-y += test_pmd_perf.c
>
> +ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
> +$(eval $(call linked_tar_resource,test_resource_tar,test_resource.c))
> +SRCS-y += test_pci.c
> +$(eval $(call linked_tar_resource,test_pci_sysfs,test_pci_sysfs))
> +LDLIBS += -larchive
> +endif
> +
I don't like this very much. I think, the linked_tar_resource can be
disabled at the place of its definition. What about:
ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
define linked_tar_resource
...
endef
else
linked_tar_resource =
endif
...
SRCS-$(CONFIG_RTE_APP_TEST_RESOURCE_TAR) += test_pci.c
...
ifeq ($(CONFIG_RTE_APP_TEST_RESOURCE_TAR),y)
LDLIBS += -larchive
endif
> ifeq ($(CONFIG_RTE_LIBRTE_TABLE),y)
> SRCS-y += test_table.c
> SRCS-$(CONFIG_RTE_LIBRTE_PIPELINE) += test_table_pipeline.c
> @@ -194,7 +198,6 @@ CFLAGS += $(WERROR_FLAGS)
> CFLAGS += -D_GNU_SOURCE
>
> LDLIBS += -lm
> -LDLIBS += -larchive
>
> # Disable VTA for memcpy test
> ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
> diff --git a/app/test/resource.c b/app/test/resource.c
> index 8c42eea..0e2b62c 100644
> --- a/app/test/resource.c
> +++ b/app/test/resource.c
> @@ -33,8 +33,6 @@
>
> #include <stdio.h>
> #include <string.h>
> -#include <archive.h>
> -#include <archive_entry.h>
> #include <errno.h>
> #include <sys/queue.h>
>
> @@ -97,6 +95,10 @@ int resource_fwrite_file(const struct resource *r, const
> char *fname)
> return ret;
> }
>
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
> +#include <archive.h>
> +#include <archive_entry.h>
> +
> static int do_copy(struct archive *r, struct archive *w)
> {
> const void *buf;
> @@ -295,6 +297,8 @@ fail:
> return -1;
> }
>
> +#endif /* RTE_APP_TEST_RESOURCE_TAR */
> +
This looks OK.
> void resource_register(struct resource *r)
> {
> TAILQ_INSERT_TAIL(&resource_list, r, next);
> diff --git a/app/test/test_mp_secondary.c b/app/test/test_mp_secondary.c
> index 4dfe418..f66b68f 100644
> --- a/app/test/test_mp_secondary.c
> +++ b/app/test/test_mp_secondary.c
> @@ -245,6 +245,7 @@ run_object_creation_tests(void)
> printf("# Checked rte_lpm_create() OK\n");
> #endif
>
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
> /* Run a test_pci call */
> if (test_pci() != 0) {
> printf("PCI scan failed in secondary\n");
> @@ -252,6 +253,7 @@ run_object_creation_tests(void)
> return -1;
> } else
> printf("PCI scan succeeded in secondary\n");
> +#endif
Is it right to call a test from another test? I think this is
wrong... A user should first test the PCI and then the mp_seconday...
Or?
>
> return 0;
> }
> @@ -266,9 +268,11 @@ test_mp_secondary(void)
> {
> if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
> if (!test_pci_run) {
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
> printf("=== Running pre-requisite test of test_pci\n");
> test_pci();
> printf("=== Requisite test done\n");
> +#endif
Similar here.
> }
> return run_secondary_instances();
> }
> diff --git a/app/test/test_resource.c b/app/test/test_resource.c
> index 1e85040..39a6468 100644
> --- a/app/test/test_resource.c
> +++ b/app/test/test_resource.c
> @@ -85,6 +85,7 @@ static int test_resource_c(void)
> return 0;
> }
>
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
> REGISTER_LINKED_RESOURCE(test_resource_tar);
>
> static int test_resource_tar(void)
> @@ -111,6 +112,8 @@ static int test_resource_tar(void)
> return 0;
> }
>
> +#endif /* RTE_APP_TEST_RESOURCE_TAR */
> +
This looks OK.
> static int test_resource(void)
> {
> if (test_resource_dpdk())
> @@ -119,8 +122,10 @@ static int test_resource(void)
> if (test_resource_c())
> return -1;
>
> +#ifdef RTE_APP_TEST_RESOURCE_TAR
> if (test_resource_tar())
> return -1;
> +#endif /* RTE_APP_TEST_RESOURCE_TAR */
This looks OK.
>
> return 0;
> }
> diff --git a/config/common_base b/config/common_base
> index 47c26f6..b9ba405 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -546,6 +546,7 @@ CONFIG_RTE_INSECURE_FUNCTION_WARNING=n
> # Compile the test application
> #
> CONFIG_RTE_APP_TEST=y
> +CONFIG_RTE_APP_TEST_RESOURCE_TAR=n
>
> #
> # Compile the PMD test application
> diff --git a/doc/guides/linux_gsg/sys_reqs.rst
> b/doc/guides/linux_gsg/sys_reqs.rst
> index 959709e..b321544 100644
> --- a/doc/guides/linux_gsg/sys_reqs.rst
> +++ b/doc/guides/linux_gsg/sys_reqs.rst
> @@ -101,6 +101,9 @@ Compilation of the DPDK
> * libpcap headers and libraries (libpcap-devel) to compile and use the
> libpcap-based poll-mode driver.
> This driver is disabled by default and can be enabled by setting
> ``CONFIG_RTE_LIBRTE_PMD_PCAP=y`` in the build time config file.
>
> +* libarchive headers and library are needed for some unit tests using tar
> to get their resources.
> +
> +
> Running DPDK Applications
> -------------------------
>
> diff --git a/scripts/test-build.sh b/scripts/test-build.sh
> index 48539c1..9a11f94 100755
> --- a/scripts/test-build.sh
> +++ b/scripts/test-build.sh
> @@ -35,6 +35,7 @@ default_path=$PATH
> # Load config options:
> # - AESNI_MULTI_BUFFER_LIB_PATH
> # - DPDK_BUILD_TEST_CONFIGS (defconfig1+option1+option2 defconfig2)
> +# - DPDK_DEP_ARCHIVE
> # - DPDK_DEP_CFLAGS
> # - DPDK_DEP_LDFLAGS
> # - DPDK_DEP_MOFED (y/[n])
> @@ -111,6 +112,7 @@ reset_env ()
> {
> export PATH=$default_path
> unset CROSS
> + unset DPDK_DEP_ARCHIVE
> unset DPDK_DEP_CFLAGS
> unset DPDK_DEP_LDFLAGS
> unset DPDK_DEP_MOFED
> @@ -149,6 +151,8 @@ config () # <directory> <target> <options>
> sed -ri 's,(PCI_CONFIG=)n,\1y,' $1/.config
> sed -ri 's,(LIBRTE_IEEE1588=)n,\1y,' $1/.config
> sed -ri 's,(BYPASS=)n,\1y,' $1/.config
> + test "$DPDK_DEP_ARCHIVE" != y || \
> + sed -ri 's,(RESOURCE_TAR=)n,\1y,' $1/.config
> test "$DPDK_DEP_MOFED" != y || \
> sed -ri 's,(MLX._PMD=)n,\1y,' $1/.config
> test "$DPDK_DEP_SZE" != y || \
This look OK.
Regards
Jan
