From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 5627F325B for ; Wed, 6 Dec 2017 01:40:44 +0100 (CET) Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Dec 2017 16:40:41 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.45,366,1508828400"; d="scan'208";a="9419247" Received: from irsmsx101.ger.corp.intel.com ([163.33.3.153]) by FMSMGA003.fm.intel.com with ESMTP; 05 Dec 2017 16:40:32 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.67]) by IRSMSX101.ger.corp.intel.com ([169.254.1.22]) with mapi id 14.03.0319.002; Wed, 6 Dec 2017 00:40:32 +0000 From: "Ananyev, Konstantin" To: Thomas Monjalon , Matan Azrad , "Neil Horman (nhorman@tuxdriver.com)" , "Wu, Jingjing" , =?iso-8859-1?Q?Ga=EBtan_Rivet?= CC: "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership Thread-Index: AdNuKdZBZJ2hOMO6QZ+FK6vy960jQA== Date: Wed, 6 Dec 2017 00:40:31 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772585FAC50B8@irsmsx105.ger.corp.intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDA3MmE4MjEtMTU0Ni00YjljLTk5MWEtNzQ4NjcwZmRiN2RiIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImFVRGVFOHdhemwxT0syMUVlMnNjRHZ1QXdibGRPalZoMzNsVXdycXQrSlE9In0= x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.181] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 2/5] 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: Wed, 06 Dec 2017 00:40:44 -0000 >=20 >=20 > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Tuesday, December 5, 2017 3:50 PM > To: Ananyev, Konstantin > Cc: Matan Azrad ; Neil Horman = ; Ga=EBtan Rivet ; Wu, > Jingjing ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership >=20 > 05/12/2017 16:13, Ananyev, Konstantin: > > > > Hi Thomas, > > > > > Hi, > > > > > I will give my view on locking and synchronization in a different ema= il. > > > Let's discuss about the API here. > > > > > 05/12/2017 12:12, Ananyev, Konstantin: > > > >> From: Matan Azrad [mailto:matan@mellanox.com] > > >> > From: Ananyev, Konstantin [mailto:konstantin.ananyev@intel.com] > > > > > > > > If the goal is just to have an ability to recognize is that dev= ice is managed by > > > > > > another device (failsafe, bonding, etc.), then I think all we n= eed is a pointer > > > > > > to rte_eth_dev_data of the owner (NULL would mean no owner). > > > > > > > > > > I think string is better than a pointer from the next reasons: > > > > > 1. It is more human friendly than pointers for debug and printing= . > > > > > > > > We can have a function that would take an owner pointer and produce= nice > > > > pretty formatted text explanation: "owned by fail-safe device at po= rt X" or so. > > > > > I don't think it is possible or convenient to have such function. > > > > Why do you think it is not possible? >=20 > Because of applications being the owner (discussion below). >=20 > > > Keep in mind that the owner can be an application thread. > > > If you prefer using a single function pointer (may help for > > > atomic implementation), we can allocate an owner structure containing > > > a name as a string to identify the owner in human readable format. > > > Then we just have to set the pointer of this struct to rte_eth_dev_da= ta. > > > > Basically you'd like to have an ability to set something different then > > pointer to rte_eth_dev_data as an owner, right? >=20 > No, it can be a pointer, or an id, I don't care. My question was about a pointer to a specific struct: rte_eth_dev_data. As I understand you want it to be a pointer to something else? Probably to some new struct rte_eth_dev_owner or so... >=20 > > I think this is possible too, just not sure it will useful. > > > > > > What I meant - this api to set/get ownership should be sort of inte= rnal to ethdev layer. > > > > Let say it would be used for failsafe/bonding (any other compound) = device that needs > > > > to own/manage several low-level devices. > > > > So in normal situation user wouldn't need to use that API directly = at all. > > > > > Again, the application may use this API to declare its ownership. > > > > Could you explain that a bit: what would mean 'application declares an = ownership on device'? > > Does it mean that no other application will be allowed to do any contro= l op on that device > > till application will clear its ownership? > > I.E. make sure that at each moment only one particular thread can modif= y device configuration? > > Or would it be totally informal and second application will be free to = ignore it? >=20 > It is an information. > It will avoid a library taking ownership on a port controlled by the app. Ok, let's step back a bit. As I can see there are 2 separate issues/design choices inside rte_ethdev := ) : 1) control API is not MT safe - at any given moment 2 (or more) threads (/p= rocesses) can try to change config of a given device. Right now we leave that sync effort to the upper layer. 2) Even with the same thread 2 (or more) DPDK entities (high level PMD/thir= d party lib, etc.) can try to manage the same device. I.E. the device can be managed by bonding device, but application mistak= enly can try to manage it on its own instead of relying on the bonding devic= e to do so. Again right now we leave it up to the upper layer to keep track which de= vice is managed by what DPDK entity. So what problem are you guys trying to solve with your patch? Is it 1) or 2) below or might be something else? Konstantin >=20 > > If it will be the second one - I personally don't see much point in it. > > If it the first one - then simplest and most straightforward way would = be - > > introduce a mutex (either per device or just per whole rte_eth_dev[]) a= nd force > > each control op to grab it at entrance release at exit. >=20 > No, a mutex does not provide any information. >=20 > > > And anwyway, it may be interesting from an application point of view > > > to be able to list every devices and their internal owners. > > > > Yes sure application is free to call 'get' to retrieve information etc. > > What I am saying for normal operation - application don't have to call = that API. > > I.E. - we don't need to change testpmd, etc. apps because that API was = introduced. >=20 > Yes the ports can stay without any owner if the application does not fill= it. Konstantin