drivers/net/ethernet/intel/idpf/idpf_txrx.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
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 <mmyangfl@gmail.com>
---
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
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of David Yang
> Sent: Monday, January 19, 2026 5:27 PM
> To: netdev@vger.kernel.org
> Cc: David Yang <mmyangfl@gmail.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; 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>; Pavan Kumar Linga <pavan.kumar.linga@intel.com>;
> Burra, Phani R <phani.r.burra@intel.com>; Willem de Bruijn
> <willemb@google.com>; Alan Brady <alan.brady@intel.com>; Samudrala,
> Sridhar <sridhar.samudrala@intel.com>; Hay, Joshua A
> <joshua.a.hay@intel.com>; intel-wired-lan@lists.osuosl.org; linux-
> kernel@vger.kernel.org
> 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 <mmyangfl@gmail.com>
> ---
> 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 <aleksandr.loktionov@intel.com>
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 <mmyangfl@gmail.com>
> ---
> 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
On Wed, Jan 21, 2026 at 12:50 AM Paul Menzel <pmenzel@molgen.mpg.de> 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 <mmyangfl@gmail.com>
> > ---
> > 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
[Cc: Remove bouncing alan.brady@intel.com and pavan.kumar.linga@intel.com]
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 <mmyangfl@gmail.com>
>> ---
>> 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
On Mon, Jan 19, 2026 at 5:28 PM David Yang <mmyangfl@gmail.com> 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 <mmyangfl@gmail.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
It seems ovs_vport_get_upcall_stats() has a similar bug, are you
interested to fix it as well ?
Thanks !
On Tue, Jan 20, 2026 at 1:59 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Mon, Jan 19, 2026 at 5:28 PM David Yang <mmyangfl@gmail.com> 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 <mmyangfl@gmail.com>
> > ---
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
>
> 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.
© 2016 - 2026 Red Hat, Inc.