[dpdk-dev] Hello Ferruh, Neil,

Ferruh Yigit ferruh.yigit at intel.com
Tue Feb 21 15:39:01 CET 2017


On 2/21/2017 1:42 PM, Shreyansh Jain wrote:
> Thanks for the suggestions about rte_* renaming in DPAA2 PMD.
> I create a draft patch for a single symbol change. (applies over v7
> of DPAA2 PMD)
> 
> Can you tell me if this is the direction you were suggesting?
> 
> I see two issues in this approach which are somewhat problematic for
> me to change all the symbols:
> 1) We saw a drop of over 5% when I replaced only 3 symbols (one
>    of the most used ones, just for sampling). This also means that
>    when more of such symbols are replaced, it would bring further
>    drop. This was case when I used the Shared library approach.
>    (*) I am not well versed with gcc symbol aliasing to comment for
>        why such a drop would happen. But multiple test cycles confirm
>        this.
> 2) I have to include a new header in almost all the source files for PMD/
>    Pool/Bus etc. This is besides the STATIC_SYMBOL macros across the
>    code. Essentially, any internal repo patch cannot be directly
>    transposed to DPDK repo. Increased effort for each internal->
>    external release
> 
> Overall, I would like you to consider if this effort for changing names
> for exposed symbols is really useful or not.

As you showed below, this works for exporting proper APIs, but not sure
if this change worth or not.

> 
> There is another approach - that of not using a drivers/common library.
> This again is problematic for us - NXP DPAA2 being a hardware, the lib
> and state for Crypto and Net hardware is tied together - so, having
> multiple instances of library breaks either of Crypto or Net PMD.

Isn't is possible to keep folder structure same, but produce single library.
Because these don't provide any API to the user application, perhaps not
need to be library at all.

Assuming that bus and pool won't be required without a driver existing,
is it possible have a single driver library that contains others?

For net driver, dependency is as following:
  bus_fslmc  --> common_dpaa2_qbman
  pool_dpaa2 --> bus_fslmc, common_dpaa2_qbman
  pmd_dpaa2  --> pool_dpaa2, bus_fslmc, common_dpaa2_qbman

So generating only "librte_pmd_dpaa2" which include above dependent ones.

For cryptodev pmd, I assume it has dependency to same modules:
  pmd_crypto --> pool_dpaa2, bus_fslmc, common_dpaa2_qbman

And this will generate only crypto pmd library, including all again.


This will create duplication in binaries, but I think easier to manage
library dependencies.


And for above case, as far as I know, both net and crypto libraries can
be linked against a binary even there are duplicate symbols. Are you
getting error here?

