From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-eopbgr60041.outbound.protection.outlook.com [40.107.6.41]) by dpdk.org (Postfix) with ESMTP id 77FDA1B1DA for ; Sat, 23 Dec 2017 23:36:38 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=y4XKfDuseUhHdrUuiCtRCEB4f2vUrxhsFjUhzzbVOKM=; b=DTTJQXSny4xtXQUytS2hRYOTITQk74/GQOm7P3JzylEYtcYtlmgtC/5QpWk8aQC3mYTpLCeWfwPYdi1M7E0M4dcgDRzTin2IOD8olM59FrvLdlezsHiPViMRFqSkHC4drkmz1U2hr18PeUyFi/GDadkf6FbVhe5RI+9P9HM42Us= Received: from HE1PR0502MB3659.eurprd05.prod.outlook.com (10.167.127.17) by HE1PR0502MB3659.eurprd05.prod.outlook.com (10.167.127.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.345.14; Sat, 23 Dec 2017 22:36:34 +0000 Received: from HE1PR0502MB3659.eurprd05.prod.outlook.com ([fe80::adf6:87de:2761:8e4]) by HE1PR0502MB3659.eurprd05.prod.outlook.com ([fe80::adf6:87de:2761:8e4%13]) with mapi id 15.20.0345.013; Sat, 23 Dec 2017 22:36:34 +0000 From: Matan Azrad To: Neil Horman CC: Thomas Monjalon , "dev@dpdk.org" , Bruce Richardson , "Ananyev, Konstantin" , =?iso-8859-1?Q?Ga=EBtan_Rivet?= , "Wu, Jingjing" Thread-Topic: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership Thread-Index: AQHTaEBDeXaioloFskiHncvIK1YNTKMs3xqAgAANj4CAAX1kAIAC1eJggAA+JgCAABoCUIAByaUAgAAcouCAAFA1gIAAeA4QgABKFgCABNAHAIAAD8IAgBS7KwCAAAoiAIAADZrAgAAcqgCAAAb+4IABKkGAgAIGoqA= Date: Sat, 23 Dec 2017 22:36:34 +0000 Message-ID: References: <20171130123611.GA20914@hmswarspite.think-freely.org> <5212147.QN8ImyqEg2@xps> <20171208123142.GA6955@hmswarspite.think-freely.org> <1567916.dnd6Z652YM@xps> <20171221174304.GB23958@hmswarspite.think-freely.org> <20171221201420.GC23958@hmswarspite.think-freely.org> <20171222142651.GA1222@hmswarspite.think-freely.org> In-Reply-To: <20171222142651.GA1222@hmswarspite.think-freely.org> Accept-Language: en-US, he-IL Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=matan@mellanox.com; x-originating-ip: [85.64.136.190] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; HE1PR0502MB3659; 6:5qRej7J9D8disgAjyI5yyMM00l5nAU3EDbj5WvMK6TCSR7vLmYIy/rEe3v/gCATKBfBQWdiAFM/SE00FPhuOvaEUwCuCRS8sxZsxZFjZIBOH7gHKZA1GWqGbkG5PefAJFAcVCtogGyNY1xEmSw4ZriraCBotg2l/+P1hlAJLXvh7hwen0CArgrbcWItDu8Xk+99SzwcLwzm9P4+E0WW5Ib7ZG7+JCuU9MltJIcfgizujUGsFGZKaPkNOAFMiLykitXfHVzy2JkkCJc856vTq7PQQ214NaMSf17twfxTrnVeBGivNmzyQ0wD7FJhKbapgj2KB5trAMwVn/Pfou8tWvrbCswrmQEeGFQBuwqG94y4=; 5:BjQ0wVpPXoAMywoSS9ZTu92gqB1pCilp/bV5qcv2i9zxJoujozHQ4jtBkLpNXBb7yrPM49JlNFiUdadHmnJTUx1NLbvsaB5N3lcr8ZSVagLHQEDoGDoFDPOExJNHpw2ig8JnDR+/Dl8w5H3j0tAXQGuLdc21JdmZVK13L6Eb1Rg=; 24:RT0asIHuS/vtfYCWswYB2sl2UApYGDFpVZhvWiUmPY49HuADAGE4nEPy8IGA+ZVdFJl/otH2wSX37lXXXGVxmaVCWkyEU/jmOhCh05uX4uc=; 7:kBO8Txk+SZLTNgK90ZrwxGyLMw6JKtjZOpTkTg2FiDoStU33xz+C4IufNe8EZ1fA+eUWuamu/5gVUDLmGLWYTxPjhEjxhrCQ3jCPyiKG9S0O3rIaZYM9Qs1x5ILWlDKdDIFnc6vQimVH+E3rQUUIe/y6tqOM77gvjT0RNIvbjRVhAC+c4cqPd8eQ6ex7uBSfCyL250FF/nyTO+ucb8WWqpI0bjhfO6Lme/jV2KmsYHX00PTGWYiZhQBhGahkLtS8 x-ms-exchange-antispam-srfa-diagnostics: SSOS; x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: a6c5c96e-77bb-4a6e-042b-08d54a559f20 x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(4534020)(4602075)(4627115)(201703031133081)(201702281549075)(48565401081)(5600026)(4604075)(3008032)(2017052603307)(7153060); SRVR:HE1PR0502MB3659; x-ms-traffictypediagnostic: HE1PR0502MB3659: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(60795455431006)(100405760836317)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040470)(2401047)(8121501046)(5005006)(3231023)(3002001)(10201501046)(93006095)(93001095)(6055026)(6041268)(20161123558120)(20161123562045)(20161123560045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(6072148)(201708071742011); SRVR:HE1PR0502MB3659; BCL:0; PCL:0; RULEID:(100000803101)(100110400095); SRVR:HE1PR0502MB3659; x-forefront-prvs: 0530FCB552 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(396003)(376002)(39380400002)(39860400002)(366004)(13464003)(76104003)(51444003)(189003)(199004)(24454002)(3846002)(6436002)(55016002)(6916009)(81156014)(8936002)(14454004)(6116002)(9686003)(5660300001)(97736004)(7736002)(74316002)(54906003)(305945005)(316002)(93886005)(478600001)(86362001)(4326008)(66066001)(76176011)(6506007)(53546011)(102836004)(5250100002)(106356001)(7696005)(99286004)(33656002)(59450400001)(25786009)(2900100001)(2906002)(105586002)(229853002)(81166006)(8676002)(3280700002)(2950100002)(53936002)(6246003)(3660700001)(68736007); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0502MB3659; H:HE1PR0502MB3659.eurprd05.prod.outlook.com; FPR:; SPF:None; PTR:InfoNoRecords; MX:1; A:1; LANG:en; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: HqIGQRcuQPX+q1Y0NIULr1wvj+MBMpMXFLbQ0V7t4WCJXF0YHH70SzpREzi8JGeb2KoXHnEenCvJecu5xXcrqg== spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: a6c5c96e-77bb-4a6e-042b-08d54a559f20 X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Dec 2017 22:36:34.3581 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0502MB3659 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: Sat, 23 Dec 2017 22:36:38 -0000 Hi=20 > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Friday, December 22, 2017 4:27 PM > To: Matan Azrad > Cc: Thomas Monjalon ; dev@dpdk.org; Bruce > Richardson ; Ananyev, Konstantin > ; Ga=EBtan Rivet ; > Wu, Jingjing > Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership >=20 > On Thu, Dec 21, 2017 at 09:57:43PM +0000, Matan Azrad wrote: > > > -----Original Message----- > > > From: Neil Horman [mailto:nhorman@tuxdriver.com] > > > Sent: Thursday, December 21, 2017 10:14 PM > > > To: Matan Azrad > > > Cc: Thomas Monjalon ; dev@dpdk.org; Bruce > > > Richardson ; Ananyev, Konstantin > > > ; Ga=EBtan Rivet > > > ; Wu, Jingjing > > > Subject: Re: [dpdk-dev] [PATCH 2/5] ethdev: add port ownership > > > > > > On Thu, Dec 21, 2017 at 07:37:06PM +0000, Matan Azrad wrote: > > > > Hi > > > > > > > > > > > > > > I think we need to clearly describe what is the > > > > > > > > tread-safety policy in DPDK (especially in ethdev as a firs= t > example). > > > > > > > > Let's start with obvious things: > > > > > > > > > > > > > > > > 1/ A queue is not protected for races with multiple Rx or = Tx > > > > > > > > - no planned change because of performance > > > > > purpose > > > > > > > > 2/ The list of devices is racy > > > > > > > > - to be fixed with atomics > > > > > > > > 3/ The configuration of different devices is thread-safe > > > > > > > > - the configurations are different per-device > > > > > > > > 4/ The configuration of a given device is racy > > > > > > > > - can be managed by the owner of the device > > > > > > > > 5/ The device ownership is racy > > > > > > > > - to be fixed with atomics > > > > > > > > > > > > > > > > What am I missing? > > > > > > > > > > > > > > > > Thank you Thomas for this order. > > > > Actually the port ownership is a good opportunity to redefine the > > > > synchronization rules in ethdev :) > > > > > > > > > > > There is fan out to consider here: > > > > > > > > > > > > > > 1) Is device configuration racy with ownership? That is to > > > > > > > say, can I change ownership of a device safely while another > > > > > > > thread that currently owns it modifies its configuration? > > > > > > > > > > > > If an entity steals ownership to another one, either it is > > > > > > agreed earlier, or it is done by a central authority. > > > > > > When it is acked that ownership can be moved, there should not > > > > > > be any configuration in progress. > > > > > > So it is more a communication issue than a race. > > > > > > > > > > > But if thats the case (specifically that mutual exclusion > > > > > between port ownership and configuration is an exercize left to > > > > > an application developer), then port ownership itself is largely > > > > > meaningless within the dpdk, because the notion of who owns the > > > > > port needs to be codified within the application anyway. > > > > > > > > > > > > > Bruce, As I understand it, only the dpdk entity who took ownership > > > > of a > > > port successfully can configure the device by default, if other dpdk > > > entities want to configure it too they must to be synchronized with > > > the port owner while it is not recommended after the port ownership > integration. > > > > > > > Can you clarify what you mean by "it is not recommended after the > > > port ownership integration"? > > > > Sure, > > The new defining of ethdev synchronization doesn't recommend to > manage a port by 2 different dpdk entities, it can be done but not > recommended. > > > Ok, thats just not what you said above. Your suggestion made it sound li= ke > you thought that after the integration of a port ownership model, that > multiple dpdk entries should not synchronize with one another, which made > no sense. >=20 Ok, I can see a dual meaning in my sentence, sorry for that, I think we agr= ee here. > > > I think there is consensus that the port owner must be the only > > > entitiy to operate on a port (be that configuration/frame rx/tx, or > > > some other operation). > > > > Your question above caused me to think that you don't understand it, Ho= w > can someone who is not the port owner to change the port owner? > > Changing the port owner, like port configuration and port release must = be > done by the owner itself except the case that there is no owner to the po= rt. > > See the API rte_eth_dev_owner_remove. > > > See above, your phrasing I don't think accurately reflected what you mean= t > to convey. Or at least thats not how I read it >=20 > > > Multithreaded operation on a port always means some level of > > > synchronization between application threads and the dpdk library, > > Yes. > > >but I'm not sure why that would be different if we introduced a more > > > concrete notion of port ownership via a new library. > > > > > > > What do you mean by "new library"?, port is an ethdev instance and shou= ld > be managed by ethdev. > > > I'm referring to the port ownership api that you proposed. Apologies, I > should not have used the term "new library", but rather "new api". >=20 > > > > So, for example, if the dpdk entity is an application, the > > application should > > >> take ownership of the port and manage the synchronization of this > > >> port configuration between the application threads and its EAL host > > >> thread callbacks, no other dpdk entity should configure the same > > >> port because they should fail when they try to take ownership of the > same port too. > > > > > Well, failing is one good approach, yes, blocking on port > > > relenquishment could be another. I'd recommend an API with the > following interface: > > > > > > rte_port_ownership_claim(int port_id) - blocks execution of the > > > calling thread until the previous owner releases ownership, then > > > claims it and returns > > > > > > rte_port_ownership_release(int port_id) - releases ownership of > > > port, or returns error if the port was not owned by this execution > > > context > > > > > > rte_port_owernship_try_claim(int port_id) - same as > > > rte_port_ownership_claim, but fails if the port is already owned. > > > > > > That would give the option for both semantics. > > > > I think the current APIs are better because of the next reasons: > > - It defines well who is the owner. > Theres no reason you can't integrate some ownership nonce to the above > API as well, thats easy to add. The relevant part is the ability to excl= ude > those who are not owners (that is to say, block their progress until owne= rship > is released by a preceding entity). >=20 > > - The owner structure includes string to allow better debug and printin= g for > humans. > I've got no problem with any such internals, its really the synchronizati= on that > I'm after. >=20 > > Did you read it? > Yes, I don't see why you would think I hadn't, I think I've been very cle= ar in > my understanding of you initial patch. Have you taken the time to > understand my concerns? > OK, Just it looks like you suggested a new APIs instead of V1 APIs. Your concerns are about the races in port ownership management. I agree with it only after Thomas redefining of port synchronization rules. Mean that if the port creation will be race safe and the new rules will be = documented, the port ownership should be race safe too. =20 > > I can add there an API that wait until the port ownership is released a= s you > suggested in V2. > > > I think that would be good. >=20 > > > > Each dpdk entity which wants to take ownership must to be able to > > > >synchronize the port configuration in its level. > > > > > Can you elaborate on what you mean by level here? Are you > > > envisioning a scheme in which multiple execution contexts might own > > > a port for various non-conflicting purposes? > > > > Sure, > > 1) Application with 2 threads wanting to configure the same port: > > level =3D application code. > > > > a. The main thread should create owner > identifier(rte_eth_dev_owner_new). > > b. The main thread should take the port > ownership(rte_eth_dev_owner_set). > > c. Synchronization between the two threads should be done for the > conflicted configurations by the application. > > d. when the application finishes the port usage it should release the > owner(rte_eth_dev_owner_remove). > > > > 2) Fail-safe PMD manages 2 sub-devices (2 ports) and uses alarm for > hotplug detections which can configure the 2 ports(by the host thread). > > Level =3D fail-safe code. > > a. Application starts the eal and the fail-safe driver probing functio= n is > called. > > b. Fail-safe probes the 2 sub-devices(2 ports are created) and takes > ownership for them. > > c. Failsafe creates itself port and leaves it ownerless. > > d. Failsafe starts the hotplug alarm mechanism. > > e. Application tries to take ownership for all ports and success only > for failsafe port. > > f. Application start to configure the failsafe port asynchronously to > failsafe hotplug alarm. > > g. Failsafe must use synchronization between failsafe alarm callback > code and failsafe configuration APIs called by the application because th= ey > both try to configure the same sub-devices ports. > > h. When fail-safe finishes with the two sub devices it should release > the ports owner. > > > Ok, this I would describe as different use cases rather than parallel > ownership, in that in both cases there is still a single execution contex= t which > is responsible for all aspects of a given port (which is fine with me, I'= m just > trying to be clear in our description of the model). >=20 Agree. Can you find a realistic scenario that a non-single execution entity must t= o manage a port and have problems with the port races synchronization manag= ement?=20 =20 >=20 > > > > > > > > > > > > > > > > 2) Is device configuration racy with device addition/removal? > > > > > > > That is to say, can one thread remove a device while another > > > configures it. > > > > > > > > > > > > I think it is the same as two threads configuring the same > > > > > > device (item 4/ above). It can be managed with port ownership. > > > > > > > > > > > Only if you assert that application is required to have the > > > > > owning port be responsible for the ports deletion, which we can > > > > > say, but that leads to the issue above again. > > > > > > > > > > > > > > As Thomas said in item 2 the port creation must be synchronized by > > > > ethdev > > > and we need to add it there. > > > > I think it is obvious that port removal must to be done only by > > > > the port > > > owner. > > > > > > > You say that, but its obvious to you as a developer who has looked > > > extensively at the code. It may well be less so to a consumer who > > > is not an active member of the community, for instance one who > > > obtains the dpdk via pre-built package. > > > > > > > Yes I can understand, but new rules should be documented and be > adjusted easy easy by the customers, no? > Ostensibly, it should be easy, yes, but in practice its a bit murkier. F= or > instance, What if an application wants to enable packet capture on an > interface via rte_pdump_enable? Does preforming that action require that > the execution context which calls that function own the port before doing > so? Digging through the code suggests to me that it (suprisingly) does n= ot, > because all that function does is set a socket to record packets too, but= I > would have intuitively thought that enabling packet capture would require > turning off the mac filter table in the hardware, and so would have requi= red > ownership >=20 > Conversely, using the same example, calling rte_pdump_init, using the > model from your last patch, would require that the calling execution cont= ext > ensured that , at the time the cli application issued the monnitor reques= t, > that the port be unowned, because the pdump main thread needs to set > rx_tx callbacks on the requested port, which I belive constitutes a > configuration change needing port ownership. >=20 > My point being, I think saying that ownership is easy and obvious isn't > accurate. Agree, as a finger rule all the port relation APIs should require ownership= taking, but it will take time to learn when we don't need to take ownershi= p. > If we are to leave proper synchrnization of access to devices up to > the application, we either need to: >=20 > 1) Assume downstream users are intimately familiar with the code or > 2) Exhaustively document the conditions under which ownership needs to be > held >=20 > (1) is a non starter, and 2 I think is a fairly large undertaking, but un= less we > are willing to codify synchronization in the code explicitly (via locking= ), (2) is > what we have to do. >=20 Agree. Maybe it will be good to document each relevant API if it requires ownershi= p taking or not in .h files, what do you think? =20 > Neil