* [dpdk-dev] CALL to eth PMD maintainers: complete closing of port @ 2019-04-18 10:59 Thomas Monjalon 2019-04-18 10:59 ` Thomas Monjalon ` (3 more replies) 0 siblings, 4 replies; 22+ messages in thread From: Thomas Monjalon @ 2019-04-18 10:59 UTC (permalink / raw) To: 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, Ferruh Yigit, 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 Cc: dev 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. The migration to the new behaviour is done in 4 drivers: git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ena enic mlx5 vmxnet3 Following drivers should be migrated: ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u af_packet af_xdp ark atlantic avp axgbe bnx2x bnxt bonding cxgbe dpaa dpaa2 e1000 enetc failsafe fm10k i40e iavf ice ifc ixgbe kni liquidio mlx4 mvneta mvpp2 netvsc nfb nfp null octeontx pcap qede ring sfc softnic szedata2 tap thunderx vdev_netvsc vhost virtio Please let's progress smoothly on this topic, thanks. The concerned maintainers (Cc) can be found with the following command: devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) ^ permalink raw reply [flat|nested] 22+ messages in thread
* [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-18 10:59 [dpdk-dev] CALL to eth PMD maintainers: complete closing of port Thomas Monjalon @ 2019-04-18 10:59 ` Thomas Monjalon 2019-04-28 6:57 ` Matan Azrad ` (2 subsequent siblings) 3 siblings, 0 replies; 22+ messages in thread From: Thomas Monjalon @ 2019-04-18 10:59 UTC (permalink / raw) To: 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, Ferruh Yigit, 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 Cc: dev 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. The migration to the new behaviour is done in 4 drivers: git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ena enic mlx5 vmxnet3 Following drivers should be migrated: ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u af_packet af_xdp ark atlantic avp axgbe bnx2x bnxt bonding cxgbe dpaa dpaa2 e1000 enetc failsafe fm10k i40e iavf ice ifc ixgbe kni liquidio mlx4 mvneta mvpp2 netvsc nfb nfp null octeontx pcap qede ring sfc softnic szedata2 tap thunderx vdev_netvsc vhost virtio Please let's progress smoothly on this topic, thanks. The concerned maintainers (Cc) can be found with the following command: devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-18 10:59 [dpdk-dev] CALL to eth PMD maintainers: complete closing of port Thomas Monjalon 2019-04-18 10:59 ` Thomas Monjalon @ 2019-04-28 6:57 ` Matan Azrad 2019-04-28 6:57 ` Matan Azrad 2019-04-28 16:17 ` Stephen Hemminger 2019-04-29 16:51 ` Ferruh Yigit 2020-08-03 18:50 ` Thomas Monjalon 3 siblings, 2 replies; 22+ messages in thread From: Matan Azrad @ 2019-04-28 6:57 UTC (permalink / raw) To: Thomas Monjalon, 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, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Shahaf Shuler, Yongseok Koh, Zyta Szpak, Liron Himi, Alan Winkowski, Tomasz Duszynski, Stephen Hemminger, kys, haiyangz, 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 Cc: dev Hi Thomas vdev_netvsc has no close API - should I change something there? > From: Thomas Monjalon <thomas@monjalon.net> > 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: > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2 > Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7 > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7 > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA > %3D&reserved=0 > " > 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. > > The migration to the new behaviour is done in 4 drivers: > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 > ena > enic > mlx5 > vmxnet3 > > Following drivers should be migrated: > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l > RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > af_packet > af_xdp > ark > atlantic > avp > axgbe > bnx2x > bnxt > bonding > cxgbe > dpaa > dpaa2 > e1000 > enetc > failsafe > fm10k > i40e > iavf > ice > ifc > ixgbe > kni > liquidio > mlx4 > mvneta > mvpp2 > netvsc > nfb > nfp > null > octeontx > pcap > qede > ring > sfc > softnic > szedata2 > tap > thunderx > vdev_netvsc > vhost > virtio > > Please let's progress smoothly on this topic, thanks. > > The concerned maintainers (Cc) can be found with the following command: > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 - > type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | > sort | uniq -u) > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-28 6:57 ` Matan Azrad @ 2019-04-28 6:57 ` Matan Azrad 2019-04-28 16:17 ` Stephen Hemminger 1 sibling, 0 replies; 22+ messages in thread From: Matan Azrad @ 2019-04-28 6:57 UTC (permalink / raw) To: Thomas Monjalon, 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, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Shahaf Shuler, Yongseok Koh, Zyta Szpak, Liron Himi, Alan Winkowski, Tomasz Duszynski, Stephen Hemminger, kys, haiyangz, 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 Cc: dev Hi Thomas vdev_netvsc has no close API - should I change something there? > From: Thomas Monjalon <thomas@monjalon.net> > 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: > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2 > Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7 > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7 > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA > %3D&reserved=0 > " > 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. > > The migration to the new behaviour is done in 4 drivers: > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 > ena > enic > mlx5 > vmxnet3 > > Following drivers should be migrated: > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l > RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > af_packet > af_xdp > ark > atlantic > avp > axgbe > bnx2x > bnxt > bonding > cxgbe > dpaa > dpaa2 > e1000 > enetc > failsafe > fm10k > i40e > iavf > ice > ifc > ixgbe > kni > liquidio > mlx4 > mvneta > mvpp2 > netvsc > nfb > nfp > null > octeontx > pcap > qede > ring > sfc > softnic > szedata2 > tap > thunderx > vdev_netvsc > vhost > virtio > > Please let's progress smoothly on this topic, thanks. > > The concerned maintainers (Cc) can be found with the following command: > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 - > type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | > sort | uniq -u) > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-28 6:57 ` Matan Azrad 2019-04-28 6:57 ` Matan Azrad @ 2019-04-28 16:17 ` Stephen Hemminger 2019-04-28 16:17 ` Stephen Hemminger 1 sibling, 1 reply; 22+ messages in thread From: Stephen Hemminger @ 2019-04-28 16:17 UTC (permalink / raw) To: Matan Azrad Cc: Thomas Monjalon, 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, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Shahaf Shuler, Yongseok Koh, Zyta Szpak, Liron Himi, Alan Winkowski, Tomasz Duszynski, Stephen Hemminger, kys, haiyangz, 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 On Sun, 28 Apr 2019 06:57:59 +0000 Matan Azrad <matan@mellanox.com> wrote: > Hi Thomas > > vdev_netvsc has no close API - should I change something there? > > > From: Thomas Monjalon <thomas@monjalon.net> > > 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: > > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2 > > Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7 > > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7 > > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a > > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA > > %3D&reserved=0 > > " > > 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. > > > > The migration to the new behaviour is done in 4 drivers: > > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 > > ena > > enic > > mlx5 > > vmxnet3 > > > > Following drivers should be migrated: > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l > > RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > > af_packet > > af_xdp > > ark > > atlantic > > avp > > axgbe > > bnx2x > > bnxt > > bonding > > cxgbe > > dpaa > > dpaa2 > > e1000 > > enetc > > failsafe > > fm10k > > i40e > > iavf > > ice > > ifc > > ixgbe > > kni > > liquidio > > mlx4 > > mvneta > > mvpp2 > > netvsc > > nfb > > nfp > > null > > octeontx > > pcap > > qede > > ring > > sfc > > softnic > > szedata2 > > tap > > thunderx > > vdev_netvsc > > vhost > > virtio > > > > Please let's progress smoothly on this topic, thanks. > > > > The concerned maintainers (Cc) can be found with the following command: > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 - > > type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | > > sort | uniq -u) > > > I have a version still testing ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-28 16:17 ` Stephen Hemminger @ 2019-04-28 16:17 ` Stephen Hemminger 0 siblings, 0 replies; 22+ messages in thread From: Stephen Hemminger @ 2019-04-28 16:17 UTC (permalink / raw) To: Matan Azrad Cc: Thomas Monjalon, 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, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Shahaf Shuler, Yongseok Koh, Zyta Szpak, Liron Himi, Alan Winkowski, Tomasz Duszynski, Stephen Hemminger, kys, haiyangz, 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 On Sun, 28 Apr 2019 06:57:59 +0000 Matan Azrad <matan@mellanox.com> wrote: > Hi Thomas > > vdev_netvsc has no close API - should I change something there? > > > From: Thomas Monjalon <thomas@monjalon.net> > > 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: > > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2 > > Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7 > > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7 > > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a > > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA > > %3D&reserved=0 > > " > > 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. > > > > The migration to the new behaviour is done in 4 drivers: > > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 > > ena > > enic > > mlx5 > > vmxnet3 > > > > Following drivers should be migrated: > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l > > RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > > af_packet > > af_xdp > > ark > > atlantic > > avp > > axgbe > > bnx2x > > bnxt > > bonding > > cxgbe > > dpaa > > dpaa2 > > e1000 > > enetc > > failsafe > > fm10k > > i40e > > iavf > > ice > > ifc > > ixgbe > > kni > > liquidio > > mlx4 > > mvneta > > mvpp2 > > netvsc > > nfb > > nfp > > null > > octeontx > > pcap > > qede > > ring > > sfc > > softnic > > szedata2 > > tap > > thunderx > > vdev_netvsc > > vhost > > virtio > > > > Please let's progress smoothly on this topic, thanks. > > > > The concerned maintainers (Cc) can be found with the following command: > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 - > > type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | > > sort | uniq -u) > > > I have a version still testing ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-18 10:59 [dpdk-dev] CALL to eth PMD maintainers: complete closing of port Thomas Monjalon 2019-04-18 10:59 ` Thomas Monjalon 2019-04-28 6:57 ` Matan Azrad @ 2019-04-29 16:51 ` Ferruh Yigit 2019-04-29 16:51 ` Ferruh Yigit 2019-04-29 20:30 ` Thomas Monjalon 2020-08-03 18:50 ` Thomas Monjalon 3 siblings, 2 replies; 22+ messages in thread From: Ferruh Yigit @ 2019-04-29 16:51 UTC (permalink / raw) To: Thomas Monjalon, 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 Cc: dev 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() ? ) - clean ethdev generic resources (rte_eth_dev_release_port() ? ) - remove device resources, which already done by remove APIs (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 If agreed on above, the task is: - '.dev_close()' - stop forwarding - clean PMD private resources - '.remove()' - call '.dev_close()' 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-29 16:51 ` Ferruh Yigit @ 2019-04-29 16:51 ` Ferruh Yigit 2019-04-29 20:30 ` Thomas Monjalon 1 sibling, 0 replies; 22+ messages in thread From: Ferruh Yigit @ 2019-04-29 16:51 UTC (permalink / raw) To: Thomas Monjalon, 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 Cc: dev 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() ? ) - clean ethdev generic resources (rte_eth_dev_release_port() ? ) - remove device resources, which already done by remove APIs (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 If agreed on above, the task is: - '.dev_close()' - stop forwarding - clean PMD private resources - '.remove()' - call '.dev_close()' 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-29 16:51 ` Ferruh Yigit 2019-04-29 16:51 ` Ferruh Yigit @ 2019-04-29 20:30 ` Thomas Monjalon 2019-04-29 20:30 ` Thomas Monjalon 2019-04-30 12:45 ` Nithin Dabilpuram 1 sibling, 2 replies; 22+ messages in thread From: Thomas Monjalon @ 2019-04-29 20:30 UTC (permalink / raw) 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 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-29 20:30 ` Thomas Monjalon @ 2019-04-29 20:30 ` Thomas Monjalon 2019-04-30 12:45 ` Nithin Dabilpuram 1 sibling, 0 replies; 22+ messages in thread From: Thomas Monjalon @ 2019-04-29 20:30 UTC (permalink / raw) 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 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-29 20:30 ` Thomas Monjalon 2019-04-29 20:30 ` Thomas Monjalon @ 2019-04-30 12:45 ` Nithin Dabilpuram 2019-04-30 12:45 ` Nithin Dabilpuram 2019-04-30 14:12 ` Thomas Monjalon 1 sibling, 2 replies; 22+ messages in thread From: Nithin Dabilpuram @ 2019-04-30 12:45 UTC (permalink / raw) To: Thomas Monjalon Cc: Ferruh Yigit, 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 On Mon, Apr 29, 2019 at 10:30:00PM +0200, Thomas Monjalon wrote: > 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. I have a related query. Even after rte_eth_dev_close(), we have to call rte_eal_hotplug_remove() to free up common resource if any and remove the device from application ? And rte_eal_hotplug_remove() would still call drivers .remove function for this. > > > 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. > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-30 12:45 ` Nithin Dabilpuram @ 2019-04-30 12:45 ` Nithin Dabilpuram 2019-04-30 14:12 ` Thomas Monjalon 1 sibling, 0 replies; 22+ messages in thread From: Nithin Dabilpuram @ 2019-04-30 12:45 UTC (permalink / raw) To: Thomas Monjalon Cc: Ferruh Yigit, 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 On Mon, Apr 29, 2019 at 10:30:00PM +0200, Thomas Monjalon wrote: > 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. I have a related query. Even after rte_eth_dev_close(), we have to call rte_eal_hotplug_remove() to free up common resource if any and remove the device from application ? And rte_eal_hotplug_remove() would still call drivers .remove function for this. > > > 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. > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-30 12:45 ` Nithin Dabilpuram 2019-04-30 12:45 ` Nithin Dabilpuram @ 2019-04-30 14:12 ` Thomas Monjalon 2019-04-30 14:12 ` Thomas Monjalon 1 sibling, 1 reply; 22+ messages in thread From: Thomas Monjalon @ 2019-04-30 14:12 UTC (permalink / raw) To: Nithin Dabilpuram Cc: Ferruh Yigit, 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 30/04/2019 14:45, Nithin Dabilpuram: > On Mon, Apr 29, 2019 at 10:30:00PM +0200, Thomas Monjalon wrote: > > 29/04/2019 18:51, Ferruh Yigit: > > > 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. > I have a related query. Even after rte_eth_dev_close(), we have to call > rte_eal_hotplug_remove() to free up common resource if any > and remove the device from application ? > And rte_eal_hotplug_remove() would still call drivers .remove function for this. Yes if you really want to free the whole device, you should call the .remove op from rte_eal_hotplug_remove() or rte_dev_remove(). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-30 14:12 ` Thomas Monjalon @ 2019-04-30 14:12 ` Thomas Monjalon 0 siblings, 0 replies; 22+ messages in thread From: Thomas Monjalon @ 2019-04-30 14:12 UTC (permalink / raw) To: Nithin Dabilpuram Cc: Ferruh Yigit, 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 30/04/2019 14:45, Nithin Dabilpuram: > On Mon, Apr 29, 2019 at 10:30:00PM +0200, Thomas Monjalon wrote: > > 29/04/2019 18:51, Ferruh Yigit: > > > 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. > I have a related query. Even after rte_eth_dev_close(), we have to call > rte_eal_hotplug_remove() to free up common resource if any > and remove the device from application ? > And rte_eal_hotplug_remove() would still call drivers .remove function for this. Yes if you really want to free the whole device, you should call the .remove op from rte_eal_hotplug_remove() or rte_dev_remove(). ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2019-04-18 10:59 [dpdk-dev] CALL to eth PMD maintainers: complete closing of port Thomas Monjalon ` (2 preceding siblings ...) 2019-04-29 16:51 ` Ferruh Yigit @ 2020-08-03 18:50 ` Thomas Monjalon 2020-09-12 11:25 ` Thomas Monjalon 3 siblings, 1 reply; 22+ messages in thread From: Thomas Monjalon @ 2020-08-03 18:50 UTC (permalink / raw) To: John W. Linville, Qi Zhang, Shepard Siegel, Ed Czeck, John Miller, Igor Russkikh, Pavel Belous, Matt Peters, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Xiao Wang, Qiming Yang, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Zyta Szpak, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Jerin Jacob, Bruce Richardson, Andrew Rybchenko, Jasvinder Singh, Cristian Dumitrescu, Keith Wiles, Maciej Czekaj, Maxime Coquelin, Zhihong Wang, Yong Wang, Anatoly Burakov, Beilei Xing, Jingjing Wu Cc: Rosen Xu, dev, Ciara Loftus, Steven Webster, Somalapuram Amaranath, xavier.huwei, Sachin Saxena, Wei Zhao, Jeff Guo, Igor Chauskin, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Min Hu (Connor), Yisen Zhuang, Alfredo Cardigliano, Jakub Grajciar, Viacheslav Ovsiienko, Long Li, Martin Spinler, Heinrich Kuhn, Harman Kalra, Nithin Dabilpuram, Kiran Kumar K, Akhil Goyal 18/04/2019 12:59, Thomas Monjalon: > 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. For the record, it did not happen in 19.11. > It means the work should be ideally finished during the summer. > > The migration to the new behaviour is done in 4 drivers: > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 > ena > enic > mlx5 > vmxnet3 > > Following drivers should be migrated: > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u [...] The progress in April 2019 was 4 of 46 (9%). > Please let's progress smoothly on this topic, thanks. More than one year later, the progress is 26 of 53 (49%). > The concerned maintainers (Cc) can be found with the following command: > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) We cannot wait forever. Temporary cannot be longer than 2 years. I am going to send a deprecation notice to remove the "temporary" flag RTE_ETH_DEV_CLOSE_REMOVE. It will break drivers which are not migrated. It will probably help to find motivation in new priorities. More details on what to do can be found in this mail thread: http://inbox.dpdk.org/dev/1748144.UFpUr2FPnr@xps/ ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-08-03 18:50 ` Thomas Monjalon @ 2020-09-12 11:25 ` Thomas Monjalon 2020-09-13 22:16 ` Thomas Monjalon 0 siblings, 1 reply; 22+ messages in thread From: Thomas Monjalon @ 2020-09-12 11:25 UTC (permalink / raw) To: John W. Linville, Qi Zhang, Shepard Siegel, Ed Czeck, John Miller, Igor Russkikh, Pavel Belous, Matt Peters, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Xiao Wang, Qiming Yang, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Zyta Szpak, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Jerin Jacob, Bruce Richardson, Andrew Rybchenko, Jasvinder Singh, Cristian Dumitrescu, Keith Wiles, Maciej Czekaj, Maxime Coquelin, Zhihong Wang, Yong Wang, Anatoly Burakov, Beilei Xing, Jingjing Wu, dev Cc: Rosen Xu, dev, Ciara Loftus, Steven Webster, Somalapuram Amaranath, xavier.huwei, Sachin Saxena, Wei Zhao, Jeff Guo, Igor Chauskin, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Min Hu (Connor), Yisen Zhuang, Alfredo Cardigliano, Jakub Grajciar, Viacheslav Ovsiienko, Long Li, Martin Spinler, Heinrich Kuhn, Harman Kalra, Nithin Dabilpuram, Kiran Kumar K, Akhil Goyal, ferruh.yigit 03/08/2020 20:50, Thomas Monjalon: > 18/04/2019 12:59, Thomas Monjalon: > > 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. > > For the record, it did not happen in 19.11. > > > Following drivers should be migrated: > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > [...] > > The progress in April 2019 was 4 of 46 (9%). > > > Please let's progress smoothly on this topic, thanks. > > More than one year later, the progress is 26 of 53 (49%). > > > The concerned maintainers (Cc) can be found with the following command: > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) > > We cannot wait forever. Temporary cannot be longer than 2 years. > I am going to send a deprecation notice to remove the "temporary" > flag RTE_ETH_DEV_CLOSE_REMOVE. The deprecation notice was merged in 20.08: http://mails.dpdk.org/archives/dev/2020-August/177314.html > It will break drivers which are not migrated. > It will probably help to find motivation in new priorities. > > More details on what to do can be found in this mail thread: > http://inbox.dpdk.org/dev/1748144.UFpUr2FPnr@xps/ Summary: * The freeing of private port resources must be moved in the PMD from the ".remove(device)" function to the ".dev_close(port)" function. * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be freed, it must be set to NULL in ".dev_close" PMD function to protect from subsequent rte_eth_dev_release_port() freeing. * Note 1: The generic resources are freed in rte_eth_dev_release_port(), after ".dev_close" is called in rte_eth_dev_close(), but not when calling ".dev_close" directly from the ".remove" PMD function. That's why rte_eth_dev_release_port() must still be called explicitly from ".remove(device)" after calling the ".dev_close" PMD function. * Note 2: If a device can have multiple ports, the common resources must be freed only in the ".remove(device)" function. * Note 3: The port is supposed to be in a stopped state when it is closed. If it is not the case, it is free to the PMD implementation how to react when trying to close a non-stopped port: either try to stop it automatically or just return an error. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-09-12 11:25 ` Thomas Monjalon @ 2020-09-13 22:16 ` Thomas Monjalon 2020-09-14 9:20 ` Rahul Lakkireddy ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Thomas Monjalon @ 2020-09-13 22:16 UTC (permalink / raw) To: John W. Linville, Qi Zhang, Shepard Siegel, Ed Czeck, John Miller, Igor Russkikh, Pavel Belous, Matt Peters, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Xiao Wang, Qiming Yang, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Zyta Szpak, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Jerin Jacob, Bruce Richardson, Andrew Rybchenko, Jasvinder Singh, Cristian Dumitrescu, Keith Wiles, Maciej Czekaj, Maxime Coquelin, Zhihong Wang, Yong Wang, Anatoly Burakov, Beilei Xing, Jingjing Wu, dev Cc: Rosen Xu, dev, Ciara Loftus, Steven Webster, Somalapuram Amaranath, xavier.huwei, Sachin Saxena, Wei Zhao, Jeff Guo, Igor Chauskin, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Min Hu (Connor), Yisen Zhuang, Alfredo Cardigliano, Jakub Grajciar, Viacheslav Ovsiienko, Long Li, Martin Spinler, Heinrich Kuhn, Harman Kalra, Nithin Dabilpuram, Kiran Kumar K, Akhil Goyal, ferruh.yigit, Thomas Monjalon The patches for removing RTE_ETH_DEV_CLOSE_REMOVE are sent: https://patches.dpdk.org/project/dpdk/list/?series=12173 11 drivers are not supporting the new behaviour correctly: bnx2x, cxgbe, dpaa, dpaa2, enetc, ionic, ipn3ke, liquidio, nfp, pfe, qede If you are the maintainer of one of these drivers, you can still consider fixing it in the next days. 12/09/2020 13:25, Thomas Monjalon: > 03/08/2020 20:50, Thomas Monjalon: > > 18/04/2019 12:59, Thomas Monjalon: > > > 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. > > > > For the record, it did not happen in 19.11. > > > > > Following drivers should be migrated: > > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > > [...] > > > > The progress in April 2019 was 4 of 46 (9%). > > > > > Please let's progress smoothly on this topic, thanks. > > > > More than one year later, the progress is 26 of 53 (49%). > > > > > The concerned maintainers (Cc) can be found with the following command: > > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) > > > > We cannot wait forever. Temporary cannot be longer than 2 years. > > I am going to send a deprecation notice to remove the "temporary" > > flag RTE_ETH_DEV_CLOSE_REMOVE. > > The deprecation notice was merged in 20.08: > http://mails.dpdk.org/archives/dev/2020-August/177314.html > > > It will break drivers which are not migrated. > > It will probably help to find motivation in new priorities. > > > > More details on what to do can be found in this mail thread: > > http://inbox.dpdk.org/dev/1748144.UFpUr2FPnr@xps/ > > Summary: > > * The freeing of private port resources must be moved in the PMD > from the ".remove(device)" function to the ".dev_close(port)" function. > > * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be freed, > it must be set to NULL in ".dev_close" PMD function to protect from > subsequent rte_eth_dev_release_port() freeing. > > * Note 1: > The generic resources are freed in rte_eth_dev_release_port(), > after ".dev_close" is called in rte_eth_dev_close(), but not when > calling ".dev_close" directly from the ".remove" PMD function. > That's why rte_eth_dev_release_port() must still be called explicitly > from ".remove(device)" after calling the ".dev_close" PMD function. > > * Note 2: > If a device can have multiple ports, the common resources must be freed > only in the ".remove(device)" function. > > * Note 3: > The port is supposed to be in a stopped state when it is closed. > If it is not the case, it is free to the PMD implementation > how to react when trying to close a non-stopped port: > either try to stop it automatically or just return an error. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-09-13 22:16 ` Thomas Monjalon @ 2020-09-14 9:20 ` Rahul Lakkireddy 2020-09-22 3:04 ` Xu, Rosen 2020-09-25 4:22 ` Rasesh Mody 2 siblings, 0 replies; 22+ messages in thread From: Rahul Lakkireddy @ 2020-09-14 9:20 UTC (permalink / raw) To: Thomas Monjalon Cc: John W. Linville, Qi Zhang, Shepard Siegel, Ed Czeck, John Miller, Igor Russkikh, Pavel Belous, Matt Peters, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Hemant Agrawal, Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Xiao Wang, Qiming Yang, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Zyta Szpak, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Jerin Jacob, Bruce Richardson, Andrew Rybchenko, Jasvinder Singh, Cristian Dumitrescu, Keith Wiles, Maciej Czekaj, Maxime Coquelin, Zhihong Wang, Yong Wang, Anatoly Burakov, Beilei Xing, Jingjing Wu, dev, Rosen Xu, Ciara Loftus, Steven Webster, Somalapuram Amaranath, xavier.huwei, Sachin Saxena, Wei Zhao, Jeff Guo, Igor Chauskin, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Min Hu (Connor), Yisen Zhuang, Alfredo Cardigliano, Jakub Grajciar, Viacheslav Ovsiienko, Long Li, Martin Spinler, Heinrich Kuhn, Harman Kalra, Nithin Dabilpuram, Kiran Kumar K, Akhil Goyal On Monday, September 09/14/20, 2020 at 00:16:17 +0200, Thomas Monjalon wrote: > The patches for removing RTE_ETH_DEV_CLOSE_REMOVE are sent: > https://patches.dpdk.org/project/dpdk/list/?series=12173 > > 11 drivers are not supporting the new behaviour correctly: > bnx2x, cxgbe, dpaa, dpaa2, enetc, ionic, > ipn3ke, liquidio, nfp, pfe, qede > > If you are the maintainer of one of these drivers, > you can still consider fixing it in the next days. > The RTE_ETH_DEV_CLOSE_REMOVE migration for CXGBE PMD has already been submitted two weeks ago [1]. [1] https://patches.dpdk.org/patch/76271/ Thanks, Rahul > > 12/09/2020 13:25, Thomas Monjalon: > > 03/08/2020 20:50, Thomas Monjalon: > > > 18/04/2019 12:59, Thomas Monjalon: > > > > 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. > > > > > > For the record, it did not happen in 19.11. > > > > > > > Following drivers should be migrated: > > > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u > > > [...] > > > > > > The progress in April 2019 was 4 of 46 (9%). > > > > > > > Please let's progress smoothly on this topic, thanks. > > > > > > More than one year later, the progress is 26 of 53 (49%). > > > > > > > The concerned maintainers (Cc) can be found with the following command: > > > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) > > > > > > We cannot wait forever. Temporary cannot be longer than 2 years. > > > I am going to send a deprecation notice to remove the "temporary" > > > flag RTE_ETH_DEV_CLOSE_REMOVE. > > > > The deprecation notice was merged in 20.08: > > http://mails.dpdk.org/archives/dev/2020-August/177314.html > > > > > It will break drivers which are not migrated. > > > It will probably help to find motivation in new priorities. > > > > > > More details on what to do can be found in this mail thread: > > > http://inbox.dpdk.org/dev/1748144.UFpUr2FPnr@xps/ > > > > Summary: > > > > * The freeing of private port resources must be moved in the PMD > > from the ".remove(device)" function to the ".dev_close(port)" function. > > > > * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be freed, > > it must be set to NULL in ".dev_close" PMD function to protect from > > subsequent rte_eth_dev_release_port() freeing. > > > > * Note 1: > > The generic resources are freed in rte_eth_dev_release_port(), > > after ".dev_close" is called in rte_eth_dev_close(), but not when > > calling ".dev_close" directly from the ".remove" PMD function. > > That's why rte_eth_dev_release_port() must still be called explicitly > > from ".remove(device)" after calling the ".dev_close" PMD function. > > > > * Note 2: > > If a device can have multiple ports, the common resources must be freed > > only in the ".remove(device)" function. > > > > * Note 3: > > The port is supposed to be in a stopped state when it is closed. > > If it is not the case, it is free to the PMD implementation > > how to react when trying to close a non-stopped port: > > either try to stop it automatically or just return an error. > > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-09-13 22:16 ` Thomas Monjalon 2020-09-14 9:20 ` Rahul Lakkireddy @ 2020-09-22 3:04 ` Xu, Rosen 2020-09-22 7:28 ` Thomas Monjalon 2020-09-25 4:22 ` Rasesh Mody 2 siblings, 1 reply; 22+ messages in thread From: Xu, Rosen @ 2020-09-22 3:04 UTC (permalink / raw) To: Thomas Monjalon, John W. Linville, Zhang, Qi Z, Shepard Siegel, Ed Czeck, John Miller, Igor Russkikh, Pavel Belous, Matt Peters, Rasesh Mody, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Wang, Xiao W, Yang, Qiming, Yigit, Ferruh, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Zyta Szpak, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Jerin Jacob, Richardson, Bruce, Andrew Rybchenko, Singh, Jasvinder, Dumitrescu, Cristian, Wiles, Keith, Maciej Czekaj, Maxime Coquelin, Wang, Zhihong, Yong Wang, Burakov, Anatoly, Xing, Beilei, Wu, Jingjing, dev Cc: dev, Loftus, Ciara, Steven Webster, Somalapuram Amaranath, xavier.huwei, Sachin Saxena, Zhao1, Wei, Guo, Jia, Igor Chauskin, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Min Hu (Connor), Yisen Zhuang, Alfredo Cardigliano, Jakub Grajciar, Viacheslav Ovsiienko, Long Li, Martin Spinler, Heinrich Kuhn, Harman Kalra, Nithin Dabilpuram, Kiran Kumar K, Akhil Goyal, Yigit, Ferruh Hi Thomas, Ipn3ke is based on ifpga_bus, and all ethdevs created from ipn3ke are representors. So it's no need to add RTE_ETH_DEV_CLOSE_REMOVE flags into ipn3ke driver. Thanks, Rosen > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Monday, September 14, 2020 6:16 > To: John W. Linville <linville@tuxdriver.com>; Zhang, Qi Z > <qi.z.zhang@intel.com>; Shepard Siegel <shepard.siegel@atomicrules.com>; > Ed Czeck <ed.czeck@atomicrules.com>; John Miller > <john.miller@atomicrules.com>; Igor Russkikh > <igor.russkikh@aquantia.com>; Pavel Belous <pavel.belous@aquantia.com>; > Matt Peters <matt.peters@windriver.com>; Rasesh Mody > <rmody@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>; Ajit > Khaparde <ajit.khaparde@broadcom.com>; Somnath Kotur > <somnath.kotur@broadcom.com>; Chas Williams <chas3@att.com>; Rahul > Lakkireddy <rahul.lakkireddy@chelsio.com>; Hemant Agrawal > <hemant.agrawal@nxp.com>; Marcin Wojtas <mw@semihalf.com>; Michal > Krawczyk <mk@semihalf.com>; Guy Tzalik <gtzalik@amazon.com>; Evgeny > Schemeilin <evgenys@amazon.com>; Gagandeep Singh <g.singh@nxp.com>; > John Daley <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>; > Gaetan Rivet <grive@u256.net>; Wang, Xiao W <xiao.w.wang@intel.com>; > Yang, Qiming <qiming.yang@intel.com>; Yigit, Ferruh > <ferruh.yigit@intel.com>; Shijith Thotton <sthotton@marvell.com>; > Srisivasubramanian Srinivasan <srinivasan@marvell.com>; Matan Azrad > <matan@mellanox.com>; Shahaf Shuler <shahafs@mellanox.com>; Zyta > Szpak <zr@semihalf.com>; Liron Himi <lironh@marvell.com>; Stephen > Hemminger <sthemmin@microsoft.com>; K. Y. Srinivasan > <kys@microsoft.com>; Haiyang Zhang <haiyangz@microsoft.com>; Jerin > Jacob <jerinj@marvell.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Andrew Rybchenko > <arybchenko@solarflare.com>; Singh, Jasvinder > <jasvinder.singh@intel.com>; Dumitrescu, Cristian > <cristian.dumitrescu@intel.com>; Wiles, Keith <keith.wiles@intel.com>; > Maciej Czekaj <mczekaj@marvell.com>; Maxime Coquelin > <maxime.coquelin@redhat.com>; Wang, Zhihong > <zhihong.wang@intel.com>; Yong Wang <yongwang@vmware.com>; > Burakov, Anatoly <anatoly.burakov@intel.com>; Xing, Beilei > <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > dev@dpdk.org > Cc: Xu, Rosen <rosen.xu@intel.com>; dev@dpdk.org; Loftus, Ciara > <ciara.loftus@intel.com>; Steven Webster <steven.webster@windriver.com>; > Somalapuram Amaranath <asomalap@amd.com>; > xavier.huwei@huawei.com; Sachin Saxena <sachin.saxena@nxp.com>; > Zhao1, Wei <wei.zhao1@intel.com>; Guo, Jia <jia.guo@intel.com>; Igor > Chauskin <igorch@amazon.com>; Ziyang Xuan <xuanziyang2@huawei.com>; > Xiaoyun Wang <cloud.wangxiaoyun@huawei.com>; Guoyang Zhou > <zhouguoyang@huawei.com>; Min Hu (Connor) <humin29@huawei.com>; > Yisen Zhuang <yisen.zhuang@huawei.com>; Alfredo Cardigliano > <cardigliano@ntop.org>; Jakub Grajciar <jgrajcia@cisco.com>; Viacheslav > Ovsiienko <viacheslavo@mellanox.com>; Long Li <longli@microsoft.com>; > Martin Spinler <spinler@cesnet.cz>; Heinrich Kuhn > <heinrich.kuhn@netronome.com>; Harman Kalra <hkalra@marvell.com>; > Nithin Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K > <kirankumark@marvell.com>; Akhil Goyal <akhil.goyal@nxp.com>; Yigit, > Ferruh <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net> > Subject: Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of > port > > The patches for removing RTE_ETH_DEV_CLOSE_REMOVE are sent: > https://patches.dpdk.org/project/dpdk/list/?series=12173 > > 11 drivers are not supporting the new behaviour correctly: > bnx2x, cxgbe, dpaa, dpaa2, enetc, ionic, > ipn3ke, liquidio, nfp, pfe, qede > > If you are the maintainer of one of these drivers, you can still consider fixing > it in the next days. > > > 12/09/2020 13:25, Thomas Monjalon: > > 03/08/2020 20:50, Thomas Monjalon: > > > 18/04/2019 12:59, Thomas Monjalon: > > > > 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. > > > > > > For the record, it did not happen in 19.11. > > > > > > > Following drivers should be migrated: > > > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; > > > > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | > > > > sort | uniq -u > > > [...] > > > > > > The progress in April 2019 was 4 of 46 (9%). > > > > > > > Please let's progress smoothly on this topic, thanks. > > > > > > More than one year later, the progress is 26 of 53 (49%). > > > > > > > The concerned maintainers (Cc) can be found with the following > command: > > > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 > > > > -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l > > > > RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) > > > > > > We cannot wait forever. Temporary cannot be longer than 2 years. > > > I am going to send a deprecation notice to remove the "temporary" > > > flag RTE_ETH_DEV_CLOSE_REMOVE. > > > > The deprecation notice was merged in 20.08: > > http://mails.dpdk.org/archives/dev/2020-August/177314.html > > > > > It will break drivers which are not migrated. > > > It will probably help to find motivation in new priorities. > > > > > > More details on what to do can be found in this mail thread: > > > http://inbox.dpdk.org/dev/1748144.UFpUr2FPnr@xps/ > > > > Summary: > > > > * The freeing of private port resources must be moved in the PMD from > > the ".remove(device)" function to the ".dev_close(port)" function. > > > > * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be > > freed, it must be set to NULL in ".dev_close" PMD function to protect > > from subsequent rte_eth_dev_release_port() freeing. > > > > * Note 1: > > The generic resources are freed in rte_eth_dev_release_port(), after > > ".dev_close" is called in rte_eth_dev_close(), but not when calling > > ".dev_close" directly from the ".remove" PMD function. > > That's why rte_eth_dev_release_port() must still be called explicitly > > from ".remove(device)" after calling the ".dev_close" PMD function. > > > > * Note 2: > > If a device can have multiple ports, the common resources must be > > freed only in the ".remove(device)" function. > > > > * Note 3: > > The port is supposed to be in a stopped state when it is closed. > > If it is not the case, it is free to the PMD implementation how to > > react when trying to close a non-stopped port: > > either try to stop it automatically or just return an error. > > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-09-22 3:04 ` Xu, Rosen @ 2020-09-22 7:28 ` Thomas Monjalon 2020-09-27 5:42 ` Xu, Rosen 0 siblings, 1 reply; 22+ messages in thread From: Thomas Monjalon @ 2020-09-22 7:28 UTC (permalink / raw) To: Xu, Rosen Cc: Zhang, Qi Z, Wang, Xiao W, Yang, Qiming, Yigit, Ferruh, Richardson, Bruce, Andrew Rybchenko, Wang, Zhihong, Xing, Beilei, Wu, Jingjing, dev, Zhao1, Wei, Guo, Jia, Yigit, Ferruh 22/09/2020 05:04, Xu, Rosen: > Hi Thomas, > > Ipn3ke is based on ifpga_bus, and all ethdevs created from ipn3ke are representors. > So it's no need to add RTE_ETH_DEV_CLOSE_REMOVE flags into ipn3ke driver. I don't understand how it is related. Do you mean it will work fine with the new close behaviour? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-09-22 7:28 ` Thomas Monjalon @ 2020-09-27 5:42 ` Xu, Rosen 0 siblings, 0 replies; 22+ messages in thread From: Xu, Rosen @ 2020-09-27 5:42 UTC (permalink / raw) To: Thomas Monjalon Cc: Zhang, Qi Z, Wang, Xiao W, Yang, Qiming, Yigit, Ferruh, Richardson, Bruce, Andrew Rybchenko, Wang, Zhihong, Xing, Beilei, Wu, Jingjing, dev, Zhao1, Wei, Guo, Jia, Yigit, Ferruh Hi, > -----Original Message----- > From: Thomas Monjalon <thomas@monjalon.net> > Sent: Tuesday, September 22, 2020 15:29 > To: Xu, Rosen <rosen.xu@intel.com> > Cc: Zhang, Qi Z <qi.z.zhang@intel.com>; Wang, Xiao W > <xiao.w.wang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Yigit, > Ferruh <ferruh.yigit@intel.com>; Richardson, Bruce > <bruce.richardson@intel.com>; Andrew Rybchenko > <arybchenko@solarflare.com>; Wang, Zhihong <zhihong.wang@intel.com>; > Xing, Beilei <beilei.xing@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; > dev@dpdk.org; Zhao1, Wei <wei.zhao1@intel.com>; Guo, Jia > <jia.guo@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com> > Subject: Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of > port > > 22/09/2020 05:04, Xu, Rosen: > > Hi Thomas, > > > > Ipn3ke is based on ifpga_bus, and all ethdevs created from ipn3ke are > representors. > > So it's no need to add RTE_ETH_DEV_CLOSE_REMOVE flags into ipn3ke > driver. > > I don't understand how it is related. > Do you mean it will work fine with the new close behaviour? yes ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port 2020-09-13 22:16 ` Thomas Monjalon 2020-09-14 9:20 ` Rahul Lakkireddy 2020-09-22 3:04 ` Xu, Rosen @ 2020-09-25 4:22 ` Rasesh Mody 2 siblings, 0 replies; 22+ messages in thread From: Rasesh Mody @ 2020-09-25 4:22 UTC (permalink / raw) To: Thomas Monjalon, John W. Linville, Qi Zhang, Shepard Siegel, Ed Czeck, John Miller, Igor Russkikh, Pavel Belous, Matt Peters, Shahed Shaikh, Ajit Khaparde, Somnath Kotur, Chas Williams, Rahul Lakkireddy, Hemant Agrawal, Marcin Wojtas, Michal Krawczyk, Guy Tzalik, Evgeny Schemeilin, Gagandeep Singh, John Daley, Hyong Youb Kim, Gaetan Rivet, Xiao Wang, Qiming Yang, Ferruh Yigit, Shijith Thotton, Srisivasubramanian Srinivasan, Matan Azrad, Shahaf Shuler, Zyta Szpak, Liron Himi, Stephen Hemminger, K. Y. Srinivasan, Haiyang Zhang, Jerin Jacob Kollanukkaran, Bruce Richardson, Andrew Rybchenko, Jasvinder Singh, Cristian Dumitrescu, Keith Wiles, Maciej Czekaj [C], Maxime Coquelin, Zhihong Wang, Yong Wang, Anatoly Burakov, Beilei Xing, Jingjing Wu, dev Cc: Rosen Xu, dev, Ciara Loftus, Steven Webster, Somalapuram Amaranath, xavier.huwei, Sachin Saxena, Wei Zhao, Jeff Guo, Igor Chauskin, Ziyang Xuan, Xiaoyun Wang, Guoyang Zhou, Min Hu (Connor), Yisen Zhuang, Alfredo Cardigliano, Jakub Grajciar, Viacheslav Ovsiienko, Long Li, Martin Spinler, Heinrich Kuhn, Harman Kalra, Nithin Kumar Dabilpuram, Kiran Kumar Kokkilagadda, Akhil Goyal, ferruh.yigit Hi Thomas, >From: dev <dev-bounces@dpdk.org> On Behalf Of Thomas Monjalon >Sent: Sunday, September 13, 2020 3:16 PM > >The patches for removing RTE_ETH_DEV_CLOSE_REMOVE are sent: > https://urldefense.proofpoint.com/v2/url?u=https- >3A__patches.dpdk.org_project_dpdk_list_-3Fseries- >3D12173&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Vhi2FR3R84xPMUtUhj >NPxoiMSxcj1IW0xDKEoZ0F00o&m=9C_CX6dJtCkd6pU6mUDB8eV- >EorzP6ErRLFuCc-dtTg&s=8L7RxyHhOuIXf3MBurNOeqvGa-gGhH1NY9vnoj- >fXM4&e= > >11 drivers are not supporting the new behaviour correctly: > bnx2x, cxgbe, dpaa, dpaa2, enetc, ionic, > ipn3ke, liquidio, nfp, pfe, qede > >If you are the maintainer of one of these drivers, you can still consider fixing it >in the next days. > The fixes for bnx2x and qede PMDs are posted. https://patches.dpdk.org/patch/78779/ https://patches.dpdk.org/patch/78780/ Thanks! -Rasesh > >12/09/2020 13:25, Thomas Monjalon: >> 03/08/2020 20:50, Thomas Monjalon: >> > 18/04/2019 12:59, Thomas Monjalon: >> > > 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: >> > > >> > > https://urldefense.proofpoint.com/v2/url?u=http-3A__git.dpdk.org_d >> > > pdk_commit_-3Fid- >3D23ea57a2a&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=V >> > > >hi2FR3R84xPMUtUhjNPxoiMSxcj1IW0xDKEoZ0F00o&m=9C_CX6dJtCkd6pU6m >UDB8 >> > > eV-EorzP6ErRLFuCc- >dtTg&s=23dp27AoThshb5NflUTFh3HH3M_KPwa8Svd2TOeN9 >> > > gs&e= >> > > " >> > > 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. >> > >> > For the record, it did not happen in 19.11. >> > >> > > Following drivers should be migrated: >> > > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; >> > > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | >> > > sort | uniq -u >> > [...] >> > >> > The progress in April 2019 was 4 of 46 (9%). >> > >> > > Please let's progress smoothly on this topic, thanks. >> > >> > More than one year later, the progress is 26 of 53 (49%). >> > >> > > The concerned maintainers (Cc) can be found with the following >command: >> > > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 >> > > -maxdepth 1 -type d | cut -d/ -f-3 ; git grep -l >> > > RTE_ETH_DEV_CLOSE_REMOVE drivers ) | sort | uniq -u) >> > >> > We cannot wait forever. Temporary cannot be longer than 2 years. >> > I am going to send a deprecation notice to remove the "temporary" >> > flag RTE_ETH_DEV_CLOSE_REMOVE. >> >> The deprecation notice was merged in 20.08: >> >> https://urldefense.proofpoint.com/v2/url?u=http-3A__mails.dpdk.org_arc >> hives_dev_2020- >2DAugust_177314.html&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ& >> >r=Vhi2FR3R84xPMUtUhjNPxoiMSxcj1IW0xDKEoZ0F00o&m=9C_CX6dJtCkd6p >U6mUDB8e >> V-EorzP6ErRLFuCc-dtTg&s=PRJ- >nWVm0NO6WrpQDWaI0ocSwaIDowsMQNWrif0FDgg&e= >> >> > It will break drivers which are not migrated. >> > It will probably help to find motivation in new priorities. >> > >> > More details on what to do can be found in this mail thread: >> > >> > https://urldefense.proofpoint.com/v2/url?u=http-3A__inbox.dpdk.org_d >> > ev_1748144.UFpUr2FPnr- >40xps_&d=DwICAg&c=nKjWec2b6R0mOyPaz7xtfQ&r=Vhi >> > >2FR3R84xPMUtUhjNPxoiMSxcj1IW0xDKEoZ0F00o&m=9C_CX6dJtCkd6pU6mU >DB8eV-E >> > orzP6ErRLFuCc- >dtTg&s=00JuVYCjq_EZW27VmszpI4E8Yq_dN9oGjewjBTaCZ9s&e= >> >> Summary: >> >> * The freeing of private port resources must be moved in the PMD from >> the ".remove(device)" function to the ".dev_close(port)" function. >> >> * If a generic resource (.mac_addrs or .hash_mac_addrs) cannot be >> freed, it must be set to NULL in ".dev_close" PMD function to protect >> from subsequent rte_eth_dev_release_port() freeing. >> >> * Note 1: >> The generic resources are freed in rte_eth_dev_release_port(), after >> ".dev_close" is called in rte_eth_dev_close(), but not when calling >> ".dev_close" directly from the ".remove" PMD function. >> That's why rte_eth_dev_release_port() must still be called explicitly >> from ".remove(device)" after calling the ".dev_close" PMD function. >> >> * Note 2: >> If a device can have multiple ports, the common resources must be >> freed only in the ".remove(device)" function. >> >> * Note 3: >> The port is supposed to be in a stopped state when it is closed. >> If it is not the case, it is free to the PMD implementation how to >> react when trying to close a non-stopped port: >> either try to stop it automatically or just return an error. > > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-09-27 5:42 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-18 10:59 [dpdk-dev] CALL to eth PMD maintainers: complete closing of port Thomas Monjalon 2019-04-18 10:59 ` Thomas Monjalon 2019-04-28 6:57 ` Matan Azrad 2019-04-28 6:57 ` Matan Azrad 2019-04-28 16:17 ` Stephen Hemminger 2019-04-28 16:17 ` Stephen Hemminger 2019-04-29 16:51 ` Ferruh Yigit 2019-04-29 16:51 ` Ferruh Yigit 2019-04-29 20:30 ` Thomas Monjalon 2019-04-29 20:30 ` Thomas Monjalon 2019-04-30 12:45 ` Nithin Dabilpuram 2019-04-30 12:45 ` Nithin Dabilpuram 2019-04-30 14:12 ` Thomas Monjalon 2019-04-30 14:12 ` Thomas Monjalon 2020-08-03 18:50 ` Thomas Monjalon 2020-09-12 11:25 ` Thomas Monjalon 2020-09-13 22:16 ` Thomas Monjalon 2020-09-14 9:20 ` Rahul Lakkireddy 2020-09-22 3:04 ` Xu, Rosen 2020-09-22 7:28 ` Thomas Monjalon 2020-09-27 5:42 ` Xu, Rosen 2020-09-25 4:22 ` Rasesh Mody
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).