Re: [Intel-wired-lan] [PATCH net v1] net/sched: cls_u32: feed u32_replace_hw_knode() with correct set of flags

From: Jamal Hadi Salim <[email protected]> 
Sent: Monday, August 10, 2026 4:00 PM

>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.
>>
>

Hi Jamal

>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.

ohh now i see tc_setup_cb_add() indeed won't make it fail as always
returns >= 0
probably would be also worth to clean it in order to not confuse people,
wdut?

>So IMO, this needs a v2.

Will be prepared, sure

Thanks for the suggestions!

>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.

cc tag already there, isn't it? or do i need to do any extra step?

regards,
jedrek

>
>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]>