From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf0-f171.google.com (mail-pf0-f171.google.com [209.85.192.171]) by dpdk.org (Postfix) with ESMTP id DEEFD1B991 for ; Fri, 11 May 2018 00:29:40 +0200 (CEST) Received: by mail-pf0-f171.google.com with SMTP id c10-v6so1729298pfi.12 for ; Thu, 10 May 2018 15:29:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=GqhMFhp4K4TO+3O9BFQaxzFamowykLaJwp8lj1V9DSc=; b=N5ZHWJp57RHzONe4dyUFQ6Wrx4ATw2mrVI0PzUV+AufyXUusyk67glqRXQdP4ADkhR VZvttgnQzqnfjXH0HYDIOVk+6VnjByJKhTK0f/dKBTDD4gyYn7ZmhcE1N465OWyGmMef +ihvrrjpvldBzpbc8i8quXJLFQXyK7ozv5dnMsD1p1Xab2XG92gYyTtI/GaEC237UzWP lskO0TA4v8qdzKWEk72i0N3uJNAWk6y0HbWLosEHESCCwDMabUpBIxb+pUU0nhIbw/Ns Y/4WIB/dD4xf3n8L7JwZ4eRiRDYnGgKYwrgCx4S+v4m0IaV28rcrcHRQR71/eFGO0PnL ynaQ== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=GqhMFhp4K4TO+3O9BFQaxzFamowykLaJwp8lj1V9DSc=; b=SqkH/dgkCcJIxf206vWLYCCybIIk9wDjWMZOUBzNf7Aaq0HiIVJ23+lKVVVNBqh5Vv ppLEo2AqSBmlNrk192TjSrOwtqbBPz/JvzWdGYC1xwolZ+aVyf2tYHN1nWhG+ej3EOty JL2YArQ2Sp3ibnlsWk3xwgMlmA5/wSAhysoMBIvmWTe6l28ner7ymu2ZzYbflE94188W i6NH7vLqT1tWdhVCa6Gkeql1VTFDCMyUQ/5fs7Y7LwlVNt45vm2KijzhNuK8Qmlm+bGb hOUxclZXtuwnxYKC/y65Vmx5RLocGr+kjBEkHGMOFULhf7GgEgIVEIgXk+8Kw1dB7x9J 3bPg== X-Gm-Message-State: ALKqPwdPvQpvOXJHHRC7Nnc5biCoxpDTNuCpReR+pBiRyXgfirFcidu/ M4iQPzrm9Kcbfomwe9mpfYy/7A== X-Google-Smtp-Source: AB8JxZrDoIZc+kKpQKcrlM425I4G3q+Mis8Tg+aEaIDjZjl3kngED6doJUSjL10YPb6fu1T7jnzuxA== X-Received: by 2002:a65:5cc6:: with SMTP id b6-v6mr2396291pgt.84.1525991380057; Thu, 10 May 2018 15:29:40 -0700 (PDT) Received: from xeon-e3 (204-195-35-107.wavecable.com. [204.195.35.107]) by smtp.gmail.com with ESMTPSA id h17-v6sm3162691pfn.80.2018.05.10.15.29.39 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 10 May 2018 15:29:39 -0700 (PDT) Date: Thu, 10 May 2018 15:29:37 -0700 From: Stephen Hemminger To: Thomas Monjalon Cc: dev@dpdk.org, Matan Azrad , stable@dpdk.org Message-ID: <20180510152937.78edb13b@xeon-e3> In-Reply-To: <3197799.d7CsaPIZEu@xps> References: <20180509094337.26112-1-thomas@monjalon.net> <20180509094337.26112-8-thomas@monjalon.net> <20180510133356.0c4bacb0@xeon-e3> <3197799.d7CsaPIZEu@xps> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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: Thu, 10 May 2018 22:29:41 -0000 On Fri, 11 May 2018 00:10:19 +0200 Thomas Monjalon wrote: > 10/05/2018 22:33, Stephen Hemminger: > > On Wed, 9 May 2018 11:43:33 +0200 > > Thomas Monjalon wrote: > > > > > > > > +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; > > > +} > > > + > > > > Not sure about this. The code it self is correct, but it creates > > a racy semantic. > > > > If caller doesn't already hold a lock then there is no guarantee that > > the device returned won't be destroyed by some other thread > > It is an old high level design decision in DPDK: > We do not hold a lock during the whole life of a port. > So it is the application responsibility to not mess its own ports. > The consequence is that one port must be managed by only one thread. > > We can discuss the original thread design but it is out of the > scope of this patchset. > > > or that the name was just allocated by some other process. > > It does not say which process allocated the port, yes. > But the name is unique among processes. > So the process knows for sure what to do with the port having this name. For future, I would like to change rte_eth_devices from an array of structures to an array of pointers. Reserving a port could be done with atomic exchange, and keep a bitmap as hint for next free port to choose. When supporting tunnels etc, it makes sense to support lots of ports (like > 16 bit); and devices may come and go. Also, change link state in eth device into full operational state value. That should be enough to cover both tunnel and failsafe usage, and existing state value can go away. The ownership model should also be expressed more as functional operations in the device model. It needs to be used consistently in multiple places, allow more layering and also have more error checks builtin.