[PATCH net v2 0/4] net/sched: cls_route: fix bucket retention and handle recomputation

[PATCH net v2 0/4] net/sched: cls_route: fix bucket retention and handle recomputation

[Victor Nogueira]

Patch 1 is the v1 patch, unchanged. route4_change() can move an existing
filter to a different top-level bucket, since route4_set_parms()
recomputes the handle from TCA_ROUTE4_TO/FROM/IIF. The filter is
unlinked from the old bucket, but the bucket itself is never freed once
it goes empty, so route4_delete() keeps reporting *last=false after the
last live filter is gone. That pins the empty tcf_proto and leaks it.
The filters linked to a bucket are refcounted now, and the bucket is
dropped from head->table[] as soon as the count reaches zero.

Reviewing v1, Sashiko pointed out that the duplicate scan in
route4_set_parms() compares against the wrong handle [1]. Patches 2 and
3 fix the two symptoms of that.

Patch 2 makes the scan compare against nhandle. f->handle is the handle
the filter has before the update, not the one it is about to be linked
under, so a change that moves a filter into a chain already holding
nhandle misses the collision and links a second filter under the same
handle. The newcomer is then unreachable: route4_get() returns the
incumbent, and route4_classify() stops at the first filter whose f->id
matches.

Patch 3 handles the mirror case. An in-place replace computes an nhandle
that the filter being replaced already carries, so the scan finds that
filter and rejects the request with -EEXIST. The older filter is passed
to route4_set_parms() and skipped in the scan. Skipping it alone would
rename the filter it replaces: the 0x7F00 order bits are carried in no
attribute and were folded into nhandle on the create path alone, so an
order 1 filter came back as order 0, and a sibling sharing its key could
then no longer be replaced at all. They are carried over now whenever
the request builds the key the filter already has, which leaves a
request that does change the key renaming the filter as before.

Patch 4 adds tdc coverage for all three, including the cross-bucket move
case Sashiko noted route.json had no test for.

[1] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260829205422.854785-1-victor%40mojatatu.com

v1 -> v2:
  - Added patches 2, 3 and 4. Patch 1 is unchanged.

Victor Nogueira (4):
  net/sched: cls_route: free emptied bucket on filter move
  net/sched: cls_route: Reject handle aliasing
  net/sched: cls_route: Fix in-place replace
  selftests/tc-testing: Add cls_route bucket move and change tests

 net/sched/cls_route.c                         |  76 ++++---
 .../tc-testing/tc-tests/filters/route.json    | 210 ++++++++++++++++++
 2 files changed, 249 insertions(+), 37 deletions(-)

-- 
2.55.0

[PATCH net v2 1/4] net/sched: cls_route: free emptied bucket on filter move

[Victor Nogueira]

route4_change can move an existing filter to a different top-level
bucket: route4_set_parms recomputes the handle from TCA_ROUTE4_TO/
FROM/IIF, and the handle-mismatch check is gated on the 'new' flag, so
for an existing filter the new handle may differ from the old one and
land in a different bucket. When this happens, the filter is unlinked
from the old bucket, but the bucket itself is never freed once it goes
empty. The stale empty bucket remains in head->table[], causing
route4_delete to report *last=false even after the last live filter is
gone. That pins the empty tcf_proto and causes a leak.

Fix this by refcounting the filters linked to a bucket and freeing the
bucket when the count drops to zero. The existing scan in route4_delete
goes away with it.

The count is updated at all sites that link or unlink a filter during add,
change and delete, and the bucket is dropped from head->table[] as soon as
it reaches zero.

Conditions to recreate the bug:
  CONFIG_NET_CLS_ROUTE4=y, CONFIG_NET_SCH_INGRESS=y, CONFIG_NET_CLS_ACT=y.

  tc qdisc replace dev lo clsact
  tc filter add dev lo ingress protocol ip pref 100 route from 1 to 1
  tc filter change dev lo ingress protocol ip pref 100 handle 0x10001 \
    route from 1 to 2
  tc filter del dev lo ingress protocol ip pref 100 handle 0x10002 \
    route from 1 to 2
  tc filter show dev lo ingress | grep -c 'pref 100 route chain 0 '

