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

[Jamal Hadi Salim <jhs@mojatatu.com>]

On 16-02-23 08:20 AM, Daniel Borkmann wrote:
> On 02/23/2016 01:09 PM, Jamal Hadi Salim wrote:
>> On 16-02-22 11:47 AM, Daniel Borkmann wrote:
> [...]
>>> So, basically this is a L2 encap with TLVs, right?
>>>
>>> And as TLVs you have skb->mark, skb->priority, skb->hash,
>>> skb->queue_mapping
>>> that you transfer from one machine to another, where on the destination,
>>> you
>>> are applying the above meta data to the skb itself. And, configuration
>>> is via
>>> tc.
>>>
>>> I couldn't parse from the commit log what the real world use case is,
>>> resp.
>>> who is going to use this infrastructure?
>>>
>>> Do you have some typical setup, where the above needs to be transferred
>>> in the
>>> encap and restored?
>>
>> I am assuming you are asking this for the sake of people who dont
>> have context (and not for yourself)?
>> I added a pointer to the paper. It is 6 pages and simple to read.
>> Isnt that sufficient? I dont want to write a novel here. Some could
>> argue that in fact i am already writing a novel in commit 1/5.
>
> Ok, the paper talks about, quote:
>
>   - Pipeline-stage Indexing.
>   - OAM information - example turn on some packet debug information on a
> need basis.
>   - Exception handling information - example VXLAN service handling.
>   - Authentication and authorization information.
>   - Versioning information.
>   - Compliance information.
>   - Service Identifiers.
>

Which is now added to the cover letter.

> As your primary examples, you provide skb->mark, skb->priority, skb->hash,
> skb->queue_mapping for encapsulating, f.e. how do you use skb>hash in this
> scenario? What's the use-case?

These are basic metadata. The question to ask is what could one use
skb->hash for. Today it is used to select a cpu to balance to.
I dont understand what your concern is: I could pass a string
as metadata if i wanted to. I chose to pass skb metadata;
some of which i actually use. Only one i dont use is queue
map - but because it is still an skb metadata so i wrote code
to pass it.

>> They need to be separated to make them unique. These are basic
>> metadatum; I have a few others lined up - but i just wanted to start
>> with these because they are obvious to see. What i mulled over is
>> to send one big patch or several. In the end it seemed cleaner to
>> send separate patches.
>
> But just to make them unique, you don't need to add extra modules for
> this ... just having a module for encoding one skb member seems like
> total overdesign to me. If you really want them to be separate ops,
> you can still include them into act_ife.c itself.
>

These things will evolve. I would rather leave them the way they
are for that reason and they serve as good examples of how one
would new metadatum.
Example: skb->mark may end up evolving to include masks.

cheers,
jamal