[Intel-wired-lan] [PATCH v2] idpf: increment completion queue next_to_clean in sw marker wait routine

[Loktionov, Aleksandr]

From: Li Li <boolli at google.com>
Sent: Monday, January 5, 2026 8:39 AM
To: Loktionov, Aleksandr <aleksandr.loktionov at intel.com>
Cc: Nguyen, Anthony L <anthony.l.nguyen at intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel at intel.com>; David S. Miller <davem at davemloft.net>; Jakub Kicinski <kuba at kernel.org>; Eric Dumazet <edumazet at google.com>; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org; linux-kernel at vger.kernel.org; David Decotigny <decot at google.com>; Singhai, Anjali <anjali.singhai at intel.com>; Samudrala, Sridhar <sridhar.samudrala at intel.com>; Brian Vazquez <brianvv at google.com>; Tantilov, Emil S <emil.s.tantilov at intel.com>
Subject: Re: [Intel-wired-lan] [PATCH v2] idpf: increment completion queue next_to_clean in sw marker wait routine



On Sun, Jan 4, 2026 at 11:19 PM Loktionov, Aleksandr <aleksandr.loktionov at intel.com<mailto:aleksandr.loktionov at intel.com>> wrote:


> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces at osuosl.org<mailto:intel-wired-lan-bounces at osuosl.org>> On Behalf
> Of Li Li via Intel-wired-lan
> Sent: Monday, January 5, 2026 7:47 AM
> To: Nguyen, Anthony L <anthony.l.nguyen at intel.com<mailto:anthony.l.nguyen at intel.com>>; Kitszel,
> Przemyslaw <przemyslaw.kitszel at intel.com<mailto:przemyslaw.kitszel at intel.com>>; David S. Miller
> <davem at davemloft.net<mailto:davem at davemloft.net>>; Jakub Kicinski <kuba at kernel.org<mailto:kuba at kernel.org>>; Eric Dumazet
> <edumazet at google.com<mailto:edumazet at google.com>>; intel-wired-lan at lists.osuosl.org<mailto:intel-wired-lan at lists.osuosl.org>
> Cc: netdev at vger.kernel.org<mailto:netdev at vger.kernel.org>; linux-kernel at vger.kernel.org<mailto:linux-kernel at vger.kernel.org>; David
> Decotigny <decot at google.com<mailto:decot at google.com>>; Singhai, Anjali
> <anjali.singhai at intel.com<mailto:anjali.singhai at intel.com>>; Samudrala, Sridhar
> <sridhar.samudrala at intel.com<mailto:sridhar.samudrala at intel.com>>; Brian Vazquez <brianvv at google.com<mailto:brianvv at google.com>>;
> Tantilov, Emil S <emil.s.tantilov at intel.com<mailto:emil.s.tantilov at intel.com>>; Li Li
> <boolli at google.com<mailto:boolli at google.com>>
> Subject: [Intel-wired-lan] [PATCH v2] idpf: increment completion queue
> next_to_clean in sw marker wait routine
>
> Currently, in idpf_wait_for_sw_marker_completion(), when an
> IDPF_TXD_COMPLT_SW_MARKER packet is found, the routine breaks out of
> the for loop and does not increment the next_to_clean counter. This
> causes the subsequent NAPI polls to run into the same
> IDPF_TXD_COMPLT_SW_MARKER packet again and print out the following:
>
>     [   23.261341] idpf 0000:05:00.0 eth1: Unknown TX completion type:
> 5
>
> Instead, we should increment next_to_clean regardless when an
> IDPF_TXD_COMPLT_SW_MARKER packet is found.
>
> Tested: with the patch applied, we do not see the errors above from
> NAPI polls anymore.
>
> Signed-off-by: Li Li <boolli at google.com<mailto:boolli at google.com>>
> ---
> Changes in v2:
>  - Initialize idpf_tx_queue *target to NULL to suppress the "'target'
>    uninitialized when 'if' statement is true warning".
>
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 69bab7187e541..452d0a9e83a4f 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -2326,7 +2326,7 @@ void idpf_wait_for_sw_marker_completion(const
> struct idpf_tx_queue *txq)
>
>       do {
>               struct idpf_splitq_4b_tx_compl_desc *tx_desc;
> -             struct idpf_tx_queue *target;
> +             struct idpf_tx_queue *target = NULL;
Linux kernel is against premature initialization just to silence a compiler.
The target variable is dereferenced at idpf_queue_clear(SW_MARKER, target))
but can remain uninitialized if execution jumps to the next: label via a goto
before target is assigned.
Isn't it?
That is correct. When the following if statement (line 2341-2343) evaluates to true:

  if (FIELD_GET(IDPF_TXD_COMPLQ_COMPL_TYPE_M, ctype_gen) !=
   IDPF_TXD_COMPLT_SW_MARKER)
    goto next;

Then the initialization at line 2346:

  target = complq->txq_grp->txqs[id];

would be skipped, making "target" uninitialized.

Therefore, in this patch, I need to initialize "target" to NULL.

The ‘NULL’ target variable can be dereferenced at idpf_queue_clear(SW_MARKER, target)), isn’t it?




>               u32 ctype_gen, id;
>
>               tx_desc = flow ? &complq->comp[ntc].common :
> @@ -2346,14 +2346,14 @@ void idpf_wait_for_sw_marker_completion(const
> struct idpf_tx_queue *txq)
>               target = complq->txq_grp->txqs[id];
>
>               idpf_queue_clear(SW_MARKER, target);
> -             if (target == txq)
> -                     break;
>
>  next:
>               if (unlikely(++ntc == complq->desc_count)) {
>                       ntc = 0;
>                       gen_flag = !gen_flag;
>               }
> +             if (target == txq)
Are tou sure that incremented ntc value is ever written back to complq->next_to_clean?

Yes, the value of "ntc" is written back to "complq->next_to_clean" at the end of the function
 (at line 2360):

  complq->next_to_clean = ntc;
Thank you, I don’t see it from the patch.


> +                     break;
>       } while (time_before(jiffies, timeout));
>
>       idpf_queue_assign(GEN_CHK, complq, gen_flag);
> --
> 2.52.0.351.gbe84eed79e-goog
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osuosl.org/pipermail/intel-wired-lan/attachments/20260105/05007b61/attachment-0001.htm>