[v1] e1000e: roll back registered MSI-X IRQs on failure

[PATCH net] e1000e: roll back registered MSI-X IRQs on failure

Posted by Runyu Xiao 1 week, 3 days ago

e1000_request_msix() requests the RX, TX, and other-cause interrupt
handlers sequentially. If a later request_irq() fails, the function
returns without releasing handlers that were registered earlier. The
caller then disables MSI-X and falls back to MSI or legacy interrupts,
leaving those handlers registered against disabled MSI-X vectors.

Free all handlers registered before the failing request in reverse order
before returning the error. The failed vector is not freed, and the
existing fallback path can then disable MSI-X without retaining stale
handlers.

A QEMU e1000e test with deterministic failure injection at the third
request_irq() reproduced a residual IRQ entry and warning on the
unfixed kernel. The fixed kernel reached the same fallback without the
residual entry. The injection is deliberate and does not claim that a
third request_irq() failure occurs spontaneously during normal operation.

Fixes: 4662e82b2cb4 ("e1000e: add support for new 82574L part")
Cc: stable@vger.kernel.org
Assisted-by: LLM Codex
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/net/ethernet/intel/e1000e/netdev.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 844f31ab37ad4..f55aec340342b 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -2139,7 +2139,7 @@ static int e1000_request_msix(struct e1000_adapter *adapter)
 			  e1000_intr_msix_tx, 0, adapter->tx_ring->name,
 			  netdev);
 	if (err)
-		return err;
+		goto err_irq;
 	adapter->tx_ring->itr_register = adapter->hw.hw_addr +
 	    E1000_EITR_82574(vector);
 	adapter->tx_ring->itr_val = adapter->itr;
@@ -2148,11 +2148,16 @@ static int e1000_request_msix(struct e1000_adapter *adapter)
 	err = request_irq(adapter->msix_entries[vector].vector,
 			  e1000_msix_other, 0, netdev->name, netdev);
 	if (err)
-		return err;
+		goto err_irq;
 
 	e1000_configure_msix(adapter);
 
 	return 0;
+
+err_irq:
+	while (vector)
+		free_irq(adapter->msix_entries[--vector].vector, netdev);
+	return err;
 }
 
 /**
-- 
2.34.1

Re: [PATCH net] e1000e: roll back registered MSI-X IRQs on failure

Posted by Paul Menzel 1 week, 3 days ago

Dear Runyu,


Thank you for your patch.

Am 14.09.26 um 16:20 schrieb Runyu Xiao:
> e1000_request_msix() requests the RX, TX, and other-cause interrupt
> handlers sequentially. If a later request_irq() fails, the function
> returns without releasing handlers that were registered earlier. The
> caller then disables MSI-X and falls back to MSI or legacy interrupts,
> leaving those handlers registered against disabled MSI-X vectors.
> 
> Free all handlers registered before the failing request in reverse order
> before returning the error. The failed vector is not freed, and the
> existing fallback path can then disable MSI-X without retaining stale
> handlers.
> 
> A QEMU e1000e test with deterministic failure injection at the third
> request_irq() reproduced a residual IRQ entry and warning on the
> unfixed kernel. The fixed kernel reached the same fallback without the
> residual entry. The injection is deliberate and does not claim that a
> third request_irq() failure occurs spontaneously during normal operation.

Could you please amend the commit message, and give more information how 
to reproduce this exactly, and get the warning?

> Fixes: 4662e82b2cb4 ("e1000e: add support for new 82574L part")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM Codex

Please add the version.

> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 844f31ab37ad4..f55aec340342b 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -2139,7 +2139,7 @@ static int e1000_request_msix(struct e1000_adapter *adapter)
>   			  e1000_intr_msix_tx, 0, adapter->tx_ring->name,
>   			  netdev);
>   	if (err)
> -		return err;
> +		goto err_irq;
>   	adapter->tx_ring->itr_register = adapter->hw.hw_addr +
>   	    E1000_EITR_82574(vector);
>   	adapter->tx_ring->itr_val = adapter->itr;
> @@ -2148,11 +2148,16 @@ static int e1000_request_msix(struct e1000_adapter *adapter)
>   	err = request_irq(adapter->msix_entries[vector].vector,
>   			  e1000_msix_other, 0, netdev->name, netdev);
>   	if (err)
> -		return err;
> +		goto err_irq;
>   
>   	e1000_configure_msix(adapter);
>   
>   	return 0;
> +
> +err_irq:
> +	while (vector)
> +		free_irq(adapter->msix_entries[--vector].vector, netdev);
> +	return err;
>   }
>   
>   /**

The diff looks good.


Kind regards,

Paul

RE: [PATCH net] e1000e: roll back registered MSI-X IRQs on failure

Posted by Loktionov, Aleksandr 1 week, 3 days ago


> -----Original Message-----
> From: Runyu Xiao <runyu.xiao@seu.edu.cn>
> Sent: Monday, September 14, 2026 4:20 PM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>
> Cc: Andrew Lunn <andrew+netdev@lunn.ch>; David S . Miller
> <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub
> Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; intel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; Runyu Xiao
> <runyu.xiao@seu.edu.cn>; jianhao.xu@seu.edu.cn
> Subject: [PATCH net] e1000e: roll back registered MSI-X IRQs on
> failure
> 
> e1000_request_msix() requests the RX, TX, and other-cause interrupt
> handlers sequentially. If a later request_irq() fails, the function
> returns without releasing handlers that were registered earlier. The
> caller then disables MSI-X and falls back to MSI or legacy interrupts,
> leaving those handlers registered against disabled MSI-X vectors.
> 
> Free all handlers registered before the failing request in reverse
> order before returning the error. The failed vector is not freed, and
> the existing fallback path can then disable MSI-X without retaining
> stale handlers.
> 
> A QEMU e1000e test with deterministic failure injection at the third
> request_irq() reproduced a residual IRQ entry and warning on the
> unfixed kernel. The fixed kernel reached the same fallback without the
> residual entry. The injection is deliberate and does not claim that a
> third request_irq() failure occurs spontaneously during normal
> operation.
> 
> Fixes: 4662e82b2cb4 ("e1000e: add support for new 82574L part")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM Codex
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
> ---
>  drivers/net/ethernet/intel/e1000e/netdev.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 844f31ab37ad4..f55aec340342b 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -2139,7 +2139,7 @@ static int e1000_request_msix(struct
> e1000_adapter *adapter)
>  			  e1000_intr_msix_tx, 0, adapter->tx_ring->name,
>  			  netdev);
>  	if (err)
> -		return err;
> +		goto err_irq;
>  	adapter->tx_ring->itr_register = adapter->hw.hw_addr +
>  	    E1000_EITR_82574(vector);
>  	adapter->tx_ring->itr_val = adapter->itr; @@ -2148,11 +2148,16
> @@ static int e1000_request_msix(struct e1000_adapter *adapter)
>  	err = request_irq(adapter->msix_entries[vector].vector,
>  			  e1000_msix_other, 0, netdev->name, netdev);
>  	if (err)
> -		return err;
> +		goto err_irq;
> 
>  	e1000_configure_msix(adapter);
> 
>  	return 0;
> +
> +err_irq:
> +	while (vector)
> +		free_irq(adapter->msix_entries[--vector].vector,
> netdev);
> +	return err;
>  }
> 
>  /**
> --
> 2.34.1


Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>