From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id CF9C1A568 for ; Sat, 20 Jan 2018 15:02:55 +0100 (CET) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 2C8D92093D; Sat, 20 Jan 2018 09:02:55 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute1.internal (MEProxy); Sat, 20 Jan 2018 09:02:55 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-sender :x-me-sender:x-sasl-enc; s=mesmtp; bh=oNCeVHtMoh29i6GIvW7thPauWy QDidkBG2qwwrpH3u4=; b=sLf1waa1xeS1+AXq44BEK5pNK1Jz46ftfpfcOQa2t6 Zepy+LdCAufyeQVQZ7w1029D5czOKxgkvESekmrqnZK04cWfMZL/9J3uGBOmrPdT C17Mq33KUkNE+KUiHisjc7N6/9ia68Mbj/Hgu8gZRIBx+zTd6utWlTUybx4AlwBE 4= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=oNCeVH tMoh29i6GIvW7thPauWyQDidkBG2qwwrpH3u4=; b=mgVmDmcVL0hBMxKAeGBvgo PUPd/r4JPmzZHUItGBu/585pAci3SlSHbKfVLm3M0PobdB9t+XZxX4CSGSjWI1DZ xPRQ1tmR+KYCd6rH4VZAH13+X7omZmQuA6JvOtT196ovReODawSwTcinbu4Ik/E3 5CL2b2oBuOdTNdZ3xbObh3+yTs8mINrHHeoXsjedkRJbZOKHf/jgUkEEWL5FFgos e6+7yz9yAJXdqA6eWAl7ewZDoPGy1zeVipvZPH/wu3gugw7lnYi4X/mUsfK60J+X V0y2Ydp2XkjAGIFsD1lk8h+HWNDt6EVUOXQyLaYkO+1NRO2n+GvfNLEyQ702IkhA == X-ME-Sender: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id C8E2724608; Sat, 20 Jan 2018 09:02:54 -0500 (EST) From: Thomas Monjalon To: "Ananyev, Konstantin" Cc: Neil Horman , dev@dpdk.org, Matan Azrad , "Richardson, Bruce" , Gaetan Rivet , "Wu, Jingjing" Date: Sat, 20 Jan 2018 15:02:19 +0100 Message-ID: <2168714.32XEBxS4BN@xps> In-Reply-To: <2601191342CEEE43887BDE71AB97725886281227@irsmsx105.ger.corp.intel.com> References: <20180118131017.GA1622@hmswarspite.think-freely.org> <20180119194739.GF9519@hmswarspite.think-freely.org> <2601191342CEEE43887BDE71AB97725886281227@irsmsx105.ger.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership 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: Sat, 20 Jan 2018 14:02:56 -0000 20/01/2018 13:54, Ananyev, Konstantin: > Hi Neil, > > > ----- Message----- > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > Sent: Friday, January 19, 2018 7:48 PM > > To: Thomas Monjalon > > Cc: dev@dpdk.org; Matan Azrad ; Richardson, Bruce ; Ananyev, Konstantin > > ; Gaetan Rivet ; Wu, Jingjing > > Subject: Re: [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership > > > > On Fri, Jan 19, 2018 at 07:12:36PM +0100, Thomas Monjalon wrote: > > > 19/01/2018 18:43, Neil Horman: > > > > On Fri, Jan 19, 2018 at 06:17:51PM +0100, Thomas Monjalon wrote: > > > > > 19/01/2018 16:27, Neil Horman: > > > > > > On Fri, Jan 19, 2018 at 03:13:47PM +0100, Thomas Monjalon wrote: > > > > > > > 19/01/2018 14:30, Neil Horman: > > > > > > > > So it seems like the real point of contention that we need to settle here is, > > > > > > > > what codifies an 'owner'. Must it be a specific execution context, or can we > > > > > > > > define any arbitrary section of code as being an owner? I would agrue against > > > > > > > > the latter. > > > > > > > > > > > > > > This is the first thing explained in the cover letter: > > > > > > > "2. The port usage synchronization will be managed by the port owner." > > > > > > > There is no intent to manage the threads synchronization for a given port. > > > > > > > It is the responsibility of the owner (a code object) to configure its > > > > > > > port via only one thread. > > > > > > > It is consistent with not trying to manage threads synchronization > > > > > > > for Rx/Tx on a given queue. > > > > > > > > > > > > > > > > > > > > Yes, in his cover letter, and I contend that notion is an invalid design point. > > > > > > By codifying an area of code as an 'owner', rather than an execution context, > > > > > > you're defining the notion of heirarchy, not ownership. That is to say, > > > > > > you want to codify the notion that there are top level ports that the > > > > > > application might see, and some of those top level ports are parents to > > > > > > subordinate ports, which only the parent port should access directly. If thats > > > > > > all you want to encode, there are far easier ways to do it: > > > > > > > > > > > > struct rte_eth_shared_data { > > > > > > < existing bits > > > > > > > struct rte_eth_port_list { > > > > > > struct rte_eth_port_list *children; > > > > > > struct rte_eth_port_list *parent; > > > > > > }; > > > > > > }; > > > > > > > > > > > > > > > > > > Build an api around a structure like that, so that the parent/child relationship > > > > > > is globally clear, and this would be much easier, especially if you want to > > > > > > continue asserting that the notion of synchronization/exclusion is an exercise > > > > > > left to the application. > > > > > > > > > > Not only Neil. > > > > > An owner can be something else than a port. > > > > > An owner can be an app process (multi-processes). > > > > > An owner can be a library. > > > > > The intent is really to solve the generic problem of which code > > > > > is managing a port. > > > > > > > > > I don't see how this precludes any part of what you just said. Define the > > > > rte_eth_port_list externally to the shared_data struct and allow any object you > > > > want to allocate it, then anything you want to control a heirarchy of ports can > > > > do so without issue, and the structure is far more clear than an opaque id that > > > > carries subtle semantic ordering with it. > > > > > > Sorry, I don't understand. Please could you rephrase? > > > > > > > Sure, I'm saying the fact that you want an owner to be an object > > (library/port/process) rather than strictly an execution context > > (process/thread) doesn't preclude what I'm proposing above. You can create a > > generic version of the strcture I propose above like so: > > > > struct rte_obj_heirarchy { > > struct rte_obj_heirarchy *children; > > struct rte_obj_heirarchy *parent; > > void *owner_data; /* optional */ > > }; > > > > And embed that structure in any object you would like to give a representative > > heirarchy to, you then have a fairly simple api > > > > struct rte_obj_heirarchy *heirarchy_alloc(); > > bool heirarchy_set(struct rte_obj_heirarchy *parent, struct rte_obj_heirarcy *child) > > void heirarchy_release(struct rte_obj_heirarchy *obj) > > > > That gives you the privately held list relationship I think you are in part > > looking for (i.e. the ability for a failsafe device to iterate over the ports it > > is in control of), without the awkwardness of the ordinal priority that the > > current implementation imposes. > > > > In summary, if what you want is ownership in the strictest sense of the word > > (i.e. mutually exclusive access, which I think makes sense), then using a lock > > and flag is really the simplest way to go. If instead what you want is a > > heirarchical relationship where you can iterate over a limited set of objects > > (the failsafe child port example), then the above is what you want. > > > > > > The soution Matan is providing does some of each of these things, but comes with > > very odd side effects > > > > It offers a level of mutual exclusion, in that only one > > object can own another at a time, but does so in a way that introduces this very > > atypical ordinality (once an ownership object is created with owner_new, any > > previously created ownership object will be denied the ability to take ownership > > of a port) > > Why is that? > As I understand current code: any owner id between 1 and next_owner_id > is considered as valid. Yes, Neil sent another email to explain it was a review mistake.