Re: [PATCH bpf-next v2] bpf, mprog: Defer RCU synchronization until peer reuse

["Alexei Starovoitov" <alexei.starovoitov@gmail.com>]

On Sun, Nov 15, 2026 at 06:45 AM Qi Wu <shijing.wuqi@linux.alibaba.com> wrote:
> An attach within a grace period of the previous change still waits
> (e.g. a tight loop); spaced-out attaches do not. Detach retains its
> post-swap synchronization; peer reuse can add a conditional wait
> before a detach. Functional checks pass and a 300-cycle attach/detach

The extra wait before a detach was my suggestion, sorry.
I looked at the next attach only.

A detach that follows a detach or a replace now waits twice.
The first detach waits in tcx_entry_sync(). The old entry has no users
after that. Then bpf_mprog_commit() takes a new state, and the next
bpf_mprog_peer() waits for it. Nothing needs that wait.
A replace doesn't swap, but it takes a new state too.
To close N links of one hook takes 2N-1 grace periods under RTNL.
Today it takes N.

Grace periods that the second change waits for, when it comes within
a grace period of the first one:

  first     second    today   v2   diff below
  -------   -------   -----   --   ----------
  attach    attach      1     1        1
  detach    attach      1     1        0
  replace   attach      1     1        0
  attach    detach      1     2        2
  detach    detach      1     2        1
  replace   detach      1     2        1

In the attach/detach row the attach saves one wait.
The sum is the same as today.

Pls add "detach after detach" to the table of numbers.

> @@ -243,6 +251,7 @@ static inline void bpf_mprog_commit(struct bpf_mprog_entry *entry)
>  	bpf_mprog_revision_new(entry);
> +	entry->parent->rcu_state = start_poll_synchronize_rcu();

Only the swap in attach needs the state.
Lets take it there, in place of the sync that goes away.
Detach and replace leave the old state alone. bpf_mprog_peer() waited
for it already, so the next change doesn't wait.
The build is clean on x86-64. I did not boot it:

diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
index 82e608e..75a1a56 100644
--- a/drivers/net/netkit.c
+++ b/drivers/net/netkit.c
@@ -772,7 +772,7 @@ int netkit_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 	if (!ret) {
 		if (entry != entry_new) {
 			netkit_entry_update(dev, entry_new);
-			netkit_entry_sync();
+			bpf_mprog_defer_sync(entry);
 		}
 		bpf_mprog_commit(entry);
 	}
@@ -854,7 +854,7 @@ static int netkit_link_prog_attach(struct bpf_link *link, u32 flags,
 	if (!ret) {
 		if (entry != entry_new) {
 			netkit_entry_update(dev, entry_new);
-			netkit_entry_sync();
+			bpf_mprog_defer_sync(entry);
 		}
 		bpf_mprog_commit(entry);
 	}
diff --git a/include/linux/bpf_mprog.h b/include/linux/bpf_mprog.h
index 0b9f4ca..2e180a4 100644
--- a/include/linux/bpf_mprog.h
+++ b/include/linux/bpf_mprog.h
@@ -30,8 +30,8 @@
  *   if (!ret) {
  *       if (entry != entry_new) {
  *           // swap @entry to @entry_new at attach location
- *           // ensure there are no inflight users of @entry:
- *           synchronize_rcu();
+ *           // nothing is released, no need to wait for users of @entry:
+ *           bpf_mprog_defer_sync(entry);
  *       }
  *       bpf_mprog_commit(entry);
  *   } else {
@@ -104,9 +104,13 @@
  *
  * Fast path accesses the active bpf_mprog_entry within RCU critical section
  * (in case of tcx it runs in NAPI which provides RCU protection there,
- * other users might need explicit rcu_read_lock()). The bpf_mprog_commit()
- * assumes that for the old bpf_mprog_entry there are no inflight users
- * anymore.
+ * other users might need explicit rcu_read_lock()). In the detach case the
+ * bpf_mprog_commit() assumes that for the old bpf_mprog_entry there are no
+ * inflight users anymore. In the attach case nothing is released, and the
+ * old bpf_mprog_entry only has to stay intact until its users are gone:
+ * bpf_mprog_defer_sync() records the RCU state after the swap, and
+ * bpf_mprog_peer() waits for it before the old bpf_mprog_entry is written
+ * again.
  *
  * The READ_ONCE()/WRITE_ONCE() pairing for bpf_mprog_fp's prog access is for
  * the replacement case where we don't swap the bpf_mprog_entry.
@@ -147,6 +151,7 @@ struct bpf_mprog_bundle {
 	struct bpf_mprog_cp cp_items[BPF_MPROG_MAX];
 	struct bpf_prog *ref;
 	atomic64_t revision;
+	unsigned long rcu_state;
 	u32 count;
 };

@@ -158,12 +163,23 @@ struct bpf_tuple {
 static inline struct bpf_mprog_entry *
 bpf_mprog_peer(const struct bpf_mprog_entry *entry)
 {
+	/*
+	 * The peer was active until the last swap and the caller is going
+	 * to overwrite it, therefore ensure that there are no inflight
+	 * users on it anymore. Paired with bpf_mprog_defer_sync().
+	 */
+	cond_synchronize_rcu(entry->parent->rcu_state);
 	if (entry == &entry->parent->a)
 		return &entry->parent->b;
 	else
 		return &entry->parent->a;
 }

+static inline void bpf_mprog_defer_sync(struct bpf_mprog_entry *entry)
+{
+	entry->parent->rcu_state = start_poll_synchronize_rcu();
+}
+
 static inline void bpf_mprog_bundle_init(struct bpf_mprog_bundle *bundle)
 {
 	BUILD_BUG_ON(sizeof(bundle->a.fp_items[0]) > sizeof(u64));
@@ -172,6 +188,7 @@ static inline void bpf_mprog_bundle_init(struct bpf_mprog_bundle *bundle)

 	memset(bundle, 0, sizeof(*bundle));
 	atomic64_set(&bundle->revision, 1);
+	bundle->rcu_state = get_completed_synchronize_rcu();
 	bundle->a.parent = bundle;
 	bundle->b.parent = bundle;
 }
diff --git a/kernel/bpf/tcx.c b/kernel/bpf/tcx.c
index 02db011..12a1620 100644
--- a/kernel/bpf/tcx.c
+++ b/kernel/bpf/tcx.c
@@ -42,7 +42,7 @@ int tcx_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 	if (!ret) {
 		if (entry != entry_new) {
 			tcx_entry_update(dev, entry_new, ingress);
-			tcx_entry_sync();
+			bpf_mprog_defer_sync(entry);
 			tcx_skeys_inc(ingress);
 		}
 		bpf_mprog_commit(entry);
@@ -156,7 +156,7 @@ static int tcx_link_prog_attach(struct bpf_link *link, u32 flags, u32 id_or_fd,
 	if (!ret) {
 		if (entry != entry_new) {
 			tcx_entry_update(dev, entry_new, ingress);
-			tcx_entry_sync();
+			bpf_mprog_defer_sync(entry);
 			tcx_skeys_inc(ingress);
 		}
 		bpf_mprog_commit(entry);

pw-bot: cr