[PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode

[PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode

[Miguel Gazquez (Schneider Electric)]

From: Victor Nogueira <victor@mojatatu.com>

[ Upstream commit 9cb36faedeafb9720ac236aeae2ea57091d90a09 ]

When u32_replace_hw_knode fails, we need to undo the tcf_bind_filter
operation done at u32_set_parms.

Fixes: d34e3e181395 ("net: cls_u32: Add support for skip-sw flag to tc u32 classifier.")
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Reviewed-by: Pedro Tammela <pctammela@mojatatu.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Miguel Gazquez (Schneider Electric) <miguel.gazquez@bootlin.com>
---
 net/sched/cls_u32.c | 40 ++++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e501390ccd75..5221ce128526 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -709,8 +709,23 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
 	[TCA_U32_FLAGS]		= { .type = NLA_U32 },
 };
 
+static void u32_unbind_filter(struct tcf_proto *tp, struct tc_u_knode *n,
+			      struct nlattr **tb)
+{
+	if (tb[TCA_U32_CLASSID])
+		tcf_unbind_filter(tp, &n->res);
+}
+
+static void u32_bind_filter(struct tcf_proto *tp, struct tc_u_knode *n,
+			    unsigned long base, struct nlattr **tb)
+{
+	if (tb[TCA_U32_CLASSID]) {
+		n->res.classid = nla_get_u32(tb[TCA_U32_CLASSID]);
+		tcf_bind_filter(tp, &n->res, base);
+	}
+}
+
 static int u32_set_parms(struct net *net, struct tcf_proto *tp,
-			 unsigned long base,
 			 struct tc_u_knode *n, struct nlattr **tb,
 			 struct nlattr *est, bool ovr,
 			 struct netlink_ext_ack *extack)
@@ -756,10 +771,6 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
 		if (ht_old)
 			ht_old->refcnt--;
 	}
-	if (tb[TCA_U32_CLASSID]) {
-		n->res.classid = nla_get_u32(tb[TCA_U32_CLASSID]);
-		tcf_bind_filter(tp, &n->res, base);
-	}
 
 	if (ifindex >= 0)
 		n->ifindex = ifindex;
@@ -898,16 +909,20 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		if (!new)
 			return -ENOMEM;
 
-		err = u32_set_parms(net, tp, base, new, tb,
-				    tca[TCA_RATE], ovr, extack);
+		err = u32_set_parms(net, tp, new, tb,tca[TCA_RATE],
+				    ovr, extack);
 
 		if (err) {
 			__u32_destroy_key(new);
 			return err;
 		}
 
+		u32_bind_filter(tp, new, base, tb);
+
 		err = u32_replace_hw_knode(tp, new, flags, extack);
 		if (err) {
+			u32_unbind_filter(tp, new, tb);
+
 			__u32_destroy_key(new);
 			return err;
 		}
@@ -1108,15 +1123,18 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 	}
 #endif
 
