From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from rcdn-iport-6.cisco.com (rcdn-iport-6.cisco.com [173.37.86.77]) by dpdk.org (Postfix) with ESMTP id 5CD471AEF0 for ; Tue, 19 Sep 2017 07:31:39 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=2779; q=dns/txt; s=iport; t=1505799099; x=1507008699; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=YdY4xnWonheFjSKuY5+O9TZuW2MP1c0NVLmaA2/tfE8=; b=RBwdaSyWraVLb9hUy3x/+Ah+3OlkbgzTq/wthJ2xVqSIFId0nyJPYB7H EulzRGopfK3ZdNO7EXcN6hxVJu6fq1HQTMv/MiEPblrbpb/uV3cFnXFKK mtSwNYK1NqukFYEn6cPzsvUHOKGwewuZz1Gdi+hzzxej2I0fSZQhYkBMY g=; X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: =?us-ascii?q?A0CZAAD8qsBZ/4UNJK1cGQEBAQEBAQEBA?= =?us-ascii?q?QEBBwEBAQEBg1pkbicHjg6Ra5YlghIKI4UYAoRNPxgBAgEBAQEBAQFrKIUYAQE?= =?us-ascii?q?BBDo/DAQCAQgRBAEBAR4JBzIUCQgCBA4FCIorEKt6iygBAQEBAQEBAQEBAQEBA?= =?us-ascii?q?QEBAQEBAQEdgyuCAoFQgWODKIUqhUEFoQsCh1qMcYJ3kAuVCgIRGQGBOAEfOIE?= =?us-ascii?q?NdxWFYhyBZ3YBh0uBDwEBAQ?= X-IronPort-AV: E=Sophos;i="5.42,416,1500940800"; d="scan'208";a="297145984" Received: from alln-core-11.cisco.com ([173.36.13.133]) by rcdn-iport-6.cisco.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 19 Sep 2017 05:31:38 +0000 Received: from XCH-RCD-009.cisco.com (xch-rcd-009.cisco.com [173.37.102.19]) by alln-core-11.cisco.com (8.14.5/8.14.5) with ESMTP id v8J5VciX029846 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Tue, 19 Sep 2017 05:31:38 GMT Received: from xch-rcd-007.cisco.com (173.37.102.17) by XCH-RCD-009.cisco.com (173.37.102.19) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Tue, 19 Sep 2017 00:31:37 -0500 Received: from xch-rcd-007.cisco.com ([173.37.102.17]) by XCH-RCD-007.cisco.com ([173.37.102.17]) with mapi id 15.00.1263.000; Tue, 19 Sep 2017 00:31:37 -0500 From: "John Daley (johndale)" To: Thomas Monjalon CC: Ferruh Yigit , "dev@dpdk.org" , Sergio Gonzalez Monroy Thread-Topic: [PATCH] net/enic: fix multi-process operation Thread-Index: AQHTKy/6U2XhmEhrXUae/JNfSKlKIKK7hz2AgAAQQICAABQhoA== Date: Tue, 19 Sep 2017 05:31:37 +0000 Message-ID: References: <20170911185833.11458-1-johndale@cisco.com> <488ca130-7a8e-223f-5b9e-50bdab9b93f2@intel.com> <4171800.AgbnscgTn2@xps> In-Reply-To: <4171800.AgbnscgTn2@xps> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.19.145.150] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] net/enic: fix multi-process operation 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: Tue, 19 Sep 2017 05:31:40 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas@monjalon.net] > Sent: Monday, September 18, 2017 3:25 PM > To: John Daley (johndale) > Cc: Ferruh Yigit ; dev@dpdk.org; Sergio Gonzalez > Monroy > Subject: Re: [PATCH] net/enic: fix multi-process operation >=20 > 18/09/2017 23:27, Ferruh Yigit: > > On 9/11/2017 7:58 PM, John Daley wrote: > > > - Use rte_malloc() instead of malloc() for the per device 'vdev' stru= cture > > > so that it can be shared across processes. > > > - Only initialize the device if the process type is RTE_PROC_PRIMARY > > > - Only allow the primary process to do queue setup, start/stop, promi= sc > > > allmulticast, mac add/del, mtu. > [...] > > > --- a/drivers/net/enic/enic_ethdev.c > > > +++ b/drivers/net/enic/enic_ethdev.c > > > @@ -142,6 +142,10 @@ enicpmd_dev_filter_ctrl(struct rte_eth_dev > > > *dev, static void enicpmd_dev_tx_queue_release(void *txq) { > > > ENICPMD_FUNC_TRACE(); > > > + > > > + if (rte_eal_process_type() !=3D RTE_PROC_PRIMARY) > > > + return; > > > + > > > > Hi John, > > > > I am not sure about these updates. Agree that these functions should > > know process type, but all others PMDs don't do this. I looked at mlx5 and it does something similar with its mlx5_is_secondary()= in functions that modify fields in its shared private structure. > > > > Added a few more people for comment, but as far I understand its > > application responsibility to NOT call these functions if it is > > secondary process. > > > > For device init/uninit, that is part of eal_init() and have to be > > called both for primary and secondary process and PMD needs to protect > > it, for other functions application's responsibility. I put the checks into the PMD because I didn't want to trust the app and I = didn't see checks in the library functions. I assumed that is why it was do= ne in mlx5. I was afraid that the secondary may try to write fields in the = shared structure and cause some silent bad behavior, like if secondary sets= the MTU then the primary does, then secondary assumes it was different tha= n what it is, or something like that. >=20 > Yes for now it is the policy. > But it is a gray area and it could be clearer with my "ownership proposal= ": > http://dpdk.org/ml/archives/dev/2017-September/074656.html > A secondary process could manage the ports it owns. I think the ownership concept would help make DPDK more flexible and potent= ially cleaner. Perhaps ownership checks could be pushed the lib functions, = like rte_eth_dev_set_mtu(), and remove all such checks in the PMD(s).=20 >=20 > Feel free to comment the proposal.