Fixes: 1e052be69d04 ("net_sched: destroy proto tp when all filters are gone")
Reported-by: Vega <vega@nebusec.ai>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 net/sched/cls_route.c | 45 +++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 0d1324c90583..17b0ebb76662 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 #include <linux/string.h>
 #include <linux/errno.h>
+#include <linux/refcount.h>
 #include <linux/skbuff.h>
 #include <net/dst.h>
 #include <net/route.h>
@@ -41,6 +42,7 @@ struct route4_head {
 struct route4_bucket {
 	/* 16 FROM buckets + 16 IIF buckets + 1 wildcard bucket */
 	struct route4_filter __rcu	*ht[16 + 16 + 1];
+	refcount_t			filters_ref;
 	struct rcu_head			rcu;
 };
 
@@ -336,7 +338,7 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last,
 	struct route4_filter *nf;
 	struct route4_bucket *b;
 	unsigned int h = 0;
-	int i, h1;
+	int h1;
 
 	if (!head || !f)
 		return -EINVAL;
@@ -362,23 +364,14 @@ static int route4_delete(struct tcf_proto *tp, void *arg, bool *last,
 			tcf_exts_get_net(&f->exts);
 			tcf_queue_work(&f->rwork, route4_delete_filter_work);
 
-			/* Strip RTNL protected tree */
-			for (i = 0; i <= 32; i++) {
-				struct route4_filter *rt;
-
-				rt = rtnl_dereference(b->ht[i]);
-				if (rt)
-					goto out;
+			if (refcount_dec_and_test(&b->filters_ref)) {
+				RCU_INIT_POINTER(head->table[to_hash(h)], NULL);
+				kfree_rcu(b, rcu);
 			}
-
-			/* OK, session has no flows */
-			RCU_INIT_POINTER(head->table[to_hash(h)], NULL);
-			kfree_rcu(b, rcu);
 			break;
 		}
 	}
 
-out:
 	*last = true;
 	for (h1 = 0; h1 <= 256; h1++) {
 		if (rcu_access_pointer(head->table[h1])) {
@@ -459,6 +452,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		if (b == NULL)
 			return -ENOBUFS;
 
+		refcount_set(&b->filters_ref, 1);
 		rcu_assign_pointer(head->table[h1], b);
 	} else {
 		unsigned int h2 = from_hash(nhandle >> 16);
@@ -468,6 +462,8 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		     fp = rtnl_dereference(fp->next))
 			if (fp->handle == f->handle)
 				return -EEXIST;
+
+		refcount_inc(&b->filters_ref);
 	}
 
 	if (tb[TCA_ROUTE4_TO])
@@ -500,7 +496,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 	struct route4_filter *fold, *f1, *pfp, *f = NULL;
 	struct route4_bucket *b;
 	struct nlattr *tb[TCA_ROUTE4_MAX + 1];
-	unsigned int h, th;
+	unsigned int h;
 	int err;
 	bool new = true;
 
@@ -560,17 +556,20 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 	rcu_assign_pointer(*fp, f);
 
 	if (fold) {
-		th = to_hash(fold->handle);
+		b = fold->bkt;
 		h = from_hash(fold->handle >> 16);
-		b = rtnl_dereference(head->table[th]);
-		if (b) {
-			fp = &b->ht[h];
-			for (pfp = rtnl_dereference(*fp); pfp;
-			     fp = &pfp->next, pfp = rtnl_dereference(*fp)) {
-				if (pfp == fold) {
-					rcu_assign_pointer(*fp, fold->next);
-					break;
+		fp = &b->ht[h];
+		for (pfp = rtnl_dereference(*fp); pfp;
+		     fp = &pfp->next, pfp = rtnl_dereference(*fp)) {
+			if (pfp == fold) {
+				rcu_assign_pointer(*fp, fold->next);
+				if (refcount_dec_and_test(&b->filters_ref)) {
+					unsigned int th = to_hash(fold->handle);
+
+					RCU_INIT_POINTER(head->table[th], NULL);
+					kfree_rcu(b, rcu);
 				}
+				break;
 			}
 		}
 	}
-- 
2.55.0

[PATCH net v2 2/4] net/sched: cls_route: Reject handle aliasing

[Victor Nogueira]

route4_set_parms() rejects a duplicate by scanning the destination chain
for f->handle, but f->handle is the handle the filter has before the
update, not the one it is about to be linked under. The comparison and
the insertion therefore use different handles, which causes breakage.

When a change moves the filter to a chain that already holds nhandle,
the scan looks for the old handle instead, misses the collision and
links a second filter with the same handle:

  tc filter add dev lo ingress protocol ip pref 100 \
    route from 1 to 1 classid 1:1 action ok
  tc filter add dev lo ingress protocol ip pref 100 \
    route from 2 to 2 classid 1:2 action drop
  tc filter change dev lo ingress protocol ip pref 100 handle 0x10001 \
    route from 2 to 2 classid 1:1 action ok
  tc filter show dev lo ingress
  ... fh 0x00020002 flowid 1:2 to 2 from 2
  ... fh 0x00020002 flowid 1:1 to 2 from 2

The newcomer is appended after the incumbent, and both end up with the
same f->id. route4_get() returns the first match, so the second filter
can no longer be addressed by handle, and route4_classify() stops at the
first filter whose f->id matches. The second filter is dumped but is
effectively dead.

Fix this by comparing against nhandle.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260829205422.854785-1-victor%40mojatatu.com
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 net/sched/cls_route.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 17b0ebb76662..9710b77d379c 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -460,8 +460,12 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		for (fp = rtnl_dereference(b->ht[h2]);
 		     fp;
 		     fp = rtnl_dereference(fp->next))
-			if (fp->handle == f->handle)
+			if (fp->handle == nhandle) {
+				NL_SET_ERR_MSG_FMT(extack,
+						   "Handle %x is already in use",
+						   nhandle);
 				return -EEXIST;
+			}
 
 		refcount_inc(&b->filters_ref);
 	}
-- 
2.55.0

Re: [PATCH net v2 2/4] net/sched: cls_route: Reject handle aliasing

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] net/sched/cls_route.c:route4_set_parms() — the duplicate-
  handle scan rewritten by this patch (line 463: `if (fp->handle ==…

--

net/sched: cls_route: Reject handle aliasing

route4_set_parms() scanned the destination chain for the filter's old
handle instead of the handle it is about to be linked under, so a change
that moves a filter into a chain already holding nhandle missed the
collision and created two filters with the same handle.  The patch makes
the scan compare against nhandle and adds an extack message.

Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260829205422.854785-1-victor%40mojatatu.com

> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 17b0ebb766626..9710b77d379c4 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -460,8 +460,12 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
>  		for (fp = rtnl_dereference(b->ht[h2]);
>  		     fp;
>  		     fp = rtnl_dereference(fp->next))
> -			if (fp->handle == f->handle)
> +			if (fp->handle == nhandle) {
> +				NL_SET_ERR_MSG_FMT(extack,
> +						   "Handle %x is already in use",
> +						   nhandle);
>  				return -EEXIST;
> +			}

[Severity: Medium]

Should this scan skip the filter that is being replaced?  When
route4_change() is called with an existing filter, fold is still linked in
its bucket chain while route4_set_parms() runs, and f was seeded from it:

net/sched/cls_route.c:route4_change() {
	...
	if (fold) {
		f->id = fold->id;
		f->iif = fold->iif;
		f->handle = fold->handle;

		f->tp = fold->tp;
		f->bkt = fold->bkt;
		new = false;
	}

	err = route4_set_parms(net, tp, base, f, handle, head, tb,
			       tca[TCA_RATE], new, flags, extack);
	...
}

fold is unlinked from b->ht[] only after route4_set_parms() returns
successfully, so the new scan can walk into fold itself.

Two things seem to follow from that.  First, for an in-place change that
keeps the same to/from, nhandle equals fold->handle and the scan matches
fold, returning -EEXIST for the very filter the user asked to modify.  The
old code compared fp->handle == f->handle, which is also fold->handle, so
the self-match itself is pre-existing and not introduced here, but the new
extack now reports "Handle %x is already in use" for the object being
modified, which reads as a foreign collision.

Second, does this newly allow a filter to be silently renumbered?  The
0x7F00 order bits are only folded in for the create path:

net/sched/cls_route.c:route4_set_parms() {
	...
	if (handle && new) {
		nhandle |= handle & 0x7F00;
	...
}

So for something like

  tc filter change ... pref 100 handle 0x10101 route from 1 to 1 classid 1:9

nhandle comes out as 0x10001 while fold->handle is 0x10101.  to_hash() uses
only id & 0xFF plus the 0x8000 bit, and from_hash(nhandle >> 16) is
unchanged, so both handles land in the same b->ht[] chain and fold really is
visible to the scan.  With the old comparison against f->handle the scan
matched fold and the change was refused; with the comparison against
nhandle it no longer matches, the change is accepted, and

	f->handle = nhandle;

renumbers the filter from 0x10101 to 0x10001.  After that, del or replace by
handle 0x10101 no longer addresses the intended filter.  Was that change of
behaviour intended here?

There is no memory-safety consequence either way: the -EEXIST return happens
before refcount_inc(&b->filters_ref) and before the f->handle / f->bkt
assignments.

For what it is worth, both halves appear to be addressed later in the same
series by "net/sched: cls_route: Fix in-place replace" (ba2f28f8), which
turns the test into

	if (fp != fold && fp->handle == nhandle) {

and carries the order bits over with

	if (handle && (!fold || nhandle == (handle & ~0x7F00)))
		nhandle |= handle & 0x7F00;

so the concern is limited to the intermediate tree state at this commit.
Would it make sense to fold the fp != fold exclusion and the 0x7F00
carry-over into this patch so no bisect point renumbers filters?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907192133.2639067-1-victor%40mojatatu.com

Re: [PATCH net v2 2/4] net/sched: cls_route: Reject handle aliasing

[Paolo Abeni]

On 9/9/26 12:22 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 · Low: 0
> 
> - [Medium] net/sched/cls_route.c:route4_set_parms() — the duplicate-
>   handle scan rewritten by this patch (line 463: `if (fp->handle ==…
> 
> --
> 
> net/sched: cls_route: Reject handle aliasing
> 
> route4_set_parms() scanned the destination chain for the filter's old
> handle instead of the handle it is about to be linked under, so a change
> that moves a filter into a chain already holding nhandle missed the
> collision and created two filters with the same handle.  The patch makes
> the scan compare against nhandle and adds an extack message.
> 
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260829205422.854785-1-victor%40mojatatu.com
> 
>> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
>> index 17b0ebb766626..9710b77d379c4 100644
>> --- a/net/sched/cls_route.c
>> +++ b/net/sched/cls_route.c
>> @@ -460,8 +460,12 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
>>  		for (fp = rtnl_dereference(b->ht[h2]);
>>  		     fp;
>>  		     fp = rtnl_dereference(fp->next))
>> -			if (fp->handle == f->handle)
>> +			if (fp->handle == nhandle) {
>> +				NL_SET_ERR_MSG_FMT(extack,
>> +						   "Handle %x is already in use",
>> +						   nhandle);
>>  				return -EEXIST;
>> +			}
> 
> [Severity: Medium]
> 
> Should this scan skip the filter that is being replaced?  When
> route4_change() is called with an existing filter, fold is still linked in
> its bucket chain while route4_set_parms() runs, and f was seeded from it:
> 
> net/sched/cls_route.c:route4_change() {
> 	...
> 	if (fold) {
> 		f->id = fold->id;
> 		f->iif = fold->iif;
> 		f->handle = fold->handle;
> 
> 		f->tp = fold->tp;
> 		f->bkt = fold->bkt;
> 		new = false;
> 	}
> 
> 	err = route4_set_parms(net, tp, base, f, handle, head, tb,
> 			       tca[TCA_RATE], new, flags, extack);
> 	...
> }
> 
> fold is unlinked from b->ht[] only after route4_set_parms() returns
> successfully, so the new scan can walk into fold itself.
> 
> Two things seem to follow from that.  First, for an in-place change that
> keeps the same to/from, nhandle equals fold->handle and the scan matches
> fold, returning -EEXIST for the very filter the user asked to modify.  The
> old code compared fp->handle == f->handle, which is also fold->handle, so
> the self-match itself is pre-existing and not introduced here, but the new
> extack now reports "Handle %x is already in use" for the object being
> modified, which reads as a foreign collision.
> 
> Second, does this newly allow a filter to be silently renumbered?  The
> 0x7F00 order bits are only folded in for the create path:
> 
> net/sched/cls_route.c:route4_set_parms() {
> 	...
> 	if (handle && new) {
> 		nhandle |= handle & 0x7F00;
> 	...
> }
> 
> So for something like
> 
>   tc filter change ... pref 100 handle 0x10101 route from 1 to 1 classid 1:9
> 
> nhandle comes out as 0x10001 while fold->handle is 0x10101.  to_hash() uses
> only id & 0xFF plus the 0x8000 bit, and from_hash(nhandle >> 16) is
> unchanged, so both handles land in the same b->ht[] chain and fold really is
> visible to the scan.  With the old comparison against f->handle the scan
> matched fold and the change was refused; with the comparison against
> nhandle it no longer matches, the change is accepted, and
> 
> 	f->handle = nhandle;
> 
> renumbers the filter from 0x10101 to 0x10001.  After that, del or replace by
> handle 0x10101 no longer addresses the intended filter.  Was that change of
> behaviour intended here?
> 
> There is no memory-safety consequence either way: the -EEXIST return happens
> before refcount_inc(&b->filters_ref) and before the f->handle / f->bkt
> assignments.
> 
> For what it is worth, both halves appear to be addressed later in the same
> series by "net/sched: cls_route: Fix in-place replace" 
FTR: due to the above point, this whole wall of text is pure noise IMHO.
Still there are a few comments referring pre-existing issues that could
deserve some follow-up, please have a look at the full report.

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907192133.2639067-1-victor%40mojatatu.com

/P

Re: [PATCH net v2 2/4] net/sched: cls_route: Reject handle aliasing

[Victor Nogueira]

On Thu, Sep 10, 2026 at 6:27 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On 9/9/26 12:22 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 · Low: 0
> >
> > - [Medium] net/sched/cls_route.c:route4_set_parms() — the duplicate-
> >   handle scan rewritten by this patch (line 463: `if (fp->handle ==…
> > [...]
> > For what it is worth, both halves appear to be addressed later in the same
> > series by "net/sched: cls_route: Fix in-place replace"
> FTR: due to the above point, this whole wall of text is pure noise IMHO.
> Still there are a few comments referring pre-existing issues that could
> deserve some follow-up, please have a look at the full report.
>
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260907192133.2639067-1-victor%40mojatatu.com

Got it, will take a closer look with Jamal.

cheers,
Victor

[PATCH net v2 3/4] net/sched: cls_route: Fix in-place replace

[Victor Nogueira]

Building on the previous patch, route4_set_parms rejects a duplicate by
scanning the destination chain for nhandle, but the scan doesn't exclude
the older version it is replacing, so an in-place replace will match
the older version's handle and fail.

Fix this by passing the older filter as a parameter to route4_set_parms
(replacing "new") and skipping it in the scan.

Excluding the older version is not enough on its own. nhandle is built
out of TCA_ROUTE4_TO, TCA_ROUTE4_FROM and TCA_ROUTE4_IIF alone, while the
0x7F00 bits, which only tell apart filters sharing one key, are folded in
on the create path. Letting the replace through would therefore rename
the filter it replaces: replacing handle 0x10101 stored it back as
0x10001, and a sibling at 0x10201 could then no longer be replaced at
all, since its own nhandle collided with the renamed filter.

	tc filter add ... handle 0x10101 route from 1 to 1 classid 1:1
	tc filter add ... handle 0x10201 route from 1 to 1 classid 1:2
	tc filter replace ... handle 0x10101 route from 1 to 1 classid 1:9
	... fh 0x00010001 flowid 1:9 to 1 from 1
	... fh 0x00010201 flowid 1:2 to 1 from 1
	tc filter replace ... handle 0x10201 route from 1 to 1 classid 1:8
	Error: Handle 10001 is already in use.

So carry those bits over when the key the request builds is the key the
older filter already has. An in-place replace then keeps the handle
userspace named the filter by, while a request that does change the key
still renames it, as it did before.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260829205422.854785-1-victor%40mojatatu.com
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 net/sched/cls_route.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 9710b77d379c..0f211f030fd9 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -393,8 +393,9 @@ static const struct nla_policy route4_policy[TCA_ROUTE4_MAX + 1] = {
 static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 			    unsigned long base, struct route4_filter *f,
 			    u32 handle, struct route4_head *head,
-			    struct nlattr **tb, struct nlattr *est, int new,
-			    u32 flags, struct netlink_ext_ack *extack)
+			    struct nlattr **tb, struct nlattr *est,
+			    struct route4_filter *fold, u32 flags,
+			    struct netlink_ext_ack *extack)
 {
 	u32 id = 0, to = 0, nhandle = 0x8000;
 	struct route4_filter *fp;
@@ -407,7 +408,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		return err;
 
 	if (tb[TCA_ROUTE4_TO]) {
-		if (new && handle & 0x8000) {
+		if (!fold && handle & 0x8000) {
 			NL_SET_ERR_MSG(extack, "Invalid handle");
 			return -EINVAL;
 		}
@@ -430,14 +431,14 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 	} else
 		nhandle |= 0xFFFF << 16;
 
-	if (handle && new) {
+	if (handle && (!fold || nhandle == (handle & ~0x7F00)))
 		nhandle |= handle & 0x7F00;
-		if (nhandle != handle) {
-			NL_SET_ERR_MSG_FMT(extack,
-					   "Handle mismatch constructed: %x (expected: %x)",
-					   handle, nhandle);
-			return -EINVAL;
-		}
+
+	if (handle && !fold && nhandle != handle) {
+		NL_SET_ERR_MSG_FMT(extack,
+				   "Handle mismatch constructed: %x (expected: %x)",
+				   handle, nhandle);
+		return -EINVAL;
 	}
 
 	if (!nhandle) {
@@ -460,7 +461,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
 		for (fp = rtnl_dereference(b->ht[h2]);
 		     fp;
 		     fp = rtnl_dereference(fp->next))
-			if (fp->handle == nhandle) {
+			if (fp != fold && fp->handle == nhandle) {
 				NL_SET_ERR_MSG_FMT(extack,
 						   "Handle %x is already in use",
 						   nhandle);
@@ -502,7 +503,6 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 	struct nlattr *tb[TCA_ROUTE4_MAX + 1];
 	unsigned int h;
 	int err;
-	bool new = true;
 
 	if (!handle) {
 		NL_SET_ERR_MSG(extack, "Creating with handle of 0 is invalid");
@@ -539,11 +539,10 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 
 		f->tp = fold->tp;
 		f->bkt = fold->bkt;
-		new = false;
 	}
 
 	err = route4_set_parms(net, tp, base, f, handle, head, tb,
-			       tca[TCA_RATE], new, flags, extack);
+			       tca[TCA_RATE], fold, flags, extack);
 	if (err < 0)
 		goto errout;
 
-- 
2.55.0

[PATCH net v2 4/4] selftests/tc-testing: Add cls_route bucket move and change tests

[Victor Nogueira]

Add 4 tdc tests for the cls_route bugs fixed earlier in this series:

- Delete a route filter that was moved to another bucket (a7d2):
  Validates that deleting a filter, and making a bucket empty, does not
  leave a dangling empty bucket
- Try to change a route filter onto an already used handle (c05a):
  Validates that attempting to change an existing filter's handle to an
  already taken one fails
- Replace a route filter that shares its key with another filter (3f21):
  Validates that an in-place replace keeps the handle userspace named
  the filter by, rather than dropping the 0x7F00 bits from it
- Replace both route filters sharing a key (9d0e):
  Validates that replacing one of the two does not make the other one
  unreplaceable

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 .../tc-testing/tc-tests/filters/route.json    | 210 ++++++++++++++++++
 1 file changed, 210 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/filters/route.json b/tools/testing/selftests/tc-testing/tc-tests/filters/route.json
index 05cedca67cca..2d5843aebd72 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/filters/route.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/filters/route.json
@@ -202,5 +202,215 @@
         "teardown": [
             "$TC qdisc del dev $DEV1 parent root drr"
         ]
+    },
+    {
+        "id": "a7d2",
+        "name": "Delete a route filter that was moved to another bucket",
+        "category": [
+            "filter",
+            "route"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 route from 1 to 1 classid 1:1",
+            "$TC filter change dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10001 route from 1 to 2 classid 1:1",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 200 route from 5 to 5 classid 1:5"
+        ],
+        "cmdUnderTest": "$TC filter del dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10002 route from 1 to 2",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -j filter show dev $DEV1 parent ffff:",
+        "matchJSON": [
+            {
+                "protocol": "ip",
+                "pref": 200,
+                "kind": "route",
+                "chain": 0
+            },
+            {
+                "protocol": "ip",
+                "pref": 200,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x50005",
+                    "flowid": "1:5"
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    },
+    {
+        "id": "c05a",
+        "name": "Try to change a route filter onto an already used handle",
+        "category": [
+            "filter",
+            "route"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 route from 1 to 1 classid 1:1 action ok",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 route from 2 to 2 classid 1:2 action drop"
+        ],
+        "cmdUnderTest": "$TC filter change dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10001 route from 2 to 2 classid 1:1 action ok",
+        "expExitCode": "2",
+        "verifyCmd": "$TC -j filter show dev $DEV1 parent ffff:",
+        "matchJSON": [
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0
+            },
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x10001",
+                    "flowid": "1:1",
+                    "actions": [
+                        {
+                            "order": 1,
+                            "kind": "gact",
+                            "control_action": {
+                                "type": "pass"
+                            }
+                        }
+                    ]
+                }
+            },
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x20002",
+                    "flowid": "1:2",
+                    "actions": [
+                        {
+                            "order": 1,
+                            "kind": "gact",
+                            "control_action": {
+                                "type": "drop"
+                            }
+                        }
+                    ]
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    },
+    {
+        "id": "3f21",
+        "name": "Replace a route filter that shares its key with another filter",
+        "category": [
+            "filter",
+            "route"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10101 route from 1 to 1 classid 1:1",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10201 route from 1 to 1 classid 1:2"
+        ],
+        "cmdUnderTest": "$TC filter replace dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10101 route from 1 to 1 classid 1:9",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -j filter show dev $DEV1 parent ffff:",
+        "matchJSON": [
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0
+            },
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x10101",
+                    "flowid": "1:9"
+                }
+            },
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x10201",
+                    "flowid": "1:2"
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
+    },
+    {
+        "id": "9d0e",
+        "name": "Replace both route filters sharing a key",
+        "category": [
+            "filter",
+            "route"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10101 route from 1 to 1 classid 1:1",
+            "$TC filter add dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10201 route from 1 to 1 classid 1:2",
+            "$TC filter replace dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10101 route from 1 to 1 classid 1:9"
+        ],
+        "cmdUnderTest": "$TC filter replace dev $DEV1 parent ffff: protocol ip prio 100 handle 0x10201 route from 1 to 1 classid 1:8",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -j filter show dev $DEV1 parent ffff:",
+        "matchJSON": [
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0
+            },
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x10101",
+                    "flowid": "1:9"
+                }
+            },
+            {
+                "protocol": "ip",
+                "pref": 100,
+                "kind": "route",
+                "chain": 0,
+                "options": {
+                    "fh": "0x10201",
+                    "flowid": "1:8"
+                }
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress"
+        ]
     }
 ]
-- 
2.55.0

Re: [PATCH net v2 0/4] net/sched: cls_route: fix bucket retention and handle recomputation

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon,  7 Sep 2026 16:21:29 -0300 you wrote:
> Patch 1 is the v1 patch, unchanged. route4_change() can move an existing
> filter to a different top-level bucket, since route4_set_parms()
> recomputes the handle from TCA_ROUTE4_TO/FROM/IIF. The filter is
> unlinked from the old bucket, but the bucket itself is never freed once
> it goes empty, so route4_delete() keeps reporting *last=false after the
> last live filter is gone. That pins the empty tcf_proto and leaks it.
> The filters linked to a bucket are refcounted now, and the bucket is
> dropped from head->table[] as soon as the count reaches zero.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/4] net/sched: cls_route: free emptied bucket on filter move
    https://git.kernel.org/netdev/net/c/1853f30cf5c8
  - [net,v2,2/4] net/sched: cls_route: Reject handle aliasing
    https://git.kernel.org/netdev/net/c/b74a8455a2f2
  - [net,v2,3/4] net/sched: cls_route: Fix in-place replace
    https://git.kernel.org/netdev/net/c/41e85e54e564
  - [net,v2,4/4] selftests/tc-testing: Add cls_route bucket move and change tests
    https://git.kernel.org/netdev/net/c/e190a7aabbea

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html