Hi,
On Fri, Aug 7, 2026 at 6:23 AM Jedrzej Jagielski
<[email protected]> wrote:
>
> Fix u32_change() to pass proper variable to u32_replace_hw_knode().
>
> When trying to offload u32 cmd by adding skip_sw attributes it gets
> ignored and it cannot be correctly processed by HW and tc_cls_u32_offload
> struct which is then passed to driver lacks skip_sw var enabled.
>
> u32_replace_hw_knode is fed with the TCA_ACT_* flags (bits 16+)
> instead of TCA_CLS_* flags (bits 0-4) which actually stores the flags
> parsed by tc, so tc_skip_sw() always returns false. This leads to
> ignoring some of the attibutes which are meant to be configured on filter
> setup.
>
> None of the TCA_ACT_FLAGS_* is actually used within
> u32_replace_hw_knode(), so there is no point in passing them.
>
> Looks like commit 695176bfe5de ("net_sched: refactor TC action init API")
> shadowed the local flags variable which used to be set with
> nla_get_u32(tb[TCA_U32_FLAGS]) with the flags as the new function param
> while not replacing for none of the u32_replace_hw_knode() calls.
>
The sashiko reviews look legit to me. Mainly: your change makes
u32_replace_hw_knode() able to fail for the first time (with the
action flags it always returned 0), which turns three previously-dead
error paths into reachable bugs.
So IMO, this needs a v2.
Suggestions for v2 from the sashiskos (I typically suck at these
summaries, so may miss something):
- Release n->ht_down on the create-path error (e.g. drop the reference
explicitly before kfree(n), or reuse __u32_destroy_key()).
- Consider switching the update path to tc_setup_cb_replace() (as
cls_bpf's cls_bpf_offload_cmd does) so old/new flags and in_hw_count
are accounted for correctly, and the old rule is re-installed on
failure.
- Mention in the commit message that skip_sw add/replace commands that
used to succeed (and were silently skipped by u32_classify()) will now
return -EOPNOTSUPP or -EINVAL when the block/driver cannot offload.
Also seems worth Cc: stable backport.
cheers,
jamal
> Fixes: 695176bfe5de ("net_sched: refactor TC action init API")
> Cc: <[email protected]>
> Reviewed-by: Przemek Kitszel <[email protected]>
> Reviewed-by: Marcin Szycik <[email protected]>
> Signed-off-by: Jedrzej Jagielski <[email protected]>
> ---
> net/sched/cls_u32.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index 8f30cc82181d..dc6e455e64ec 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -935,7 +935,7 @@ static int u32_change(struct net *net, struct sk_buff
> *in_skb,
>
> u32_bind_filter(tp, new, base, tb);
>
> - err = u32_replace_hw_knode(tp, new, flags, extack);
> + err = u32_replace_hw_knode(tp, new, userflags, extack);
> if (err) {
> u32_unbind_filter(tp, new, tb);
>
> @@ -1161,7 +1161,7 @@ static int u32_change(struct net *net, struct sk_buff
> *in_skb,
> struct tc_u_knode __rcu **ins;
> struct tc_u_knode *pins;
>
> - err = u32_replace_hw_knode(tp, n, flags, extack);
> + err = u32_replace_hw_knode(tp, n, userflags, extack);
> if (err)
> goto errunbind;
>
> --
> 2.31.1
>
