| Message ID | 20210122175804.772207-1-thomas@monjalon.net (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Ferruh Yigit |
| Headers | show |
| Series | [1/1] ethdev: fix handling of close failure | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-testing | warning | Testing issues |
| ci/iol-mellanox-Functional | success | Functional Testing PASS |
| ci/iol-intel-Performance | fail | Performance Testing issues |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/intel-Testing | success | Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
On 1/22/21 8:58 PM, Thomas Monjalon wrote: > If a failure happens when closing a port, > it was unnecessarily failing again in the function eth_err(), > because of a check against HW removal cause. > Indeed there is a big chance the port is released at this point. > Given the port is in the middle (or at the end) of a close process, > checking the error cause by accessing the port is a non-sense. > The error check is replaced by a simple return in the close function. > > Bugzilla ID: 624 > Fixes: 8a5a0aad5d3e ("ethdev: allow close function to return an error") > Cc: stable@dpdk.org > > Reported-by: Anatoly Burakov <anatoly.burakov@intel.com> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
On 22-Jan-21 5:58 PM, Thomas Monjalon wrote: > If a failure happens when closing a port, > it was unnecessarily failing again in the function eth_err(), > because of a check against HW removal cause. > Indeed there is a big chance the port is released at this point. > Given the port is in the middle (or at the end) of a close process, > checking the error cause by accessing the port is a non-sense. > The error check is replaced by a simple return in the close function. > > Bugzilla ID: 624 > Fixes: 8a5a0aad5d3e ("ethdev: allow close function to return an error") > Cc: stable@dpdk.org > > Reported-by: Anatoly Burakov <anatoly.burakov@intel.com> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > --- Tested-by: Anatoly Burakov <anatoly.burakov@intel.com>
25/01/2021 10:13, Andrew Rybchenko: > On 1/22/21 8:58 PM, Thomas Monjalon wrote: > > If a failure happens when closing a port, > > it was unnecessarily failing again in the function eth_err(), > > because of a check against HW removal cause. > > Indeed there is a big chance the port is released at this point. > > Given the port is in the middle (or at the end) of a close process, > > checking the error cause by accessing the port is a non-sense. > > The error check is replaced by a simple return in the close function. > > > > Bugzilla ID: 624 > > Fixes: 8a5a0aad5d3e ("ethdev: allow close function to return an error") > > Cc: stable@dpdk.org > > > > Reported-by: Anatoly Burakov <anatoly.burakov@intel.com> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> Applied
25/01/2021 13:37, Thomas Monjalon: > 25/01/2021 10:13, Andrew Rybchenko: > > On 1/22/21 8:58 PM, Thomas Monjalon wrote: > > > If a failure happens when closing a port, > > > it was unnecessarily failing again in the function eth_err(), > > > because of a check against HW removal cause. > > > Indeed there is a big chance the port is released at this point. > > > Given the port is in the middle (or at the end) of a close process, > > > checking the error cause by accessing the port is a non-sense. > > > The error check is replaced by a simple return in the close function. > > > > > > Bugzilla ID: 624 > > > Fixes: 8a5a0aad5d3e ("ethdev: allow close function to return an error") > > > Cc: stable@dpdk.org > > > > > > Reported-by: Anatoly Burakov <anatoly.burakov@intel.com> > > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > > > > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> > > Applied Sorry please ignore this wrong message, patch not applied. (will be considered by Ferruh)
On 1/25/2021 9:13 AM, Andrew Rybchenko wrote: > On 1/22/21 8:58 PM, Thomas Monjalon wrote: >> If a failure happens when closing a port, >> it was unnecessarily failing again in the function eth_err(), >> because of a check against HW removal cause. >> Indeed there is a big chance the port is released at this point. >> Given the port is in the middle (or at the end) of a close process, >> checking the error cause by accessing the port is a non-sense. >> The error check is replaced by a simple return in the close function. >> >> Bugzilla ID: 624 >> Fixes: 8a5a0aad5d3e ("ethdev: allow close function to return an error") >> Cc: stable@dpdk.org >> >> Reported-by: Anatoly Burakov <anatoly.burakov@intel.com> >> Signed-off-by: Thomas Monjalon <thomas@monjalon.net> > > Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru> > Applied to dpdk-next-net/main, thanks.
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c index daf5f24f7e..89455a432e 100644 --- a/lib/librte_ethdev/rte_ethdev.c +++ b/lib/librte_ethdev/rte_ethdev.c @@ -1820,7 +1820,7 @@ rte_eth_dev_close(uint16_t port_id) rte_ethdev_trace_close(port_id); *lasterr = rte_eth_dev_release_port(dev); - return eth_err(port_id, firsterr); + return firsterr; } int
If a failure happens when closing a port, it was unnecessarily failing again in the function eth_err(), because of a check against HW removal cause. Indeed there is a big chance the port is released at this point. Given the port is in the middle (or at the end) of a close process, checking the error cause by accessing the port is a non-sense. The error check is replaced by a simple return in the close function. Bugzilla ID: 624 Fixes: 8a5a0aad5d3e ("ethdev: allow close function to return an error") Cc: stable@dpdk.org Reported-by: Anatoly Burakov <anatoly.burakov@intel.com> Signed-off-by: Thomas Monjalon <thomas@monjalon.net> --- lib/librte_ethdev/rte_ethdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)