[dpdk-dev] [PATCH 07/11] ethdev: add lock to port allocation check

[Thomas Monjalon]

09/05/2018 14:21, Gaëtan Rivet:
> Hi,
> 
> On Wed, May 09, 2018 at 11:43:33AM +0200, Thomas Monjalon wrote:
> > From: Matan Azrad <matan at mellanox.com>
> > 
> > When comparing the port name, there can be a race condition with
> > a thread allocating a new port and writing the name at the same time.
> > It can lead to match with a partial name by error.
> > 
> > The check of the port is now considered as a critical section
> > protected with locks.
> > 
> > This fix will be even more required for multi-process when the
> > port availability will rely only on the name, in a following patch.
> > 
> > Fixes: 84934303a17c ("ethdev: synchronize port allocation")
> > Cc: stable at dpdk.org
> > 
> > Signed-off-by: Matan Azrad <matan at mellanox.com>
> > Acked-by: Thomas Monjalon <thomas at monjalon.net>
> > ---
> >  lib/librte_ethdev/rte_ethdev.c | 22 +++++++++++++++++++---
> >  1 file changed, 19 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
> > index ae86d0ba7..357be2dca 100644
> > --- a/lib/librte_ethdev/rte_ethdev.c
> > +++ b/lib/librte_ethdev/rte_ethdev.c
> > @@ -227,8 +227,8 @@ rte_eth_dev_shared_data_prepare(void)
> >  	rte_spinlock_unlock(&rte_eth_shared_data_lock);
> >  }
> >  
> > -struct rte_eth_dev *
> > -rte_eth_dev_allocated(const char *name)
> > +static struct rte_eth_dev *
> > +rte_eth_dev_allocated_lock_free(const char *name)
> 
> A suggestion about the naming here.
> Reading subsequent patches, we can see this function being used during
> ethdev allocation routines. The _lock_free suffix is a little
> misleading, as for an instant one can think that there is something
> being freed about an allocated ethdev lock.
> 
> I would suggest
> 
>   * rte_eth_dev_allocated_nolock
>     or
>   * rte_eth_dev_allocated_lockless
>     (or even rte_eth_lockless_dev_allocated)
> 
> instead.

Good suggestions. I vote for rte_eth_dev_allocated_nolock.
Thanks