> 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.
>
> It was only being used for extra paranoid, things might change in
> the future mode anyway, so remove it and just use rte_mempool_full.
How about keeping the dpdk_mp_full() function in a following manner:
static int
dpdk_mp_full(const struct rte_mempool *mp) OVS_REQUIRES(dpdk_mp_mutex)
{
/* XXX: A comment about possible false positives and why we're not
* worrying about them so much. */
return rte_mempool_full(mp);
}
?
I feel that we still need some explanation inside the code.
What do you think?
Best regards, Ilya Maximets.
>
> The mempools are still removed later and are checked to see
> that the buffers are back in it. Previously the mempools were removed
> straight away and there was no checking.
>
> 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 | 22 ++--------------------
> 1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 87152a7..ede1e5c 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -527,22 +527,4 @@ ovs_rte_pktmbuf_init(struct rte_mempool *mp OVS_UNUSED,
> }
>
> -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.
> - */
> - 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;
> -}
> -
> /* Free unused mempools. */
> static void
> @@ -553,5 +535,5 @@ dpdk_mp_sweep(void)
> ovs_mutex_lock(&dpdk_mp_mutex);
> LIST_FOR_EACH_SAFE (dmp, next, list_node, &dpdk_mp_free_list) {
> - if (dpdk_mp_full(dmp->mp)) {
> + if (rte_mempool_full(dmp->mp)) {
> VLOG_DBG("Freeing mempool \"%s\"", dmp->mp->name);
> ovs_list_remove(&dmp->list_node);
> @@ -666,5 +648,5 @@ dpdk_mp_release(struct rte_mempool *mp)
>
> ovs_mutex_lock(&dpdk_mp_mutex);
> - if (dpdk_mp_full(mp)) {
> + if (rte_mempool_full(mp)) {
> VLOG_DBG("Freeing mempool \"%s\"", mp->name);
> rte_mempool_free(mp);
> --
> 1.8.3.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
