DPDK patches and discussions
 help / color / mirror / Atom feed
* [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&amp;data=02%7
> C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> %3D&amp;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&amp;data=02%7
> C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> %3D&amp;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&amp;data=02%7
> > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> > %3D&amp;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&amp;data=02%7
> > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> > %3D&amp;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-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

* 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

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).