Re: tc: u32: Wrong sample hash calculation

Hi Phil,

On 2021-01-22 8:59 a.m., Phil Sutter wrote:

Jamal,

On Fri, Jan 22, 2021 at 06:25:22AM -0500, Jamal Hadi Salim wrote:
[...]

quoted

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.

Vague recollection that it didnt work. I will have to dig deeper in old
email exchanges. Its been like close to 20 years (if you consider there
was work in progress about 1-2 years before that final submission) ;->

cheers,
jamal