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

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

[Qi Wu]

bpf_mprog_attach() swaps the active entry and the caller then waits
for a grace period (tcx_entry_sync() / netkit_entry_sync()) before
returning. But attach releases nothing: the old entry only has to
stay intact until the next attach or detach overwrites it as the
peer. The unconditional wait buys nothing on the attach path, yet it
runs inside rtnl_lock() and serializes the whole machine's network
control plane. Creating containers in bulk pays 2N grace periods for
N devices, one first attach per direction.

Borrow the scheme used by mini_qdisc_pair_swap() in commit 267463823adb
("net: sch: eliminate unnecessary RCU waits in mini_qdisc_pair_swap()"):
record the grace-period state after the swap and only wait before the
old buffer is written again. bpf_mprog_commit() records
start_poll_synchronize_rcu(), bpf_mprog_peer() calls
cond_synchronize_rcu() before returning the peer to be overwritten,
and a new bundle starts with get_completed_synchronize_rcu() so the
first attach never waits. This removes the attach-path sync from both
tcx and netkit with no 'created' special case, and covers a clsact
qdisc having published the entry first.

A/B on the same bpf-next base (8835113107b2), identical configs apart
from LOCALVERSION, lock/RCU debugging off, 64 vCPU KVM guest. Average
bpf() syscall latency, 30 iterations per scenario:

  scenario                 base        v2        speedup
  first link attach        11.375 ms   277.1 us    41x
  first prog attach        13.226 ms   264.2 us    50x
  clsact qdisc first       12.547 ms   114.5 us   110x

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
churn under live traffic shows no splats. BPF CI:
https://github.com/kernel-patches/bpf/pull/14821 (39 passed, 0 failed).

The implementation and measurement were produced with the help of an
AI coding assistant; the change was reviewed by the submitter.

Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Qi Wu <shijing.wuqi@linux.alibaba.com>
---
 drivers/net/netkit.c      |  8 ++------
 include/linux/bpf_mprog.h | 23 ++++++++++++++++-------
 kernel/bpf/tcx.c          |  2 --
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
index 82e608e733d9..aa15fedadaeb 100644
--- a/drivers/net/netkit.c
+++ b/drivers/net/netkit.c
@@ -770,10 +770,8 @@ int netkit_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 			       attr->attach_flags, attr->relative_fd,
 			       attr->expected_revision);
 	if (!ret) {
-		if (entry != entry_new) {
+		if (entry != entry_new)
 			netkit_entry_update(dev, entry_new);
-			netkit_entry_sync();
-		}
 		bpf_mprog_commit(entry);
 	}
 out:
@@ -852,10 +850,8 @@ static int netkit_link_prog_attach(struct bpf_link *link, u32 flags,
 	ret = bpf_mprog_attach(entry, &entry_new, link->prog, link, NULL, flags,
 			       id_or_fd, revision);
 	if (!ret) {
-		if (entry != entry_new) {
+		if (entry != entry_new)
 			netkit_entry_update(dev, entry_new);
-			netkit_entry_sync();
-		}
 		bpf_mprog_commit(entry);
 	}
 	return ret;
diff --git a/include/linux/bpf_mprog.h b/include/linux/bpf_mprog.h
index 0b9f4caeeb0a..de0021a66214 100644
--- a/include/linux/bpf_mprog.h
+++ b/include/linux/bpf_mprog.h
@@ -28,11 +28,8 @@
  *   [...]
  *   ret = bpf_mprog_attach(entry, &entry_new, [...]);
  *   if (!ret) {
- *       if (entry != entry_new) {
+ *       if (entry != entry_new)
  *           // swap @entry to @entry_new at attach location
- *           // ensure there are no inflight users of @entry:
- *           synchronize_rcu();
- *       }
  *       bpf_mprog_commit(entry);
  *   } else {
  *       // error path, bail out, propagate @ret
@@ -104,9 +101,12 @@
  *
  * 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_commit() records the RCU state after the swap, 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 +147,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,6 +159,12 @@ 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.
+	 */
+	cond_synchronize_rcu(entry->parent->rcu_state);
 	if (entry == &entry->parent->a)
 		return &entry->parent->b;
 	else
@@ -172,6 +179,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;
 }
@@ -243,6 +251,7 @@ static inline void bpf_mprog_commit(struct bpf_mprog_entry *entry)
 {
 	bpf_mprog_complete_release(entry);
 	bpf_mprog_revision_new(entry);
+	entry->parent->rcu_state = start_poll_synchronize_rcu();
 }
 
 static inline u64 bpf_mprog_revision(struct bpf_mprog_entry *entry)
diff --git a/kernel/bpf/tcx.c b/kernel/bpf/tcx.c
index 02db0113b8e7..0079be0b16f0 100644
--- a/kernel/bpf/tcx.c
+++ b/kernel/bpf/tcx.c
@@ -42,7 +42,6 @@ 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();
 			tcx_skeys_inc(ingress);
 		}
 		bpf_mprog_commit(entry);
