From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by dpdk.org (Postfix) with ESMTP id A42641B3CD for ; Wed, 9 May 2018 14:21:32 +0200 (CEST) Received: by mail-wm0-f65.google.com with SMTP id f8-v6so27451539wmc.4 for ; Wed, 09 May 2018 05:21:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=hg9aeNWvjj194oY5qbWR0pEboX/+D5YoV4BnMGJUfak=; b=EeNeztFD4bxzp+Nm+h30K5c4h8x/Hrjk6NxmEdNrcmRL8nL36Bk1oww79frK40Kthl HfWdsCbj0CUorisWNGKYhHHj1uroh80TeNq9gA5B+NXsFGzW9SEm3yPrlkSoxYtmu2M0 rxaWB/e+CqXk2Eg7gr5IM9LaX6TDFarV1+UtCU0gCqFxAhBAcz2COWBJFgN4AjHMiWe1 aHq9/39GtzEGKEXYG7C1eyQm+uwtAKKt4/LirksTep1e9eXYUuykoPyfMGV4bSucaFVn paQK/HrdDk6FwgbmppRJ2C7avPA61o2bay3JerprHPhvZWcSb53IR/JAWMBKG4GB6znp 4fuw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=hg9aeNWvjj194oY5qbWR0pEboX/+D5YoV4BnMGJUfak=; b=YvCBHCKpwVQ+97KQT5xuV2Z0QgXy1UUzxi12NUzpwb0tt4WWi2cDpS56yzDjm8m4CQ /dGm9GaPQ3YiLraRVJVa4yaRNuzrW/+u3PKszT8pPux9rMcOO6IUuqOlQ2hdQP7H9Fk6 Qqr8V5lnPrV80hHEh6pfII8iKquSOEKfP1cf67O9z+rN9gro4bzlHJfDV36ZLeSJjTDv Krw36b3mHb+m0WGwbmCMWaLfgXhrHeRi1HaPuZnAyyEbEC1INdoKre3ip21wtFp83FMX KfHRqh0sFHRQSD6+znO4K2sh0O7eDi76i3vvhW0F5M4Gqr2A3sxmKW21kJArZlUUF61G xD1w== X-Gm-Message-State: ALQs6tBx42M2ALue/BjG6iSzPknifhDNSvGV649plyjIpivPBKqhdQeg +Jd4vZRCo9XVKceLlzWfKbx5sQ== X-Google-Smtp-Source: AB8JxZo6vlZ6li5H0iG2FineUYJjNn7L8+1mAGkS2nNO62C3E/XH7jJ/TlRK3Ds0AoSBks1a6tQdRA== X-Received: by 2002:a50:f695:: with SMTP id d21-v6mr58078347edn.142.1525868492317; Wed, 09 May 2018 05:21:32 -0700 (PDT) Received: from bidouze.vm.6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id p21-v6sm14760285edq.33.2018.05.09.05.21.31 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 09 May 2018 05:21:31 -0700 (PDT) Date: Wed, 9 May 2018 14:21:17 +0200 From: =?iso-8859-1?Q?Ga=EBtan?= Rivet To: Thomas Monjalon Cc: dev@dpdk.org, Matan Azrad , stable@dpdk.org Message-ID: <20180509122116.ljistjpz3dfljdqo@bidouze.vm.6wind.com> References: <20180509094337.26112-1-thomas@monjalon.net> <20180509094337.26112-8-thomas@monjalon.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180509094337.26112-8-thomas@monjalon.net> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [dpdk-dev] [PATCH 07/11] ethdev: add lock to port allocation check X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 May 2018 12:21:32 -0000 Hi, On Wed, May 09, 2018 at 11:43:33AM +0200, Thomas Monjalon wrote: > From: Matan Azrad > > 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@dpdk.org > > Signed-off-by: Matan Azrad > Acked-by: Thomas Monjalon > --- > 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. > { > unsigned i; > > @@ -240,6 +240,22 @@ rte_eth_dev_allocated(const char *name) > return NULL; > } > > +struct rte_eth_dev * > +rte_eth_dev_allocated(const char *name) > +{ > + struct rte_eth_dev *ethdev; > + > + rte_eth_dev_shared_data_prepare(); > + > + rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock); > + > + ethdev = rte_eth_dev_allocated_lock_free(name); > + > + rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock); > + > + return ethdev; > +} > + > static uint16_t > rte_eth_dev_find_free_port(void) > { > @@ -286,7 +302,7 @@ rte_eth_dev_allocate(const char *name) > goto unlock; > } > > - if (rte_eth_dev_allocated(name) != NULL) { > + if (rte_eth_dev_allocated_lock_free(name) != NULL) { > ethdev_log(ERR, > "Ethernet Device with name %s already allocated!", > name); > -- > 2.16.2 > -- Gaëtan Rivet 6WIND