Hi David,
> -----Original Message-----
> From: Coyle, David <david.co...@intel.com>
> Sent: Friday, April 3, 2020 5:37 PM
> To: dev@dpdk.org
> Cc: Doherty, Declan <declan.dohe...@intel.com>; Trahe, Fiona
> <fiona.tr...@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.gua...@intel.com>; Ryan, Brendan <brendan.r...@intel.com>;
> shreyansh.j...@nxp.com; hemant.agra...@nxp.com; Coyle, David
> <david.co...@intel.com>; O'loingsigh, Mairtin <mairtin.oloings...@intel.com>
> Subject: [PATCH v2 1/4] raw/common: add multi-function interface
>
> The multi-function interface provides a flexible and extensible way of
> combining
> one or more packet processing functions into a single operation. The interface
> can be used by applications to send the combined operations to a optimized
> software or hardware accelerator via a raw device.
>
> Signed-off-by: David Coyle <david.co...@intel.com>
> Signed-off-by: Mairtin o Loingsigh <mairtin.oloings...@intel.com>
> ---
>
> In particular, looking for feedback on the meson script changes that were
> required to build the drivers/raw/common/multi_fn directory. Thank you.
>
> config/common_base | 5 +
> drivers/meson.build | 5 +
> drivers/raw/Makefile | 1 +
> drivers/raw/common/Makefile | 8 +
> drivers/raw/common/meson.build | 7 +
> drivers/raw/common/multi_fn/Makefile | 27 ++
> drivers/raw/common/multi_fn/meson.build | 9 +
> .../multi_fn/rte_common_multi_fn_version.map | 11 +
> drivers/raw/common/multi_fn/rte_multi_fn.c | 166 +++++++++
> drivers/raw/common/multi_fn/rte_multi_fn.h | 350 ++++++++++++++++++
> .../raw/common/multi_fn/rte_multi_fn_driver.h | 55 +++
> meson.build | 4 +
> mk/rte.app.mk | 1 +
> 13 files changed, 649 insertions(+)
> create mode 100644 drivers/raw/common/Makefile create mode 100644
> drivers/raw/common/meson.build create mode 100644
> drivers/raw/common/multi_fn/Makefile
> create mode 100644 drivers/raw/common/multi_fn/meson.build
> create mode 100644
> drivers/raw/common/multi_fn/rte_common_multi_fn_version.map
> create mode 100644 drivers/raw/common/multi_fn/rte_multi_fn.c
> create mode 100644 drivers/raw/common/multi_fn/rte_multi_fn.h
> create mode 100644 drivers/raw/common/multi_fn/rte_multi_fn_driver.h
>
> diff --git a/config/common_base b/config/common_base index
> c31175f9d..4f004968b 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -818,6 +818,11 @@
> CONFIG_RTE_LIBRTE_PMD_OCTEONTX2_EP_RAWDEV=y
> #
> CONFIG_RTE_LIBRTE_PMD_NTB_RAWDEV=y
>
> +#
> +# Compile multi-fn raw device interface #
> +CONFIG_RTE_LIBRTE_MULTI_FN_COMMON=n
This can be enabled by default, right? It doesn't have any external dependency.
...
> +++ b/drivers/raw/common/multi_fn/rte_common_multi_fn_version.map
> @@ -0,0 +1,11 @@
> +EXPERIMENTAL {
> + global:
> +
> + rte_multi_fn_session_create;
> + rte_multi_fn_session_destroy;
> + rte_multi_fn_op_pool_create;
> + rte_multi_fn_op_bulk_alloc;
> + rte_multi_fn_op_free;
This list should be sorted alphabetically.
> +
> + local: *;
> +};
> diff --git a/drivers/raw/common/multi_fn/rte_multi_fn.c
> b/drivers/raw/common/multi_fn/rte_multi_fn.c
> new file mode 100644
> index 000000000..4f8e7fd94
> --- /dev/null
> +++ b/drivers/raw/common/multi_fn/rte_multi_fn.c
> @@ -0,0 +1,166 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation.
> + */
> +
> +#include <ctype.h>
> +#include <stdio.h>
...
> +#include <rte_rawdev.h>
A bunch of these includes are not needed.
>From what I could see, only <inttypes.h>, <rte_log.h>, <rte_common.h>,
<rte_rawdev.h> and <rte_mempool.h> are needed, apart from the two below.
> +
> +#include "rte_multi_fn_driver.h"
> +#include "rte_multi_fn.h"
...
> +++ b/drivers/raw/common/multi_fn/rte_multi_fn.h
> @@ -0,0 +1,350 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation.
> + */
> +
> +#ifndef _RTE_MULTI_FN_H_
> +#define _RTE_MULTI_FN_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <rte_compat.h>
> +#include <rte_common.h>
> +#include <rte_mbuf.h>
> +#include <rte_memory.h>
> +#include <rte_mempool.h>
> +#include <rte_comp.h>
> +#include <rte_crypto.h>
> +#include <rte_rawdev.h>
Only <rte_comp.h> and <rte_crypto.h> are needed.
> +
...
> +__rte_experimental
> +static inline void
This is a public API, so I'd say this shouldn't be inline, right?
> +rte_multi_fn_op_free(struct rte_multi_fn_op *op) {
> + if (op != NULL && op->mempool != NULL)
> + rte_mempool_put(op->mempool, op);
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_MULTI_FN_H_ */
> diff --git a/drivers/raw/common/multi_fn/rte_multi_fn_driver.h
> b/drivers/raw/common/multi_fn/rte_multi_fn_driver.h
> new file mode 100644
> index 000000000..7e1e57fa3
> --- /dev/null
> +++ b/drivers/raw/common/multi_fn/rte_multi_fn_driver.h
> @@ -0,0 +1,55 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2020 Intel Corporation.
> + */
> +
> +#ifndef _RTE_MULTI_FN_DRIVER_H_
> +#define _RTE_MULTI_FN_DRIVER_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <rte_compat.h>
> +#include <rte_common.h>
> +#include <rte_rawdev.h>
> +#include <rte_multi_fn.h>
> +
Only <rte_rawdev.h> and <rte_multi_fn.h> are needed.
Actually, since rte_multi_fn.h is in the same folder, you can use double quotes.
