[PATCH 1/1] examples/l2fwd-jobstats: add delay to show stats

[PATCH 1/1] examples/l2fwd-jobstats: add delay to show stats

[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

[PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats

[Rakesh Kudurumalla]

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

[PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

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

Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability

[Thomas Monjalon]

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);
>  	}

RE: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

[-- 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 --]

Re: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability

[Thomas Monjalon]

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?

RE: [EXTERNAL] Re: [PATCH v3 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

> -----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

[PATCH v4 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

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

[PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

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

Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability

[Stephen Hemminger]

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

RE: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

> -----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.

RE: [EXTERNAL] Re: [PATCH v5 1/1] examples/l2fwd-jobstats: fix lock availability

[Rakesh Kudurumalla]

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.

[PATCH v2 1/1] examples/l2fwd-jobstats: add delay to show stats

[Rakesh Kudurumalla]

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