On 4/3/15 4:04 PM, Alexei Starovoitov wrote:
On 4/3/15 3:54 PM, Daniel Borkmann wrote:
On 04/04/2015 12:17 AM, Alexei Starovoitov wrote:
...
1. there shouldn't be a choice at all for bpf. Because not pulling l2
means it's bug.
Yep, correct. You would also loose context for a possible dissection,
at best you only have skb->protocol.
2. adding a flag means adding it to iproute2 with default off and making
users forgetting it from time to time and have no way of knowing why
their programs all of a sudden stopped working.
classic falls under the same rules. It doesn't make sense at all to run
a program on packet without L2 header. It's very odd both for classic
and extended programs.
Yep.
Two 'if' conditions in critical path is bogus argument, since these
checks would be there in ingress as well. Same critical path.
Why bogus? There would be no such test on the normal egress path,
where this is irrelevant. I wasn't talking about ingress here.
I see the point regarding the user option. So, why not adding a flag
to tcf_proto_ops a la `.flags = CLS_REQUIRES_L2` that gets propagated
to tcf_proto, and only ingress_enqueue() would need to test if the
classifier imposes that requirement, so it can push/pull.
ok. that sounds better, but neither tcf_proto nor tcf_proto_ops have
'flags' field today... well, I guess it's time to add flags there.
Probably add 'flags' to tcf_proto_ops only and do fl->ops->flags in
ingress_enqueue()?
Will respin.
nope. will take it back.
that doesn't work, since this check cannot be done in ingress_enqueue(),
because it sees the pointer to first filter only, so both TCQ_F_INGRESS
flag and CLS_REQUIRES_L2 flag need to be checked inside
tc_classify_compat() which is a lot worse than my current patch.
So I prefer this patch still :)
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
