[PATCH net v2 0/3] net/sched: cls_bpf: accept dev-bound programs on shared blocks

[PATCH net v2 0/3] net/sched: cls_bpf: accept dev-bound programs on shared blocks

[Jamal Hadi Salim]

This series fixes the regression where a device-bound SCHED_CLS program
attached through a shared block is rejected even when it is bound to a
netdev that owns the block, and closes the bind/reoffload replay hole a
later-joining device would otherwise exploit.

Patch 1 gives netdevsim's cls_bpf TC block callback the device-match
check nfp already has; the callback is the authoritative per-device
acceptance boundary and the defect there predates the cls_bpf core
change.

Patch 2 makes the core attach-time check derive the target device from
the shared block's member set (block->q is NULL for a shared block) and
require every member to match, mirroring the broadcast fan-out of the
offload path.  Patch 1 is a prerequisite of patch 2 on v7.2+ trees:
commit 120977e2c096 ("net/sched: cls_bpf: reject dev-bound programs
bound to a different device") made the replay path unreachable through
shared blocks as a side effect, and patch 2 re-exposes it, so a stable
tree must take both together.

Patch 3 adds shared-block and late-join coverage to bpf_offload.py; it
uses the existing sample_ret0.bpf.o with an explicit `type classifier`
override instead of adding a new sample object.

---
Changelog:

v1 -> v2
- Patch 1 (netdevsim): enforce bpf_offload_dev_match() on the cls_bpf TC
  block callback, closing the bind/reoffload replay path (nipa High).
- Patch 2 (cls_bpf): reject an empty shared block; reformat the comment
  (gemini + nipa Low).
- Patch 3 (selftests): new shared-block + late-join coverage in
  bpf_offload.py.
- Split the v2 draft into three patches; patches 1 and 2 each carry their
  own Fixes: and stable metadata (the selftest companion carries neither).

Jamal Hadi Salim (3):
  netdevsim: reject cls_bpf programs bound to another device
  net/sched: cls_bpf: accept dev-bound programs on shared blocks
  selftests: net: exercise dev-bound cls_bpf on shared blocks


 drivers/net/netdevsim/bpf.c                |  12 ++
 net/sched/cls_bpf.c                        |  28 ++++-
 tools/testing/selftests/net/bpf_offload.py | 139 +++++++++++++++++++++
 3 files changed, 175 insertions(+), 4 deletions(-)

-- 
2.43.0

[PATCH net v2 1/3] netdevsim: reject cls_bpf programs bound to another device

[Jamal Hadi Salim]

The issue:
nsim_bpf_setup_tc_block_cb() installs any device-bound SCHED_CLS program
without checking that its offload device matches the target netdev.  The
core cls_bpf attach path rejects a mismatching program, but a netdev that
joins a shared block later is replayed the stored filter by
cls_bpf_reoffload() before it is inserted into block->ports, so the
attach-time check cannot see it.  The driver callback is the
authoritative per-device acceptance boundary and must enforce the match
itself, as nfp already does in nfp_net_bpf_offload().

Without the check, replaying a device-bound program onto a different
bpf_offload_dev installs it into that device's netdevsim state, which
trips the WARN in nsim_bpf_uninit() (ns->bpf_offloaded set on a foreign
device) and can dereference a NULL prog->aux->offload in
nsim_prog_set_loaded().

The Fix:
Reject a device-bound program whose offload device does not match the
target netdev, with -EINVAL and an extack so the rejected replay is not
silent.  The check is intentionally narrower than nfp's: it fires only
for a dev-bound program, so nsim's bpf_tc_non_bound_accept mode (which
admits an unbound program onto the device) is unaffected.  Teardown is
never blocked: the guard is on the program being installed and an unbind
passes cls_bpf.prog = NULL.

Conditions to recreate the bug:
  - CONFIG_NETDEVSIM=y, CONFIG_NET_CLS_BPF=y
  - load and pin a dev-bound SCHED_CLS program for netdevsim device B
  - install the program on a shared block owned by B
  - add an independently backed netdevsim device C to that block; the
    bind replays the filter to C's callback, and C's
    ports/0/bpf_offloaded_id takes the B-bound program id.

The reported Sashiko review is at
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260809094418.901607-1-jhs@mojatatu.com

Fixes: 31d3ad832948 ("netdevsim: add bpf offload support")
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/netdev/20260809094418.901607-1-jhs@mojatatu.com/
Link: https://lore.kernel.org/netdev/QDISC-PYC7.v1.20261003103747@mojatatu.com/
Cc: stable@vger.kernel.org
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 -> v2 (v2 of an on-list patch; v1 posted 2026-10-04):

- Finding (nipa Sashiko High, v1) -- FIXED: the
  every-member predicate is enforced only at add/replace, but a netdev
  joining a shared block later is replayed the stored filter by
  cls_bpf_reoffload() before it is inserted into block->ports
  (tcf_block_offload_bind precedes the xa_insert).  Neither
  cls_bpf_reoffload() nor nsim_bpf_setup_tc_block_cb() checked
  bpf_offload_dev_match(), so the B-bound program was installed on the
  joining device (re-opening the 120977e2c096 mixed-block defect).
  Fixed by enforcing the device match in nsim_bpf_setup_tc_block_cb()
  (drivers/net/netdevsim/bpf.c), mirroring nfp.
- Style (gemini + nipa Low) -- FIXED: the shared-block comment opens with
  /* on its own line.
- The companion selftest exercises shared-block and late-join coverage,
  pinning both the all-members predicate and this callback's replay gate.
- Split the single v2 draft into [PATCH net v2 1/3] netdevsim,
  2/3 cls_bpf and 3/3 selftests, each with its own Fixes: and stable
  metadata.
---
 drivers/net/netdevsim/bpf.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 16aa88278398..a73ea8b07682 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -144,6 +144,18 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
 		return -EOPNOTSUPP;
 	}
 
+	/*
+	 * A dev-bound program can be replayed here when a netdev joins a
+	 * shared block, after the core attach-time check has run; enforce
+	 * the device match as nfp does.
+	 */
+	if (prog && bpf_prog_is_dev_bound(prog->aux) &&
+	    !bpf_offload_dev_match(prog, ns->netdev)) {
+		NSIM_EA(cls_bpf->common.extack,
+			"program bound to a different device");
+		return -EINVAL;
+	}
+
 	if (cls_bpf->command != TC_CLSBPF_OFFLOAD)
 		return -EOPNOTSUPP;
 
-- 
2.43.0

Re: [PATCH net v2 1/3] netdevsim: reject cls_bpf programs bound to another device

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] On the tree this patch applies to, the bug described in the
  commit message cannot happen.

Pre-existing issues:
- [Medium] Offload accounting imbalance when a shared-block late-join
  replay is rolled back.

--

netdevsim: reject cls_bpf programs bound to another device

This adds a check to nsim_bpf_setup_tc_block_cb(). The check rejects a
device-bound cls_bpf program whose offload device does not match the
target netdev, and returns -EINVAL with an extack message. The commit
message says this closes a shared-block late-join replay path through
cls_bpf_reoffload().

Link: https://lore.kernel.org/netdev/20260809094418.901607-1-jhs@mojatatu.com/
Link: https://lore.kernel.org/netdev/QDISC-PYC7.v1.20261003103747@mojatatu.com/

> diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
> index 16aa882783984..e6799947db834 100644
> --- a/drivers/net/netdevsim/bpf.c
> +++ b/drivers/net/netdevsim/bpf.c
> @@ -144,6 +144,18 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	/*
> +	 * A dev-bound program can be replayed here when a netdev joins a
> +	 * shared block, after the core attach-time check has run; enforce
> +	 * the device match as nfp does.
> +	 */

[Severity: Low]

Can this replay path be reached on the tree this patch applies to? The
commit message says:

  The core cls_bpf attach path rejects a mismatching program, but a
  netdev that joins a shared block later is replayed the stored filter by
  cls_bpf_reoffload() before it is inserted into block->ports, so the
  attach-time check cannot see it.

