[dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[dpdk-dev] CALL to eth PMD maintainers: complete closing of port

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

[dpdk-dev] CALL to eth PMD maintainers: complete closing of port

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Matan Azrad]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Matan Azrad]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Stephen Hemminger]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Stephen Hemminger]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[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() ? )
- 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.

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[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() ? )
- 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.

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Thomas Monjalon]

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.

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Thomas Monjalon]

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.

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Nithin Dabilpuram]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Nithin Dabilpuram]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Thomas Monjalon]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Thomas Monjalon]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

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

> 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/

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Rahul Lakkireddy]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

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

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Thomas Monjalon]

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?

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Rasesh Mody]

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

Re: [dpdk-dev] CALL to eth PMD maintainers: complete closing of port

[Xu, Rosen]

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