Re: [iproute PATCH] tc: u32: Fix key folding in sample option

On Thu, Feb 04, 2021 at 10:28:26AM -0500, Jamal Hadi Salim wrote:

On 2021-02-04 9:50 a.m., Phil Sutter wrote:

quoted

On Thu, Feb 04, 2021 at 09:34:01AM -0500, Jamal Hadi Salim wrote:

quoted

On 2021-02-04 9:04 a.m., Phil Sutter wrote:

quoted

Jamal,

On Thu, Feb 04, 2021 at 08:19:55AM -0500, Jamal Hadi Salim wrote:

quoted

I couldnt tell by inspection if what used to work before continues to.
In particular the kernel version does consider the divisor when folding.

That's correct. And so does tc. What's the matter?

tc assumes 256 when undefined. Maybe man page needs to be
updated to state we need divisor specified otherwise default
is 256.

tc-u32.8 mentions the default in 'sample' option description. Specifying
divisor is mandatory when creating a hash table, so that path is
covered, too. I still don't get how this is related to my patch, though.

It is a general comment related to this code (that you are modifying).
You mentioned divisor in your earlier email as part of the syntax for
sample. So it made me wonder:
Does the bucket placement assume a specific number of buckets in a
table? Example if i had a hash table with 4 buckets, would the sample
then pick the correct bucket? Would it be also correct for 32 buckets,
etc. Or it didnt matter before and it doesnt matter now.

My patch doesn't change how divisor is applied. And yes, with a smaller
than 256 buckets hash table, specifying the divisor along with sample is
necessary.

quoted

quoted

quoted

quoted

Two examples that currently work, if you can try them:

Both lack information about the used hashkey and divisor.

quoted

Most used scheme:
---
tc filter add dev $DEV parent 999:0  protocol ip prio 10 u32 \
ht 2:: \
sample ip protocol 1 0xff match ip src 1.2.3.4/32 flowid 1:10 \
action ok
----

htid before: 0x201000
htid after: 0x201000

Ok, this is the most common use-case. So we are good.

Whatever.

Meaning?

Things are always fine if we only consider the positive results.

Cheers, Phil