Re: [PATCH net-next v2 0/3] ] TC datapath hash api

Jamal Hadi Salim
2020-07-10T12:04:43Z

On 2020-07-09 8:19 a.m., Jiri Pirko wrote:
> Thu, Jul 09, 2020 at 01:00:26PM CEST, jhs@mojatatu.com wrote:
>> On 2020-07-08 10:45 a.m., Jiri Pirko wrote:
>>> Wed, Jul 08, 2020 at 03:54:14PM CEST, jhs@mojatatu.com wrote:
>>>> On 2020-07-07 6:05 a.m., Jiri Pirko wrote:

>> Nothing to do with how a driver offloads. That part is fine.
>>
>> By "flower's algorithm" I mean the fact you have to parse and
>> create the flow cache from scratch on ingress - that slows down
>> the ingress path. Compare, from cpu cycles pov, to say fw
>
> Could you point to the specific code please?
>
> The skb->hash is only accessed if the user sets it up for matching.
> I don't understand what slowdown you are talking about :/
>

Compare the lookup approach taken by flower in ->classify vs fw.
Then add a few hundred(or thousands of) rules.

>
>> classifier which dereferences skbmark and uses it as a key
>> to lookup a hash table.
>> An skbhash classifier would look the same as fw in its
>> approach.
>> subtle point i was making was: if your goal was to save cpu cycles
>> by offloading the lookup(whose result you then use to do
>> less work on the host) then you need all the cycles you can
>> save.
>>
>> Main point is: classifying based on hash(and for that
>> matter any other metadata like mark) is needed as a general
>> utility for the system and should not be only available for
>> flower. The one big reason we allow all kinds of classifiers
>> in tc is in the name of "do one thing and do it well".
>
> Sure. That classifier can exist, no problem. At the same time, flower
> can match on it as well. There are already multiple examples of
> classifiers matching on the same thing. I don't see any problem there.
>

I keep pointing to the issues and we keep circling back
to your desire to add it to flower. I emphatize with the
desire to have flower as a one stop shop for all things classification
but this is at the expense of other classifiers. I too need this for
offloading as well as getting the RSS proper feature i described.ets
make progress.
You go ahead - i will submit a version to add it as a separate
hash classifier.

cheers,
jamal