From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id A96A11B1AD for ; Wed, 24 Jan 2018 19:30:31 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Jan 2018 10:30:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,408,1511856000"; d="scan'208";a="25145647" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga001.fm.intel.com with ESMTP; 24 Jan 2018 10:30:29 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.236]) by irsmsx110.ger.corp.intel.com ([163.33.3.25]) with mapi id 14.03.0319.002; Wed, 24 Jan 2018 18:30:28 +0000 From: "Ananyev, Konstantin" To: Thomas Monjalon CC: Matan Azrad , =?iso-8859-1?Q?Ga=EBtan_Rivet?= , "Wu, Jingjing" , "dev@dpdk.org" , Neil Horman , "Richardson, Bruce" Thread-Topic: [dpdk-dev] [PATCH v3 7/7] app/testpmd: adjust ethdev port ownership Thread-Index: AQHTlHB0mIDt6Y+2sU2Ne78ePy1VDqOB9FHwgAC4R4CAAKpZAA== Date: Wed, 24 Jan 2018 18:30:27 +0000 Message-ID: <2601191342CEEE43887BDE71AB9772588628308D@irsmsx105.ger.corp.intel.com> References: <1515318351-4756-1-git-send-email-matan@mellanox.com> <3663694.426ICJToDY@xps> <2601191342CEEE43887BDE71AB9772588628298E@irsmsx105.ger.corp.intel.com> <1682675.gfxK9aSxec@xps> In-Reply-To: <1682675.gfxK9aSxec@xps> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNDFiNWU3MmUtY2Y5YS00ZGU1LThlNTQtYTE0NDgwZjY1ZmY0IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IldJeTFLSHpIU0tzOTNzNDNLVDJESGNyM2s3Vm5LRjB2Vm1rY0UxaE54K1U9In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v3 7/7] app/testpmd: adjust ethdev 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, 24 Jan 2018 18:30:32 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Wednesday, January 24, 2018 8:10 AM > To: Ananyev, Konstantin > Cc: Matan Azrad ; Ga=EBtan Rivet ; Wu, Jingjing ; > dev@dpdk.org; Neil Horman ; Richardson, Bruce > Subject: Re: [dpdk-dev] [PATCH v3 7/7] app/testpmd: adjust ethdev port ow= nership >=20 > 23/01/2018 22:18, Ananyev, Konstantin: > > > > > > 23/01/2018 16:18, Ananyev, Konstantin: > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konst= antin > > > > > From: Thomas Monjalon [mailto:thomas@monjalon.net] > > > > > > 23/01/2018 14:34, Ananyev, Konstantin: > > > > > > > If that' s the use case, then I think you need to set device = ownership at creation time - > > > > > > > inside dev_allocate(). > > > > > > > Again that would avoid such racing conditions inside testpmd. > > > > > > > > > > > > The devices must be allocated at a low level layer. > > > > > > > > > > No one arguing about that. > > > > > But we can provide owner id information to the low level. > > > > > > Sorry, you did not get it. > > > > Might be. > > > > > We cannot provide owner id at the low level > > > because it is not yet decided who will be the owner > > > before the port is allocated. > > > > Why is that? > > What prevents us decide who will manage that device *before* allocating= port of it? > > IMO we do have all needed information at that stage. >=20 > We don't have the information. At that point we do have dev name and all parameters, right? Plus we do have blacklist/whitelist, etc. So what else are we missing to make the decision at that point?=20 > It is a new device, it is matched by a driver which allocates a port. > I don't see where the higher level can interact here. > And even if you manage a trick, the higher level needs to read the port > informations to decide the ownership. Could you specify what particular port information it needs? >=20 > > > > > > When a new device appears (hotplug), an ethdev port should be a= llocated > > > > > > automatically if it passes the whitelist/blacklist policy test. > > > > > > Then we must decide who will manage this device. > > > > > > I suggest notifying the DPDK libs first. > > > > > > So a DPDK lib or PMD like failsafe can have the priority to tak= e the > > > > > > ownership in its notification callback. > > > > > > > > > > Possible, but seems a bit overcomplicated. > > > > > Why not just: > > > > > > > > > > Have a global variable process_default_owner_id, that would be in= it once at startup. > > > > > Have an LTS variable default_owner_id. > > > > > It will be used by rte_eth_dev_allocate() caller can set dev->own= er_id at creation time, > > > > > so port allocation and setting ownership - will be an atomic oper= ation. > > > > > At the exit rte_eth_dev_allocate() will always reset default_owne= r_id=3D0: > > > > > > > > > > rte_eth_dev_allocate(...) > > > > > { > > > > > lock(owner_lock); > > > > > > > > > > owner =3D RTE_PER_LCORE(default_owner_id); > > > > > if (owner =3D=3D 0) > > > > > owner =3D process_default_owner_id; > > > > > set_owner(port, ..., owner); > > > > > unlock(owner_lock); > > > > > RTE_PER_LCORE(default_owner_id) =3D 0; > > > > > > > > Or probably better to leave default_owner_id reset to the caller. > > > > Another thing - we can use same LTS variable in all control ops to > > > > allow/disallow changing of port configuration based on ownership. > > > > Konstantin > > > > > > > > > } > > > > > > > > > > So callers who don't need any special ownership - don't need to d= o anything. > > > > > Special callers (like failsafe) can set default_owenr_id just bef= ore calling hotplug > > > > > handling routine. > > > > > > No, hotplug will not be a routine. > > > I am talking about real hotplug, like a device which appears in the V= M. > > > This new device must be handled by EAL and probed automatically if > > > comply with whitelist/blacklist policy given by the application or us= er. > > > Real hotplug is asynchronous. > > > > By 'asynchronous' here you mean it would be handled in the EAL interrup= t thread > > or something different? >=20 > Yes, we receive an hotplug event which is processed in the event thread. >=20 > > Anyway, I suppose you do need a function inside DPDK that will do the = actual work in response > > on hotplug event, right? >=20 > Yes Ok, btw why that function has to be always called from interrupt thread? Why not to allow user to decide? Some apps have their own thread dedicated for control ops (like testpmd) For them it might be plausible to call that function from their own control= thread context. Konstantin >=20 > > That's what I refer to as 'hotplug routine' above. > > > > > We will just receive notifications that it appeared. > > > > > > Note: there is some temporary code in failsafe to manage some hotplug= . > > > This code must be removed when it will be properly handled in EAL. > > > > Ok, if it is just a temporary code, that would be removed soon - > > then it definitely seems wrong to modify tespmd (or any other user app) > > to comply with that temporary solution. >=20 > It will be modified when EAL hotplug will be implemented. >=20 > However, the ownership issue will be the same: > we don't know the owner when allocating a port.