[PATCH 1/2] net/i40e: fix return value of close operation

[PATCH 1/2] net/i40e: fix return value of close operation

[SidAli CHERRATI]

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 i40e_dev_close(), so applications checking
rte_eth_dev_close() != 0 treat a successful close as a failure.

Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: stable@dpdk.org
Signed-off-by: SidAli CHERRATI <scherrati1@gmail.com>
---
 drivers/net/intel/i40e/i40e_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/intel/i40e/i40e_ethdev.c b/drivers/net/intel/i40e/i40e_ethdev.c
index b6b2d291ee..2fe6b1e118 100644
--- a/drivers/net/intel/i40e/i40e_ethdev.c
+++ b/drivers/net/intel/i40e/i40e_ethdev.c
@@ -2764,6 +2764,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
 		ret = rte_intr_callback_unregister(intr_handle,
 				i40e_dev_interrupt_handler, dev);
 		if (ret >= 0 || ret == -ENOENT) {
+			ret = 0;
 			break;
 		} else if (ret != -EAGAIN) {
 			PMD_INIT_LOG(ERR,
-- 
2.34.1

[PATCH 2/2] net/ixgbe: fix return value of close operation

[SidAli CHERRATI]

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.

Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
Cc: stable@dpdk.org
Signed-off-by: SidAli CHERRATI <scherrati1@gmail.com>
---
 drivers/net/intel/ixgbe/ixgbe_ethdev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.c b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
index c5010f623c..6b1799d7a0 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
@@ -3121,6 +3121,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
 		ret = rte_intr_callback_unregister(intr_handle,
 				ixgbe_dev_interrupt_handler, dev);
 		if (ret >= 0 || ret == -ENOENT) {
+			ret = 0;
 			break;
 		} else if (ret != -EAGAIN) {
 			PMD_INIT_LOG(ERR,
-- 
2.34.1

Re: [PATCH 2/2] net/ixgbe: fix return value of close operation

[Bruce Richardson]

On Fri, Jul 10, 2026 at 03:46:39PM +0200, SidAli CHERRATI wrote:
> 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.
> 
> Fixes: 62024eb82756 ("ethdev: change stop operation callback to return int")
> Cc: stable@dpdk.org
> Signed-off-by: SidAli CHERRATI <scherrati1@gmail.com>
> ---
>  drivers/net/intel/ixgbe/ixgbe_ethdev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.c b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
> index c5010f623c..6b1799d7a0 100644
> --- a/drivers/net/intel/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.c
> @@ -3121,6 +3121,7 @@ ixgbe_dev_close(struct rte_eth_dev *dev)
>  		ret = rte_intr_callback_unregister(intr_handle,
>  				ixgbe_dev_interrupt_handler, dev);
>  		if (ret >= 0 || ret == -ENOENT) {
> +			ret = 0;
>  			break;

This fixes the issue of returning an invalid value from the interrupt
unregister, but it also means losing the previous value of ret from the
stop call above. I'm therefore wondering if a better fix is to use a new
temporary variable inside the do{ }while loop, and assign that to ret in
the case of an actual error, otherwise leaving ret unmodified.
In that case, we probably want to stop retries on an error too, right?

	do {
		int cb_ret = rte_intr_callback_unregister(intr_handle,
				ixgbe_dev_interrupt_handler, dev);
		if (cb_ret >= 0 || cb_ret == -ENOENT) {
			break;
		} else if (cb_ret != -EAGAIN) {
			PMD_INIT_LOG(ERR,
				"intr callback unregister failed: %d",
				cb_ret);
			ret = cb_ret;
			break;  /* on error, don't retry */
		}
		rte_delay_ms(100);
	} while (retries++ < (10 + IXGBE_LINK_UP_TIME));

IF we do need to retry on error, then this approach won't work. Maybe a
modified version of your suggestion might do instead - rather than assigning
0 to ret on success, we assign a saved off version of it from before the
loop?

What do you think?
/Bruce