Re: [dpdk-dev] [PATCH v8 3/5] kni: add app specific mempool create & free routine

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 7/23/19 8:38 AM, vattunuru@marvell.com wrote:
> From: Vamsi Attunuru <vattunuru@marvell.com>
>
> When KNI operates in IOVA = VA mode, it requires mbuf memory
> to be physically contiguous to ensure KNI kernel module could
> translate IOVA addresses properly. Patch adds a KNI specific
> mempool create routine to populate the KNI packet mbuf pool
> with memory objects that are being on a page.
>
> KNI applications need to use this mempool create & free routines
> so that mbuf related requirements in IOVA = VA mode are handled
> inside those routines based on the enabled mode.
>
> Updated the release notes with these new routine details.
>
> Signed-off-by: Vamsi Attunuru <vattunuru@marvell.com>
> Signed-off-by: Kiran Kumar K <kirankumark@marvell.com>
> ---
>   doc/guides/rel_notes/release_19_08.rst |  6 ++++
>   examples/kni/main.c                    |  6 +++-
>   lib/librte_kni/Makefile                |  1 +
>   lib/librte_kni/meson.build             |  1 +
>   lib/librte_kni/rte_kni.c               | 59 ++++++++++++++++++++++++++++++++++
>   lib/librte_kni/rte_kni.h               | 49 ++++++++++++++++++++++++++++
>   lib/librte_kni/rte_kni_version.map     |  2 ++
>   7 files changed, 123 insertions(+), 1 deletion(-)
>
> diff --git a/doc/guides/rel_notes/release_19_08.rst b/doc/guides/rel_notes/release_19_08.rst
> index 0a3f840..bd01e99 100644
> --- a/doc/guides/rel_notes/release_19_08.rst
> +++ b/doc/guides/rel_notes/release_19_08.rst
> @@ -281,6 +281,12 @@ API Changes
>     offload flag from the library. The application must set this flag if it is
>     supported by the platform and application wishes to use it.
>   
> +* kni: ``rte_kni_pktmbuf_pool_create`` ``rte_kni_pktmbuf_pool_free`` functions
> +  were introduced for KNI applications for creating & freeing packet pool.
> +  Since IOVA=VA mode was added in KNI, packet pool's mbuf memory should be
> +  physically contiguous for the KNI kernel module to work in IOVA=VA mode,
> +  this requirment was taken care in the kni packet pool creation fucntions.
> +
>   
>   ABI Changes
>   -----------
> diff --git a/examples/kni/main.c b/examples/kni/main.c
> index 4710d71..3b9c067 100644
> --- a/examples/kni/main.c
> +++ b/examples/kni/main.c
> @@ -975,7 +975,7 @@ main(int argc, char** argv)
>   		rte_exit(EXIT_FAILURE, "Could not parse input parameters\n");
>   
>   	/* Create the mbuf pool */
> -	pktmbuf_pool = rte_pktmbuf_pool_create("mbuf_pool", NB_MBUF,
> +	pktmbuf_pool = rte_kni_pktmbuf_pool_create("mbuf_pool", NB_MBUF,
>   		MEMPOOL_CACHE_SZ, 0, MBUF_DATA_SZ, rte_socket_id());
>   	if (pktmbuf_pool == NULL) {
>   		rte_exit(EXIT_FAILURE, "Could not initialise mbuf pool\n");
> @@ -1043,6 +1043,10 @@ main(int argc, char** argv)
>   			continue;
>   		kni_free_kni(port);
>   	}
> +
> +	if (pktmbuf_pool)

Typically pointer is compared to NULL, but it is not required here
anyway, since rte_mempool_free() handles NULL perfectly itself.

> +		rte_kni_pktmbuf_pool_free(pktmbuf_pool);
> +
>   	for (i = 0; i < RTE_MAX_ETHPORTS; i++)
>   		if (kni_port_params_array[i]) {
>   			rte_free(kni_port_params_array[i]);

<...>

> diff --git a/lib/librte_kni/rte_kni.h b/lib/librte_kni/rte_kni.h
> index 5699a64..7f11927 100644
> --- a/lib/librte_kni/rte_kni.h
> +++ b/lib/librte_kni/rte_kni.h
> @@ -20,6 +20,7 @@
>   #include <rte_pci.h>
>   #include <rte_memory.h>
>   #include <rte_mempool.h>
> +#include <rte_mbuf_pool_ops.h>

I don't understand why it is included here.

>   #include <rte_ether.h>
>   
>   #include <rte_kni_common.h>
>

<...>