Subject: Re: [net-next PATCH 0/5] net_sched: Add support for IFE action

From: Daniel Borkmann <[email-masked]>
Date: Fri, 26 Feb 2016 01:03:42 +0100

On 02/25/2016 11:40 PM, Jamal Hadi Salim wrote:
> On 16-02-25 04:34 PM, Daniel Borkmann wrote:
>> On 02/25/2016 01:23 PM, Jamal Hadi Salim wrote:
>>> On 16-02-24 12:48 PM, Daniel Borkmann wrote:
>>>> On 02/24/2016 01:49 PM, Jamal Hadi Salim wrote:
>> [...]
>>>>> Drivers do set the hash. My use case is slightly different.
>>>>> I have a NIC which has an embedded cavium processor. This thing
>>>>> strips off the TLV and uses the hash to select the host MSI.
>>>>> Only thing we dont use at the moment is queue_mapping.
>>>>
>>>> Ok, but the example says ingress qdisc. ;) I presume the driver for the
>>>> NIC and the offloading parts are non-public? :/
>>>
>>> Well, it is not exactly mellanox or intel - but I am sure someone would
>>> be willing to sell that NIC.
>>>
>>>> So, without them, placing
>>>> this on ingress qdisc doesn't seem much useful wrt the skb hash example,
>>>> and most people only have the software part (for ingress I mean)
>>>> available.
>>>
>>> Do you want me to take skbbhash out? I just want to get this set in then
>>> worry about adding and modifying.
>>
>> Well, if the NIC driver and offloading parts for ife are not available
>> (or cannot be made available)
>
> If you are willing to pay for one i can have one sold to you.
> Note: There is no dependency on such a NIC. You asked for an example
> of how one would use skbhash and i pointed it out to you.
> Infact nothing in the commit notes pointed to any NIC.

Yes, but that was not the point, I think we're talking past each other. ;)
The example (or better, use-case) you provided with your NIC seems useful.
But when you want to decode this with your kernel module, f.e. attached on
ingress qdisc, it's an entirely different situation. Anyway, point is subsequent
calls to skb_get_hash() will overwrite what you decoded there, that's why I
asked and tried to point this out earlier. You may want to look at sth
like __skb_set_sw_hash() ...

>> in the upstream kernel tree, then you have
>> to assume that everyone else can _only_ use this with the existing tc
>> facilities in the kernel. And as such, the whole set needs to be evaluated,
>> I think this is nothing new. ;-)
>
> Seriously this is getting to a ridiculous level now.
> I gave a talk - which you attended. I wrote a paper which you may have
> at minimal glanced at.
> I have had discussions with you on this very subject.

I think I only suggested that in case you still don't have an ethertype
assigned to this to require the user to specify one instead of some default.

> And as soon as i posted the patches your statements led from you
> needing a good use case to why not use a netdev and why i have
> all these things that are not very useful. None of which came up before.

Fair enough, didn't see that code before and just tried to review it, probably
shouldn't have, so I'm not looking further into this anymore. ;)

Thanks anyway,
Daniel