On 08/31/2017 12:22 AM, Daniel Borkmann wrote:
On 08/31/2017 12:01 AM, Cong Wang wrote:
On Wed, Aug 30, 2017 at 2:48 PM, Daniel Borkmann <dan...@iogearbox.net> wrote:
On 08/30/2017 11:30 PM, Cong Wang wrote:
[...]
Note, we still can NOT totally get rid of those class lookup in
->enqueue() because cgroup and flow filters have no way to determine
the classid at setup time, they still have to go through dynamic lookup.
[...]
---
include/net/sch_generic.h | 1 +
net/sched/cls_basic.c | 9 +++++++
net/sched/cls_bpf.c | 9 +++++++
Same is for cls_bpf as well, so bind_class wouldn't work there
either as we could return dynamic classids. bind_class cannot
be added here, too.
I think you are probably right, but the following code is
misleading there:
if (tb[TCA_BPF_CLASSID]) {
prog->res.classid = nla_get_u32(tb[TCA_BPF_CLASSID]);
tcf_bind_filter(tp, &prog->res, base);
}
If the classid is dynamic, why this tb[TCA_BPF_CLASSID]?
The prog->res.classid is the default one, but can be overridden
later depending on the specified program. cls_bpf_classify() does
after prog return (filter_res holds return code):
[...]
if (filter_res == 0)
continue;
if (filter_res != -1) {
res->class = 0;
res->classid = filter_res;
} else {
*res = prog->res;
}
[...]
Meaning in case of a match (-1), we use the default bound one,
but prog may as well return an alternative found classid if it
wants to. So both versions are possible.
But even for that case your patch looks fine to me actually, since
for dynamic classid we set class to 0. No objections from my side
then.