-	err = u32_set_parms(net, tp, base, n, tb, tca[TCA_RATE], ovr,
+	err = u32_set_parms(net, tp, n, tb, tca[TCA_RATE], ovr,
 			    extack);
+
+	u32_bind_filter(tp, n, base, tb);
+
 	if (err == 0) {
 		struct tc_u_knode __rcu **ins;
 		struct tc_u_knode *pins;
 
 		err = u32_replace_hw_knode(tp, n, flags, extack);
 		if (err)
-			goto errhw;
+			goto errunbind;
 
 		if (!tc_in_hw(n->flags))
 			n->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
@@ -1134,7 +1152,9 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
 		return 0;
 	}
 
-errhw:
+errunbind:
+	u32_unbind_filter(tp, n, tb);
+
 #ifdef CONFIG_CLS_U32_MARK
 	free_percpu(n->pcpu_success);
 #endif

---
base-commit: 587461ddf5d522bfcb50ebd55078c0dac37be496
change-id: 20261005-cve-2023-53733-61f835619316

Best regards,
--  
Miguel Gazquez (Schneider Electric) <miguel.gazquez@bootlin.com>

Re: [PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

netdev LLM review for backports (WAS: [PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode)

[Sasha Levin]

On Mon, Oct 05, 2026 at 10:33:13AM +0000, netdev-bot+sinfo@kernel.org wrote:
>Hi!
>
>This is an automated message. This series looks like a fix, but its
>commit messages seem to be missing some information:
>
> - How the issue was discovered, e.g. hit in production, hit during
>   development, syzbot report, manual code inspection, LLM or static
>   analysis tool scan.
>
> - Whether the issue was actually triggered, or is only theoretical
>   (e.g. found by code inspection). If it was triggered please include
>   the symptoms, like the stack trace or error messages.
>
>Please do not repost the series just to address the above. Instead,
>reply to this email with the missing information, so that reviewers
>can take it into account. If the series needs another revision for
>other reasons, please include the information in the commit messages
>then.
>
>The evaluation is done by an LLM so it may be wrong, if you think
>that is the case please reply and explain.

Hello,

I started seeing these reviews on proposed backports of net/ commits, and while
they are REALLY great for upstream commits, I do not think that these comments
make sense in the context of a backport.

-- 
Thanks,
Sasha

Re: netdev LLM review for backports (WAS: [PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode)

[Jakub Kicinski]

On Mon, 5 Oct 2026 10:00:46 -0400 Sasha Levin wrote:
> I started seeing these reviews on proposed backports of net/ commits, and while
> they are REALLY great for upstream commits, I do not think that these comments
> make sense in the context of a backport.

Blah, sorry about that.
Let me add a regex on the tags to exclude patches with 5.x/6.x/7.x

Re: netdev LLM review for backports (WAS: [PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode)

[Florian Fainelli]

On 10/5/26 14:21, Jakub Kicinski wrote:
> On Mon, 5 Oct 2026 10:00:46 -0400 Sasha Levin wrote:
>> I started seeing these reviews on proposed backports of net/ commits, and while
>> they are REALLY great for upstream commits, I do not think that these comments
>> make sense in the context of a backport.
> 
> Blah, sorry about that.
> Let me add a regex on the tags to exclude patches with 5.x/6.x/7.x
> 

They are useful if the backport does not cleanly cherry pick, because it 
is possible to introduce bugs in the process of back porting a specific 
commit. I don't think sashiko is quite smart enough to be able to do 
that however.
-- 
Florian

Re: netdev LLM review for backports (WAS: [PATCH 5.10.y] net: sched: cls_u32: Undo tcf_bind_filter if u32_replace_hw_knode)

[Sasha Levin]

On Mon, Oct 05, 2026 at 02:51:18PM -0700, Florian Fainelli wrote:
>On 10/5/26 14:21, Jakub Kicinski wrote:
>>On Mon, 5 Oct 2026 10:00:46 -0400 Sasha Levin wrote:
>>>I started seeing these reviews on proposed backports of net/ commits, and while
>>>they are REALLY great for upstream commits, I do not think that these comments
>>>make sense in the context of a backport.
>>
>>Blah, sorry about that.
>>Let me add a regex on the tags to exclude patches with 5.x/6.x/7.x
>>
>
>They are useful if the backport does not cleanly cherry pick, because 
>it is possible to introduce bugs in the process of back porting a 
>specific commit. I don't think sashiko is quite smart enough to be 
>able to do that however.

Oh, sorry, I agree with Florian. To be clear, my objection was with the
questions that it's asking, not the review itself:

> - How the issue was discovered, e.g. hit in production, hit during
>   development, syzbot report, manual code inspection, LLM or static
>   analysis tool scan.
>
> - Whether the issue was actually triggered, or is only theoretical
>   (e.g. found by code inspection). If it was triggered please include
>   the symptoms, like the stack trace or error messages.

-- 
Thanks,
Sasha