All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: David Carlier <devnexen@gmail.com>
Cc: <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<anthony.l.nguyen@intel.com>, <przemyslaw.kitszel@intel.com>,
	<aleksandr.loktionov@intel.com>, <advoretsky@gmail.com>,
	<stable@vger.kernel.org>
Subject: Re: [PATCH net] igc: remove napi_synchronize() in igc_down()
Date: Mon, 13 Jul 2026 11:42:34 +0200	[thread overview]
Message-ID: <alSzCokS7scCoza7@boxer> (raw)
In-Reply-To: <20260712132242.223254-1-devnexen@gmail.com>

On Sun, Jul 12, 2026 at 02:22:42PM +0100, David Carlier wrote:
> When an AF_XDP zero-copy application is killed abruptly, the XSK pool is
> torn down but NAPI keeps polling. igc_clean_rx_irq_zc() then returns the
> full budget on every poll, so napi_complete_done() never clears
> NAPI_STATE_SCHED.
> 
> igc_down() calls napi_synchronize() before napi_disable(), so it spins
> forever waiting for that bit and the interface never goes down. Drop the
> napi_synchronize() and let napi_disable() do the job -- it sets
> NAPI_STATE_DISABLE, which forces the stuck poll to complete. Reorder it
> ahead of igc_set_queue_napi() so the NAPI mapping is cleared only after
> polling has stopped, matching the recent igb fix b1e067240379.
> 
> Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

This is a mirror of what Alex Dvoretsky did on igb, correct? Did you
reproduce the same issue on your side or is it a blind shot at this
driver?

Regardless, I think it's a correct thing to do, but some clarification
would be nice.

> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 2c9e2dfd8499..b3883a5a7d7a 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -5352,9 +5352,8 @@ void igc_down(struct igc_adapter *adapter)
>  
>  	for (i = 0; i < adapter->num_q_vectors; i++) {
>  		if (adapter->q_vector[i]) {
> -			napi_synchronize(&adapter->q_vector[i]->napi);
> -			igc_set_queue_napi(adapter, i, NULL);
>  			napi_disable(&adapter->q_vector[i]->napi);
> +			igc_set_queue_napi(adapter, i, NULL);
>  		}
>  	}
>  
> -- 
> 2.53.0
> 

WARNING: multiple messages have this Message-ID (diff)
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: David Carlier <devnexen@gmail.com>
Cc: <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<anthony.l.nguyen@intel.com>, <przemyslaw.kitszel@intel.com>,
	<aleksandr.loktionov@intel.com>, <advoretsky@gmail.com>,
	<stable@vger.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH net] igc: remove napi_synchronize() in igc_down()
Date: Mon, 13 Jul 2026 11:42:34 +0200	[thread overview]
Message-ID: <alSzCokS7scCoza7@boxer> (raw)
In-Reply-To: <20260712132242.223254-1-devnexen@gmail.com>

On Sun, Jul 12, 2026 at 02:22:42PM +0100, David Carlier wrote:
> When an AF_XDP zero-copy application is killed abruptly, the XSK pool is
> torn down but NAPI keeps polling. igc_clean_rx_irq_zc() then returns the
> full budget on every poll, so napi_complete_done() never clears
> NAPI_STATE_SCHED.
> 
> igc_down() calls napi_synchronize() before napi_disable(), so it spins
> forever waiting for that bit and the interface never goes down. Drop the
> napi_synchronize() and let napi_disable() do the job -- it sets
> NAPI_STATE_DISABLE, which forces the stuck poll to complete. Reorder it
> ahead of igc_set_queue_napi() so the NAPI mapping is cleared only after
> polling has stopped, matching the recent igb fix b1e067240379.
> 
> Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

This is a mirror of what Alex Dvoretsky did on igb, correct? Did you
reproduce the same issue on your side or is it a blind shot at this
driver?

Regardless, I think it's a correct thing to do, but some clarification
would be nice.

> ---
>  drivers/net/ethernet/intel/igc/igc_main.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 2c9e2dfd8499..b3883a5a7d7a 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -5352,9 +5352,8 @@ void igc_down(struct igc_adapter *adapter)
>  
>  	for (i = 0; i < adapter->num_q_vectors; i++) {
>  		if (adapter->q_vector[i]) {
> -			napi_synchronize(&adapter->q_vector[i]->napi);
> -			igc_set_queue_napi(adapter, i, NULL);
>  			napi_disable(&adapter->q_vector[i]->napi);
> +			igc_set_queue_napi(adapter, i, NULL);
>  		}
>  	}
>  
> -- 
> 2.53.0
> 

  reply	other threads:[~2026-07-13  9:42 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12 13:22 [PATCH net] igc: remove napi_synchronize() in igc_down() David Carlier
2026-07-12 13:22 ` [Intel-wired-lan] " David Carlier
2026-07-13  9:42 ` Maciej Fijalkowski [this message]
2026-07-13  9:42   ` Maciej Fijalkowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alSzCokS7scCoza7@boxer \
    --to=maciej.fijalkowski@intel.com \
    --cc=advoretsky@gmail.com \
    --cc=aleksandr.loktionov@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=devnexen@gmail.com \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=netdev@vger.kernel.org \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.