Re: [patch net-next] tc: introduce OpenFlow classifier

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

On 03/30/15 02:26, Jiri Pirko wrote:
> Mon, Mar 30, 2015 at 02:39:31AM CEST, jhs@mojatatu.com wrote:

>> Could you have got rid of "type" above given we always have
>> a "proto" field in tc that is checked at the core?
>
> Sure it is possible. But I try to stick with the names used in OpenFlof
> documentation.
>

Ok, thats fine. Safer to stick with the spec.

>> Was there something else that was intended to go into that tp union?
>
> Following fields are missing:
>
> OXM_OF_SCTP_SRC
> OXM_OF_SCTP_DST
> OXM_OF_ICMPV4_TYPE
> OXM_OF_ICMPV4_CODE
>

It just looked strange to have a union with only one member.

>> General comments:
>> so what happens if someone adds a new field? It sounds to me like
>> given it is tied to datapath match it will never be backward compatible
>> (i.e think old tc, new kernel vs new tc, old kernel)
>
>
> Well if kernel does not understand the new field, it will simply ignore
> it.
>

More like user expectation may be different. I suppose they could query
the kernel and find that their field was not set.
BTW, another micro optimization:
You could pre-compute the user passed key/mask result per field and
in the fast path use that instead of doing f->match.key & f->match.mask

cheers,
jamal