Re: tc: u32: Wrong sample hash calculation

[Phil Sutter <phil@nwl.cc>]

Jamal,

On Fri, Jan 22, 2021 at 06:25:22AM -0500, Jamal Hadi Salim wrote:
[...]
> My gut feel is user space is the right/easier spot to fix this
> as long as it doesnt break the working setup of 8b.

One last attempt at clarifying the situation:

Back in 2004, your commit 4e54c4816bf ("[NET]: Add tc extensions
infrastructure.")[1] was applied which commented out the old hash
folding and introduced the shift/cutoff we have today:

|  @@ -90,10 +101,12 @@ static struct tc_u_common *u32_list;
|  
|  static __inline__ unsigned u32_hash_fold(u32 key, struct tc_u32_sel *sel)
|  {
| -	unsigned h = key & sel->hmask;
| +	unsigned h = (key & sel->hmask)>>sel->fshift;
|  
| +	/*
|  	h ^= h>>16;
|  	h ^= h>>8;
| +	*/
|  	return h;
|  }

In a later commit, the new code was made compile-time selected via '#ifdef
fix_u32_bug'. In that same commit, I don't see a related #define though.

Do you remember why this was changed? Seems like the old code was
problematic somehow.

Cheers, Phil

[1] https://github.com/laijs/linux-kernel-ancient-history/commit/4e54c4816bfe51c145382d272b19c2ae41e9e36f#