@@ -156,7 +155,6 @@ 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();
 			tcx_skeys_inc(ingress);
 		}
 		bpf_mprog_commit(entry);
-- 
2.47.3

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3538 bytes --]

> bpf, mprog: Defer RCU synchronization until peer reuse

TL;DR:
- Recording an RCU cookie on every commit may add an extra grace period to
  back-to-back detaches (N becomes 2N-1), a possible performance regression
- Nit: example comment in bpf_mprog.h is ambiguous without braces

> diff --git a/include/linux/bpf_mprog.h b/include/linux/bpf_mprog.h
> --- a/include/linux/bpf_mprog.h
> +++ b/include/linux/bpf_mprog.h
> @@ -28,11 +28,8 @@
>   *   [...]
>   *   ret = bpf_mprog_attach(entry, &entry_new, [...]);
>   *   if (!ret) {
> - *       if (entry != entry_new) {
> + *       if (entry != entry_new)
>   *           // swap @entry to @entry_new at attach location
> - *           // ensure there are no inflight users of @entry:
> - *           synchronize_rcu();
> - *       }
>   *       bpf_mprog_commit(entry);
>   *   } else {
>   *       // error path, bail out, propagate @ret

This isn't a bug, but with the braces removed, the only body of the if is a
comment line.  Read as C, the next statement becomes the body:

    if (entry != entry_new)
        bpf_mprog_commit(entry);

In the real callers (tcx_prog_attach(), netkit_prog_attach(),
tcx_link_update(), ...) bpf_mprog_commit() is unconditional, and it must
also run when entry == entry_new so that bpf_mprog_revision_new() bumps the
revision.  Could the braces be kept, or a placeholder statement used for
the swap, so the example is unambiguous?

[ ... ]

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

Does recording a new cookie on every commit add a grace period to detach
sequences that previously paid only one?

The detach paths (tcx_prog_detach(), tcx_link_release(),
netkit_prog_detach(), netkit_link_release()) already wait before calling
commit:

    tcx_entry_update(dev, entry_new, ingress);
    tcx_entry_sync();
    tcx_skeys_dec(ingress);
    bpf_mprog_commit(entry);

Once tcx_entry_sync() returns, the old entry has no readers left, but
commit stores a cookie that only completes after another full grace
period.

With two programs on one tcx hook detached back to back, the second detach
goes bpf_mprog_delete() -> bpf_mprog_peer() -> cond_synchronize_rcu(),
which has to run synchronize_rcu(), and then tcx_entry_sync() waits again.
That is two grace periods where there used to be one, so N back-to-back
detaches go from N to 2N-1.

A similar thing happens for replace followed by detach.  tcx_link_update(),
netkit_link_update() and BPF_F_REPLACE prog attach do not swap entries
(WARN_ON_ONCE(entry != entry_new)), so the peer is unchanged, yet commit
still overwrites rcu_state with a fresh cookie.  A detach shortly after
then pays cond_synchronize_rcu() plus tcx_entry_sync().

The commit message says "peer reuse can add a conditional wait before a
detach", but in these cases the extra wait does not protect anything.
mini_qdisc_pair_swap(), which this patch copies, only records state for the
miniq that was actually swapped out.  Should the cookie be recorded only
when a swap happened without a following synchronize_rcu(), or should the
detach paths store get_completed_synchronize_rcu() after their sync?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/43727865402

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

[Alexei Starovoitov]

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