[PATCH grout] l2: optimize bridge FDB source MAC learning

[Robin Jarry]

Morten Brørup, May 21, 2026 at 23:09:
> We started off with something simple as that. But races started
> showing up, where e.g. a packet could seemingly spend negative time in
> a shaper if the enqueueing thread had snapshotted its clock after the
> dequeueing thread had snapshotted its clock - although the enqueueing
> thread had obviously put the packet in the queue before the dequeueing
> thread took it out of the queue.
>
> So we kept the RTE_PER_LCORE() timestamps in various formats, and
> introduced a "void per_lcore_clock_update(void)" function to update
> the snapshots of the calling thread. Now, the trick is: Any burst
> processing function relying on timestamps must call this update
> function first. Thinking about the need for this requires the same way
> of thinking as for locks; you need to always consider ordering and
> races.

Ah yes, if you do cross worker processing, you'll need a way to
synchronize clocks.

> If I added a "clock" module exposing these features for public use
> (not only internal use), where in the tree should I put the files?
> There's going to be a C file with the gr_per_lcore_clock_update()
> function implementation and the RTE_DEFINE_PER_LCORE(gr_clock_xyz)
> timestamps in various formats, and an accompanying public header file
> with the function declaration and the
> RTE_DECLARE_PER_LCORE(gr_clock_xyz) timestamps in various formats.
>
> Then I can update the bridge FDB patch to use this central clock
> instead. And for now, I think updating the clock is only required
> where you put it in your example above.
>
> I'm not yet familiar with Grout naming conventions. The "void
> gr_per_lcore_clock_update(void)" function updates the RTE_PER_LCORE()
> timestamp variables for the calling thread. Any guidance for how to
> name the function?

Only symbols that are exposed via the API headers (gr_*.h) should start
with the gr_ prefix. Symbols internal to the grout daemon or grcli
should not. There are some unfortunate exceptions (one of them being
gr_datapath_loop) but we should avoid these.

How about adding the functions to modules/infra/datapath/datapath.h and
find a common prefix for them:

diff --git a/modules/infra/datapath/datapath.h b/modules/infra/datapath/datapath.h
index 2c1e58e5f8ec..4bdb9532498c 100644
--- a/modules/infra/datapath/datapath.h
+++ b/modules/infra/datapath/datapath.h
@@ -3,4 +3,21 @@
 
 #pragma once
 
+#include <rte_per_lcore.h>
+
+#include <time.h>
+
 void *gr_datapath_loop(void *priv);
+
+// Per worker timestamp value maintained by gr_datapath_loop().
+RTE_DECLARE_PER_LCORE(clock_t, timestamp_us);
+
+// Get the value of the current thread timestamp.
+static inline clock_t timestamp_us_get(void) {
+	return RTE_PER_LCORE(timestamp_us);
+}
+
+// Update the value of the current thread timestamp.
+static inline void timestamp_us_set(clock_t us) {
+	RTE_PER_LCORE(timestamp_us) = us;
+}
diff --git a/modules/infra/datapath/main_loop.c b/modules/infra/datapath/main_loop.c
index f462cfbd42dd..423a4eb91ba7 100644
--- a/modules/infra/datapath/main_loop.c
+++ b/modules/infra/datapath/main_loop.c
@@ -10,6 +10,8 @@
 #include "vec.h"
 #include "worker.h"
 
+#include <gr_clock.h>
+
 #include <rte_common.h>
 #include <rte_eal.h>
 #include <rte_errno.h>
@@ -182,6 +184,8 @@ err:
 
 static struct rte_rcu_qsbr *rcu;
 
+RTE_DEFINE_PER_LCORE(clock_t, timestamp_us);
+
 void *gr_datapath_loop(void *priv) {
 	struct stats_context ctx = {
 		.stats = NULL,
@@ -257,6 +261,7 @@ reconfig:
 	loop = 0;
 	sleep = 0;
 	timestamp = rte_rdtsc();
+	timestamp_us_set(gr_clock_us());
 	for (;;) {
 		rte_graph_walk(graph);
 
@@ -292,6 +297,7 @@ reconfig:
 			ctx.w_stats->total_cycles += cycles;
 			ctx.w_stats->loop_cycles += cycles;
 			ctx.w_stats->n_loops += HOUSEKEEPING_INTERVAL;
+			timestamp_us_set(gr_clock_us());
 		}
 	}
 
diff --git a/modules/l2/control/fdb.c b/modules/l2/control/fdb.c
index 3982cce11854..bb0fb86cc42d 100644
--- a/modules/l2/control/fdb.c
+++ b/modules/l2/control/fdb.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: BSD-3-Clause
 // Copyright (c) 2026 Robin Jarry
 
+#include "datapath.h"
 #include "event.h"
 #include "iface.h"
 #include "l2.h"
@@ -138,7 +139,7 @@ void fdb_learn(
 		fdb = data;
 	}
 
-	fdb->last_seen = gr_clock_us();
+	fdb->last_seen = timestamp_us_get();
 
 	if ((fdb->flags & GR_FDB_F_LEARN)
 	    && (fdb->iface_id != iface_id || !l3_addr_eq(&fdb->vtep, vtep))) {

Does that fit your needs?

-- 
Robin

> Please come again.