rte_intr_callback_unregister() returns the number of unregistered
callbacks on success. Since the close operation started returning
the ret variable instead of 0, this positive value leaks as the
return value of ixgbe_dev_close(), so applications checking
rte_eth_dev_close() != 0 treat a successful close as a failure.

Use a separate cb_ret variable for the unregister return value so a
successful unregister never clobbers the dev_stop() error already
stored in ret, and stop retrying once unregister fails with anything
other than -EAGAIN, since those errors are not transient.

Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: [email protected]

Signed-off-by: Sid Ali Cherrati <[email protected]>
---
v2: use a temporary cb_ret variable instead of unconditionally zeroing
    ret, and stop retrying on a non-transient unregister error, so an
    earlier error is not lost or masked by a successful/failed
    unregister

 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.c 
b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
index c5010f623c..9433515b85 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
@@ -3095,6 +3095,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
        struct rte_intr_handle *intr_handle = pci_dev->intr_handle;
        int retries = 0;
        int ret;
+       int cb_ret;
 
        PMD_INIT_FUNC_TRACE();
        if (rte_eal_process_type() != RTE_PROC_PRIMARY)
@@ -3118,14 +3119,17 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
        rte_intr_disable(intr_handle);
 
        do {
-               ret = rte_intr_callback_unregister(intr_handle,
+               cb_ret = rte_intr_callback_unregister(intr_handle,
                                ixgbe_dev_interrupt_handler, dev);
-               if (ret >= 0 || ret == -ENOENT) {
+               if (cb_ret >= 0 || cb_ret == -ENOENT) {
                        break;
-               } else if (ret != -EAGAIN) {
+               } else if (cb_ret != -EAGAIN) {
                        PMD_INIT_LOG(ERR,
                                "intr callback unregister failed: %d",
-                               ret);
+                               cb_ret);
+                       if (ret == 0)
+                               ret = cb_ret;
+                       break;
                }
                rte_delay_ms(100);
        } while (retries++ < (10 + IXGBE_LINK_UP_TIME));
-- 
2.34.1

Reply via email to