From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00042.outbound.protection.outlook.com [40.107.0.42]) by dpdk.org (Postfix) with ESMTP id B937F1BDDE for ; Tue, 26 Jun 2018 18:54:36 +0200 (CEST) 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:X-MS-Exchange-SenderADCheck; bh=VTKUPKtphIJt6zRUA1fEVQiw92eUr9VUEDL1x8bopRk=; b=xDWAHS622du1qt+MxsAJkR25CV/4dBEpgutlWOkBFC4XPJLeSSSe1efwx3Xt95ZfO5Ck41V3N578ejUi4tguf0pI4pKDZ6X4uR6oMPkJscH2SoXECuxz0fc2oYBMdPosWECclvfENehduKr3dRv3oBGPpoyO6sCZkZmw3tcMdrE= Received: from VI1PR0501MB2608.eurprd05.prod.outlook.com (10.168.137.20) by VI1PR0501MB2111.eurprd05.prod.outlook.com (10.167.196.15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.863.19; Tue, 26 Jun 2018 16:54:33 +0000 Received: from VI1PR0501MB2608.eurprd05.prod.outlook.com ([fe80::9dd0:9bdb:fd59:b615]) by VI1PR0501MB2608.eurprd05.prod.outlook.com ([fe80::9dd0:9bdb:fd59:b615%7]) with mapi id 15.20.0884.024; Tue, 26 Jun 2018 16:54:33 +0000 From: Matan Azrad To: "Zhang, Qi Z" , Thomas Monjalon , "Burakov, Anatoly" CC: "Ananyev, Konstantin" , "dev@dpdk.org" , "Richardson, Bruce" , "Yigit, Ferruh" , "Shelton, Benjamin H" , "Vangati, Narender" Thread-Topic: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to release port in local process Thread-Index: AQHUDRyBOOZkWOgLrEmS0dpaVlkVNqRygFFggAAIlQCAADW0kA== Date: Tue, 26 Jun 2018 16:54:33 +0000 Message-ID: References: <20180607123849.14439-1-qi.z.zhang@intel.com> <20180626070832.3055-1-qi.z.zhang@intel.com> <20180626070832.3055-4-qi.z.zhang@intel.com> <039ED4275CED7440929022BC67E706115323E812@SHSMSX103.ccr.corp.intel.com> In-Reply-To: <039ED4275CED7440929022BC67E706115323E812@SHSMSX103.ccr.corp.intel.com> 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; VI1PR0501MB2111; 7:Msk72eLySAAs0jwMekVmOg8XVUM8PDyvgcmZ96S1d9ErmXAz6ehIgjClGGSbwk8JS8cGCr1r296uXJIPtXX1yTRcYPiQ/aGqBpJpRfPQrfyMynDOGlsJF35126Q2Jlp5hMsucUWytkvlx40e0cWHnW6FQEnfjMrq3fM9/c7EkMnHGve3frAW3pGbO0WpddStiz1xEx1zFngcIaxpErVF/1z6yMVU/MJtmyTDLXWf7jNkQFJe0C6QBskeUEAUYQNi x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: b50e2305-1c50-469e-841e-08d5db857e36 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652020)(8989117)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(5600026)(711020)(48565401081)(2017052603328)(7153060)(7193020); SRVR:VI1PR0501MB2111; x-ms-traffictypediagnostic: VI1PR0501MB2111: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(60795455431006)(228905959029699); x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(3231254)(944501410)(52105095)(93006095)(93001095)(10201501046)(3002001)(6055026)(149027)(150027)(6041310)(20161123558120)(20161123560045)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(6072148)(201708071742011)(7699016); SRVR:VI1PR0501MB2111; BCL:0; PCL:0; RULEID:; SRVR:VI1PR0501MB2111; x-forefront-prvs: 071518EF63 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(346002)(39860400002)(376002)(136003)(396003)(13464003)(189003)(199004)(229853002)(478600001)(76176011)(86362001)(26005)(186003)(2906002)(5250100002)(66066001)(99286004)(110136005)(6436002)(14444005)(5024004)(54906003)(3846002)(316002)(53546011)(6506007)(102836004)(7696005)(33656002)(6116002)(476003)(4326008)(25786009)(9686003)(11346002)(486006)(53936002)(6246003)(14454004)(93886005)(55016002)(81156014)(105586002)(81166006)(106356001)(68736007)(7736002)(446003)(305945005)(8936002)(74316002)(5660300001)(97736004)(256004)(2900100001); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR0501MB2111; H:VI1PR0501MB2608.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: K+91kSfsBgMeI0hVHn0Xn5y9lznVGuKnmjunxVJayanWamVAuPsrfPVJUw9F3SUtRvrsn3zs3djniabiE2CyI/JUNqyqsCJuD2q23miZ9EnDqUlBAIhHUAwouv/E8STC4W24cGx+dDGd5NAD8ZGrzQsk4ym2rgAu/NkldTNCuaxKBhRrMI2ox+MotZLQWk5svpssYgoTc27QVojsGSr/RKFUN71B44MWQnUH9kXIOmh0BneIlaS9JfT+RVaLuNMuNFmAZ/smSQqc3LmGVfM2JNqxun2h6owR+7VGrZiZPis4GPrrvIPEBwhMX9fHhlH4LimLrWc34sttmK3MEHGPpwZTUOkG2kBJ5UH+i05OBYg= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: b50e2305-1c50-469e-841e-08d5db857e36 X-MS-Exchange-CrossTenant-originalarrivaltime: 26 Jun 2018 16:54:33.6022 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0501MB2111 Subject: Re: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to release port in local process 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, 26 Jun 2018 16:54:37 -0000 Hi Zhang From: Zhang, Qi Z=20 > Sent: Tuesday, June 26, 2018 4:30 PM > To: Matan Azrad ; Thomas Monjalon > ; Burakov, Anatoly > Cc: Ananyev, Konstantin ; dev@dpdk.org; > Richardson, Bruce ; Yigit, Ferruh > ; Shelton, Benjamin H > ; Vangati, Narender > > Subject: RE: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to release = port > in local process >=20 >=20 >=20 > > -----Original Message----- > > From: Zhang, Qi Z > > Sent: Tuesday, June 26, 2018 9:29 PM > > To: 'Matan Azrad' ; Thomas Monjalon > > ; Burakov, Anatoly > > Cc: Ananyev, Konstantin ; dev@dpdk.org; > > Richardson, Bruce ; Yigit, Ferruh > > ; Shelton, Benjamin H > > ; Vangati, Narender > > > > Subject: RE: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to > > release port in local process > > > > Hi Matan: > > > > > -----Original Message----- > > > From: Matan Azrad [mailto:matan@mellanox.com] > > > Sent: Tuesday, June 26, 2018 7:50 PM > > > To: Zhang, Qi Z ; Thomas Monjalon > > > ; Burakov, Anatoly > > > Cc: Ananyev, Konstantin ; > > > dev@dpdk.org; Richardson, Bruce ; Yigit, > > > Ferruh ; Shelton, Benjamin H > > > ; Vangati, Narender > > > > > > Subject: RE: [dpdk-dev] [PATCH v4 03/24] ethdev: add function to > > > release port in local process > > > > > > Hi Qi > > > > > > Please see comments\questions.. > > > > > > From: Qi Zhang > > > > Add driver API rte_eth_release_port_private to support the > > > > requirement that an ethdev only be released on secondary process, > > > > so only local state be set to unused , share data will not be > > > > reset so primary process can > > > still use it. > > > > > > > > Signed-off-by: Qi Zhang > > > > --- > > > > > > > > lib/librte_ethdev/rte_ethdev.c | 24 +++++++++++++++++++++--= - > > > > lib/librte_ethdev/rte_ethdev_driver.h | 13 +++++++++++++ > > > > 2 files changed, 34 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/lib/librte_ethdev/rte_ethdev.c > > > > b/lib/librte_ethdev/rte_ethdev.c index a9977df97..205b2ee33 100644 > > > > --- a/lib/librte_ethdev/rte_ethdev.c > > > > +++ b/lib/librte_ethdev/rte_ethdev.c > > > > @@ -359,6 +359,23 @@ rte_eth_dev_attach_secondary(const char > > > *name) } > > > > > > > > int > > > > +rte_eth_dev_release_port_private(struct rte_eth_dev *eth_dev) { > > > > + if (eth_dev =3D=3D NULL) > > > > + return -EINVAL; > > > > + > > > > + _rte_eth_dev_callback_process(eth_dev, RTE_ETH_EVENT_DESTROY, > > > > NULL); > > > > + > > > > + rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock); > > > > > > I don't think you need the lock here because there is not shared > > > data to > > reset. > > > > OK, will remove this. > > > > > > > > > + eth_dev->state =3D RTE_ETH_DEV_UNUSED; > > > > + > > > > + rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int > > > > rte_eth_dev_release_port(struct rte_eth_dev *eth_dev) { > > > > if (eth_dev =3D=3D NULL) > > > > @@ -370,9 +387,10 @@ rte_eth_dev_release_port(struct rte_eth_dev > > > > *eth_dev) > > > > > > > > rte_spinlock_lock(&rte_eth_dev_shared_data->ownership_lock); > > > > > > > > - eth_dev->state =3D RTE_ETH_DEV_UNUSED; > > > > - > > > > - memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); > > > > + if (eth_dev->state !=3D RTE_ETH_DEV_UNUSED) { > > > > > > Can you explain why not to release the shared data in case of > > > locally unused port? > > > > Now, we have rte_eth_dev_release_port be called in rte_eth_dev_detach, > > its redundant for some driver, because they already will release port > > in > > dev->remove, but I'm not sure if it is true for all drivers. rte_eth_dev_release_port() like detach port should be called only by the po= rt owner, by default is the application, so it should not be called by the = PMD, maybe need to fix PMDs which does it. > > > > On secondary process, when detach a device, it is possible that > > rte_eth_dev_release_port be called after > > rte_eth_dev_release_port_local , so it will reset share data which is > > not expected, since the primary process will fail on detach it because > rte_eth_dev_allocated will return NULL. > > > > There could be other way, but current implementation help to reuse > > exist code., I can't add more comment on this change in v5 >=20 > I can add more comment This solution is suffering from much more complexity, The port id should be the same in all the processes, so you can pass it by = the mp channel and not use rte_eth_dev_allocated. I see it like this: The initiator process asks from all the other processes to detach a port by= port id, all the other processes using rte_eth_dev_release_port_local and = sending ack back to the initiator, Then, the initiator using the non-local release to release the port. =20 Am I missing something? > > > > Regards > > Qi > > > > > > > > > + eth_dev->state =3D RTE_ETH_DEV_UNUSED; > > > > + memset(eth_dev->data, 0, sizeof(struct rte_eth_dev_data)); > > > > + } > > > > > > > > rte_spinlock_unlock(&rte_eth_dev_shared_data->ownership_lock); > > > > > > > > diff --git a/lib/librte_ethdev/rte_ethdev_driver.h > > > > b/lib/librte_ethdev/rte_ethdev_driver.h > > > > index c9c825e3f..49c27223d 100644 > > > > --- a/lib/librte_ethdev/rte_ethdev_driver.h > > > > +++ b/lib/librte_ethdev/rte_ethdev_driver.h > > > > @@ -70,6 +70,19 @@ int rte_eth_dev_release_port(struct rte_eth_dev > > > > *eth_dev); > > > > > > > > /** > > > > * @internal > > > > + * Release the specified ethdev port in local process, only set > > > > +to ethdev > > > > + * state to unused, but not reset share data since it assume > > > > +other process > > > > + * is still using it, typically it is called by secondary process. > > > > + * > > > > + * @param eth_dev > > > > + * The *eth_dev* pointer is the address of the *rte_eth_dev* struc= ture. > > > > + * @return > > > > + * - 0 on success, negative on error > > > > + */ > > > > +int rte_eth_dev_release_port_private(struct rte_eth_dev > > > > +*eth_dev); > > > > + > > > > +/** > > > > + * @internal > > > > * Release device queues and clear its configuration to force the = user > > > > * application to reconfigure it. It is for internal use only. > > > > * > > > > -- > > > > 2.13.6