Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-22 Thread Loktionov, Aleksandr



> -Original Message-
> From: Intel-wired-lan  On Behalf
> Of David Yang
> Sent: Monday, January 19, 2026 5:27 PM
> To: [email protected]
> Cc: David Yang ; Nguyen, Anthony L
> ; Kitszel, Przemyslaw
> ; Andrew Lunn ;
> David S. Miller ; Eric Dumazet
> ; Jakub Kicinski ; Paolo Abeni
> ; Pavan Kumar Linga ;
> Burra, Phani R ; Willem de Bruijn
> ; Alan Brady ; Samudrala,
> Sridhar ; Hay, Joshua A
> ; [email protected]; linux-
> [email protected]
> Subject: [Intel-wired-lan] [PATCH net] idpf: Fix data race in
> idpf_net_dim
> 
> In idpf_net_dim(), some statistics protected by u64_stats_sync, are
> read and accumulated in ignorance of possible u64_stats_fetch_retry()
> events.
> The correct way to copy statistics is already illustrated by
> idpf_add_queue_stats(). Fix this by reading them into temporary
> variables first.
> 
> Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
> Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
> Signed-off-by: David Yang 
> ---
>  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 16 +++-
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index 97a5fe766b6b..66ba645e8b90 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -3956,7 +3956,7 @@ static void idpf_update_dim_sample(struct
> idpf_q_vector *q_vector,  static void idpf_net_dim(struct
> idpf_q_vector *q_vector)  {
>   struct dim_sample dim_sample = { };
> - u64 packets, bytes;
> + u64 packets, bytes, pkts, bts;
>   u32 i;
> 
>   if (!IDPF_ITR_IS_DYNAMIC(q_vector->tx_intr_mode))
> @@ -3968,9 +3968,12 @@ static void idpf_net_dim(struct idpf_q_vector
> *q_vector)
> 
>   do {
>   start = u64_stats_fetch_begin(&txq->stats_sync);
> - packets += u64_stats_read(&txq->q_stats.packets);
> - bytes += u64_stats_read(&txq->q_stats.bytes);
> + pkts = u64_stats_read(&txq->q_stats.packets);
> + bts = u64_stats_read(&txq->q_stats.bytes);
>   } while (u64_stats_fetch_retry(&txq->stats_sync,
> start));
> +
> + packets += pkts;
> + bytes += bts;
>   }
> 
>   idpf_update_dim_sample(q_vector, &dim_sample, &q_vector-
> >tx_dim, @@ -3987,9 +3990,12 @@ static void idpf_net_dim(struct
> idpf_q_vector *q_vector)
> 
>   do {
>   start = u64_stats_fetch_begin(&rxq->stats_sync);
> - packets += u64_stats_read(&rxq->q_stats.packets);
> - bytes += u64_stats_read(&rxq->q_stats.bytes);
> + pkts = u64_stats_read(&rxq->q_stats.packets);
> + bts = u64_stats_read(&rxq->q_stats.bytes);
>   } while (u64_stats_fetch_retry(&rxq->stats_sync,
> start));
> +
> + packets += pkts;
> + bytes += bts;
>   }
> 
>   idpf_update_dim_sample(q_vector, &dim_sample, &q_vector-
> >rx_dim,
> --
> 2.51.0

Reviewed-by: Aleksandr Loktionov 


Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-20 Thread patchwork-bot+netdevbpf
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski :

On Tue, 20 Jan 2026 00:27:16 +0800 you wrote:
> In idpf_net_dim(), some statistics protected by u64_stats_sync, are read
> and accumulated in ignorance of possible u64_stats_fetch_retry() events.
> The correct way to copy statistics is already illustrated by
> idpf_add_queue_stats(). Fix this by reading them into temporary variables
> first.
> 
> Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
> Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
> Signed-off-by: David Yang 
> 
> [...]

Here is the summary with links:
  - [net] idpf: Fix data race in idpf_net_dim
https://git.kernel.org/netdev/net/c/5fbe395cd1fd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-20 Thread Yangfl
On Wed, Jan 21, 2026 at 12:50 AM Paul Menzel  wrote:
>
> Dear David,
>
>
> Thank you for your patch.
>
> Am 19.01.26 um 17:27 schrieb David Yang:
> > In idpf_net_dim(), some statistics protected by u64_stats_sync, are read
> > and accumulated in ignorance of possible u64_stats_fetch_retry() events.
> > The correct way to copy statistics is already illustrated by
> > idpf_add_queue_stats(). Fix this by reading them into temporary variables
> > first.
>
> It’d be great if you also documented a test case.
>

Sorry, I didn't get what "documente a test case" means. Triggering the
bug would require precise timing between the writer and reader. If
u64_stats_fetch_retry() returns true you already know the previous
critical section was invalid, which is documented in u64_stats_sync.h.

> > Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
> > Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
> > Signed-off-by: David Yang 
> > ---
> >   drivers/net/ethernet/intel/idpf/idpf_txrx.c | 16 +++-
> >   1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c 
> > b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > index 97a5fe766b6b..66ba645e8b90 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > @@ -3956,7 +3956,7 @@ static void idpf_update_dim_sample(struct 
> > idpf_q_vector *q_vector,
> >   static void idpf_net_dim(struct idpf_q_vector *q_vector)
> >   {
> >   struct dim_sample dim_sample = { };
> > - u64 packets, bytes;
> > + u64 packets, bytes, pkts, bts;
>
> The new variable names are ambiguous. Would _tmp or so be better?
>
> >   u32 i;
> >
> >   if (!IDPF_ITR_IS_DYNAMIC(q_vector->tx_intr_mode))
> > @@ -3968,9 +3968,12 @@ static void idpf_net_dim(struct idpf_q_vector 
> > *q_vector)
> >
> >   do {
> >   start = u64_stats_fetch_begin(&txq->stats_sync);
> > - packets += u64_stats_read(&txq->q_stats.packets);
> > - bytes += u64_stats_read(&txq->q_stats.bytes);
> > + pkts = u64_stats_read(&txq->q_stats.packets);
> > + bts = u64_stats_read(&txq->q_stats.bytes);
> >   } while (u64_stats_fetch_retry(&txq->stats_sync, start));
> > +
> > + packets += pkts;
> > + bytes += bts;
> >   }
> >
> >   idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->tx_dim,
> > @@ -3987,9 +3990,12 @@ static void idpf_net_dim(struct idpf_q_vector 
> > *q_vector)
> >
> >   do {
> >   start = u64_stats_fetch_begin(&rxq->stats_sync);
> > - packets += u64_stats_read(&rxq->q_stats.packets);
> > - bytes += u64_stats_read(&rxq->q_stats.bytes);
> > + pkts = u64_stats_read(&rxq->q_stats.packets);
> > + bts = u64_stats_read(&rxq->q_stats.bytes);
> >   } while (u64_stats_fetch_retry(&rxq->stats_sync, start));
> > +
> > + packets += pkts;
> > + bytes += bts;
> >   }
> >
> >   idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->rx_dim,
>
>
> Kind regards,
>
> Paul


Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-20 Thread Paul Menzel

[Cc: Remove bouncing [email protected] and [email protected]]

Am 20.01.26 um 17:50 schrieb Paul Menzel:

Dear David,


Thank you for your patch.

Am 19.01.26 um 17:27 schrieb David Yang:

In idpf_net_dim(), some statistics protected by u64_stats_sync, are read
and accumulated in ignorance of possible u64_stats_fetch_retry() events.
The correct way to copy statistics is already illustrated by
idpf_add_queue_stats(). Fix this by reading them into temporary variables
first.


It’d be great if you also documented a test case.


Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
Signed-off-by: David Yang 
---
  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 16 +++-
  1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c 
b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 97a5fe766b6b..66ba645e8b90 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -3956,7 +3956,7 @@ static void idpf_update_dim_sample(struct idpf_q_vector 
*q_vector,
  static void idpf_net_dim(struct idpf_q_vector *q_vector)
  {
  struct dim_sample dim_sample = { };
-    u64 packets, bytes;
+    u64 packets, bytes, pkts, bts;


The new variable names are ambiguous. Would _tmp or so be better?


  u32 i;
  if (!IDPF_ITR_IS_DYNAMIC(q_vector->tx_intr_mode))
@@ -3968,9 +3968,12 @@ static void idpf_net_dim(struct idpf_q_vector *q_vector)
  do {
  start = u64_stats_fetch_begin(&txq->stats_sync);
-    packets += u64_stats_read(&txq->q_stats.packets);
-    bytes += u64_stats_read(&txq->q_stats.bytes);
+    pkts = u64_stats_read(&txq->q_stats.packets);
+    bts = u64_stats_read(&txq->q_stats.bytes);
  } while (u64_stats_fetch_retry(&txq->stats_sync, start));
+
+    packets += pkts;
+    bytes += bts;
  }
  idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->tx_dim,
@@ -3987,9 +3990,12 @@ static void idpf_net_dim(struct idpf_q_vector *q_vector)
  do {
  start = u64_stats_fetch_begin(&rxq->stats_sync);
-    packets += u64_stats_read(&rxq->q_stats.packets);
-    bytes += u64_stats_read(&rxq->q_stats.bytes);
+    pkts = u64_stats_read(&rxq->q_stats.packets);
+    bts = u64_stats_read(&rxq->q_stats.bytes);
  } while (u64_stats_fetch_retry(&rxq->stats_sync, start));
+
+    packets += pkts;
+    bytes += bts;
  }
  idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->rx_dim,



Kind regards,

Paul


Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-20 Thread Paul Menzel

Dear David,


Thank you for your patch.

Am 19.01.26 um 17:27 schrieb David Yang:

In idpf_net_dim(), some statistics protected by u64_stats_sync, are read
and accumulated in ignorance of possible u64_stats_fetch_retry() events.
The correct way to copy statistics is already illustrated by
idpf_add_queue_stats(). Fix this by reading them into temporary variables
first.


It’d be great if you also documented a test case.


Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
Signed-off-by: David Yang 
---
  drivers/net/ethernet/intel/idpf/idpf_txrx.c | 16 +++-
  1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c 
b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index 97a5fe766b6b..66ba645e8b90 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -3956,7 +3956,7 @@ static void idpf_update_dim_sample(struct idpf_q_vector 
*q_vector,
  static void idpf_net_dim(struct idpf_q_vector *q_vector)
  {
struct dim_sample dim_sample = { };
-   u64 packets, bytes;
+   u64 packets, bytes, pkts, bts;


The new variable names are ambiguous. Would _tmp or so be better?


u32 i;
  
  	if (!IDPF_ITR_IS_DYNAMIC(q_vector->tx_intr_mode))

@@ -3968,9 +3968,12 @@ static void idpf_net_dim(struct idpf_q_vector *q_vector)
  
  		do {

start = u64_stats_fetch_begin(&txq->stats_sync);
-   packets += u64_stats_read(&txq->q_stats.packets);
-   bytes += u64_stats_read(&txq->q_stats.bytes);
+   pkts = u64_stats_read(&txq->q_stats.packets);
+   bts = u64_stats_read(&txq->q_stats.bytes);
} while (u64_stats_fetch_retry(&txq->stats_sync, start));
+
+   packets += pkts;
+   bytes += bts;
}
  
  	idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->tx_dim,

@@ -3987,9 +3990,12 @@ static void idpf_net_dim(struct idpf_q_vector *q_vector)
  
  		do {

start = u64_stats_fetch_begin(&rxq->stats_sync);
-   packets += u64_stats_read(&rxq->q_stats.packets);
-   bytes += u64_stats_read(&rxq->q_stats.bytes);
+   pkts = u64_stats_read(&rxq->q_stats.packets);
+   bts = u64_stats_read(&rxq->q_stats.bytes);
} while (u64_stats_fetch_retry(&rxq->stats_sync, start));
+
+   packets += pkts;
+   bytes += bts;
}
  
  	idpf_update_dim_sample(q_vector, &dim_sample, &q_vector->rx_dim,



Kind regards,

Paul


Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-20 Thread Yangfl
On Tue, Jan 20, 2026 at 1:59 AM Eric Dumazet  wrote:
>
> On Mon, Jan 19, 2026 at 5:28 PM David Yang  wrote:
> >
> > In idpf_net_dim(), some statistics protected by u64_stats_sync, are read
> > and accumulated in ignorance of possible u64_stats_fetch_retry() events.
> > The correct way to copy statistics is already illustrated by
> > idpf_add_queue_stats(). Fix this by reading them into temporary variables
> > first.
> >
> > Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
> > Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
> > Signed-off-by: David Yang 
> > ---
>
> Reviewed-by: Eric Dumazet 
>
> It seems ovs_vport_get_upcall_stats() has a similar bug, are you
> interested to fix it as well ?
>
> Thanks !

Sure, I'll take a look at it.


Re: [Intel-wired-lan] [PATCH net] idpf: Fix data race in idpf_net_dim

2026-01-19 Thread Eric Dumazet via Intel-wired-lan
On Mon, Jan 19, 2026 at 5:28 PM David Yang  wrote:
>
> In idpf_net_dim(), some statistics protected by u64_stats_sync, are read
> and accumulated in ignorance of possible u64_stats_fetch_retry() events.
> The correct way to copy statistics is already illustrated by
> idpf_add_queue_stats(). Fix this by reading them into temporary variables
> first.
>
> Fixes: c2d548cad150 ("idpf: add TX splitq napi poll support")
> Fixes: 3a8845af66ed ("idpf: add RX splitq napi poll support")
> Signed-off-by: David Yang 
> ---

Reviewed-by: Eric Dumazet 

It seems ovs_vport_get_upcall_stats() has a similar bug, are you
interested to fix it as well ?

Thanks !