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)
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)
Hi Thomas
vdev_netvsc has no close API - should I change something there?
> From: Thomas Monjalon <thomas@monjalon.net>
> Hi all,
>
> Since DPDK 18.11, the behaviour of the close operation is changed if
> RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver:
> port is released (i.e. totally freed and data erased) on close.
> This new behaviour is enabled per driver for a migration period.
>
> Looking at the code, you can see these comments:
> /* old behaviour: only free queue arrays */ RTE_ETHDEV_LOG(DEBUG, "Port
> closing is using an old behaviour.\n"
> "The driver %s should migrate to the new behaviour.\n",
> /* new behaviour: send event + reset state + free all data */
>
> You can find an advice in the commit:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2
> Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7
> C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> %3D&reserved=0
> "
> When enabling RTE_ETH_DEV_CLOSE_REMOVE,
> the PMD must free all its private resources for the port, in its dev_close
> function.
> It is advised to call the dev_close function in the remove function in order to
> support removing a device without closing its ports.
> "
>
> It would be great to complete this migration for the next LTS version, which
> will be 19.11.
> It means the work should be ideally finished during the summer.
>
> The migration to the new behaviour is done in 4 drivers:
> git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3
> ena
> enic
> mlx5
> vmxnet3
>
> Following drivers should be migrated:
> ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l
> RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u
> af_packet
> af_xdp
> ark
> atlantic
> avp
> axgbe
> bnx2x
> bnxt
> bonding
> cxgbe
> dpaa
> dpaa2
> e1000
> enetc
> failsafe
> fm10k
> i40e
> iavf
> ice
> ifc
> ixgbe
> kni
> liquidio
> mlx4
> mvneta
> mvpp2
> netvsc
> nfb
> nfp
> null
> octeontx
> pcap
> qede
> ring
> sfc
> softnic
> szedata2
> tap
> thunderx
> vdev_netvsc
> vhost
> virtio
>
> Please let's progress smoothly on this topic, thanks.
>
> The concerned maintainers (Cc) can be found with the following command:
> devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -
> type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) |
> sort | uniq -u)
>
Hi Thomas
vdev_netvsc has no close API - should I change something there?
> From: Thomas Monjalon <thomas@monjalon.net>
> Hi all,
>
> Since DPDK 18.11, the behaviour of the close operation is changed if
> RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver:
> port is released (i.e. totally freed and data erased) on close.
> This new behaviour is enabled per driver for a migration period.
>
> Looking at the code, you can see these comments:
> /* old behaviour: only free queue arrays */ RTE_ETHDEV_LOG(DEBUG, "Port
> closing is using an old behaviour.\n"
> "The driver %s should migrate to the new behaviour.\n",
> /* new behaviour: send event + reset state + free all data */
>
> You can find an advice in the commit:
> https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2
> Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7
> C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> %3D&reserved=0
> "
> When enabling RTE_ETH_DEV_CLOSE_REMOVE,
> the PMD must free all its private resources for the port, in its dev_close
> function.
> It is advised to call the dev_close function in the remove function in order to
> support removing a device without closing its ports.
> "
>
> It would be great to complete this migration for the next LTS version, which
> will be 19.11.
> It means the work should be ideally finished during the summer.
>
> The migration to the new behaviour is done in 4 drivers:
> git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3
> ena
> enic
> mlx5
> vmxnet3
>
> Following drivers should be migrated:
> ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l
> RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u
> af_packet
> af_xdp
> ark
> atlantic
> avp
> axgbe
> bnx2x
> bnxt
> bonding
> cxgbe
> dpaa
> dpaa2
> e1000
> enetc
> failsafe
> fm10k
> i40e
> iavf
> ice
> ifc
> ixgbe
> kni
> liquidio
> mlx4
> mvneta
> mvpp2
> netvsc
> nfb
> nfp
> null
> octeontx
> pcap
> qede
> ring
> sfc
> softnic
> szedata2
> tap
> thunderx
> vdev_netvsc
> vhost
> virtio
>
> Please let's progress smoothly on this topic, thanks.
>
> The concerned maintainers (Cc) can be found with the following command:
> devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -
> type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) |
> sort | uniq -u)
>
On Sun, 28 Apr 2019 06:57:59 +0000
Matan Azrad <matan@mellanox.com> wrote:
> Hi Thomas
>
> vdev_netvsc has no close API - should I change something there?
>
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Hi all,
> >
> > Since DPDK 18.11, the behaviour of the close operation is changed if
> > RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver:
> > port is released (i.e. totally freed and data erased) on close.
> > This new behaviour is enabled per driver for a migration period.
> >
> > Looking at the code, you can see these comments:
> > /* old behaviour: only free queue arrays */ RTE_ETHDEV_LOG(DEBUG, "Port
> > closing is using an old behaviour.\n"
> > "The driver %s should migrate to the new behaviour.\n",
> > /* new behaviour: send event + reset state + free all data */
> >
> > You can find an advice in the commit:
> > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2
> > Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7
> > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> > %3D&reserved=0
> > "
> > When enabling RTE_ETH_DEV_CLOSE_REMOVE,
> > the PMD must free all its private resources for the port, in its dev_close
> > function.
> > It is advised to call the dev_close function in the remove function in order to
> > support removing a device without closing its ports.
> > "
> >
> > It would be great to complete this migration for the next LTS version, which
> > will be 19.11.
> > It means the work should be ideally finished during the summer.
> >
> > The migration to the new behaviour is done in 4 drivers:
> > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3
> > ena
> > enic
> > mlx5
> > vmxnet3
> >
> > Following drivers should be migrated:
> > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l
> > RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u
> > af_packet
> > af_xdp
> > ark
> > atlantic
> > avp
> > axgbe
> > bnx2x
> > bnxt
> > bonding
> > cxgbe
> > dpaa
> > dpaa2
> > e1000
> > enetc
> > failsafe
> > fm10k
> > i40e
> > iavf
> > ice
> > ifc
> > ixgbe
> > kni
> > liquidio
> > mlx4
> > mvneta
> > mvpp2
> > netvsc
> > nfb
> > nfp
> > null
> > octeontx
> > pcap
> > qede
> > ring
> > sfc
> > softnic
> > szedata2
> > tap
> > thunderx
> > vdev_netvsc
> > vhost
> > virtio
> >
> > Please let's progress smoothly on this topic, thanks.
> >
> > The concerned maintainers (Cc) can be found with the following command:
> > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -
> > type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) |
> > sort | uniq -u)
> >
>
I have a version still testing
On Sun, 28 Apr 2019 06:57:59 +0000
Matan Azrad <matan@mellanox.com> wrote:
> Hi Thomas
>
> vdev_netvsc has no close API - should I change something there?
>
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Hi all,
> >
> > Since DPDK 18.11, the behaviour of the close operation is changed if
> > RTE_ETH_DEV_CLOSE_REMOVE is enabled in the driver:
> > port is released (i.e. totally freed and data erased) on close.
> > This new behaviour is enabled per driver for a migration period.
> >
> > Looking at the code, you can see these comments:
> > /* old behaviour: only free queue arrays */ RTE_ETHDEV_LOG(DEBUG, "Port
> > closing is using an old behaviour.\n"
> > "The driver %s should migrate to the new behaviour.\n",
> > /* new behaviour: send event + reset state + free all data */
> >
> > You can find an advice in the commit:
> > https://eur03.safelinks.protection.outlook.com/?url=http%3A%2F%2
> > Fgit.dpdk.org%2Fdpdk%2Fcommit%2F%3Fid%3D23ea57a2a&data=02%7
> > C01%7Cmatan%40mellanox.com%7C3fea370ec31f4d22be8708d6c3ecf74b%7
> > Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636911819926371170&a
> > mp;sdata=%2BHLHG6VK2gNLejSHRKLYtS4Qelqg%2FOD%2FUQbZwOIT9%2BA
> > %3D&reserved=0
> > "
> > When enabling RTE_ETH_DEV_CLOSE_REMOVE,
> > the PMD must free all its private resources for the port, in its dev_close
> > function.
> > It is advised to call the dev_close function in the remove function in order to
> > support removing a device without closing its ports.
> > "
> >
> > It would be great to complete this migration for the next LTS version, which
> > will be 19.11.
> > It means the work should be ideally finished during the summer.
> >
> > The migration to the new behaviour is done in 4 drivers:
> > git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3
> > ena
> > enic
> > mlx5
> > vmxnet3
> >
> > Following drivers should be migrated:
> > ( find drivers/net -mindepth 1 -maxdepth 1 -type d | cut -d/ -f3 ; git grep -l
> > RTE_ETH_DEV_CLOSE_REMOVE drivers | cut -d/ -f3 ) | sort | uniq -u
> > af_packet
> > af_xdp
> > ark
> > atlantic
> > avp
> > axgbe
> > bnx2x
> > bnxt
> > bonding
> > cxgbe
> > dpaa
> > dpaa2
> > e1000
> > enetc
> > failsafe
> > fm10k
> > i40e
> > iavf
> > ice
> > ifc
> > ixgbe
> > kni
> > liquidio
> > mlx4
> > mvneta
> > mvpp2
> > netvsc
> > nfb
> > nfp
> > null
> > octeontx
> > pcap
> > qede
> > ring
> > sfc
> > softnic
> > szedata2
> > tap
> > thunderx
> > vdev_netvsc
> > vhost
> > virtio
> >
> > Please let's progress smoothly on this topic, thanks.
> >
> > The concerned maintainers (Cc) can be found with the following command:
> > devtools/get-maintainer.sh $(( find drivers/net -mindepth 1 -maxdepth 1 -
> > type d | cut -d/ -f-3 ; git grep -l RTE_ETH_DEV_CLOSE_REMOVE drivers ) |
> > sort | uniq -u)
> >
>
I have a version still testing
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.
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.
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.
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.
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. > >
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. > >
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().
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().
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/
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.
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.
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. > > >
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.
>
>
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?
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. > >
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