From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by dpdk.org (Postfix) with ESMTP id 748B15F36 for ; Mon, 29 Apr 2019 22:30:13 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.nyi.internal (Postfix) with ESMTP id 34F9C13BB1; Mon, 29 Apr 2019 16:30:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 29 Apr 2019 16:30:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=bqRImLHi8EVi6xnaHhpgcBxY47xAzrYCu/TxMcLVmmk=; b=BGFADETE3UDL UmM5P4Ir0/m1YV92IumVRcAmKfmpFkPxa7dScVihIe2l65U6ugvCBE6oasUaxQ4j ozBRbvILW1VnAXeGK0lnDiDZuHikopwrKRODiLEhsAeKTEMf3qoZw+h9zRJU3oKm DBRke/Ilo8gdS2OeWxKhWJnhsZ4ZVKY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=bqRImLHi8EVi6xnaHhpgcBxY47xAzrYCu/TxMcLVm mk=; b=d9wJmyVBqOkz5jSU2ZvznpOnpNECuJXBS9OleEBI8GLI97vr7lLf8nrLq HAHe6IUMjrmJW/8EWrDzWsaSBo+YvWXuNF0DlEuhFM6hwVlHcd5HMxefX85xJIA0 b7NdGJAh4UWFrrpRMKO2ji7SBzN9fO/dKlMGYC1xc9EByV+BiMniZv+7ZZL4Ba1V IocuNz4lt3TMW/iZs2ENcH8HUhTeoEPPytxEmgJJiaAKVbdFbirVhrRre/ViI5nT Uwb78F+YprX0knh/w0C8GL/5ufUQvKfB2tpUzOPDTfI5CjapNtOT/calF4Gx8Yq8 q5VRd4Sf6OK22K7sv/Mxa/HYMN9kQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddriedvgdduheduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecuff homhgrihhnpeguphgukhdrohhrghenucfkphepjeejrddufeegrddvtdefrddukeegnecu rfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtne cuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id AD715103CE; Mon, 29 Apr 2019 16:30:01 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit Cc: "John W. Linville" , Xiaolong Ye , Qi Zhang , Shepard Siegel , Ed Czeck , John Miller , Igor Russkikh , Pavel Belous , Allain Legacy , Matt Peters , Ravi Kumar , Rasesh Mody , Shahed Shaikh , Ajit Khaparde , Somnath Kotur , Chas Williams , Rahul Lakkireddy , Hemant Agrawal , Shreyansh Jain , Wenzhuo Lu , Marcin Wojtas , Michal Krawczyk , Guy Tzalik , Evgeny Schemeilin , Gagandeep Singh , Pankaj Chauhan , John Daley , Hyong Youb Kim , Gaetan Rivet , Xiao Wang , Beilei Xing , Jingjing Wu , Qiming Yang , Konstantin Ananyev , Shijith Thotton , Srisivasubramanian Srinivasan , Matan Azrad , Shahaf Shuler , Yongseok Koh , Zyta Szpak , Liron Himi , Alan Winkowski , Tomasz Duszynski , Stephen Hemminger , "K. Y. Srinivasan" , Haiyang Zhang , Rastislav Cernay , Jan Remes , Alejandro Lucero , Tetsuya Mukawa , Jerin Jacob , Bruce Richardson , Andrew Rybchenko , Jasvinder Singh , Cristian Dumitrescu , Keith Wiles , Maciej Czekaj , Maxime Coquelin , Tiwei Bie , Zhihong Wang , Yong Wang , Anatoly Burakov , dev@dpdk.org Date: Mon, 29 Apr 2019 22:30:00 +0200 Message-ID: <1748144.UFpUr2FPnr@xps> In-Reply-To: References: <30528485.5cHeq7CNxZ@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 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: Mon, 29 Apr 2019 20:30:13 -0000 29/04/2019 18:51, Ferruh Yigit: > On 4/18/2019 11:59 AM, Thomas Monjalon wrote: > > Hi all, > > > > Since DPDK 18.11, the behaviour of the close operation is changed > > if RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver: > > port is released (i.e. totally freed and data erased) on close. > > This new behaviour is enabled per driver for a migration period. > > > > Looking at the code, you can see these comments: > > /* old behaviour: only free queue arrays */ > > RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n" > > "The driver %s should migrate to the new behaviour.\n", > > /* new behaviour: send event + reset state + free all data */ > > > > You can find an advice in the commit: > > http://git.dpdk.org/dpdk/commit/?id=23ea57a2a > > " > > When enabling RTE_ETH_DEV_CLOSE_REMOVE, > > the PMD must free all its private resources for the port, > > in its dev_close function. > > It is advised to call the dev_close function in the remove function > > in order to support removing a device without closing its ports. > > " > > > > It would be great to complete this migration for the next LTS > > version, which will be 19.11. > > It means the work should be ideally finished during the summer. > > I would like to detail a little more what needs to be done, mainly for the sake > of the discussion, please comment if something missing/wrong. > > There are two exit paths for driver: > 1) Hotplug remove the device > rte_dev_remove() OR rte_eal_hotplug_remove() > > 2) Application exit: > rte_eth_dev_close() > > > (1) can be called without ethdev stop and close functions called, so the path > should cover those functions. > And in the PMD entry point in this path is .remove() function. In this .remove() > function PMD should: > - stop forwarding > - clean PMD private resources (dev_close() ? ) yes > - clean ethdev generic resources (rte_eth_dev_release_port() ? ) already done in dev_close thanks to RTE_ETH_DEV_CLOSE_REMOVE > - remove device resources, which already done by remove APIs There can be some specific PMD private resources, not cleaned when closing a port, to clean in "remove". > (2) ethdev won't be usable after "rte_eth_dev_close()" and it should clear PMD > private and ethdev generic resources. > With RTE_ETH_DEV_CLOSE_REMOVE flag 'rte_eth_dev_close()' will free generic > ethdev resources (rte_eth_dev_release_port()) so PMD specific '.dev_close()' should: > - stop forwarding > - clean PMD private resources yes, but only port-related resources, not those common with other ports or features of the device. > If agreed on above, the task is: > - '.dev_close()' > - stop forwarding > - clean PMD private resources > > - '.remove()' > - call '.dev_close()' Yes, looks right. > A question is if application should call 'rte_eth_dev_stop()' explicitly before > close or should it be part of close? For (2) it makes sense app call the stop > explicitly, but for device remove it is not clear who to stop the forwarding, > for this case 'dev_close()' stopping forwarding makes things easier. "stop" is a prerequisite for "close" anyway. In most cases, the app should manage "stop" itself to properly free related resources. The question is to know whether we return an error on "close" if the port is not stopped, or we stop it implicitly. The API says: * Close a stopped Ethernet device. The device cannot be restarted! We may explicit the behaviour if the port is not stopped. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 0B6DFA0679 for ; Mon, 29 Apr 2019 22:30:15 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id DBF226CC1; Mon, 29 Apr 2019 22:30:14 +0200 (CEST) Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com [66.111.4.224]) by dpdk.org (Postfix) with ESMTP id 748B15F36 for ; Mon, 29 Apr 2019 22:30:13 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailnew.nyi.internal (Postfix) with ESMTP id 34F9C13BB1; Mon, 29 Apr 2019 16:30:12 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 29 Apr 2019 16:30:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=bqRImLHi8EVi6xnaHhpgcBxY47xAzrYCu/TxMcLVmmk=; b=BGFADETE3UDL UmM5P4Ir0/m1YV92IumVRcAmKfmpFkPxa7dScVihIe2l65U6ugvCBE6oasUaxQ4j ozBRbvILW1VnAXeGK0lnDiDZuHikopwrKRODiLEhsAeKTEMf3qoZw+h9zRJU3oKm DBRke/Ilo8gdS2OeWxKhWJnhsZ4ZVKY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=bqRImLHi8EVi6xnaHhpgcBxY47xAzrYCu/TxMcLVm mk=; b=d9wJmyVBqOkz5jSU2ZvznpOnpNECuJXBS9OleEBI8GLI97vr7lLf8nrLq HAHe6IUMjrmJW/8EWrDzWsaSBo+YvWXuNF0DlEuhFM6hwVlHcd5HMxefX85xJIA0 b7NdGJAh4UWFrrpRMKO2ji7SBzN9fO/dKlMGYC1xc9EByV+BiMniZv+7ZZL4Ba1V IocuNz4lt3TMW/iZs2ENcH8HUhTeoEPPytxEmgJJiaAKVbdFbirVhrRre/ViI5nT Uwb78F+YprX0knh/w0C8GL/5ufUQvKfB2tpUzOPDTfI5CjapNtOT/calF4Gx8Yq8 q5VRd4Sf6OK22K7sv/Mxa/HYMN9kQ== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddriedvgdduheduucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhephffvufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhhomhgr shcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqnecuff homhgrihhnpeguphgukhdrohhrghenucfkphepjeejrddufeegrddvtdefrddukeegnecu rfgrrhgrmhepmhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtne cuvehluhhsthgvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id AD715103CE; Mon, 29 Apr 2019 16:30:01 -0400 (EDT) From: Thomas Monjalon To: Ferruh Yigit Cc: "John W. Linville" , Xiaolong Ye , Qi Zhang , Shepard Siegel , Ed Czeck , John Miller , Igor Russkikh , Pavel Belous , Allain Legacy , Matt Peters , Ravi Kumar , Rasesh Mody , Shahed Shaikh , Ajit Khaparde , Somnath Kotur , Chas Williams , Rahul Lakkireddy , Hemant Agrawal , Shreyansh Jain , Wenzhuo Lu , Marcin Wojtas , Michal Krawczyk , Guy Tzalik , Evgeny Schemeilin , Gagandeep Singh , Pankaj Chauhan , John Daley , Hyong Youb Kim , Gaetan Rivet , Xiao Wang , Beilei Xing , Jingjing Wu , Qiming Yang , Konstantin Ananyev , Shijith Thotton , Srisivasubramanian Srinivasan , Matan Azrad , Shahaf Shuler , Yongseok Koh , Zyta Szpak , Liron Himi , Alan Winkowski , Tomasz Duszynski , Stephen Hemminger , "K. Y. Srinivasan" , Haiyang Zhang , Rastislav Cernay , Jan Remes , Alejandro Lucero , Tetsuya Mukawa , Jerin Jacob , Bruce Richardson , Andrew Rybchenko , Jasvinder Singh , Cristian Dumitrescu , Keith Wiles , Maciej Czekaj , Maxime Coquelin , Tiwei Bie , Zhihong Wang , Yong Wang , Anatoly Burakov , dev@dpdk.org Date: Mon, 29 Apr 2019 22:30:00 +0200 Message-ID: <1748144.UFpUr2FPnr@xps> In-Reply-To: References: <30528485.5cHeq7CNxZ@xps> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190429203000.4IbkkO73SIh6wKaWnnuJ0inXP5H9-y-x0MJFiMmxms8@z> 29/04/2019 18:51, Ferruh Yigit: > On 4/18/2019 11:59 AM, Thomas Monjalon wrote: > > Hi all, > > > > Since DPDK 18.11, the behaviour of the close operation is changed > > if RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver: > > port is released (i.e. totally freed and data erased) on close. > > This new behaviour is enabled per driver for a migration period. > > > > Looking at the code, you can see these comments: > > /* old behaviour: only free queue arrays */ > > RTE_ETHDEV_LOG(DEBUG, "Port closing is using an old behaviour.\n" > > "The driver %s should migrate to the new behaviour.\n", > > /* new behaviour: send event + reset state + free all data */ > > > > You can find an advice in the commit: > > http://git.dpdk.org/dpdk/commit/?id=23ea57a2a > > " > > When enabling RTE_ETH_DEV_CLOSE_REMOVE, > > the PMD must free all its private resources for the port, > > in its dev_close function. > > It is advised to call the dev_close function in the remove function > > in order to support removing a device without closing its ports. > > " > > > > It would be great to complete this migration for the next LTS > > version, which will be 19.11. > > It means the work should be ideally finished during the summer. > > I would like to detail a little more what needs to be done, mainly for the sake > of the discussion, please comment if something missing/wrong. > > There are two exit paths for driver: > 1) Hotplug remove the device > rte_dev_remove() OR rte_eal_hotplug_remove() > > 2) Application exit: > rte_eth_dev_close() > > > (1) can be called without ethdev stop and close functions called, so the path > should cover those functions. > And in the PMD entry point in this path is .remove() function. In this .remove() > function PMD should: > - stop forwarding > - clean PMD private resources (dev_close() ? ) yes > - clean ethdev generic resources (rte_eth_dev_release_port() ? ) already done in dev_close thanks to RTE_ETH_DEV_CLOSE_REMOVE > - remove device resources, which already done by remove APIs There can be some specific PMD private resources, not cleaned when closing a port, to clean in "remove". > (2) ethdev won't be usable after "rte_eth_dev_close()" and it should clear PMD > private and ethdev generic resources. > With RTE_ETH_DEV_CLOSE_REMOVE flag 'rte_eth_dev_close()' will free generic > ethdev resources (rte_eth_dev_release_port()) so PMD specific '.dev_close()' should: > - stop forwarding > - clean PMD private resources yes, but only port-related resources, not those common with other ports or features of the device. > If agreed on above, the task is: > - '.dev_close()' > - stop forwarding > - clean PMD private resources > > - '.remove()' > - call '.dev_close()' Yes, looks right. > A question is if application should call 'rte_eth_dev_stop()' explicitly before > close or should it be part of close? For (2) it makes sense app call the stop > explicitly, but for device remove it is not clear who to stop the forwarding, > for this case 'dev_close()' stopping forwarding makes things easier. "stop" is a prerequisite for "close" anyway. In most cases, the app should manage "stop" itself to properly free related resources. The question is to know whether we return an error on "close" if the port is not stopped, or we stop it implicitly. The API says: * Close a stopped Ethernet device. The device cannot be restarted! We may explicit the behaviour if the port is not stopped.