> 
> Any other suggestions?
> 
> -
> Shreyansh
> 
> --->8---
> 
> From b9928ed44e69d7f9f9def0f06647a8d3bc25f284 Mon Sep 17 00:00:00 2001
> From: Shreyansh Jain <shreyansh.jain at nxp.com>
> Date: Fri, 10 Feb 2017 19:18:55 +0530
> Subject: [PATCH] dpaa2: fix for rte_* symbol naming
> 
> This sample code:
> * creates nxp_compat.h containing custom version of MAP_STATIC_* and
>   BIND_DEFAULT*
>   Can't use the existing version because they are based on appending
>   a string (_v[0-9]), not prepending as required in this case
> * creates nxp_shared_symbol.h for appending rte_* to symbols
> * declares functions
> * changes map file
> 
> Hereafter:
> * for every newly introduced symbol, create NXP_*_SYMBOL mapping
> * put symbol in nxp_shared_symbols.h
> * add entry in map with rte_* prepended
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
> 
> ---
>  drivers/bus/fslmc/portal/dpaa2_hw_dpio.c           |  3 +
>  .../common/dpaa2/qbman/include/fsl_qbman_portal.h  |  1 +
>  drivers/common/dpaa2/qbman/include/nxp_compat.h    | 70 ++++++++++++++++++++++
>  .../dpaa2/qbman/include/nxp_shared_symbols.h       | 41 +++++++++++++
>  drivers/common/dpaa2/qbman/qbman_portal.c          |  2 +
>  .../dpaa2/qbman/rte_common_dpaa2_qbman_version.map |  2 +-
>  6 files changed, 118 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/common/dpaa2/qbman/include/nxp_compat.h
>  create mode 100644 drivers/common/dpaa2/qbman/include/nxp_shared_symbols.h
> 
> diff --git a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> index c80d6c5..1c157b9 100644
> --- a/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> +++ b/drivers/bus/fslmc/portal/dpaa2_hw_dpio.c
> @@ -59,9 +59,12 @@
>  
>  #include <fslmc_logs.h>
>  #include <fslmc_vfio.h>
> +#include <fsl_qbman_portal.h>
>  #include "dpaa2_hw_pvt.h"
>  #include "dpaa2_hw_dpio.h"
>  
> +#include <nxp_shared_symbols.h>
> +
>  #define NUM_HOST_CPUS RTE_MAX_LCORE
>  
>  struct dpaa2_io_portal_t dpaa2_io_portal[RTE_MAX_LCORE];
> diff --git a/drivers/common/dpaa2/qbman/include/fsl_qbman_portal.h b/drivers/common/dpaa2/qbman/include/fsl_qbman_portal.h
> index 7731772..63ffdd1 100644
> --- a/drivers/common/dpaa2/qbman/include/fsl_qbman_portal.h
> +++ b/drivers/common/dpaa2/qbman/include/fsl_qbman_portal.h
> @@ -29,6 +29,7 @@
>  #define _FSL_QBMAN_PORTAL_H
>  
>  #include <fsl_qbman_base.h>
> +#include "nxp_compat.h"
>  
>  /**
>   * DOC - QBMan portal APIs to implement the following functions:
> diff --git a/drivers/common/dpaa2/qbman/include/nxp_compat.h b/drivers/common/dpaa2/qbman/include/nxp_compat.h
> new file mode 100644
> index 0000000..edbbfde
> --- /dev/null
> +++ b/drivers/common/dpaa2/qbman/include/nxp_compat.h
> @@ -0,0 +1,70 @@
> +/*-
> + *   BSD LICENSE
> + *
> + * Copyright (C) 2014-2016 Freescale Semiconductor, Inc.
> + * Copyright (C) 2016 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in the
> + *       documentation and/or other materials provided with the distribution.
> + *     * Neither the name of Freescale Semiconductor nor the
> + *       names of its contributors may be used to endorse or promote products
> + *       derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _NXP_COMPAT_H_
> +#define _NXP_COMPAT_H_
> +#include <rte_common.h>
> +
> +#ifdef RTE_BUILD_SHARED_LIB
> +
> +/*
> + * BIND_DEFAULT_SYMBOL
> + * Creates a symbol version entry instructing the linker to bind references to
> + * symbol <b> to exported symbol <a>; it also appends the symbol version as
> + * per the map file.
> + */
> +#define NXP_SHARED_SYMBOL(b, a, n) __asm__(".symver " RTE_STR(b) ", " RTE_STR(a) "@@DPDK_" RTE_STR(n))
> +
> +/*
> + * MAP_STATIC_SYMBOL
> + * If a function has been bifurcated into multiple versions, none of which
> + * are defined as the exported symbol name in the map file, this macro can be
> + * used to alias a specific version of the symbol to its exported name.  For
> + * example, if you have 2 versions of a function foo_v1 and foo_v2, where the
> + * former is mapped to foo at DPDK_1 and the latter is mapped to foo at DPDK_2 when
> + * building a shared library, this macro can be used to map either foo_v1 or
> + * foo_v2 to the symbol foo when building a static library, e.g.:
> + * MAP_STATIC_SYMBOL(void foo(), foo_v2);
> + */
> +#define NXP_STATIC_SYMBOL(f, p)
> +
> +#else
> +/*
> + * No symbol versioning in use
> + */
> +#define NXP_SHARED_SYMBOL(b, e, n)
> +#define NXP_STATIC_SYMBOL(f, p) f __attribute__((alias(RTE_STR(p))))
> +/*
> + * RTE_BUILD_SHARED_LIB=n
> + */
> +#endif
> +
> +struct qbman_swp *rte_qbman_swp_init(const struct qbman_swp_desc *d);
> +
> +#endif /* _NXP_COMPAT_H_ */
> diff --git a/drivers/common/dpaa2/qbman/include/nxp_shared_symbols.h b/drivers/common/dpaa2/qbman/include/nxp_shared_symbols.h
> new file mode 100644
> index 0000000..6faba36
> --- /dev/null
> +++ b/drivers/common/dpaa2/qbman/include/nxp_shared_symbols.h
> @@ -0,0 +1,41 @@
> +/*-
> + *   BSD LICENSE
> + *
> + * Copyright (C) 2014-2016 Freescale Semiconductor, Inc.
> + * Copyright (C) 2016 NXP
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are met:
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in the
> + *       documentation and/or other materials provided with the distribution.
> + *     * Neither the name of Freescale Semiconductor nor the
> + *       names of its contributors may be used to endorse or promote products
> + *       derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _NXP_SHARED_SYMBOLS_H_
> +#define _NXP_SHARED_SYMBOLS_H_
> +
> +#ifdef RTE_BUILD_SHARED_LIB
> +
> +#define qbman_swp_init			rte_qbman_swp_init
> +
> +#endif
> +
> +#endif /* NXP_STATIC_SYMBOL */
> +
> +
> diff --git a/drivers/common/dpaa2/qbman/qbman_portal.c b/drivers/common/dpaa2/qbman/qbman_portal.c
> index 11116bd..f9bad4f 100644
> --- a/drivers/common/dpaa2/qbman/qbman_portal.c
> +++ b/drivers/common/dpaa2/qbman/qbman_portal.c
> @@ -178,6 +178,8 @@ struct qbman_swp *qbman_swp_init(const struct qbman_swp_desc *d)
>  	portal_idx_map[p->desc.idx] = p;
>  	return p;
>  }
> +NXP_SHARED_SYMBOL(qbman_swp_init, rte_qbman_swp_init, 17.02);
> +NXP_STATIC_SYMBOL(struct qbman_swp *rte_qbman_swp_init(const struct qbman_swp_desc *d), qbman_swp_init);
>  
>  void qbman_swp_finish(struct qbman_swp *p)
>  {
> diff --git a/drivers/common/dpaa2/qbman/rte_common_dpaa2_qbman_version.map b/drivers/common/dpaa2/qbman/rte_common_dpaa2_qbman_version.map
> index f653421..1dda776 100644
> --- a/drivers/common/dpaa2/qbman/rte_common_dpaa2_qbman_version.map
> +++ b/drivers/common/dpaa2/qbman/rte_common_dpaa2_qbman_version.map
> @@ -18,7 +18,7 @@ DPDK_17.02 {
>  	qbman_result_DQ_flags;
>  	qbman_result_has_new_result;
>  	qbman_swp_acquire;
> -	qbman_swp_init;
> +	rte_qbman_swp_init;
>  	qbman_swp_pull;
>  	qbman_swp_release;
>  	qbman_swp_send_multiple;
> 



More information about the dev mailing list