Mon, Mar 30, 2015 at 01:10:24PM CEST, jhs@mojatatu.com wrote:

On 03/30/15 02:26, Jiri Pirko wrote:

quoted

Mon, Mar 30, 2015 at 02:39:31AM CEST, jhs@mojatatu.com wrote:

quoted

quoted

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.

quoted

quoted

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.

Oh, will remove the union for now as it can be easily extended later.

quoted

quoted

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.

I believe that is the usual way thing are done using Netlink API.

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

Yes, I plan to address this as well for v2. Thanks!

cheers,
jamal