[ovs-dev] [PATCH v4 05/15] netdev-dpdk: Serialise non-pmds mbufs' alloc/free.

Ian Stokes ian.stokes at intel.com
Wed Jul 11 11:45:39 UTC 2018


On 7/10/2018 12:06 PM, Tiago Lam wrote:
> A new mutex, 'nonpmd_mp_mutex', has been introduced to serialise
> allocation and free operations by non-pmd threads on a given mempool.
> 
> free_dpdk_buf() has been modified to make use of the introduced mutex.
> 

1 comment Below Tiago.

> Signed-off-by: Tiago Lam <tiago.lam at intel.com>
> Acked-by: Eelco Chaudron <echaudro at redhat.com>
> ---
>   lib/netdev-dpdk.c | 30 +++++++++++++++++++++++++++---
>   1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index a68f78a..72b9955 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -322,6 +322,16 @@ static struct ovs_mutex dpdk_mp_mutex OVS_ACQ_AFTER(dpdk_mutex)
>   static struct ovs_list dpdk_mp_list OVS_GUARDED_BY(dpdk_mp_mutex)
>       = OVS_LIST_INITIALIZER(&dpdk_mp_list);
>   
> +/* This mutex must be used by non pmd threads when allocating or freeing
> + * mbufs through mempools, when outside of the `non_pmd_mutex` mutex, in struct
> + * dp_netdev.
> + * The reason, as pointed out in the "Known Issues" section in DPDK's EAL docs,
> + * is that the implementation on which mempool is based off is non-preemptable.
> + * Since non-pmds may end up not being pinned this could lead to the preemption
> + * between non-pmds performing operations on the same mempool, which could lead
> + * to memory corruption. */
> +static struct ovs_mutex nonpmd_mp_mutex = OVS_MUTEX_INITIALIZER;
> +
>   struct dpdk_mp {
>        struct rte_mempool *mp;
>        int mtu;
> @@ -492,6 +502,8 @@ struct netdev_rxq_dpdk {
>       dpdk_port_t port_id;
>   };
>   
> +static bool dpdk_thread_is_pmd(void);
> +
>   static void netdev_dpdk_destruct(struct netdev *netdev);
>   static void netdev_dpdk_vhost_destruct(struct netdev *netdev);
>   
> @@ -525,6 +537,12 @@ dpdk_buf_size(int mtu)
>                        NETDEV_DPDK_MBUF_ALIGN);
>   }
>   
> +static bool
> +dpdk_thread_is_pmd(void)
> +{
> +     return rte_lcore_id() != NON_PMD_CORE_ID;
> +}
> +
>   /* Allocates an area of 'sz' bytes from DPDK.  The memory is zero'ed.
>    *
>    * Unlike xmalloc(), this function can return NULL on failure. */
> @@ -535,11 +553,17 @@ dpdk_rte_mzalloc(size_t sz)
>   }
>   
>   void
> -free_dpdk_buf(struct dp_packet *p)
> +free_dpdk_buf(struct dp_packet *packet)
>   {
> -    struct rte_mbuf *pkt = (struct rte_mbuf *) p;
> +    if (!dpdk_thread_is_pmd()) {
> +        ovs_mutex_lock(&nonpmd_mp_mutex);
> +    }
>   
> -    rte_pktmbuf_free(pkt);
> +    rte_pktmbuf_free(&packet->mbuf);
> +
> +    if (!dpdk_thread_is_pmd()) {
> +        ovs_mutex_unlock(&nonpmd_mp_mutex);
> +    }

This causes build failures for travis as nonpmd_mp_mutex will not be 
held on every path through the code.

https://travis-ci.org/istokes/ovs/jobs/402193295

Ian
>   }
>   
>   static void
> 



More information about the dev mailing list