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: [email protected]
> Signed-off-by: SidAli CHERRATI <[email protected]>
> ---
> 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
