From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 2717011DE for ; Sat, 20 Jan 2018 13:54:21 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Jan 2018 04:54:20 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,386,1511856000"; d="scan'208";a="28045565" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by orsmga002.jf.intel.com with ESMTP; 20 Jan 2018 04:54:19 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.236]) by IRSMSX106.ger.corp.intel.com ([169.254.8.36]) with mapi id 14.03.0319.002; Sat, 20 Jan 2018 12:54:18 +0000 From: "Ananyev, Konstantin" To: Neil Horman , Thomas Monjalon CC: "dev@dpdk.org" , Matan Azrad , "Richardson, Bruce" , Gaetan Rivet , "Wu, Jingjing" Thread-Topic: [dpdk-dev] [PATCH v2 2/6] ethdev: add port ownership Thread-Index: AQHTh5xreoY7ZC2VXUKPLFrmK4AmzKNtEwCwgABGVwCAATRvEIAAOkGAgACPrvCAAIW5gIAE/GJggAAbIoCAAD5+YIAA/qMAgACx27CAAB8nAIAA79jQgAAU2ACAACAHAIAAMFhggAFUAYCAAA4AgIAAMK6AgAAHPICAABa7AIAAG9sAgABZWgCAAF0EgIAAJf+AgAAUvwCAAC5SgIAADCWAgAAUpwCAAB7GgIAABzcAgAAIFQCAABqPgIABHjug Date: Sat, 20 Jan 2018 12:54:18 +0000 Message-ID: <2601191342CEEE43887BDE71AB97725886281227@irsmsx105.ger.corp.intel.com> References: <20180118131017.GA1622@hmswarspite.think-freely.org> <7777073.qS0DmqPron@xps> <20180119174340.GE9519@hmswarspite.think-freely.org> <1959306.0b6nHJGtEC@xps> <20180119194739.GF9519@hmswarspite.think-freely.org> In-Reply-To: <20180119194739.GF9519@hmswarspite.think-freely.org> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDAzYTFlMDEtOWJkZS00N2FmLTg1YWMtNGJkNjY4NDk5ODY3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IkFMOTI1N0Z4RnRORWNtdVpsMXFPcGhrMDhpOWw1aGZzTkVNT2JmOTh3UE09In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 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 12:54:22 -0000 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 >=20 > 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 co= ntext, or can we > > > > > > > define any arbitrary section of code as being an owner? I wo= uld 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 config= ure its > > > > > > port via only one thread. > > > > > > It is consistent with not trying to manage threads synchronizat= ion > > > > > > 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 execut= ion context, > > > > > you're defining the notion of heirarchy, not ownership. That is t= o 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 pare= nts to > > > > > subordinate ports, which only the parent port should access direc= tly. 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/chi= ld relationship > > > > > is globally clear, and this would be much easier, especially if y= ou want to > > > > > continue asserting that the notion of synchronization/exclusion i= s 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. Defin= e 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 opaq= ue id that > > > carries subtle semantic ordering with it. > > > > Sorry, I don't understand. Please could you rephrase? > > >=20 > 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 crea= te a > generic version of the strcture I propose above like so: >=20 > struct rte_obj_heirarchy { > struct rte_obj_heirarchy *children; > struct rte_obj_heirarchy *parent; > void *owner_data; /* optional */ > }; >=20 > And embed that structure in any object you would like to give a represent= ative > heirarchy to, you then have a fairly simple api >=20 > struct rte_obj_heirarchy *heirarchy_alloc(); > bool heirarchy_set(struct rte_obj_heirarchy *parent, struct rte_obj_heira= rcy *child) > void heirarchy_release(struct rte_obj_heirarchy *obj) >=20 > That gives you the privately held list relationship I think you are in pa= rt > looking for (i.e. the ability for a failsafe device to iterate over the p= orts it > is in control of), without the awkwardness of the ordinal priority that t= he > current implementation imposes. >=20 > In summary, if what you want is ownership in the strictest sense of the w= ord > (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 obj= ects > (the failsafe child port example), then the above is what you want. >=20 >=20 > The soution Matan is providing does some of each of these things, but com= es with > very odd side effects >=20 > 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 th= is very > atypical ordinality (once an ownership object is created with owner_new, = any > previously created ownership object will be denied the ability to take ow= nership > of a port) Why is that? As I understand current code: any owner id between 1 and next_owner_id=20 is considered as valid. Konstantin >=20 > It also offers a level of filtering (in that if you can set the ownership= id of > a given set of object to the value X, you can then iterate over them by > iterating over all objects of that type, and filtering on their id), but = it > offers no clear in-memory relationship between parent and children (i.e. = if you > were to look at at an object in a debugger and see that it was owned by o= wner id > X, it would provide you with no indicator of what object held the allocat= ed > ownership object assigned id X. My proposal trades a few bytes of data i= n > exchage for a global clear, definitive heirarcy relationship. And if you= add an > api call and a spinlock, you can easily graft on mutual exclusion here, b= y > blocking access to objects that arent the immediate parent of a given obj= ect. >=20 > Neil >=20 >=20 >=20 > subsequently created object