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: > > On Thu, Feb 04, 2021 at 09:34:01AM -0500, Jamal Hadi Salim wrote: > >> On 2021-02-04 9:04 a.m., Phil Sutter wrote: > >>> Jamal, > >>> > >>> On Thu, Feb 04, 2021 at 08:19:55AM -0500, Jamal Hadi Salim wrote: > >>>> 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. > >>>> Two examples that currently work, if you can try them: > >>> > >>> Both lack information about the used hashkey and divisor. > >>> > >>>> 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