[ovs-dev] [PATCH v2] netdev-dpdk: Remove use of rte_mempool_ops_get_count.

[Stokes, Ian]

> rte_mempool_ops_get_count is not exported by DPDK so it means it cannot be
> used by OVS when using DPDK as a shared library.
> 
> Remove rte_mempool_ops_get_count but still use rte_mempool_full and
> document it's behavior.
> 

Thanks for the v2, I've merged this to dpdk_merge and backported it. It will be part of this weeks pull request.

Ian

> Fixes: 91fccdad72a2 ("netdev-dpdk: Free mempool only when no in-use
> mbufs.")
> Reported-by: Timothy Redaelli <tredaelli at redhat.com>
> Reported-by: Markos Chandras <mchandras at suse.de>
> Signed-off-by: Kevin Traynor <ktraynor at redhat.com>
> ---
>  lib/netdev-dpdk.c | 25 +++++++++++++------------
>  1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 87152a7..719140f
> 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -530,17 +530,18 @@ static int
>  dpdk_mp_full(const struct rte_mempool *mp) OVS_REQUIRES(dpdk_mp_mutex)  {
> -    unsigned ring_count;
> -    /* This logic is needed because rte_mempool_full() is not guaranteed
> to
> -     * be atomic and mbufs could be moved from mempool cache --> mempool
> ring
> -     * during the call. However, as no mbufs will be taken from the
> mempool
> -     * at this time, we can work around it by also checking the ring
> entries
> -     * separately and ensuring that they have not changed.
> +    /* At this point we want to know if all the mbufs are back
> +     * in the mempool. rte_mempool_full() is not atomic but it's
> +     * the best available and as we are no longer requesting mbufs
> +     * from the mempool, it means mbufs will not move from
> +     * 'mempool ring' --> 'mempool cache'. In rte_mempool_full()
> +     * the ring is counted before caches, so we won't get false
> +     * positives in this use case and we handle false negatives.
> +     *
> +     * If future implementations of rte_mempool_full() were to change
> +     * it could be possible for a false positive. Even that would
> +     * likely be ok, as there are additional checks during mempool
> +     * freeing but it would make things racey.
>       */
> -    ring_count = rte_mempool_ops_get_count(mp);
> -    if (rte_mempool_full(mp) && rte_mempool_ops_get_count(mp) ==
> ring_count) {
> -        return 1;
> -    }
> -
> -    return 0;
> +    return rte_mempool_full(mp);
>  }
> 
> --
> 1.8.3.1