* [PATCH 1/1] examples/l2fwd-jobstats: add delay to show stats
@ 2024-07-26 5:37 Rakesh Kudurumalla
2024-07-29 6:10 ` [PATCH v2 " Rakesh Kudurumalla
2024-07-29 6:20 ` [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
0 siblings, 2 replies; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-07-26 5:37 UTC (permalink / raw)
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla
In main_loop function only one lock is acquired
before fwd jobs has started and finished and then lock is released.
Due to this most of the time lock is not available for
show_lcore_stats() as a result stats are not updated periodically.
This patch fixes the same by adding delay before accquring lock
in loop
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
examples/l2fwd-jobstats/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ rte_delay_us(10);
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats
2024-07-26 5:37 [PATCH 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
@ 2024-07-29 6:10 ` Rakesh Kudurumalla
2024-07-30 10:03 ` [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability Rakesh Kudurumalla
2024-07-29 6:20 ` [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
1 sibling, 1 reply; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-07-29 6:10 UTC (permalink / raw)
To: thomas, ferruh.yigit, andrew.rybchenko, orika
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla, stable
This patch addresses the issue by introducing a delay
before acquiring the lock in the loop. This delay allows for better
availability of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding jobs are running.
Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
v2: updated commit message
examples/l2fwd-jobstats/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ rte_delay_us(10);
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats
2024-07-26 5:37 [PATCH 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
2024-07-29 6:10 ` [PATCH v2 " Rakesh Kudurumalla
@ 2024-07-29 6:20 ` Rakesh Kudurumalla
2026-03-02 16:23 ` Stephen Hemminger
1 sibling, 1 reply; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-07-29 6:20 UTC (permalink / raw)
To: thomas, ferruh.yigit, andrew.rybchenko, orika
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla, stable
This patch addresses the issue by introducing a delay
before acquiring the lock in the loop. This delay allows for better
availability of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding jobs are running.
Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
v2: updated commit message
examples/l2fwd-jobstats/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ rte_delay_us(10);
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
2024-07-29 6:10 ` [PATCH v2 " Rakesh Kudurumalla
@ 2024-07-30 10:03 ` Rakesh Kudurumalla
2024-07-30 16:12 ` Thomas Monjalon
2024-08-11 6:58 ` [PATCH v4 " Rakesh Kudurumalla
0 siblings, 2 replies; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-07-30 10:03 UTC (permalink / raw)
To: thomas, ferruh.yigit, andrew.rybchenko, orika
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla, stable
This patch addresses the issue by introducing a delay
before acquiring the lock in the loop. This delay allows for better
availability of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding jobs are running.
Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
v3: updated subject message
v2: updated commit description
examples/l2fwd-jobstats/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ rte_delay_us(10);
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
2024-07-30 10:03 ` [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability Rakesh Kudurumalla
@ 2024-07-30 16:12 ` Thomas Monjalon
2024-08-08 11:41 ` [EXTERNAL] " Rakesh Kudurumalla
2024-08-11 6:58 ` [PATCH v4 " Rakesh Kudurumalla
1 sibling, 1 reply; 23+ messages in thread
From: Thomas Monjalon @ 2024-07-30 16:12 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: ferruh.yigit, andrew.rybchenko, orika, dev, jerinj, ndabilpuram,
stable
Hello,
30/07/2024 12:03, Rakesh Kudurumalla:
> This patch addresses the issue by introducing a delay
Please start with describing the issue.
> before acquiring the lock in the loop. This delay allows for better
> availability of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding jobs are running.
Why a delay is better than a pause?
> @@ -542,7 +542,7 @@ l2fwd_main_loop(void)
> } while (likely(stats_read_pending == 0));
>
> rte_spinlock_unlock(&qconf->lock);
> - rte_pause();
> + rte_delay_us(10);
> }
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
2024-07-30 16:12 ` Thomas Monjalon
@ 2024-08-08 11:41 ` Rakesh Kudurumalla
2024-08-08 12:22 ` Thomas Monjalon
0 siblings, 1 reply; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-08-08 11:41 UTC (permalink / raw)
To: Thomas Monjalon
Cc: ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru,
orika@nvidia.com, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]
From: Thomas Monjalon <thomas@monjalon.net>
Sent: Tuesday, July 30, 2024 9:42 PM
To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; orika@nvidia.com; dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; stable@dpdk.org
Subject: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
Hello, 30/07/2024 12: 03, Rakesh Kudurumalla: > This patch addresses the issue by introducing a delay Please start with describing the issue. > before acquiring the lock in the loop. This delay allows for better > availability of the
Hello,
30/07/2024 12:03, Rakesh Kudurumalla:
> This patch addresses the issue by introducing a delay
Please start with describing the issue.
> before acquiring the lock in the loop. This delay allows for better
> availability of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding jobs are running.
Why a delay is better than a pause?
due to high frequency of polling in l2fwd_main_loop() rte_pause() is not yieding the processor
to display stats relating to fwd and flush execution time so added a delay achieve the same.
> @@ -542,7 +542,7 @@ l2fwd_main_loop(void)
> } while (likely(stats_read_pending == 0));
>
> rte_spinlock_unlock(&qconf->lock);
> - rte_pause();
> + rte_delay_us(10);
> }
[-- Attachment #2: Type: text/html, Size: 7516 bytes --]
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-08 11:41 ` [EXTERNAL] " Rakesh Kudurumalla
@ 2024-08-08 12:22 ` Thomas Monjalon
2024-08-11 16:00 ` Rakesh Kudurumalla
0 siblings, 1 reply; 23+ messages in thread
From: Thomas Monjalon @ 2024-08-08 12:22 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru,
orika@nvidia.com, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
08/08/2024 13:41, Rakesh Kudurumalla:
>
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, July 30, 2024 9:42 PM
> To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru; orika@nvidia.com; dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; stable@dpdk.org
> Subject: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
>
> Hello, 30/07/2024 12: 03, Rakesh Kudurumalla: > This patch addresses the issue by introducing a delay Please start with describing the issue. > before acquiring the lock in the loop. This delay allows for better > availability of the
>
>
> Hello,
>
>
>
> 30/07/2024 12:03, Rakesh Kudurumalla:
>
> > This patch addresses the issue by introducing a delay
>
>
>
> Please start with describing the issue.
You should explain the race is managed with a spinlock,
and where the threads are running (one is a timer).
> > before acquiring the lock in the loop. This delay allows for better
> > availability of the lock, ensuring that show_lcore_stats() can
> > periodically update the statistics even when forwarding jobs are running.
>
>
> Why a delay is better than a pause?
>
>
> due to high frequency of polling in l2fwd_main_loop() rte_pause() is not yieding the processor
> to display stats relating to fwd and flush execution time so added a delay achieve the same.
Which CPU did you try?
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v4 1/1] examples/l2fwd-jobstats: fix lock availability
2024-07-30 10:03 ` [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability Rakesh Kudurumalla
2024-07-30 16:12 ` Thomas Monjalon
@ 2024-08-11 6:58 ` Rakesh Kudurumalla
2024-08-11 15:59 ` [PATCH v5 " Rakesh Kudurumalla
1 sibling, 1 reply; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-08-11 6:58 UTC (permalink / raw)
To: ferruh.yigit, andrew.rybchenko, orika, monjalon.net
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla, stable
Race condition between jobstats and time metrics
for forwading and flushing is maintained using spinlock.
Timer metrics are not displayed properly due to the
frequent unavailability of the lock.This patch fixes
the issue by introducing a delay before acquiring
the lock in the loop. This delay allows for betteravailability
of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding
jobs are running.
Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
v4: Addressed cause of issue in commit message
examples/l2fwd-jobstats/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ rte_delay_us(10);
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-11 6:58 ` [PATCH v4 " Rakesh Kudurumalla
@ 2024-08-11 15:59 ` Rakesh Kudurumalla
2024-08-11 16:17 ` Stephen Hemminger
` (3 more replies)
0 siblings, 4 replies; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-08-11 15:59 UTC (permalink / raw)
To: ferruh.yigit, andrew.rybchenko, orika, thomas
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla, stable
Race condition between jobstats and time metrics
for forwarding and flushing is maintained using spinlock.
Timer metrics are not displayed properly due to the
frequent unavailability of the lock.This patch fixes
the issue by introducing a delay before acquiring
the lock in the loop. This delay allows for betteravailability
of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding
jobs are running.
Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
v5: updated cause of issue in commit message
examples/l2fwd-jobstats/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..7bb38b290f 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -542,7 +542,7 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ rte_delay_us(10);
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-08 12:22 ` Thomas Monjalon
@ 2024-08-11 16:00 ` Rakesh Kudurumalla
0 siblings, 0 replies; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-08-11 16:00 UTC (permalink / raw)
To: Thomas Monjalon
Cc: ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru,
orika@nvidia.com, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, August 8, 2024 5:52 PM
> To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> orika@nvidia.com; dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Nithin
> Kumar Dabilpuram <ndabilpuram@marvell.com>; stable@dpdk.org
> Subject: Re: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock
> availability
>
> 08/08/2024 13: 41, Rakesh Kudurumalla: > > From: Thomas Monjalon
> <thomas@ monjalon. net> > Sent: Tuesday, July 30, 2024 9: 42 PM > To:
> Rakesh Kudurumalla <rkudurumalla@ marvell. com> > Cc:
> ferruh. yigit@ amd. com; andrew. rybchenko@ oktetlabs. ru;
>
> 08/08/2024 13:41, Rakesh Kudurumalla:
> >
> > From: Thomas Monjalon <thomas@monjalon.net>
> > Sent: Tuesday, July 30, 2024 9:42 PM
> > To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> > orika@nvidia.com; dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>;
> > Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>; stable@dpdk.org
> > Subject: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix
> > lock availability
> >
> > Hello, 30/07/2024 12: 03, Rakesh Kudurumalla: > This patch addresses
> > the issue by introducing a delay Please start with describing the
> > issue. > before acquiring the lock in the loop. This delay allows for
> > better > availability of the
> >
> >
> > Hello,
> >
> >
> >
> > 30/07/2024 12:03, Rakesh Kudurumalla:
> >
> > > This patch addresses the issue by introducing a delay
> >
> >
> >
> > Please start with describing the issue.
>
> You should explain the race is managed with a spinlock, and where the
> threads are running (one is a timer).
> Updated commit message in V5
>
> > > before acquiring the lock in the loop. This delay allows for better
> > > availability of the lock, ensuring that show_lcore_stats() can
> > > periodically update the statistics even when forwarding jobs are running.
> >
> >
> > Why a delay is better than a pause?
> >
> >
> > due to high frequency of polling in l2fwd_main_loop() rte_pause() is
> > not yieding the processor to display stats relating to fwd and flush
> execution time so added a delay achieve the same.
>
> Which CPU did you try?
> arm
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-11 15:59 ` [PATCH v5 " Rakesh Kudurumalla
@ 2024-08-11 16:17 ` Stephen Hemminger
2024-08-16 5:25 ` [EXTERNAL] " Rakesh Kudurumalla
2025-05-24 15:39 ` Stephen Hemminger
` (2 subsequent siblings)
3 siblings, 1 reply; 23+ messages in thread
From: Stephen Hemminger @ 2024-08-11 16:17 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: ferruh.yigit, andrew.rybchenko, orika, thomas, dev, jerinj,
ndabilpuram, stable
On Sun, 11 Aug 2024 21:29:57 +0530
Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> Race condition between jobstats and time metrics
> for forwarding and flushing is maintained using spinlock.
> Timer metrics are not displayed properly due to the
> frequent unavailability of the lock.This patch fixes
> the issue by introducing a delay before acquiring
> the lock in the loop. This delay allows for betteravailability
> of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding
> jobs are running.
>
> Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
Would be better if this code used RCU and not a lock
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-11 16:17 ` Stephen Hemminger
@ 2024-08-16 5:25 ` Rakesh Kudurumalla
2024-10-21 7:13 ` Rakesh Kudurumalla
0 siblings, 1 reply; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-08-16 5:25 UTC (permalink / raw)
To: Stephen Hemminger
Cc: ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru,
orika@nvidia.com, thomas@monjalon.net, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Sunday, August 11, 2024 9:47 PM
> To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> orika@nvidia.com; thomas@monjalon.net; dev@dpdk.org; Jerin Jacob
> <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; stable@dpdk.org
> Subject: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock
> availability
>
> On Sun, 11 Aug 2024 21: 29: 57 +0530 Rakesh Kudurumalla
> <rkudurumalla@ marvell. com> wrote: > Race condition between jobstats
> and time metrics > for forwarding and flushing is maintained using spinlock.
> > Timer metrics are not displayed
> On Sun, 11 Aug 2024 21:29:57 +0530
> Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
>
> > Race condition between jobstats and time metrics for forwarding and
> > flushing is maintained using spinlock.
> > Timer metrics are not displayed properly due to the frequent
> > unavailability of the lock.This patch fixes the issue by introducing a
> > delay before acquiring the lock in the loop. This delay allows for
> > betteravailability of the lock, ensuring that show_lcore_stats() can
> > periodically update the statistics even when forwarding jobs are
> > running.
> >
> > Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
>
> Would be better if this code used RCU and not a lock
Currently the jobstats app uses the lock only for collecting single snapshot of different statistics and printing the same from main core. With RCU since we cannot pause the worker core to collect such a single snapshot, integrating RCU would need a full redesign of the application and would take lot of effort.
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-16 5:25 ` [EXTERNAL] " Rakesh Kudurumalla
@ 2024-10-21 7:13 ` Rakesh Kudurumalla
2024-11-04 10:17 ` Nithin Dabilpuram
0 siblings, 1 reply; 23+ messages in thread
From: Rakesh Kudurumalla @ 2024-10-21 7:13 UTC (permalink / raw)
To: Stephen Hemminger
Cc: ferruh.yigit@amd.com, andrew.rybchenko@oktetlabs.ru,
orika@nvidia.com, thomas@monjalon.net, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
ping
> -----Original Message-----
> From: Rakesh Kudurumalla
> Sent: Friday, August 16, 2024 10:55 AM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> orika@nvidia.com; thomas@monjalon.net; dev@dpdk.org; Jerin Jacob
> <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; stable@dpdk.org
> Subject: RE: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock
> availability
>
>
>
> > -----Original Message-----
> > From: Stephen Hemminger <stephen@networkplumber.org>
> > Sent: Sunday, August 11, 2024 9:47 PM
> > To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> > orika@nvidia.com; thomas@monjalon.net; dev@dpdk.org; Jerin Jacob
> > <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpuram@marvell.com>; stable@dpdk.org
> > Subject: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix
> > lock availability
> >
> > On Sun, 11 Aug 2024 21: 29: 57 +0530 Rakesh Kudurumalla
> <rkudurumalla@
> > marvell. com> wrote: > Race condition between jobstats and time
> > metrics > for forwarding and flushing is maintained using spinlock.
> > > Timer metrics are not displayed
> > On Sun, 11 Aug 2024 21:29:57 +0530
> > Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> >
> > > Race condition between jobstats and time metrics for forwarding and
> > > flushing is maintained using spinlock.
> > > Timer metrics are not displayed properly due to the frequent
> > > unavailability of the lock.This patch fixes the issue by introducing
> > > a delay before acquiring the lock in the loop. This delay allows for
> > > betteravailability of the lock, ensuring that show_lcore_stats() can
> > > periodically update the statistics even when forwarding jobs are
> > > running.
> > >
> > > Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> >
> > Would be better if this code used RCU and not a lock
>
> Currently the jobstats app uses the lock only for collecting single snapshot of
> different statistics and printing the same from main core. With RCU since we
> cannot pause the worker core to collect such a single snapshot, integrating
> RCU would need a full redesign of the application and would take lot of
> effort.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-10-21 7:13 ` Rakesh Kudurumalla
@ 2024-11-04 10:17 ` Nithin Dabilpuram
0 siblings, 0 replies; 23+ messages in thread
From: Nithin Dabilpuram @ 2024-11-04 10:17 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: Stephen Hemminger, ferruh.yigit@amd.com,
andrew.rybchenko@oktetlabs.ru, orika@nvidia.com,
thomas@monjalon.net, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
Acked-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
On Mon, Oct 21, 2024 at 12:43 PM Rakesh Kudurumalla
<rkudurumalla@marvell.com> wrote:
>
> ping
>
> > -----Original Message-----
> > From: Rakesh Kudurumalla
> > Sent: Friday, August 16, 2024 10:55 AM
> > To: Stephen Hemminger <stephen@networkplumber.org>
> > Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> > orika@nvidia.com; thomas@monjalon.net; dev@dpdk.org; Jerin Jacob
> > <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> > <ndabilpuram@marvell.com>; stable@dpdk.org
> > Subject: RE: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock
> > availability
> >
> >
> >
> > > -----Original Message-----
> > > From: Stephen Hemminger <stephen@networkplumber.org>
> > > Sent: Sunday, August 11, 2024 9:47 PM
> > > To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > > Cc: ferruh.yigit@amd.com; andrew.rybchenko@oktetlabs.ru;
> > > orika@nvidia.com; thomas@monjalon.net; dev@dpdk.org; Jerin Jacob
> > > <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> > > <ndabilpuram@marvell.com>; stable@dpdk.org
> > > Subject: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix
> > > lock availability
> > >
> > > On Sun, 11 Aug 2024 21: 29: 57 +0530 Rakesh Kudurumalla
> > <rkudurumalla@
> > > marvell. com> wrote: > Race condition between jobstats and time
> > > metrics > for forwarding and flushing is maintained using spinlock.
> > > > Timer metrics are not displayed
> > > On Sun, 11 Aug 2024 21:29:57 +0530
> > > Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> > >
> > > > Race condition between jobstats and time metrics for forwarding and
> > > > flushing is maintained using spinlock.
> > > > Timer metrics are not displayed properly due to the frequent
> > > > unavailability of the lock.This patch fixes the issue by introducing
> > > > a delay before acquiring the lock in the loop. This delay allows for
> > > > betteravailability of the lock, ensuring that show_lcore_stats() can
> > > > periodically update the statistics even when forwarding jobs are
> > > > running.
> > > >
> > > > Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > >
> > > Would be better if this code used RCU and not a lock
> >
> > Currently the jobstats app uses the lock only for collecting single snapshot of
> > different statistics and printing the same from main core. With RCU since we
> > cannot pause the worker core to collect such a single snapshot, integrating
> > RCU would need a full redesign of the application and would take lot of
> > effort.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-11 15:59 ` [PATCH v5 " Rakesh Kudurumalla
2024-08-11 16:17 ` Stephen Hemminger
@ 2025-05-24 15:39 ` Stephen Hemminger
2026-01-14 6:24 ` Stephen Hemminger
2026-03-02 6:18 ` [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention rkudurumalla
3 siblings, 0 replies; 23+ messages in thread
From: Stephen Hemminger @ 2025-05-24 15:39 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: ferruh.yigit, andrew.rybchenko, orika, thomas, dev, jerinj,
ndabilpuram, stable
On Sun, 11 Aug 2024 21:29:57 +0530
Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> Race condition between jobstats and time metrics
> for forwarding and flushing is maintained using spinlock.
> Timer metrics are not displayed properly due to the
> frequent unavailability of the lock.This patch fixes
> the issue by introducing a delay before acquiring
> the lock in the loop. This delay allows for betteravailability
> of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding
> jobs are running.
>
> Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
The original code is a mess here.
The whole idle job loop here is the problem.
It should use rte_timer_next_ticks() to know when next timer is
about to expire and use that.
And instead of fighting with spin lock, use ticket lock which will
cause in-order waiting.
Something like the following (untested):
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index 308b8edd20..9586d90ab6 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -27,7 +27,7 @@
#include <rte_ethdev.h>
#include <rte_mempool.h>
#include <rte_mbuf.h>
-#include <rte_spinlock.h>
+#include <rte_ticketlock.h>
#include <rte_errno.h>
#include <rte_jobstats.h>
@@ -80,8 +80,7 @@ struct __rte_cache_aligned lcore_queue_conf {
struct rte_jobstats idle_job;
struct rte_jobstats_context jobs_context;
- RTE_ATOMIC(uint16_t) stats_read_pending;
- rte_spinlock_t lock;
+ rte_ticketlock_t lock;
};
/* >8 End of list of queues to be polled for given lcore. */
struct lcore_queue_conf lcore_queue_conf[RTE_MAX_LCORE];
@@ -151,9 +150,7 @@ show_lcore_stats(unsigned lcore_id)
uint64_t collection_time = rte_get_timer_cycles();
/* Ask forwarding thread to give us stats. */
- rte_atomic_store_explicit(&qconf->stats_read_pending, 1, rte_memory_order_relaxed);
- rte_spinlock_lock(&qconf->lock);
- rte_atomic_store_explicit(&qconf->stats_read_pending, 0, rte_memory_order_relaxed);
+ rte_ticketlock_lock(&qconf->lock);
/* Collect context statistics. */
stats_period = ctx->state_time - ctx->start_time;
@@ -195,7 +192,7 @@ show_lcore_stats(unsigned lcore_id)
idle_exec_max = qconf->idle_job.max_exec_time;
rte_jobstats_reset(&qconf->idle_job);
- rte_spinlock_unlock(&qconf->lock);
+ rte_ticketlock_unlock(&qconf->lock);
exec -= idle_exec;
busy = exec + management;
@@ -478,11 +475,11 @@ l2fwd_main_loop(void)
unsigned lcore_id;
unsigned i, portid;
struct lcore_queue_conf *qconf;
- uint8_t stats_read_pending = 0;
- uint8_t need_manage;
+ uint64_t hz;
lcore_id = rte_lcore_id();
qconf = &lcore_queue_conf[lcore_id];
+ hz = rte_get_timer_hz();
if (qconf->n_rx_port == 0) {
RTE_LOG(INFO, L2FWD, "lcore %u has nothing to do\n", lcore_id);
@@ -502,47 +499,22 @@ l2fwd_main_loop(void)
/* Minimize impact of stats reading. 8< */
for (;;) {
- rte_spinlock_lock(&qconf->lock);
-
- do {
- rte_jobstats_context_start(&qconf->jobs_context);
-
- /* Do the Idle job:
- * - Read stats_read_pending flag
- * - check if some real job need to be executed
- */
- rte_jobstats_start(&qconf->jobs_context, &qconf->idle_job);
-
- uint64_t repeats = 0;
-
- do {
- uint8_t i;
- uint64_t now = rte_get_timer_cycles();
-
- repeats++;
- need_manage = qconf->flush_timer.expire < now;
- /* Check if we was esked to give a stats. */
- stats_read_pending = rte_atomic_load_explicit(
- &qconf->stats_read_pending,
- rte_memory_order_relaxed);
- need_manage |= stats_read_pending;
- for (i = 0; i < qconf->n_rx_port && !need_manage; i++)
- need_manage = qconf->rx_timers[i].expire < now;
+ rte_ticketlock_lock(&qconf->lock);
- } while (!need_manage);
+ rte_jobstats_context_start(&qconf->jobs_context);
+ rte_jobstats_start(&qconf->jobs_context, &qconf->idle_job);
+ rte_timer_manage();
+ rte_jobstats_context_finish(&qconf->jobs_context);
- if (likely(repeats != 1))
- rte_jobstats_finish(&qconf->idle_job, qconf->idle_job.target);
- else
- rte_jobstats_abort(&qconf->idle_job);
+ int64_t next_ticks = rte_timer_next_ticks();
- rte_timer_manage();
- rte_jobstats_context_finish(&qconf->jobs_context);
- } while (likely(stats_read_pending == 0));
+ rte_ticketlock_unlock(&qconf->lock);
- rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ if (next_ticks > 0)
+ rte_delay_us((1000000 * next_ticks) / hz);
+ else
+ rte_pause();
}
/* >8 End of minimize impact of stats reading. */
}
@@ -972,7 +944,7 @@ main(int argc, char **argv)
RTE_LCORE_FOREACH(lcore_id) {
qconf = &lcore_queue_conf[lcore_id];
- rte_spinlock_init(&qconf->lock);
+ rte_ticketlock_init(&qconf->lock);
if (rte_jobstats_context_init(&qconf->jobs_context) != 0)
rte_panic("Jobs stats context for core %u init failed\n", lcore_id);
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability
2024-08-11 15:59 ` [PATCH v5 " Rakesh Kudurumalla
2024-08-11 16:17 ` Stephen Hemminger
2025-05-24 15:39 ` Stephen Hemminger
@ 2026-01-14 6:24 ` Stephen Hemminger
2026-03-02 6:18 ` [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention rkudurumalla
3 siblings, 0 replies; 23+ messages in thread
From: Stephen Hemminger @ 2026-01-14 6:24 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: ferruh.yigit, andrew.rybchenko, orika, thomas, dev, jerinj,
ndabilpuram, stable
On Sun, 11 Aug 2024 21:29:57 +0530
Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> Race condition between jobstats and time metrics
> for forwarding and flushing is maintained using spinlock.
> Timer metrics are not displayed properly due to the
> frequent unavailability of the lock.This patch fixes
> the issue by introducing a delay before acquiring
> the lock in the loop. This delay allows for betteravailability
> of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding
> jobs are running.
>
> Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> ---
I prodded AI to review this patch and it had some good observations.
You could go farther with rte_wait_until_equal which has even better power performance.
Let me see the critical `l2fwd_main_loop` function to understand the full locking context:Now I have a complete picture. Let me provide my review.
---
## Patch Review: examples/l2fwd-jobstats: fix lock availability
### Commit Message Issues
**Errors:**
- Line 84: Missing space before "betteravailability" → should be "better availability"
- Line 82: Missing space after period: "lock.This" → should be "lock. This"
**Style (minor):** The commit message is a bit verbose for what it's doing.
### Technical Analysis
The patch addresses a real problem. Looking at the code flow:
```c
/* Main loop holds lock almost continuously */
for (;;) {
rte_spinlock_lock(&qconf->lock); // Acquire
do {
/* ... lots of work ... */
stats_read_pending = rte_atomic_load_explicit(...);
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock); // Release
rte_pause(); // <-- This is too short!
// Immediately re-acquires lock...
}
```
Meanwhile, `show_lcore_stats()` sets the flag and waits on the lock:
```c
rte_atomic_store_explicit(&qconf->stats_read_pending, 1, ...);
rte_spinlock_lock(&qconf->lock); // Starved - main loop re-acquires too fast
```
**The fix works**, but it's suboptimal:
- The 10µs delay is an arbitrary magic number
- It delays **unconditionally**, even when no stats reader is waiting
- Wastes cycles when the system is idle
### Better Option: Wait for stats completion
The `stats_read_pending` flag already exists to coordinate this handshake but isn't used fully. The cleaner fix is to make the main loop wait until the stats reader has finished:
```c
rte_spinlock_unlock(&qconf->lock);
/* Wait for stats reader to complete before reacquiring lock */
while (rte_atomic_load_explicit(&qconf->stats_read_pending,
rte_memory_order_relaxed) != 0)
rte_pause();
```
This approach:
- Uses the existing synchronization mechanism as intended
- Only yields when stats are actually being read
- Has zero overhead when no stats read is pending
- Doesn't introduce arbitrary magic constants
- Guarantees the stats reader gets the lock (not probabilistic)
### Alternative: Conditional delay
If you want to keep the delay approach, at least make it conditional:
```c
rte_spinlock_unlock(&qconf->lock);
if (stats_read_pending)
rte_delay_us(10);
else
rte_pause();
```
This avoids the unconditional 10µs penalty on every iteration.
### Verdict
**NAK as-is.** The fix works but the better solution is to properly use the existing `stats_read_pending` handshake. Would suggest the author revise to use the wait-for-completion pattern instead of an arbitrary delay.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention
2024-08-11 15:59 ` [PATCH v5 " Rakesh Kudurumalla
` (2 preceding siblings ...)
2026-01-14 6:24 ` Stephen Hemminger
@ 2026-03-02 6:18 ` rkudurumalla
2026-03-02 15:48 ` Stephen Hemminger
3 siblings, 1 reply; 23+ messages in thread
From: rkudurumalla @ 2026-03-02 6:18 UTC (permalink / raw)
To: Pawel Wodkowski, Pablo de Lara
Cc: dev, jerinj, ndabilpuram, Rakesh Kudurumalla, stable
From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
Race condition between jobstats and time metrics
for forwarding and flushing is maintained using spinlock.
Timer metrics are not displayed properly due to the
frequent unavailability of the lock.
This patch fixes the issue by introducing a delay before
acquiring the lock in the loop. This delay allows for better
availability of the lock, ensuring that show_lcore_stats() can
periodically update the statistics even when forwarding
jobs are running.
Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
Cc: stable@dpdk.org
Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
---
examples/l2fwd-jobstats/main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/examples/l2fwd-jobstats/main.c b/examples/l2fwd-jobstats/main.c
index a7cd5b4840..036bd4f951 100644
--- a/examples/l2fwd-jobstats/main.c
+++ b/examples/l2fwd-jobstats/main.c
@@ -541,7 +541,9 @@ l2fwd_main_loop(void)
} while (likely(stats_read_pending == 0));
rte_spinlock_unlock(&qconf->lock);
- rte_pause();
+ while (rte_atomic_load_explicit(&qconf->stats_read_pending,
+ rte_memory_order_relaxed) != 0)
+ rte_pause();
}
/* >8 End of minimize impact of stats reading. */
}
--
2.25.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention
2026-03-02 6:18 ` [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention rkudurumalla
@ 2026-03-02 15:48 ` Stephen Hemminger
2026-03-16 13:36 ` [EXTERNAL] " Rakesh Kudurumalla
0 siblings, 1 reply; 23+ messages in thread
From: Stephen Hemminger @ 2026-03-02 15:48 UTC (permalink / raw)
To: rkudurumalla
Cc: Pawel Wodkowski, Pablo de Lara, dev, jerinj, ndabilpuram, stable
On Mon, 2 Mar 2026 11:48:05 +0530
rkudurumalla <rkudurumalla@marvell.com> wrote:
> From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
>
> Race condition between jobstats and time metrics
> for forwarding and flushing is maintained using spinlock.
> Timer metrics are not displayed properly due to the
> frequent unavailability of the lock.
>
> This patch fixes the issue by introducing a delay before
> acquiring the lock in the loop. This delay allows for better
> availability of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding
> jobs are running.
>
> Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
This does fix the problem, but this whole application seems like
it is using an unusual architecture. The main loop per thread
spends most of its time updating job stats and only periodically
calls the actual rx/tx burst forwarding. That is upside down??
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats
2024-07-29 6:20 ` [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
@ 2026-03-02 16:23 ` Stephen Hemminger
0 siblings, 0 replies; 23+ messages in thread
From: Stephen Hemminger @ 2026-03-02 16:23 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: thomas, ferruh.yigit, andrew.rybchenko, orika, dev, jerinj,
ndabilpuram, stable
On Mon, 29 Jul 2024 11:50:37 +0530
Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> This patch addresses the issue by introducing a delay
> before acquiring the lock in the loop. This delay allows for better
> availability of the lock, ensuring that show_lcore_stats() can
> periodically update the statistics even when forwarding jobs are running.
>
> Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> Cc: stable@dpdk.org
>
> Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
A more long winded AI analysis of why this example is an architectural mess.
---
## 1. The Main Loop is Structurally Convoluted
The `l2fwd_main_loop()` is the core problem. It uses three nested loops with a spinlock held across the entire active processing path:
```
for (;;) { // outer: infinite
rte_spinlock_lock(&qconf->lock);
do { // middle: context loop
// idle job inner loop
do { // inner: busy-wait polling timers
...
} while (!need_manage);
rte_timer_manage();
} while (stats_read_pending == 0);
rte_spinlock_unlock(&qconf->lock);
rte_pause();
}
```
**Problems:**
The **spinlock is held during all packet processing**. It's only released briefly when the stats reader sets `stats_read_pending`, which means `show_lcore_stats()` on the main thread must wait for the datapath to notice the flag *and* complete a full context cycle before it can acquire the lock. This creates an unpredictable latency spike for stats collection and makes the lock contention window essentially unbounded during normal forwarding.
The **inner idle loop is a raw busy-poll** comparing `timer.expire < now` for every RX timer plus the flush timer on each iteration. With 16 ports per lcore (MAX_RX_QUEUE_PER_LCORE), that's 17 comparisons per spin. There's no `rte_pause()` inside the idle loop — only outside the outer spinlock unlock. The recent patch you may have seen (replacing `rte_pause()` with `rte_delay_us(10)` after unlock) treats the symptom, not the disease.
The **triple-loop structure makes the control flow genuinely hard to reason about**. The middle `do...while` tests `stats_read_pending` which is set by a remote lcore, but the inner loop checks it too (for `need_manage`). The idle job abort-vs-finish logic (`repeats != 1`) is a subtle optimization that's easy to misread — if the idle loop body executes exactly once, the job is aborted rather than finished, meaning the timer was already expired on entry.
## 2. The Flush Job Has an Inverted Condition (Possible Bug)
In `l2fwd_flush_job()`:
```c
if (qconf->next_flush_time[portid] <= now)
continue;
```
This **skips flushing when the time has expired**, which is backwards. You'd expect to flush when `now >= next_flush_time`, i.e. skip when `now < next_flush_time`. This means TX buffers only get flushed when they *haven't* expired yet — the opposite of the intended drain behavior. This could cause packets to sit in TX buffers far longer than the intended 100μs drain interval, contributing directly to the "slow" feeling at low traffic rates.
Additionally, `lcore_id` and `qconf` are fetched twice at the top of this function for no reason.
## 3. MAC Address Manipulation via Aliasing Violation
In `l2fwd_simple_forward()`:
```c
tmp = ð->dst_addr.addr_bytes[0];
*((uint64_t *)tmp) = 0x000000000002 + ((uint64_t)dst_port << 40);
```
This writes 8 bytes into a 6-byte MAC address through a `uint64_t*` cast, which is both a strict aliasing violation and writes 2 bytes past the MAC address into the ethertype field. The intent is to write `02:00:00:00:00:xx` as the destination MAC, and it "works" on little-endian x86 because the excess bytes happen to overwrite `src_addr` bytes that are immediately overwritten by the `rte_ether_addr_copy()` below. But this is fragile, non-portable, and undefined behavior. On a big-endian architecture this produces a completely wrong MAC.
## 4. Global Mutable State Everywhere
The application uses an excessive amount of file-scope globals:
- `l2fwd_ports_eth_addr[RTE_MAX_ETHPORTS]`
- `l2fwd_enabled_port_mask`
- `l2fwd_dst_ports[RTE_MAX_ETHPORTS]`
- `lcore_queue_conf[RTE_MAX_LCORE]`
- `tx_buffer[RTE_MAX_ETHPORTS]`
- `port_statistics[RTE_MAX_ETHPORTS]`
- `l2fwd_pktmbuf_pool`
- `hz`, `drain_tsc`, `timer_period`
These are all `RTE_MAX_ETHPORTS`/`RTE_MAX_LCORE` sized arrays, statically allocated regardless of actual usage. `port_statistics` is accessed from both the datapath lcores (writes) and the stats display callback (reads) with **no synchronization** — the stats numbers can tear on platforms without naturally atomic 64-bit loads. The `memset(&port_statistics, 0, ...)` inside the per-port init loop also clears all port stats on each port init, not just the current port's.
## 5. The Timer-Driven Architecture Adds Complexity Without Proportional Benefit
The original l2fwd example uses a simple poll loop: check TSC, drain if needed, burst RX, forward. This jobstats variant replaces that with:
- `rte_timer` for each RX port + flush timer per lcore
- `rte_jobstats` wrappers around each timer callback
- A custom `l2fwd_job_update_cb` that adjusts polling frequency via a hysteresis algorithm with asymmetric step sizes (`UPDATE_STEP_UP = 1`, `UPDATE_STEP_DOWN = 32`)
The adaptive polling idea (poll less when idle, poll more when busy) is sound in theory, but the implementation has issues. The step-up granularity of 1 TSC tick is so small relative to step-down of 32 that recovery from an idle period is extremely slow. If the system goes idle for a period and the polling interval ramps up, it takes 32x as many busy iterations to ramp back down. The target of `MAX_PKT_BURST` (32 packets) as the "optimal" value is hard-coded with no way to tune it.
The `rte_timer_manage()` call itself is not free — it walks the timer skip list on each invocation, adding overhead that a direct TSC comparison in plain l2fwd avoids.
## 6. The Double RX Burst is Questionable
```c
total_nb_rx = rte_eth_rx_burst(portid, 0, pkts_burst, MAX_PKT_BURST);
// ... forward all ...
if (total_nb_rx == MAX_PKT_BURST) {
const uint16_t nb_rx = rte_eth_rx_burst(portid, 0, pkts_burst, MAX_PKT_BURST);
// ... forward these too ...
}
```
The comment says this "allows rte_jobstats logic to see if this function must be called more frequently." The total is fed to `rte_jobstats_finish()` which compares against the target. But this means `total_nb_rx` can be up to 64 while the target is 32, and the jobstats update callback sees `result > target` and decreases the period. The asymmetric step sizes make this interaction even more unpredictable. This also means the prefetch pattern is suboptimal — the first batch is prefetched and forwarded sequentially (prefetch one, forward one), which doesn't give the prefetch enough lead time to be useful.
## 7. Argument Parsing Edge Cases
`l2fwd_parse_portmask()` returns `unsigned long` cast through `int` (0 means error), so a mask of 0 is indistinguishable from a parse error. On 64-bit systems where `unsigned long` is 64 bits but `l2fwd_enabled_port_mask` is `uint32_t`, masks above 0xFFFFFFFF silently truncate. The port mask check `(1 << portid)` throughout the code is undefined behavior for `portid >= 32` since `1` is an `int`.
## 8. No Graceful Shutdown
There's no signal handler. The application can only be killed with SIGKILL/SIGTERM, leaving ports in an undefined state (promiscuous mode enabled, potentially with traffic still queued in TX buffers). The `rte_eal_cleanup()` at the end is unreachable during normal operation since `l2fwd_main_loop()` never returns.
## Summary Recommendations
If you're looking at cleaning this up or using it as a basis for review guidelines:
The flush condition inversion should be treated as a bug fix. The main loop should be restructured to separate the stats synchronization concern from the packet processing path — a lock-free stats snapshot approach (double-buffering or seqlock) would eliminate the spinlock entirely. The MAC address write needs to use `rte_ether_addr` properly. The adaptive polling logic needs tunable parameters and a more balanced step function. And ideally, the example should be reconciled with plain l2fwd to show clearly what jobstats *adds* rather than being a near-complete fork with structural divergence.
^ permalink raw reply [flat|nested] 23+ messages in thread
* RE: [EXTERNAL] Re: [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention
2026-03-02 15:48 ` Stephen Hemminger
@ 2026-03-16 13:36 ` Rakesh Kudurumalla
2026-03-16 15:20 ` Stephen Hemminger
2026-03-17 16:40 ` Thomas Monjalon
0 siblings, 2 replies; 23+ messages in thread
From: Rakesh Kudurumalla @ 2026-03-16 13:36 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Pawel Wodkowski, Pablo de Lara, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
Hi @Stephen Hemminger,
Yes, you're correct that the architecture looks unusual. But since the primary goal here is to stress-test the stats/timer subsystem under contention rather than packet forwarding, I believe the current structure serves its purpose well.
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, March 2, 2026 9:18 PM
> To: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> Cc: Pawel Wodkowski <pawelwod@gmail.com>; Pablo de Lara
> <pablo.de.lara.guarch@intel.com>; dev@dpdk.org; Jerin Jacob
> <jerinj@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpuram@marvell.com>; stable@dpdk.org
> Subject: [EXTERNAL] Re: [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer
> stats display with lock contention
>
> On Mon, 2 Mar 2026 11: 48: 05 +0530 rkudurumalla
> <rkudurumalla@ marvell. com> wrote: > From: Rakesh Kudurumalla
> <rkudurumalla@ marvell. com> > > Race condition between jobstats and time
> metrics > for forwarding and flushing ZjQcmQRYFpfptBannerStart Prioritize
> security for external emails:
> Confirm sender and content safety before clicking links or opening
> attachments <https://us-phishalarm-
> ewt.proofpoint.com/EWT/v1/CRVmXkqW!tM3Z1f8UYnW69E-
> 8WVxabgLBnjenXs-
> Q7zZlpNFDtzEovrPgbLIXPV9yb_zhviTyHuvZsJVX17MSajtzju0guOTk$>
> Report Suspicious
>
> ZjQcmQRYFpfptBannerEnd
> On Mon, 2 Mar 2026 11:48:05 +0530
> rkudurumalla <rkudurumalla@marvell.com> wrote:
>
> > From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> >
> > Race condition between jobstats and time metrics for forwarding and
> > flushing is maintained using spinlock.
> > Timer metrics are not displayed properly due to the frequent
> > unavailability of the lock.
> >
> > This patch fixes the issue by introducing a delay before acquiring the
> > lock in the loop. This delay allows for better availability of the
> > lock, ensuring that show_lcore_stats() can periodically update the
> > statistics even when forwarding jobs are running.
> >
> > Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
>
> This does fix the problem, but this whole application seems like it is using an
> unusual architecture. The main loop per thread spends most of its time
> updating job stats and only periodically calls the actual rx/tx burst forwarding.
> That is upside down??
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention
2026-03-16 13:36 ` [EXTERNAL] " Rakesh Kudurumalla
@ 2026-03-16 15:20 ` Stephen Hemminger
2026-03-17 16:39 ` Thomas Monjalon
2026-03-17 16:40 ` Thomas Monjalon
1 sibling, 1 reply; 23+ messages in thread
From: Stephen Hemminger @ 2026-03-16 15:20 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: Pawel Wodkowski, Pablo de Lara, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, stable@dpdk.org
On Mon, 16 Mar 2026 13:36:30 +0000
Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
> Hi @Stephen Hemminger,
>
> Yes, you're correct that the architecture looks unusual. But since the primary goal here is to stress-test the stats/timer subsystem under contention rather than packet forwarding, I believe the current structure serves its purpose well.
But it also exposes lots of issues. I would argue that if it is a stress test it should be in tests/
not examples. Examples are intended as guidance for how users should use the API.
This is not the example that we should guide users towards.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention
2026-03-16 15:20 ` Stephen Hemminger
@ 2026-03-17 16:39 ` Thomas Monjalon
0 siblings, 0 replies; 23+ messages in thread
From: Thomas Monjalon @ 2026-03-17 16:39 UTC (permalink / raw)
To: Rakesh Kudurumalla, Stephen Hemminger
Cc: Pawel Wodkowski, Pablo de Lara, dev@dpdk.org, Jerin Jacob,
Nithin Kumar Dabilpuram, techboard
16/03/2026 16:20, Stephen Hemminger:
> On Mon, 16 Mar 2026 13:36:30 +0000
> Rakesh Kudurumalla <rkudurumalla@marvell.com> wrote:
>
> > Hi @Stephen Hemminger,
> >
> > Yes, you're correct that the architecture looks unusual. But since the primary goal here is to stress-test the stats/timer subsystem under contention rather than packet forwarding, I believe the current structure serves its purpose well.
>
>
> But it also exposes lots of issues. I would argue that if it is a stress test it should be in tests/
> not examples. Examples are intended as guidance for how users should use the API.
>
> This is not the example that we should guide users towards.
I agree.
It could be discussed in the techboard.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [EXTERNAL] Re: [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention
2026-03-16 13:36 ` [EXTERNAL] " Rakesh Kudurumalla
2026-03-16 15:20 ` Stephen Hemminger
@ 2026-03-17 16:40 ` Thomas Monjalon
1 sibling, 0 replies; 23+ messages in thread
From: Thomas Monjalon @ 2026-03-17 16:40 UTC (permalink / raw)
To: Rakesh Kudurumalla
Cc: Stephen Hemminger, dev, Pawel Wodkowski, Pablo de Lara,
dev@dpdk.org, Jerin Jacob, Nithin Kumar Dabilpuram,
stable@dpdk.org
16/03/2026 14:36, Rakesh Kudurumalla:
> Hi @Stephen Hemminger,
>
> Yes, you're correct that the architecture looks unusual. But since the primary goal here is to stress-test the stats/timer subsystem under contention rather than packet forwarding, I believe the current structure serves its purpose well.
>
> From: Stephen Hemminger <stephen@networkplumber.org>
> > > From: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> > >
> > > Race condition between jobstats and time metrics for forwarding and
> > > flushing is maintained using spinlock.
> > > Timer metrics are not displayed properly due to the frequent
> > > unavailability of the lock.
> > >
> > > This patch fixes the issue by introducing a delay before acquiring the
> > > lock in the loop. This delay allows for better availability of the
> > > lock, ensuring that show_lcore_stats() can periodically update the
> > > statistics even when forwarding jobs are running.
> > >
> > > Fixes: 204896f8d66c ("examples/l2fwd-jobstats: add new example")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Rakesh Kudurumalla <rkudurumalla@marvell.com>
> >
> > This does fix the problem, but this whole application seems like it is using an
> > unusual architecture. The main loop per thread spends most of its time
> > updating job stats and only periodically calls the actual rx/tx burst forwarding.
> > That is upside down??
We can discuss the future of this example.
For now, the fix is applied, thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-03-17 16:40 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-26 5:37 [PATCH 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
2024-07-29 6:10 ` [PATCH v2 " Rakesh Kudurumalla
2024-07-30 10:03 ` [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability Rakesh Kudurumalla
2024-07-30 16:12 ` Thomas Monjalon
2024-08-08 11:41 ` [EXTERNAL] " Rakesh Kudurumalla
2024-08-08 12:22 ` Thomas Monjalon
2024-08-11 16:00 ` Rakesh Kudurumalla
2024-08-11 6:58 ` [PATCH v4 " Rakesh Kudurumalla
2024-08-11 15:59 ` [PATCH v5 " Rakesh Kudurumalla
2024-08-11 16:17 ` Stephen Hemminger
2024-08-16 5:25 ` [EXTERNAL] " Rakesh Kudurumalla
2024-10-21 7:13 ` Rakesh Kudurumalla
2024-11-04 10:17 ` Nithin Dabilpuram
2025-05-24 15:39 ` Stephen Hemminger
2026-01-14 6:24 ` Stephen Hemminger
2026-03-02 6:18 ` [PATCH v6 1/1] examples/l2fwd-jobstats: fix timer stats display with lock contention rkudurumalla
2026-03-02 15:48 ` Stephen Hemminger
2026-03-16 13:36 ` [EXTERNAL] " Rakesh Kudurumalla
2026-03-16 15:20 ` Stephen Hemminger
2026-03-17 16:39 ` Thomas Monjalon
2026-03-17 16:40 ` Thomas Monjalon
2024-07-29 6:20 ` [PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats Rakesh Kudurumalla
2026-03-02 16:23 ` Stephen Hemminger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox