RE: [PATCH net v2 0/3] net/sched: u32_change() fixes

>From: Kitszel, Przemyslaw <[email protected]> 
>Sent: Monday, August 17, 2026 2:43 PM
>
>On 8/13/26 14:22, Jedrzej Jagielski wrote:
>> sorry for spam, previous series get treated as incomplete so resending
>> 
>> ALREADY ON NETDEV, SASHIKO RUN
>
>hehe
>ofc, the above does no harm here

:D

>
>for the series:
>Reviewed-by: Przemek Kitszel <[email protected]>
>
>--
>
>Sashiko has found one more *preexisting* bug, perhaps it could be
>handled by one more fix in future series?:
>https://sashiko.dev/#/patchset/20260813122242.1690024-1-jedrzej.jagielski%40intel.com

yeah, i saw that and i thought about extending the series 

>
> > @@ -938,14 +938,6 @@ static int u32_change(struct net *net, struct 
>sk_buff *in_skb,
> >  		err = u32_replace_hw_knode(tp, new, userflags, extack);
> >  		if (err) {
> >  			u32_unbind_filter(tp, new, tb);
> > -
> > -			if (tb[TCA_U32_LINK]) {
> > -				struct tc_u_hnode *ht_old;
> > -
> > -				ht_old = rtnl_dereference(n->ht_down);
> > -				if (ht_old)
> > -					refcount_inc(&ht_old->refcnt);
> > -			}
> >  			__u32_destroy_key(new);
> >  			return err;
> >  		}
>This isn't a bug introduced by this patch, but looking at u32_change(), 
>there
>appears to be a pre-existing issue with how manual hash table handles are
>tracked in the IDR.
>When a user creates a u32 hash table with a manual handle (for example,
>0x80100000), u32_change() inserts the full handle directly into
>tp_c->handle_idr:
>net/sched/cls_u32.c:u32_change() {
>     ...
>		} else {
>			err = idr_alloc_u32(&tp_c->handle_idr, ht, &handle,
>					    handle, GFP_KERNEL);
>     ...
>}
>However, when destroying the node later, u32_destroy_hnode() removes the
>mapped ID rather than the full handle:
>net/sched/cls_u32.c:u32_destroy_hnode() {
>     ...
>		idr_remove(&tp_c->handle_idr, handle2id(ht->handle));
>     ...
>}
>For a handle like 0x80100000, handle2id() returns 1. Does this mean the IDR
>entry at 0x80100000 is permanently leaked, while index 1 is erroneously 
>freed?
>If index 1 is already in use by an active auto-generated hash table, could
>freeing it here allow gen_new_htid() to issue the same handle again to a new
>hash table, causing collisions?