In net/sched/cls_bpf.c, cls_bpf_prog_from_efd() does this attach check:

	dev = block->q ? qdisc_dev(block->q) : NULL;
	if (!dev || !bpf_offload_dev_match(fp, dev)) {
		NL_SET_ERR_MSG(extack,
			       "Program is bound to a different device");

In net/sched/cls_api.c, tcf_block_create() never sets block->q for a
shared block:

	/* Don't store q pointer for blocks which are shared */
	if (!tcf_block_shared(block))
		block->q = q;

So it looks like every dev-bound program on a shared block is rejected
at attach time. Wouldn't step 3 of the reproducer ("install the program
on a shared block owned by B") fail with "Program is bound to a
different device"?

If so, cls_bpf_reoffload() never holds a dev-bound filter on a shared
block that it could replay to a joining device. A non-shared block has
only one netdev, and that netdev already passed the attach check.

The late-join path seems to open only with the companion patch
"net/sched: cls_bpf: accept dev-bound programs on shared blocks". That
patch replaces the block->q check with the every-member predicate
mentioned in the v1 -> v2 notes. This comment only becomes accurate once
that patch is applied.

Could the commit message and this comment say that the cls_bpf patch
opens the replay path? Or could they describe the path that the Fixes:
and Cc: stable tags actually cover on older trees, which is direct
attach before 120977e2c096? The code change itself looks fine.

> +	if (prog && bpf_prog_is_dev_bound(prog->aux) &&
> +	    !bpf_offload_dev_match(prog, ns->netdev)) {
> +		NSIM_EA(cls_bpf->common.extack,
> +			"program bound to a different device");
> +		return -EINVAL;
> +	}

[Severity: Medium]

This is a pre-existing issue that this patch did not introduce, but the
new -EINVAL return adds another way to reach it. The companion cls_bpf
patch lets a dev-bound program sit on a shared block. After that, what
happens to the offload counters when this rejection triggers a rollback?

Say device B holds a skip_sw cls_bpf filter with a B-bound program on a
shared block. So in_hw_count == 1, TCA_CLS_FLAGS_IN_HW is set, and
block->offloadcnt == 1. Then netdevsim device C, which has a different
bpf_offload_dev, binds the block:

tcf_block_bind()
  tcf_block_playback_offloads(add=true)
    cls_bpf_reoffload()
      tc_setup_cb_reoffload()
        nsim_bpf_setup_tc_block_cb()   <- returns -EINVAL here

The filter is skip_sw, so the error propagates and
tcf_block_playback_offloads() jumps to err_playback_remove. That replays
add=false over every filter, including the one whose add just failed.

With prog == NULL the new guard is skipped. ns->bpf_offloaded is NULL,
so it is != oldprog, and C's callback takes this path:

	if (ns->bpf_offloaded != oldprog) {
		oldprog = NULL;
		if (!cls_bpf->prog)
			return 0;

tc_setup_cb_reoffload() treats that 0 as a successful removal and calls
tc_cls_offload_cnt_update(..., 1, false):

	*cnt -= diff;
	if (!*cnt)
		tcf_block_offload_dec(block, flags);

Wouldn't in_hw_count then drop from 1 to 0, clearing IN_HW and
decrementing block->offloadcnt, while B still has the program offloaded?
Each later failed join would decrement the u32 in_hw_count again and
wrap it.

Once block->offloadcnt reads 0, tcf_block_offload_in_use() returns
false. Could a device without ndo_setup_tc then bind a block that still
carries a skip_sw filter?

The same imbalance looks reachable on the baseline through netdevsim's
existing rejections, which only apply to adds. One case is an unbound
skip_sw program that B accepts (bpf_tc_non_bound_accept=1) but a joining
C rejects with the default bpf_tc_non_bound_accept=0. Another is the
-EBUSY "driver and netdev offload states mismatch" path.

The root cause seems to have two parts. The core rolls back without
tracking which callbacks accepted the add. Also, netdevsim returns 0 when
asked to remove a program it does not hold.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com

Re: [PATCH net v2 1/3] netdevsim: reject cls_bpf programs bound to another device

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 5:03 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
>
> Low priority / nit picks for your consideration:
>
> - [Low] On the tree this patch applies to, the bug described in the
>   commit message cannot happen.
>
> Pre-existing issues:
> - [Medium] Offload accounting imbalance when a shared-block late-join
>   replay is rolled back.
>

Unless something else which is substantial shows up I will not be
sending an update to this patch.


cheers,
jamal

> --
>
> netdevsim: reject cls_bpf programs bound to another device
>
> This adds a check to nsim_bpf_setup_tc_block_cb(). The check rejects a
> device-bound cls_bpf program whose offload device does not match the
> target netdev, and returns -EINVAL with an extack message. The commit
> message says this closes a shared-block late-join replay path through
> cls_bpf_reoffload().
>
> Link: https://lore.kernel.org/netdev/20260809094418.901607-1-jhs@mojatatu.com/
> Link: https://lore.kernel.org/netdev/QDISC-PYC7.v1.20261003103747@mojatatu.com/
>
> > diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
> > index 16aa882783984..e6799947db834 100644
> > --- a/drivers/net/netdevsim/bpf.c
> > +++ b/drivers/net/netdevsim/bpf.c
> > @@ -144,6 +144,18 @@ int nsim_bpf_setup_tc_block_cb(enum tc_setup_type type,
> >               return -EOPNOTSUPP;
> >       }
> >
> > +     /*
> > +      * A dev-bound program can be replayed here when a netdev joins a
> > +      * shared block, after the core attach-time check has run; enforce
> > +      * the device match as nfp does.
> > +      */
>
> [Severity: Low]
>
> Can this replay path be reached on the tree this patch applies to? The
> commit message says:
>
>   The core cls_bpf attach path rejects a mismatching program, but a
>   netdev that joins a shared block later is replayed the stored filter by
>   cls_bpf_reoffload() before it is inserted into block->ports, so the
>   attach-time check cannot see it.
>
> In net/sched/cls_bpf.c, cls_bpf_prog_from_efd() does this attach check:
>
>         dev = block->q ? qdisc_dev(block->q) : NULL;
>         if (!dev || !bpf_offload_dev_match(fp, dev)) {
>                 NL_SET_ERR_MSG(extack,
>                                "Program is bound to a different device");
>
> In net/sched/cls_api.c, tcf_block_create() never sets block->q for a
> shared block:
>
>         /* Don't store q pointer for blocks which are shared */
>         if (!tcf_block_shared(block))
>                 block->q = q;
>
> So it looks like every dev-bound program on a shared block is rejected
> at attach time. Wouldn't step 3 of the reproducer ("install the program
> on a shared block owned by B") fail with "Program is bound to a
> different device"?
>
> If so, cls_bpf_reoffload() never holds a dev-bound filter on a shared
> block that it could replay to a joining device. A non-shared block has
> only one netdev, and that netdev already passed the attach check.
>
> The late-join path seems to open only with the companion patch
> "net/sched: cls_bpf: accept dev-bound programs on shared blocks". That
> patch replaces the block->q check with the every-member predicate
> mentioned in the v1 -> v2 notes. This comment only becomes accurate once
> that patch is applied.
>
> Could the commit message and this comment say that the cls_bpf patch
> opens the replay path? Or could they describe the path that the Fixes:
> and Cc: stable tags actually cover on older trees, which is direct
> attach before 120977e2c096? The code change itself looks fine.
>
> > +     if (prog && bpf_prog_is_dev_bound(prog->aux) &&
> > +         !bpf_offload_dev_match(prog, ns->netdev)) {
> > +             NSIM_EA(cls_bpf->common.extack,
> > +                     "program bound to a different device");
> > +             return -EINVAL;
> > +     }
>
> [Severity: Medium]
>
> This is a pre-existing issue that this patch did not introduce, but the
> new -EINVAL return adds another way to reach it. The companion cls_bpf
> patch lets a dev-bound program sit on a shared block. After that, what
> happens to the offload counters when this rejection triggers a rollback?
>
> Say device B holds a skip_sw cls_bpf filter with a B-bound program on a
> shared block. So in_hw_count == 1, TCA_CLS_FLAGS_IN_HW is set, and
> block->offloadcnt == 1. Then netdevsim device C, which has a different
> bpf_offload_dev, binds the block:
>
> tcf_block_bind()
>   tcf_block_playback_offloads(add=true)
>     cls_bpf_reoffload()
>       tc_setup_cb_reoffload()
>         nsim_bpf_setup_tc_block_cb()   <- returns -EINVAL here
>
> The filter is skip_sw, so the error propagates and
> tcf_block_playback_offloads() jumps to err_playback_remove. That replays
> add=false over every filter, including the one whose add just failed.
>
> With prog == NULL the new guard is skipped. ns->bpf_offloaded is NULL,
> so it is != oldprog, and C's callback takes this path:
>
>         if (ns->bpf_offloaded != oldprog) {
>                 oldprog = NULL;
>                 if (!cls_bpf->prog)
>                         return 0;
>
> tc_setup_cb_reoffload() treats that 0 as a successful removal and calls
> tc_cls_offload_cnt_update(..., 1, false):
>
>         *cnt -= diff;
>         if (!*cnt)
>                 tcf_block_offload_dec(block, flags);
>
> Wouldn't in_hw_count then drop from 1 to 0, clearing IN_HW and
> decrementing block->offloadcnt, while B still has the program offloaded?
> Each later failed join would decrement the u32 in_hw_count again and
> wrap it.
>
> Once block->offloadcnt reads 0, tcf_block_offload_in_use() returns
> false. Could a device without ndo_setup_tc then bind a block that still
> carries a skip_sw filter?
>
> The same imbalance looks reachable on the baseline through netdevsim's
> existing rejections, which only apply to adds. One case is an unbound
> skip_sw program that B accepts (bpf_tc_non_bound_accept=1) but a joining
> C rejects with the default bpf_tc_non_bound_accept=0. Another is the
> -EBUSY "driver and netdev offload states mismatch" path.
>
> The root cause seems to have two parts. The core rolls back without
> tracking which callbacks accepted the add. Also, netdevsim returns 0 when
> asked to remove a program it does not hold.
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com
>

[PATCH net v2 2/3] net/sched: cls_bpf: accept dev-bound programs on shared blocks

[Jamal Hadi Salim]

The issue:
Follow-up to commit 120977e2c096 ("net/sched: cls_bpf: reject dev-bound
programs bound to a different device"), which derived the target device
from block->q:

	dev = block->q ? qdisc_dev(block->q) : NULL;

tcf_block_create() leaves block->q NULL for shared blocks, so a
device-bound program attached through a shared block is rejected with
-EINVAL even when it is bound to a netdev that owns the block.  The
offload path (cls_bpf_offload_cmd()) never consults block->q and drives
the per-device callbacks for shared blocks as before.

The Fix:
Accept the attach on a shared block when bpf_offload_dev_match() matches
every netdev bound to the block, and reject an empty block.  Every member
must match because the offload path broadcasts the program to every
callback registered on the block, so an any-member check would re-admit
the wrong-device attach on a mixed block.  Every dev-bound SCHED_CLS
attach is skip_sw, which becomes err_stop and aborts the offload on the
first failing callback, so the all-members rule is the offload path's own
semantics.

Iterate the public block->ports xarray (913b47d3424e, v6.8-rc1) rather
than block->owner_list, which is private to cls_api.c.  The shared-block
rejection reports "Program is not bound to every device in the block",
distinct from the non-shared message, because a member can exist without
a registered callback (netdevsim registers its block callback
ingress-only).

Context note:
This patch is not an independent fix: it re-exposes the shared-block
replay path that commit 120977e2c096 ("net/sched: cls_bpf: reject
dev-bound programs bound to a different device") had closed, so patch 1
must be applied first, including in stable.  A stable tree taking this
patch alone would re-enable shared-block attaches on a kernel whose
netdevsim callback is still unchecked.

Conditions to recreate the bug:
  - CONFIG_NETDEVSIM=y, CONFIG_NET_CLS_BPF=y
  - load a dev-bound SCHED_CLS program for netdevsim device B
    (prog_ifindex=B), pin it in bpffs
  - tc qdisc add dev B ingress_block 22 clsact
  - tc filter add block 22 ingress bpf da object-pinned <pin> skip_sw
  Unfixed, the filter add fails with -EINVAL ("Program is bound to a
  different device"); fixed, it succeeds.  Adding a second, independently
  backed netdevsim device to the same block makes the attach fail again.

Fixes: 120977e2c096 ("net/sched: cls_bpf: reject dev-bound programs bound to a different device")
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/netdev/20260809094418.901607-1-jhs@mojatatu.com/
Link: https://lore.kernel.org/netdev/QDISC-PYC7.v1.20261003103747@mojatatu.com/
Cc: stable@vger.kernel.org
Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 -> v2 (v2 of an on-list patch; v1 posted 2026-10-04):

- Finding (nipa Sashiko High, v1) -- FIXED: the
  every-member predicate is enforced only at add/replace, but a netdev
  joining a shared block later is replayed the stored filter by
  cls_bpf_reoffload() before it is inserted into block->ports
  (tcf_block_offload_bind precedes the xa_insert).  Neither
  cls_bpf_reoffload() nor nsim_bpf_setup_tc_block_cb() checked
  bpf_offload_dev_match(), so the B-bound program was installed on the
  joining device (re-opening the 120977e2c096 mixed-block defect).
  Fixed by enforcing the device match in nsim_bpf_setup_tc_block_cb()
  (drivers/net/netdevsim/bpf.c), mirroring nfp.
- Style (gemini + nipa Low) -- FIXED: the shared-block comment opens with
  /* on its own line.
- The companion selftest exercises shared-block and late-join coverage,
  pinning both the all-members predicate and this callback's replay gate.
- Split the single v2 draft into [PATCH net v2 1/3] netdevsim,
  2/3 cls_bpf and 3/3 selftests, each with its own Fixes: and stable
  metadata.
---
 net/sched/cls_bpf.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 188cf0f949dd..bbd2dc2f518f 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -392,11 +392,31 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
 	if (bpf_prog_is_dev_bound(fp->aux)) {
 		struct tcf_block *block = tp->chain->block;
 		struct net_device *dev;
+		unsigned long ifindex;
+		bool match = false;
+
+		/* A shared block has no qdisc of its own, but it may bind
+		 * several netdevs and the program is offloaded to all of them;
+		 * require a match with every member.
+		 */
+		if (!tcf_block_shared(block)) {
+			dev = qdisc_dev(tcf_block_q(block));
+			match = bpf_offload_dev_match(fp, dev);
+			if (!match)
+				NL_SET_ERR_MSG(extack,
+					       "Program is bound to a different device");
+		} else {
+			xa_for_each(&block->ports, ifindex, dev) {
+				match = bpf_offload_dev_match(fp, dev);
+				if (!match)
+					break;
+			}
+			if (!match)
+				NL_SET_ERR_MSG(extack,
+					       "Program is not bound to every device in the block");
+		}
 
-		dev = block->q ? qdisc_dev(block->q) : NULL;
-		if (!dev || !bpf_offload_dev_match(fp, dev)) {
-			NL_SET_ERR_MSG(extack,
-				       "Program is bound to a different device");
+		if (!match) {
 			bpf_prog_put(fp);
 			return -EINVAL;
 		}
-- 
2.43.0

Re: [PATCH net v2 2/3] net/sched: cls_bpf: accept dev-bound programs on shared blocks

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The v1->v2 notes say: "Style (gemini + nipa Low) -- FIXED: the
  shared-block comment opens with /* on its own line." The diff does not…
- [Low] The commit message ("matches every netdev bound to the block"),
  the code comment ("require a match with every member") and the extack…
- [Low] The stable metadata does not express the patch's known
  dependencies.

--

net/sched: cls_bpf: accept dev-bound programs on shared blocks

This patch follows up commit 120977e2c096, which took the target device
from block->q and so rejected every dev-bound cls_bpf program attached
through a shared block. On shared blocks, cls_bpf_prog_from_efd() now
walks block->ports and accepts the attach only when
bpf_offload_dev_match() succeeds for every entry. It rejects an empty
block.

Link: https://lore.kernel.org/netdev/20260809094418.901607-1-jhs@mojatatu.com/
Link: https://lore.kernel.org/netdev/QDISC-PYC7.v1.20261003103747@mojatatu.com/

> Fixes: 120977e2c096 ("net/sched: cls_bpf: reject dev-bound programs bound to a different device")
> Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
> [ ... ]
> Cc: stable@vger.kernel.org

[Severity: Low]
Do these tags give stable tooling enough to backport this safely?

Commit 120977e2c096 carries "Fixes: 2b3486bc2d23" ("bpf: Introduce
device-bound XDP programs", v6.3). Stable trees from v6.3 onward, such
as 6.6.y, can therefore pick it up and inherit the shared-block
rejection.

This follow-up iterates block->ports in cls_bpf_prog_from_efd(). That
field was added by 913b47d3424e ("net/sched: Introduce tc block netdev
tracking infra", v6.8-rc1), so the patch will not build on those older
trees. They would keep rejecting every dev-bound attach on a shared
block.

The ordering against the netdevsim patch ("patch 1 must be applied
first, including in stable") is only stated in prose. Would a version
bound on the Cc: stable line, or a prerequisite annotation, express
both dependencies in a form the tooling can use?

> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> index 188cf0f949dd4..07ad9556889d4 100644
> --- a/net/sched/cls_bpf.c
> +++ b/net/sched/cls_bpf.c
> @@ -392,11 +392,31 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
>  	if (bpf_prog_is_dev_bound(fp->aux)) {
>  		struct tcf_block *block = tp->chain->block;
>  		struct net_device *dev;
> +		unsigned long ifindex;
> +		bool match = false;
> +
> +		/* A shared block has no qdisc of its own, but it may bind
> +		 * several netdevs and the program is offloaded to all of them;
> +		 * require a match with every member.
> +		 */

[Severity: Low]
This isn't a bug, but the v1 -> v2 notes say:

  Style (gemini + nipa Low) -- FIXED: the shared-block comment opens
  with /* on its own line.

This comment in cls_bpf_prog_from_efd() still has text on the opening
line. The only new comment in the series with /* on its own line is in
nsim_bpf_setup_tc_block_cb(), in the netdevsim patch.

Was the style fix lost when v2 was split into three patches?

> +		if (!tcf_block_shared(block)) {
> +			dev = qdisc_dev(tcf_block_q(block));
> +			match = bpf_offload_dev_match(fp, dev);
> +			if (!match)
> +				NL_SET_ERR_MSG(extack,
> +					       "Program is bound to a different device");
> +		} else {
> +			xa_for_each(&block->ports, ifindex, dev) {
> +				match = bpf_offload_dev_match(fp, dev);
> +				if (!match)
> +					break;
> +			}

[Severity: Low]
Does block->ports really hold every netdev bound to the shared block?
The commit message ("matches every netdev bound to the block"), the
comment above and the extack text all assume it does.

In tcf_block_get_ext(), the offload callback is bound for every binder
type, but the device only goes into ports when tcf_block_tracks_dev()
is true:

net/sched/cls_api.c:tcf_block_get_ext() {
    ...
	err = tcf_block_offload_bind(block, q, ei, extack);
	if (err)
		goto err_block_offload_bind;

	if (tcf_block_tracks_dev(block, ei)) {
		err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL);
    ...
}

tcf_block_tracks_dev() only accepts FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS
and FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS.

sch_red binds shared blocks through tcf_qevent_init() using
FLOW_BLOCK_BINDER_TYPE_RED_EARLY_DROP and FLOW_BLOCK_BINDER_TYPE_RED_MARK.
Those netdevs own the block and receive the cls_bpf offload broadcast,
but this loop never visits them.

That leads to two cases:

- If a block's only binders are RED qevents, ports is empty. A dev-bound
  attach then fails with "Program is not bound to every device in the
  block", which is misleading. The old code also returned -EINVAL here,
  so the return value has not changed.

- On a mixed clsact plus qevent block, the check passes without looking
  at the qevent member.

The mixed case is safe in-tree today only because of other checks. nfp
and netdevsim register their callbacks ingress-only, so a qevent bind
raises nooffloaddevcnt, and tc_setup_cb_add() rejects skip_sw:

	/* Make sure all netdevs sharing this block are offload-capable. */
	if (block->nooffloaddevcnt && err_stop) {
		ok_count = -EOPNOTSUPP;
		goto err_unlock;
	}

Both drivers also re-check bpf_offload_dev_match() in their callbacks.

Should the commit message, comment and extack say that this check only
covers clsact-tracked members? The rest of the protection comes from
nooffloaddevcnt and the driver callbacks.

> +			if (!match)
> +				NL_SET_ERR_MSG(extack,
> +					       "Program is not bound to every device in the block");
> +		}

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com

Re: [PATCH net v2 2/3] net/sched: cls_bpf: accept dev-bound programs on shared blocks

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 5:03 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found more severe
> issue(s) in other patches of this series. This mail is only an FYI, in case
> you want to address the nit picks below when you respin.
>
> Low priority / nit picks for your consideration:
>
> - [Low] The v1->v2 notes say: "Style (gemini + nipa Low) -- FIXED: the
>   shared-block comment opens with /* on its own line." The diff does not…
> - [Low] The commit message ("matches every netdev bound to the block"),
>   the code comment ("require a match with every member") and the extack…
> - [Low] The stable metadata does not express the patch's known
>   dependencies.
>

Unless something else which is substantial shows up I will not be
sending an update to this patch.

cheers,
jamal

> --
>
> net/sched: cls_bpf: accept dev-bound programs on shared blocks
>
> This patch follows up commit 120977e2c096, which took the target device
> from block->q and so rejected every dev-bound cls_bpf program attached
> through a shared block. On shared blocks, cls_bpf_prog_from_efd() now
> walks block->ports and accepts the attach only when
> bpf_offload_dev_match() succeeds for every entry. It rejects an empty
> block.
>
> Link: https://lore.kernel.org/netdev/20260809094418.901607-1-jhs@mojatatu.com/
> Link: https://lore.kernel.org/netdev/QDISC-PYC7.v1.20261003103747@mojatatu.com/
>
> > Fixes: 120977e2c096 ("net/sched: cls_bpf: reject dev-bound programs bound to a different device")
> > Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
> > [ ... ]
> > Cc: stable@vger.kernel.org
>
> [Severity: Low]
> Do these tags give stable tooling enough to backport this safely?
>
> Commit 120977e2c096 carries "Fixes: 2b3486bc2d23" ("bpf: Introduce
> device-bound XDP programs", v6.3). Stable trees from v6.3 onward, such
> as 6.6.y, can therefore pick it up and inherit the shared-block
> rejection.
>
> This follow-up iterates block->ports in cls_bpf_prog_from_efd(). That
> field was added by 913b47d3424e ("net/sched: Introduce tc block netdev
> tracking infra", v6.8-rc1), so the patch will not build on those older
> trees. They would keep rejecting every dev-bound attach on a shared
> block.
>
> The ordering against the netdevsim patch ("patch 1 must be applied
> first, including in stable") is only stated in prose. Would a version
> bound on the Cc: stable line, or a prerequisite annotation, express
> both dependencies in a form the tooling can use?
>
> > diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> > index 188cf0f949dd4..07ad9556889d4 100644
> > --- a/net/sched/cls_bpf.c
> > +++ b/net/sched/cls_bpf.c
> > @@ -392,11 +392,31 @@ static int cls_bpf_prog_from_efd(struct nlattr **tb, struct cls_bpf_prog *prog,
> >       if (bpf_prog_is_dev_bound(fp->aux)) {
> >               struct tcf_block *block = tp->chain->block;
> >               struct net_device *dev;
> > +             unsigned long ifindex;
> > +             bool match = false;
> > +
> > +             /* A shared block has no qdisc of its own, but it may bind
> > +              * several netdevs and the program is offloaded to all of them;
> > +              * require a match with every member.
> > +              */
>
> [Severity: Low]
> This isn't a bug, but the v1 -> v2 notes say:
>
>   Style (gemini + nipa Low) -- FIXED: the shared-block comment opens
>   with /* on its own line.
>
> This comment in cls_bpf_prog_from_efd() still has text on the opening
> line. The only new comment in the series with /* on its own line is in
> nsim_bpf_setup_tc_block_cb(), in the netdevsim patch.
>
> Was the style fix lost when v2 was split into three patches?
>
> > +             if (!tcf_block_shared(block)) {
> > +                     dev = qdisc_dev(tcf_block_q(block));
> > +                     match = bpf_offload_dev_match(fp, dev);
> > +                     if (!match)
> > +                             NL_SET_ERR_MSG(extack,
> > +                                            "Program is bound to a different device");
> > +             } else {
> > +                     xa_for_each(&block->ports, ifindex, dev) {
> > +                             match = bpf_offload_dev_match(fp, dev);
> > +                             if (!match)
> > +                                     break;
> > +                     }
>
> [Severity: Low]
> Does block->ports really hold every netdev bound to the shared block?
> The commit message ("matches every netdev bound to the block"), the
> comment above and the extack text all assume it does.
>
> In tcf_block_get_ext(), the offload callback is bound for every binder
> type, but the device only goes into ports when tcf_block_tracks_dev()
> is true:
>
> net/sched/cls_api.c:tcf_block_get_ext() {
>     ...
>         err = tcf_block_offload_bind(block, q, ei, extack);
>         if (err)
>                 goto err_block_offload_bind;
>
>         if (tcf_block_tracks_dev(block, ei)) {
>                 err = xa_insert(&block->ports, dev->ifindex, dev, GFP_KERNEL);
>     ...
> }
>
> tcf_block_tracks_dev() only accepts FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS
> and FLOW_BLOCK_BINDER_TYPE_CLSACT_EGRESS.
>
> sch_red binds shared blocks through tcf_qevent_init() using
> FLOW_BLOCK_BINDER_TYPE_RED_EARLY_DROP and FLOW_BLOCK_BINDER_TYPE_RED_MARK.
> Those netdevs own the block and receive the cls_bpf offload broadcast,
> but this loop never visits them.
>
> That leads to two cases:
>
> - If a block's only binders are RED qevents, ports is empty. A dev-bound
>   attach then fails with "Program is not bound to every device in the
>   block", which is misleading. The old code also returned -EINVAL here,
>   so the return value has not changed.
>
> - On a mixed clsact plus qevent block, the check passes without looking
>   at the qevent member.
>
> The mixed case is safe in-tree today only because of other checks. nfp
> and netdevsim register their callbacks ingress-only, so a qevent bind
> raises nooffloaddevcnt, and tc_setup_cb_add() rejects skip_sw:
>
>         /* Make sure all netdevs sharing this block are offload-capable. */
>         if (block->nooffloaddevcnt && err_stop) {
>                 ok_count = -EOPNOTSUPP;
>                 goto err_unlock;
>         }
>
> Both drivers also re-check bpf_offload_dev_match() in their callbacks.
>
> Should the commit message, comment and extack say that this check only
> covers clsact-tracked members? The rest of the protection comes from
> nooffloaddevcnt and the driver callbacks.
>
> > +                     if (!match)
> > +                             NL_SET_ERR_MSG(extack,
> > +                                            "Program is not bound to every device in the block");
> > +             }
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com
>

[PATCH net v2 3/3] selftests: net: exercise dev-bound cls_bpf on shared blocks

[Jamal Hadi Salim]

The issue:
bpf_offload.py only exercises per-device ingress qdiscs today, so neither
the shared-block accept path nor the late-join replay is tested.

The Fix:
Add a test for a device-bound SCHED_CLS program attached through a shared
block.  It loads a dev-bound SCHED_CLS program for a netdevsim device and
checks:
  - the program attaches to a shared block owned by its device, and the
    filter teardown leaves the device unloaded;
  - a block that also holds an independently backed device rejects the
    attach, because the offload path broadcasts the program to all
    members;
  - an independently backed device that joins a populated block is not
    loaded by the bind/replay path, and the driver's extack is asserted;
  - a second port of the same netdevsim device (same offload device) is
    accepted together with the bound port.

The program is loaded from the existing sample_ret0.bpf.o with an explicit
`type classifier` override, so no new sample object is needed.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
v1 -> v2 (v2 of an on-list patch; v1 posted 2026-10-04):

- New selftest: shared-block + late-join coverage in bpf_offload.py.
- Split the single v2 draft into three patches; this companion carries no
  Fixes: and no Cc: stable (a selftest is not backported; it depends on
  both code patches).
---
 tools/testing/selftests/net/bpf_offload.py | 139 +++++++++++++++++++++
 1 file changed, 139 insertions(+)

diff --git a/tools/testing/selftests/net/bpf_offload.py b/tools/testing/selftests/net/bpf_offload.py
index c856d266c8f3..87e244d8ff05 100755
--- a/tools/testing/selftests/net/bpf_offload.py
+++ b/tools/testing/selftests/net/bpf_offload.py
@@ -452,6 +452,38 @@ class BpfNetdevSim(NetdevSim):
         self.tc_add_ingress()
         self.wait_for_flush(bound=bound, total=total)
 
+    def tc_add_block(self, block, fail=True, include_stderr=False):
+        return tc("qdisc add dev %s ingress_block %d clsact" %
+                  (self['ifname'], block), fail=fail,
+                  include_stderr=include_stderr)
+
+    def tc_del_block(self, block):
+        tc("qdisc del dev %s clsact" % (self['ifname']), fail=False)
+
+    def tc_add_block_filter(self, block, bpf, op="add", prio=None, handle=None,
+                            da=False, verbose=False, skip_sw=False,
+                            skip_hw=False, fail=True, include_stderr=False):
+        spec = ""
+        if prio is not None:
+            spec += " prio %d" % (prio)
+        if handle:
+            spec += " handle %s" % (handle)
+
+        params = ""
+        if da:
+            params += " da"
+        if verbose:
+            params += " verbose"
+        if skip_sw:
+            params += " skip_sw"
+        if skip_hw:
+            params += " skip_hw"
+
+        return tc("filter {op} block {block} {spec} bpf {bpf} {params}"
+                  .format(op=op, block=block, spec=spec, bpf=bpf,
+                          params=params),
+                  fail=fail, include_stderr=include_stderr)
+
     def tc_show_ingress(self, expected=None):
         # No JSON support, oh well...
         flags = ["skip_sw", "skip_hw", "in_hw"]
@@ -629,6 +661,111 @@ def test_spurios_extack(sim, obj, skip_hw, needle):
                             include_stderr=True)
     check_no_extack(res, needle)
 
+def test_tc_shared_block():
+    """
+    Device-bound cls_bpf program attached through a shared block.
+    """
+    # A shared block has no qdisc, so the target device is derived from the
+    # block's bound netdevs and the program must match every member, since
+    # the offload path broadcasts it to every callback on the block.  A
+    # netdev joining a populated block later is replayed the stored filter
+    # before it is inserted into block->ports, so the driver callback must
+    # enforce the device match as well.
+    simdev = BpfNetdevSimDev()
+    sim, = simdev.nsims
+    sim.set_ethtool_tc_offloads(True)
+
+    start_test("Test TC shared-block load dev-bound program...")
+    bpftool_prog_load("sample_ret0.bpf.o", "/sys/fs/bpf/tc_shared",
+                      prog_type="classifier", dev=sim['ifname'])
+
+    start_test("Test TC shared-block dev-bound attach...")
+    # The pinned program is dev-bound to this device; attaching it through
+    # a shared block must succeed even though block->q is NULL.
+    sim.tc_add_block(22)
+    # err is kept (include_stderr) for post-mortem value on a failure.
+    ret, _, err = sim.tc_add_block_filter(22, bpf_pinned("/sys/fs/bpf/tc_shared"),
+                                          da=True, skip_sw=True,
+                                          fail=False, include_stderr=True)
+    fail(ret != 0, "dev-bound program rejected on a matching shared block")
+    fail(sim.dfs_read("bpf_offloaded_id") == "0",
+         "dev-bound program not offloaded on a matching shared block")
+    sim.tc_del_block(22)
+
+    start_test("Test TC shared-block broadcast to a foreign member...")
+    # Add an independently backed device to the SAME block before the
+    # attach.  The program is bound to sim's device only, and the offload
+    # path fans it out to every member, so the attach must be rejected.
+    simdev2 = BpfNetdevSimDev()
+    sim2, = simdev2.nsims
+    sim2.set_ethtool_tc_offloads(True)
+    sim.tc_add_block(40)
+    sim2.tc_add_block(40)
+    ret, _, err = sim.tc_add_block_filter(40, bpf_pinned("/sys/fs/bpf/tc_shared"),
+                                          da=True, skip_sw=True,
+                                          fail=False, include_stderr=True)
+    fail(ret == 0, "dev-bound program broadcast to a foreign shared-block member")
+    check_extack(err, "Program is not bound to every device in the block.", args)
+    sim2.tc_del_block(40)
+    sim.tc_del_block(40)
+    simdev2.remove()
+
+    start_test("Test TC shared-block late join replay...")
+    # Build a block owned by sim's device with the matching program
+    # installed, then let an independently backed device join it.  The
+    # join replays the stored filter to the joining device's callback
+    # before the device is recorded in block->ports; the driver must
+    # reject the replay so the foreign device is never loaded.
+    # sim2 is created here (not inherited) so the TC-offload state it
+    # depends on is explicit.
+    simdev2 = BpfNetdevSimDev()
+    sim2, = simdev2.nsims
+    sim2.set_ethtool_tc_offloads(True)
+    sim.tc_add_block(30)
+    ret, _, err = sim.tc_add_block_filter(30, bpf_pinned("/sys/fs/bpf/tc_shared"),
+                                          da=True, skip_sw=True,
+                                          fail=False, include_stderr=True)
+    fail(ret != 0, "dev-bound program rejected on a matching shared block")
+    ret, _, err = sim2.tc_add_block(30, fail=False, include_stderr=True)
+    fail(ret == 0, "foreign device joined a populated dev-bound shared block")
+    check_extack_nsim(err, "program bound to a different device.", args)
+    fail(sim2.dfs_read("bpf_offloaded_id") != "0",
+         "replayed program loaded onto a foreign device")
+    # The add above was expected to fail, so the qdisc may not exist; the
+    # delete is harmless and only cleans up when it does.
+    sim2.tc_del_block(30)
+    sim.tc_del_block(30)
+    simdev2.remove()
+
+    start_test("Test TC shared-block same-offload-device member...")
+    # A second port of the same netdevsim device shares the offload device,
+    # so a program bound to one port may legitimately be offloaded to both.
+    simdev3 = BpfNetdevSimDev(2)
+    sim3, sim3b = simdev3.nsims
+    sim3.set_ethtool_tc_offloads(True)
+    sim3b.set_ethtool_tc_offloads(True)
+    bpftool_prog_load("sample_ret0.bpf.o", "/sys/fs/bpf/tc_shared_multi",
+                      prog_type="classifier", dev=sim3['ifname'])
+    sim3.tc_add_block(50)
+    sim3b.tc_add_block(50)
+    # err kept for post-mortem value on a failure.
+    ret, _, err = sim3.tc_add_block_filter(50, bpf_pinned("/sys/fs/bpf/tc_shared_multi"),
+                                           da=True, skip_sw=True,
+                                           fail=False, include_stderr=True)
+    fail(ret != 0, "same-offload-device member rejected on a shared block")
+    fail(sim3.dfs_read("bpf_offloaded_id") == "0",
+         "program not offloaded to the bound port")
+    fail(sim3b.dfs_read("bpf_offloaded_id") == "0",
+         "program not offloaded to the same-offload-device port")
+    sim3b.tc_del_block(50)
+    sim3.tc_del_block(50)
+
+    simdev3.remove()
+    simdev.remove()
+    rm("/sys/fs/bpf/tc_shared_multi")
+    rm("/sys/fs/bpf/tc_shared")
+    bpftool_prog_list_wait(expected=0)
+
 def test_multi_prog(simdev, sim, obj, modename, modeid):
     start_test("Test multi-attachment XDP - %s + offload..." %
                (modename or "default", ))
@@ -1349,6 +1486,8 @@ try:
     ret, out = bpftool("prog show %s" % (progB), fail=False)
     fail(ret != 0, "couldn't get information about orphaned program")
 
+    test_tc_shared_block()
+
     print("%s: OK" % (os.path.basename(__file__)))
 
 finally:
-- 
2.43.0

Re: [PATCH net v2 3/3] selftests: net: exercise dev-bound cls_bpf on shared blocks

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 (2)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says the first subtest checks that "the
  program attaches to a shared block owned by its device, and the filter…

Pre-existing issues:
- [Medium] This problem wasn't introduced by this patch, but the late-
  join subtest (block 30) runs a core offload-accounting imbalance and…
- [Low] This problem wasn't introduced by this patch, but the same-
  offload-device subtest hits a netdevsim state-tracking defect.
- [Low] This problem wasn't introduced by this patch, but if one of the
  new subtests fails, a netdevsim device can be left behind.

--

selftests: net: exercise dev-bound cls_bpf on shared blocks

This adds test_tc_shared_block() to bpf_offload.py. The test attaches a
device-bound SCHED_CLS program to netdevsim through shared tc blocks. It
covers four cases: attach on a block owned by the bound device, rejection
when a foreign device is a member, rejection of a foreign device that
joins late during replay, and acceptance on a second port of the same
netdevsim device.

> diff --git a/tools/testing/selftests/net/bpf_offload.py b/tools/testing/selftests/net/bpf_offload.py
> index c856d266c8f36..8c1da9b51be5a 100755
> --- a/tools/testing/selftests/net/bpf_offload.py
> +++ b/tools/testing/selftests/net/bpf_offload.py

[ ... ]

> @@ -629,6 +661,111 @@ def test_spurios_extack(sim, obj, skip_hw, needle):
>                              include_stderr=True)
>      check_no_extack(res, needle)
>  
> +def test_tc_shared_block():

[ ... ]

> +    start_test("Test TC shared-block dev-bound attach...")
> +    # The pinned program is dev-bound to this device; attaching it through
> +    # a shared block must succeed even though block->q is NULL.
> +    sim.tc_add_block(22)
> +    # err is kept (include_stderr) for post-mortem value on a failure.
> +    ret, _, err = sim.tc_add_block_filter(22, bpf_pinned("/sys/fs/bpf/tc_shared"),
> +                                          da=True, skip_sw=True,
> +                                          fail=False, include_stderr=True)
> +    fail(ret != 0, "dev-bound program rejected on a matching shared block")
> +    fail(sim.dfs_read("bpf_offloaded_id") == "0",
> +         "dev-bound program not offloaded on a matching shared block")
> +    sim.tc_del_block(22)

[Severity: Medium]
The commit message says this subtest checks that "the program attaches to
a shared block owned by its device, and the filter teardown leaves the
device unloaded". Is the teardown part checked anywhere?

After sim.tc_del_block(22), nothing reads bpf_offloaded_id again or waits
for the flush. sim.tc_del_block(30) has the same gap. So does the
sim3b.tc_del_block(50) / sim3.tc_del_block(50) pair, which covers the
unbind remove replay and the last-member block destroy.

The tc_del_block() helper also ignores failure:

    def tc_del_block(self, block):
        tc("qdisc del dev %s clsact" % (self['ifname']), fail=False)

The only check left is bpftool_prog_list_wait(expected=0) at the end. It
runs after simdev3.remove(), simdev.remove() and the rm of both pins. It
counts programs, not per-port offload state, so it can't tell an unload
caused by filter teardown from one caused by device removal.

Suppose the remove replay through
cls_bpf_reoffload()->nsim_bpf_setup_tc_block_cb()->nsim_bpf_offload(ns,
NULL, true) stopped clearing ns->bpf_offloaded:

For blocks 30 and 50, the only sign would be the
WARN_ON(ns->bpf_offloaded) in nsim_bpf_uninit() in dmesg.

For block 22, the failure would show up later as -EBUSY ("driver and
netdev offload states mismatch") on the block 30 attach. The test would
then report it as "dev-bound program rejected on a matching shared block".

Could a check like this be added after each tc_del_block() for sim, sim3
and sim3b, while the devices still exist?

    fail(sim.dfs_read("bpf_offloaded_id") != "0", ...)

It could also be worth checking that sim3 is still loaded after sim3b
leaves block 50. Otherwise, the teardown claim could be dropped from the
commit message.

> +
> +    start_test("Test TC shared-block broadcast to a foreign member...")
> +    # Add an independently backed device to the SAME block before the
> +    # attach.  The program is bound to sim's device only, and the offload
> +    # path fans it out to every member, so the attach must be rejected.
> +    simdev2 = BpfNetdevSimDev()

[Severity: Low]
This is a pre-existing issue, not one introduced by this patch, but can a
failure in one of these subtests leave a netdevsim device behind?

clean_up() loops over the same list that each removal modifies:

    for dev in devs:
        dev.remove()

and BpfNetdevSimDev.remove() does:

    def remove(self):
        super().remove()
        devs.remove(self)

As a result, every removal skips the next entry in the list.

Here simdev and simdev2 (or simdev3) are both alive. A fail() in the
broadcast, late-join or same-offload-device subtest raises before the
local simdevN.remove(). The outer finally then calls clean_up(), which
removes the first device and skips the second. The second device and its
clsact qdisc are left in place.

Looping over a copy of the list (for dev in list(devs):) would avoid
this.

[ ... ]

> +    sim.tc_add_block(30)
> +    ret, _, err = sim.tc_add_block_filter(30, bpf_pinned("/sys/fs/bpf/tc_shared"),
> +                                          da=True, skip_sw=True,
> +                                          fail=False, include_stderr=True)
> +    fail(ret != 0, "dev-bound program rejected on a matching shared block")
> +    ret, _, err = sim2.tc_add_block(30, fail=False, include_stderr=True)
> +    fail(ret == 0, "foreign device joined a populated dev-bound shared block")
> +    check_extack_nsim(err, "program bound to a different device.", args)
> +    fail(sim2.dfs_read("bpf_offloaded_id") != "0",
> +         "replayed program loaded onto a foreign device")

[Severity: Medium]
This isn't a bug introduced by this patch, but the rejected join here
seems to break the core offload accounting, and these assertions can't
catch it.

When sim2 joins block 30, tcf_block_bind()->tcf_block_playback_offloads()
with add=true replays the filter to sim2's callback. The callback rejects
it with -EINVAL. Because the filter is skip_sw, tc_setup_cb_reoffload()
returns the error and the playback unwinds:

net/sched/cls_api.c:tcf_block_playback_offloads() {
    ...
err_playback_remove:
	tcf_proto_put(tp, true, NULL);
	tcf_chain_put(chain);
	tcf_block_playback_offloads(block, cb, cb_priv, false, offload_in_use,
				    extack);
    ...
}

This sends a remove to the same sim2 callback, including for the tp whose
add just failed. nsim returns success for removing a program it never
held:

drivers/net/netdevsim/bpf.c:nsim_bpf_setup_tc_block_cb() {
    ...
	if (ns->bpf_offloaded != oldprog) {
		oldprog = NULL;
		if (!cls_bpf->prog)
			return 0;
    ...
}

So tc_setup_cb_reoffload() calls tc_cls_offload_cnt_update() with
add=false. The filter's in_hw_count drops from 1 (sim's real offload) to
0. TCA_CLS_FLAGS_IN_HW is cleared and block->offloadcnt drops to 0, while
sim still has the program loaded.

When sim.tc_del_block(30) runs, the unbind decrements in_hw_count again.
The u32 wraps to 0xFFFFFFFF and stays there until block destruction calls
tc_cls_offload_cnt_reset(). Until then, tcf_block_offload_in_use() is
false, so tcf_block_offload_bind() skips its "dev has offload disabled"
check.

Would it be worth also checking sim's offload state and the filter's
in_hw flag after the rejected join?

The same remove-returns-0 behaviour can already be reached through nsim's
-EBUSY add rejection path, and nfp's callback has the same shape.

[ ... ]

> +    fail(sim3b.dfs_read("bpf_offloaded_id") == "0",
> +         "program not offloaded to the same-offload-device port")
> +    sim3b.tc_del_block(50)
> +    sim3.tc_del_block(50)

[Severity: Low]
This is a pre-existing netdevsim issue, not one introduced by this patch,
but with this teardown order the program's debugfs loaded state is wrong
while sim3 is still offloaded.

netdevsim tracks loaded state in a single per-program boolean that every
port shares:

drivers/net/netdevsim/bpf.c:nsim_prog_set_loaded() {
    ...
	state = prog->aux->offload->dev_priv;
	state->is_loaded = loaded;
}

sim3b.tc_del_block(50) replays a remove to sim3b. That runs
nsim_bpf_offload(ns, NULL, true), which calls
nsim_prog_set_loaded(ns->bpf_offloaded, false). This clears is_loaded
even though sim3's ns->bpf_offloaded still points at the program.

Until sim3 leaves the block, bpf_bound_progs/<id>/loaded reads as not
loaded. The WARN(state->is_loaded, ...) in nsim_bpf_destroy_prog() also
won't fire in this case.

The existing XDP multi-dev ASIC tests already follow the same pattern.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com

Re: [PATCH net v2 3/3] selftests: net: exercise dev-bound cls_bpf on shared blocks

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 5:03 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 (2)
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The commit message says the first subtest checks that "the
>   program attaches to a shared block owned by its device, and the filter…
>
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but the late-
>   join subtest (block 30) runs a core offload-accounting imbalance and…
> - [Low] This problem wasn't introduced by this patch, but the same-
>   offload-device subtest hits a netdevsim state-tracking defect.
> - [Low] This problem wasn't introduced by this patch, but if one of the
>   new subtests fails, a netdevsim device can be left behind.
>

Unless something else which is substantial shows up I will not be
sending an update to this patch.

cheers,
jamal

> --
>
> selftests: net: exercise dev-bound cls_bpf on shared blocks
>
> This adds test_tc_shared_block() to bpf_offload.py. The test attaches a
> device-bound SCHED_CLS program to netdevsim through shared tc blocks. It
> covers four cases: attach on a block owned by the bound device, rejection
> when a foreign device is a member, rejection of a foreign device that
> joins late during replay, and acceptance on a second port of the same
> netdevsim device.
>
> > diff --git a/tools/testing/selftests/net/bpf_offload.py b/tools/testing/selftests/net/bpf_offload.py
> > index c856d266c8f36..8c1da9b51be5a 100755
> > --- a/tools/testing/selftests/net/bpf_offload.py
> > +++ b/tools/testing/selftests/net/bpf_offload.py
>
> [ ... ]
>
> > @@ -629,6 +661,111 @@ def test_spurios_extack(sim, obj, skip_hw, needle):
> >                              include_stderr=True)
> >      check_no_extack(res, needle)
> >
> > +def test_tc_shared_block():
>
> [ ... ]
>
> > +    start_test("Test TC shared-block dev-bound attach...")
> > +    # The pinned program is dev-bound to this device; attaching it through
> > +    # a shared block must succeed even though block->q is NULL.
> > +    sim.tc_add_block(22)
> > +    # err is kept (include_stderr) for post-mortem value on a failure.
> > +    ret, _, err = sim.tc_add_block_filter(22, bpf_pinned("/sys/fs/bpf/tc_shared"),
> > +                                          da=True, skip_sw=True,
> > +                                          fail=False, include_stderr=True)
> > +    fail(ret != 0, "dev-bound program rejected on a matching shared block")
> > +    fail(sim.dfs_read("bpf_offloaded_id") == "0",
> > +         "dev-bound program not offloaded on a matching shared block")
> > +    sim.tc_del_block(22)
>
> [Severity: Medium]
> The commit message says this subtest checks that "the program attaches to
> a shared block owned by its device, and the filter teardown leaves the
> device unloaded". Is the teardown part checked anywhere?
>
> After sim.tc_del_block(22), nothing reads bpf_offloaded_id again or waits
> for the flush. sim.tc_del_block(30) has the same gap. So does the
> sim3b.tc_del_block(50) / sim3.tc_del_block(50) pair, which covers the
> unbind remove replay and the last-member block destroy.
>
> The tc_del_block() helper also ignores failure:
>
>     def tc_del_block(self, block):
>         tc("qdisc del dev %s clsact" % (self['ifname']), fail=False)
>
> The only check left is bpftool_prog_list_wait(expected=0) at the end. It
> runs after simdev3.remove(), simdev.remove() and the rm of both pins. It
> counts programs, not per-port offload state, so it can't tell an unload
> caused by filter teardown from one caused by device removal.
>
> Suppose the remove replay through
> cls_bpf_reoffload()->nsim_bpf_setup_tc_block_cb()->nsim_bpf_offload(ns,
> NULL, true) stopped clearing ns->bpf_offloaded:
>
> For blocks 30 and 50, the only sign would be the
> WARN_ON(ns->bpf_offloaded) in nsim_bpf_uninit() in dmesg.
>
> For block 22, the failure would show up later as -EBUSY ("driver and
> netdev offload states mismatch") on the block 30 attach. The test would
> then report it as "dev-bound program rejected on a matching shared block".
>
> Could a check like this be added after each tc_del_block() for sim, sim3
> and sim3b, while the devices still exist?
>
>     fail(sim.dfs_read("bpf_offloaded_id") != "0", ...)
>
> It could also be worth checking that sim3 is still loaded after sim3b
> leaves block 50. Otherwise, the teardown claim could be dropped from the
> commit message.
>
> > +
> > +    start_test("Test TC shared-block broadcast to a foreign member...")
> > +    # Add an independently backed device to the SAME block before the
> > +    # attach.  The program is bound to sim's device only, and the offload
> > +    # path fans it out to every member, so the attach must be rejected.
> > +    simdev2 = BpfNetdevSimDev()
>
> [Severity: Low]
> This is a pre-existing issue, not one introduced by this patch, but can a
> failure in one of these subtests leave a netdevsim device behind?
>
> clean_up() loops over the same list that each removal modifies:
>
>     for dev in devs:
>         dev.remove()
>
> and BpfNetdevSimDev.remove() does:
>
>     def remove(self):
>         super().remove()
>         devs.remove(self)
>
> As a result, every removal skips the next entry in the list.
>
> Here simdev and simdev2 (or simdev3) are both alive. A fail() in the
> broadcast, late-join or same-offload-device subtest raises before the
> local simdevN.remove(). The outer finally then calls clean_up(), which
> removes the first device and skips the second. The second device and its
> clsact qdisc are left in place.
>
> Looping over a copy of the list (for dev in list(devs):) would avoid
> this.
>
> [ ... ]
>
> > +    sim.tc_add_block(30)
> > +    ret, _, err = sim.tc_add_block_filter(30, bpf_pinned("/sys/fs/bpf/tc_shared"),
> > +                                          da=True, skip_sw=True,
> > +                                          fail=False, include_stderr=True)
> > +    fail(ret != 0, "dev-bound program rejected on a matching shared block")
> > +    ret, _, err = sim2.tc_add_block(30, fail=False, include_stderr=True)
> > +    fail(ret == 0, "foreign device joined a populated dev-bound shared block")
> > +    check_extack_nsim(err, "program bound to a different device.", args)
> > +    fail(sim2.dfs_read("bpf_offloaded_id") != "0",
> > +         "replayed program loaded onto a foreign device")
>
> [Severity: Medium]
> This isn't a bug introduced by this patch, but the rejected join here
> seems to break the core offload accounting, and these assertions can't
> catch it.
>
> When sim2 joins block 30, tcf_block_bind()->tcf_block_playback_offloads()
> with add=true replays the filter to sim2's callback. The callback rejects
> it with -EINVAL. Because the filter is skip_sw, tc_setup_cb_reoffload()
> returns the error and the playback unwinds:
>
> net/sched/cls_api.c:tcf_block_playback_offloads() {
>     ...
> err_playback_remove:
>         tcf_proto_put(tp, true, NULL);
>         tcf_chain_put(chain);
>         tcf_block_playback_offloads(block, cb, cb_priv, false, offload_in_use,
>                                     extack);
>     ...
> }
>
> This sends a remove to the same sim2 callback, including for the tp whose
> add just failed. nsim returns success for removing a program it never
> held:
>
> drivers/net/netdevsim/bpf.c:nsim_bpf_setup_tc_block_cb() {
>     ...
>         if (ns->bpf_offloaded != oldprog) {
>                 oldprog = NULL;
>                 if (!cls_bpf->prog)
>                         return 0;
>     ...
> }
>
> So tc_setup_cb_reoffload() calls tc_cls_offload_cnt_update() with
> add=false. The filter's in_hw_count drops from 1 (sim's real offload) to
> 0. TCA_CLS_FLAGS_IN_HW is cleared and block->offloadcnt drops to 0, while
> sim still has the program loaded.
>
> When sim.tc_del_block(30) runs, the unbind decrements in_hw_count again.
> The u32 wraps to 0xFFFFFFFF and stays there until block destruction calls
> tc_cls_offload_cnt_reset(). Until then, tcf_block_offload_in_use() is
> false, so tcf_block_offload_bind() skips its "dev has offload disabled"
> check.
>
> Would it be worth also checking sim's offload state and the filter's
> in_hw flag after the rejected join?
>
> The same remove-returns-0 behaviour can already be reached through nsim's
> -EBUSY add rejection path, and nfp's callback has the same shape.
>
> [ ... ]
>
> > +    fail(sim3b.dfs_read("bpf_offloaded_id") == "0",
> > +         "program not offloaded to the same-offload-device port")
> > +    sim3b.tc_del_block(50)
> > +    sim3.tc_del_block(50)
>
> [Severity: Low]
> This is a pre-existing netdevsim issue, not one introduced by this patch,
> but with this teardown order the program's debugfs loaded state is wrong
> while sim3 is still offloaded.
>
> netdevsim tracks loaded state in a single per-program boolean that every
> port shares:
>
> drivers/net/netdevsim/bpf.c:nsim_prog_set_loaded() {
>     ...
>         state = prog->aux->offload->dev_priv;
>         state->is_loaded = loaded;
> }
>
> sim3b.tc_del_block(50) replays a remove to sim3b. That runs
> nsim_bpf_offload(ns, NULL, true), which calls
> nsim_prog_set_loaded(ns->bpf_offloaded, false). This clears is_loaded
> even though sim3's ns->bpf_offloaded still points at the program.
>
> Until sim3 leaves the block, bpf_bound_progs/<id>/loaded reads as not
> loaded. The WARN(state->is_loaded, ...) in nsim_bpf_destroy_prog() also
> won't fire in this case.
>
> The existing XDP multi-dev ASIC tests already follow the same pattern.
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com

Re: [PATCH net v2 3/3] selftests: net: exercise dev-bound cls_bpf on shared blocks

[Jamal Hadi Salim]

On Sun, Oct 11, 2026 at 5:03 PM <netdev-bot+sashiko@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider.
>
> Critical: 0 · High: 0 · Medium: 1 (1) · Low: 0 (2)
>
> If you believe none of the issues are legitimate, please reply to each
> issue of Medium severity or higher and explain why the AI is wrong.
> If any of them are legitimate and you plan to respin, please let patchwork
> know by including "pw-bot: cr" as a separate line at the end of your reply
> (one such reply per series is enough).
>
> - [Medium] The commit message says the first subtest checks that "the
>   program attaches to a shared block owned by its device, and the filter…
>
> Pre-existing issues:
> - [Medium] This problem wasn't introduced by this patch, but the late-
>   join subtest (block 30) runs a core offload-accounting imbalance and…
> - [Low] This problem wasn't introduced by this patch, but the same-
>   offload-device subtest hits a netdevsim state-tracking defect.
> - [Low] This problem wasn't introduced by this patch, but if one of the
>   new subtests fails, a netdevsim device can be left behind.
>

Unless something else which is substantial shows up I will not be
sending an update to this patch.

cheers,
jamal
> --
>
> selftests: net: exercise dev-bound cls_bpf on shared blocks
>
> This adds test_tc_shared_block() to bpf_offload.py. The test attaches a
> device-bound SCHED_CLS program to netdevsim through shared tc blocks. It
> covers four cases: attach on a block owned by the bound device, rejection
> when a foreign device is a member, rejection of a foreign device that
> joins late during replay, and acceptance on a second port of the same
> netdevsim device.
>
> > diff --git a/tools/testing/selftests/net/bpf_offload.py b/tools/testing/selftests/net/bpf_offload.py
> > index c856d266c8f36..8c1da9b51be5a 100755
> > --- a/tools/testing/selftests/net/bpf_offload.py
> > +++ b/tools/testing/selftests/net/bpf_offload.py
>
> [ ... ]
>
> > @@ -629,6 +661,111 @@ def test_spurios_extack(sim, obj, skip_hw, needle):
> >                              include_stderr=True)
> >      check_no_extack(res, needle)
> >
> > +def test_tc_shared_block():
>
> [ ... ]
>
> > +    start_test("Test TC shared-block dev-bound attach...")
> > +    # The pinned program is dev-bound to this device; attaching it through
> > +    # a shared block must succeed even though block->q is NULL.
> > +    sim.tc_add_block(22)
> > +    # err is kept (include_stderr) for post-mortem value on a failure.
> > +    ret, _, err = sim.tc_add_block_filter(22, bpf_pinned("/sys/fs/bpf/tc_shared"),
> > +                                          da=True, skip_sw=True,
> > +                                          fail=False, include_stderr=True)
> > +    fail(ret != 0, "dev-bound program rejected on a matching shared block")
> > +    fail(sim.dfs_read("bpf_offloaded_id") == "0",
> > +         "dev-bound program not offloaded on a matching shared block")
> > +    sim.tc_del_block(22)
>
> [Severity: Medium]
> The commit message says this subtest checks that "the program attaches to
> a shared block owned by its device, and the filter teardown leaves the
> device unloaded". Is the teardown part checked anywhere?
>
> After sim.tc_del_block(22), nothing reads bpf_offloaded_id again or waits
> for the flush. sim.tc_del_block(30) has the same gap. So does the
> sim3b.tc_del_block(50) / sim3.tc_del_block(50) pair, which covers the
> unbind remove replay and the last-member block destroy.
>
> The tc_del_block() helper also ignores failure:
>
>     def tc_del_block(self, block):
>         tc("qdisc del dev %s clsact" % (self['ifname']), fail=False)
>
> The only check left is bpftool_prog_list_wait(expected=0) at the end. It
> runs after simdev3.remove(), simdev.remove() and the rm of both pins. It
> counts programs, not per-port offload state, so it can't tell an unload
> caused by filter teardown from one caused by device removal.
>
> Suppose the remove replay through
> cls_bpf_reoffload()->nsim_bpf_setup_tc_block_cb()->nsim_bpf_offload(ns,
> NULL, true) stopped clearing ns->bpf_offloaded:
>
> For blocks 30 and 50, the only sign would be the
> WARN_ON(ns->bpf_offloaded) in nsim_bpf_uninit() in dmesg.
>
> For block 22, the failure would show up later as -EBUSY ("driver and
> netdev offload states mismatch") on the block 30 attach. The test would
> then report it as "dev-bound program rejected on a matching shared block".
>
> Could a check like this be added after each tc_del_block() for sim, sim3
> and sim3b, while the devices still exist?
>
>     fail(sim.dfs_read("bpf_offloaded_id") != "0", ...)
>
> It could also be worth checking that sim3 is still loaded after sim3b
> leaves block 50. Otherwise, the teardown claim could be dropped from the
> commit message.
>
> > +
> > +    start_test("Test TC shared-block broadcast to a foreign member...")
> > +    # Add an independently backed device to the SAME block before the
> > +    # attach.  The program is bound to sim's device only, and the offload
> > +    # path fans it out to every member, so the attach must be rejected.
> > +    simdev2 = BpfNetdevSimDev()
>
> [Severity: Low]
> This is a pre-existing issue, not one introduced by this patch, but can a
> failure in one of these subtests leave a netdevsim device behind?
>
> clean_up() loops over the same list that each removal modifies:
>
>     for dev in devs:
>         dev.remove()
>
> and BpfNetdevSimDev.remove() does:
>
>     def remove(self):
>         super().remove()
>         devs.remove(self)
>
> As a result, every removal skips the next entry in the list.
>
> Here simdev and simdev2 (or simdev3) are both alive. A fail() in the
> broadcast, late-join or same-offload-device subtest raises before the
> local simdevN.remove(). The outer finally then calls clean_up(), which
> removes the first device and skips the second. The second device and its
> clsact qdisc are left in place.
>
> Looping over a copy of the list (for dev in list(devs):) would avoid
> this.
>
> [ ... ]
>
> > +    sim.tc_add_block(30)
> > +    ret, _, err = sim.tc_add_block_filter(30, bpf_pinned("/sys/fs/bpf/tc_shared"),
> > +                                          da=True, skip_sw=True,
> > +                                          fail=False, include_stderr=True)
> > +    fail(ret != 0, "dev-bound program rejected on a matching shared block")
> > +    ret, _, err = sim2.tc_add_block(30, fail=False, include_stderr=True)
> > +    fail(ret == 0, "foreign device joined a populated dev-bound shared block")
> > +    check_extack_nsim(err, "program bound to a different device.", args)
> > +    fail(sim2.dfs_read("bpf_offloaded_id") != "0",
> > +         "replayed program loaded onto a foreign device")
>
> [Severity: Medium]
> This isn't a bug introduced by this patch, but the rejected join here
> seems to break the core offload accounting, and these assertions can't
> catch it.
>
> When sim2 joins block 30, tcf_block_bind()->tcf_block_playback_offloads()
> with add=true replays the filter to sim2's callback. The callback rejects
> it with -EINVAL. Because the filter is skip_sw, tc_setup_cb_reoffload()
> returns the error and the playback unwinds:
>
> net/sched/cls_api.c:tcf_block_playback_offloads() {
>     ...
> err_playback_remove:
>         tcf_proto_put(tp, true, NULL);
>         tcf_chain_put(chain);
>         tcf_block_playback_offloads(block, cb, cb_priv, false, offload_in_use,
>                                     extack);
>     ...
> }
>
> This sends a remove to the same sim2 callback, including for the tp whose
> add just failed. nsim returns success for removing a program it never
> held:
>
> drivers/net/netdevsim/bpf.c:nsim_bpf_setup_tc_block_cb() {
>     ...
>         if (ns->bpf_offloaded != oldprog) {
>                 oldprog = NULL;
>                 if (!cls_bpf->prog)
>                         return 0;
>     ...
> }
>
> So tc_setup_cb_reoffload() calls tc_cls_offload_cnt_update() with
> add=false. The filter's in_hw_count drops from 1 (sim's real offload) to
> 0. TCA_CLS_FLAGS_IN_HW is cleared and block->offloadcnt drops to 0, while
> sim still has the program loaded.
>
> When sim.tc_del_block(30) runs, the unbind decrements in_hw_count again.
> The u32 wraps to 0xFFFFFFFF and stays there until block destruction calls
> tc_cls_offload_cnt_reset(). Until then, tcf_block_offload_in_use() is
> false, so tcf_block_offload_bind() skips its "dev has offload disabled"
> check.
>
> Would it be worth also checking sim's offload state and the filter's
> in_hw flag after the rejected join?
>
> The same remove-returns-0 behaviour can already be reached through nsim's
> -EBUSY add rejection path, and nfp's callback has the same shape.
>
> [ ... ]
>
> > +    fail(sim3b.dfs_read("bpf_offloaded_id") == "0",
> > +         "program not offloaded to the same-offload-device port")
> > +    sim3b.tc_del_block(50)
> > +    sim3.tc_del_block(50)
>
> [Severity: Low]
> This is a pre-existing netdevsim issue, not one introduced by this patch,
> but with this teardown order the program's debugfs loaded state is wrong
> while sim3 is still offloaded.
>
> netdevsim tracks loaded state in a single per-program boolean that every
> port shares:
>
> drivers/net/netdevsim/bpf.c:nsim_prog_set_loaded() {
>     ...
>         state = prog->aux->offload->dev_priv;
>         state->is_loaded = loaded;
> }
>
> sim3b.tc_del_block(50) replays a remove to sim3b. That runs
> nsim_bpf_offload(ns, NULL, true), which calls
> nsim_prog_set_loaded(ns->bpf_offloaded, false). This clears is_loaded
> even though sim3's ns->bpf_offloaded still points at the program.
>
> Until sim3 leaves the block, bpf_bound_progs/<id>/loaded reads as not
> loaded. The WARN(state->is_loaded, ...) in nsim_bpf_destroy_prog() also
> won't fire in this case.
>
> The existing XDP multi-dev ASIC tests already follow the same pattern.
>
> [ ... ]
>
> --
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/QDISC-PYC7.v2.20261010165947%40mojatatu.com