Re: [PATCH bpf-next] bpf, tcx: Skip RCU synchronization when the entry was newly created

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

On Sat, Nov 14, 2026 at 04:40 AM Qi Wu <shijing.wuqi@linux.alibaba.com> wrote:
> Both preconditions for the grace period are therefore absent and the
> synchronize_rcu() is pure overhead. It runs inside the rtnl_lock()
> critical section, so it also serializes the whole machine's network
> control plane. This is visible when creating containers or sandboxes
> in bulk: each new netdev needs one first attach per direction, i.e.
> 2N grace periods for N devices, all serialized under RTNL.

No attach has to wait after the swap. Not only the first one.
Attach releases nothing. The old entry has to stay intact only until
the next attach or detach overwrites it as the peer.

mini_qdisc_pair_swap() has the same a/b scheme in the same hook.
It stores start_poll_synchronize_rcu() after the swap and waits
before it writes to the old buffer again. See
commit 267463823adb ("net: sch: eliminate unnecessary RCU waits in mini_qdisc_pair_swap()")

Lets do the same in bpf_mprog.
A new bundle starts with get_completed_synchronize_rcu().
Then there is no need for the 'created' check.
netkit needs no special case either.

  attach                     this patch   poll in bpf_mprog
  -------------------------  -----------  ----------------------------
  tcx, first on the device   no wait      no wait
  tcx, clsact came first     waits        no wait
  tcx, prog #2..N            waits        waits only when the previous
                                          change is younger than a
                                          grace period
  netkit                     waits        same as tcx

The diff is at the end.

> Detach, replace, multi-program chains and the error rollback path are
> unchanged.

Bulk removal has the same 2N problem.
unregister_netdevice_many_notify() calls tcx_uninstall() for both
directions of every device. Each call that finds an entry waits for
a grace period under RTNL.
synchronize_net() is expedited under RTNL.
Should tcx_entry_sync() and netkit_entry_sync() use it?

> The numbers below come from an A/B pair built from the same bpf-next
> base (2e14a351662d) with identical configs apart from LOCALVERSION, and
> all lock/RCU debugging disabled. 64 vCPU KVM guest, CONFIG_HZ=1000,

Way too long. That's 100+ lines of commit log for a two line change.
Keep the problem, the idea and one table of numbers.

> -			tcx_entry_sync();
> +			/* A newly created entry was never published, so
> +			 * there are no inflight RCU users to wait for.
> +			 */
> +			if (!created)
> +				tcx_entry_sync();

bpf_mprog_commit() records the state after detach and replace too.
They don't need it, but the next attach is never slower than today.
This is what I have in mind. The x86-64 build passes, no test was run:

diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
index 82e608e..aa15fed 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 0b9f4ca..e74430a 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,11 @@ 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 +178,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 +250,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 02db011..0079be0 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);

pw-bot: cr