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

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

[Qi Wu]

tcx_prog_attach() and tcx_link_prog_attach() call tcx_entry_sync()
unconditionally after tcx_entry_update(). tcx_entry_sync() is a bare
synchronize_rcu() whose stated purpose is to ensure there are no
inflight users of the bpf_mprog_entry that got swapped out, so that
the next attach can safely reuse it as the peer and bpf_mprog_commit()
can safely drop the old prog reference.

When tcx_entry_fetch_or_create() reports created == true, the entry
being swapped out is bundle.a of a freshly allocated tcx_entry that
has never been published: dev->tcx_{ingress,egress} was NULL, so
sch_handle_ingress() and sch_handle_egress() always dereferenced NULL
and returned early. No RCU reader can hold a reference to it.
entry->parent->ref is also NULL on this path, because
bpf_mprog_mark_for_release() is only called from bpf_mprog_delete(),
so bpf_mprog_commit() does not drop any prog reference either.

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.

Skip tcx_entry_sync() when created is true. Note that created means
"this direction had no tcx_entry before", not "this is the first BPF
program": if a clsact or ingress qdisc was created first,
ingress_init()/clsact_init() already published bundle.a via
tcx_entry_update(), so created is false and the sync is still
performed. This mirrors the existing behaviour of those two functions,
which publish a newly created entry without any synchronize_rcu().

Detach, replace, multi-program chains and the error rollback path are
unchanged. The replace path already skips the sync today, since
bpf_mprog_replace() returns entry_new == entry.

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,
CONFIG_NO_HZ_FULL=y. 30 iterations per scenario, plus ftrace kprobes
on tcx_prog_attach, tcx_link_attach and synchronize_rcu filtered by
common_pid.

bpf() syscall latency, base -> patched:

  scenario                     base        patched     change
  first link attach (ingress)  12.730 ms   263.8 us    -97.9%  48x
  first link attach (egress)   12.824 ms   286.0 us    -97.8%  45x
  first prog attach (fd API)   12.462 ms   273.5 us    -97.8%  46x
  attach #1 on a device         9.870 ms   204.8 us    -97.9%  48x
  bulk, per device (2 dirs)    13.746 ms   296.2 us    -97.8%  46x

Where the time went, from the ftrace attribution:

  scenario          base total  base sync  share   patched total  sync
  first link        12.113 ms   11.923 ms  98.4%   243.5 us       none
  first prog        11.435 ms   11.260 ms  98.5%   293.7 us       none
  bulk, 64 attaches  6.560 ms    6.552 ms  99.9%     7.4 us       none

Creating 32 veth devices and attaching tcx to ingress and egress on
each (64 first attaches, all serialized under rtnl_lock):

  base     440.184 ms
  patched    9.930 ms      -97.7%, 44x faster

BPF_PROG_QUERY, which takes RTNL but performs no sync, averages 0.8 us
on both kernels, so lock acquisition itself is not the cost.

The created=false paths keep their grace period, as intended:

  scenario                     base sync   patched sync
  clsact qdisc first           11.924 ms   11.972 ms
  attach #2..N on a device      6.749 ms    7.303 ms

synchronize_rcu() call counts were checked per caller and match the
expectation exactly in all 12 cases: the attach-path counts drop to
zero for created=true scenarios, stay identical for created=false
scenarios, and every detach/release count is unchanged.

Functional checks pass on the patched kernel: attachment ordering
(BPF_F_BEFORE/BPF_F_AFTER), BPF_F_REPLACE, expected_revision/-ESTALE,
duplicate attach/-EEXIST, full detach and re-attach.

A concurrency test runs flood ping through a veth pair split across
two net namespaces while repeatedly attaching and detaching a chain
of tcx links. Each cycle detaches everything, which frees the bundle,
so the next cycle hits created == true again: 300 cycles exercised
300 created=true and 900 created=false attaches under live traffic,
with no failures and no RCU or memory-safety splats in dmesg.

BPF CI was run on this change via kernel-patches/bpf pull request #14821:

  https://github.com/kernel-patches/bpf/pull/14821

42 checks passed and none failed, across x86_64 gcc-15, x86_64 llvm-21,
aarch64 gcc-15 and s390x gcc-15. This covers test_progs (including
tc_links, tc_opts and tc_netkit), test_progs_no_alu32, test_progs_cpuv4,
test_progs_parallel, test_progs_no_alu32_parallel, test_maps and
test_verifier. The tc_links cases exercise both branches of this change:
dev_mixed attaches tcx before creating the clsact hook (created == true),
while chain_classic creates the hook first (created == false).

netkit has the same pattern: netkit_entry_fetch() falls back to
&nk->bundle.a when nk->active is NULL, so its first attach also waits
for a grace period on an entry that was never published. Addressing that
is left to a separate change to keep this one minimal.

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

Signed-off-by: Qi Wu <shijing.wuqi@linux.alibaba.com>
---
 kernel/bpf/tcx.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/tcx.c b/kernel/bpf/tcx.c
index 02db0113b8e7..7652069271e0 100644
--- a/kernel/bpf/tcx.c
+++ b/kernel/bpf/tcx.c
@@ -42,7 +42,11 @@ 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();
+			/* A newly created entry was never published, so
+			 * there are no inflight RCU users to wait for.
+			 */
+			if (!created)
+				tcx_entry_sync();
 			tcx_skeys_inc(ingress);
 		}
 		bpf_mprog_commit(entry);
@@ -156,7 +160,11 @@ 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();
+			/* A newly created entry was never published, so
+			 * there are no inflight RCU users to wait for.
+			 */
+			if (!created)
+				tcx_entry_sync();
 			tcx_skeys_inc(ingress);
 		}
 		bpf_mprog_commit(entry);
-- 
2.47.3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] bpf: multi-line comment style in tcx_prog_attach() and
  tcx_link_prog_attach()
--

commit 2e14928eb20fcdc45d5b96fe78599b9ad93e5381
Author: Qi Wu <shijing.wuqi@linux.alibaba.com>

bpf, tcx: Skip RCU synchronization when the entry was newly created

This commit skips calling tcx_entry_sync() in tcx_prog_attach() and
tcx_link_prog_attach() when the tcx_entry was newly created and never
published to RCU readers.

> diff --git a/kernel/bpf/tcx.c b/kernel/bpf/tcx.c
> index 02db0113b8e7c..7652069271e0c 100644
> --- a/kernel/bpf/tcx.c
> +++ b/kernel/bpf/tcx.c
> @@ -42,7 +42,11 @@ 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();
> +			/* A newly created entry was never published, so
> +			 * there are no inflight RCU users to wait for.
> +			 */
> +			if (!created)
> +				tcx_entry_sync();
>  			tcx_skeys_inc(ingress);
>  		}
>  		bpf_mprog_commit(entry);
> @@ -156,7 +160,11 @@ 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();
> +			/* A newly created entry was never published, so
> +			 * there are no inflight RCU users to wait for.
> +			 */
> +			if (!created)
> +				tcx_entry_sync();
>  			tcx_skeys_inc(ingress);
>  		}
>  		bpf_mprog_commit(entry);

[Severity: Low]
This isn't a bug, but BPF code uses the multi-line comment style where the
opening /* is on its own line and the comment text starts on the next line.
Could both comments in tcx_prog_attach() and tcx_link_prog_attach() be
formatted that way?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114124039.111360-1-shijing.wuqi@linux.alibaba.com?part=1

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

[Alexei Starovoitov]

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