Message ID | CAM_iQpVF8bHN8WNNH5kHyanj+fLE6kbsFOwQA+t_z_=UK_v=+w@mail.gmail.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On 09/28/2014 06:40 PM, Cong Wang wrote: > On Sun, Sep 28, 2014 at 10:50 AM, John Fastabend > <john.fastabend@gmail.com> wrote: >> >> >> I don't think we need this change, (or perhaps it needs to be a bit >> more complete if something is missing) take a look a >> tcf_exts_validate(), notice the policer is added to act->list so the >> if block in dump() work out, > > I thought it's clear that act->list is correct here. :) > > I was thinking the dump logic was wrong, but you are right that > it should match the logic in validate. Actually the bug is we forgot > to copy exts->type, I am going to submit a patch below: > > diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c > index 77147c8..aad6a67 100644 > --- a/net/sched/cls_api.c > +++ b/net/sched/cls_api.c > @@ -549,6 +549,7 @@ void tcf_exts_change(struct tcf_proto *tp, struct > tcf_exts *dst, > tcf_tree_lock(tp); > list_splice_init(&dst->actions, &tmp); > list_splice(&src->actions, &dst->actions); > + dst->type = src->type; > tcf_tree_unlock(tp); > tcf_action_destroy(&tmp, TCA_ACT_UNBIND); > #endif > Yep it does seem to be missing... Although with the latest code can we just drop tcf_exts_change() and call tcf_exts_validate() directly. I'll check in the morning but a quick glance makes me think it should be OK and simplifies things. .John
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 77147c8..aad6a67 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -549,6 +549,7 @@ void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst, tcf_tree_lock(tp); list_splice_init(&dst->actions, &tmp); list_splice(&src->actions, &dst->actions); + dst->type = src->type; tcf_tree_unlock(tp); tcf_action_destroy(&tmp, TCA_ACT_UNBIND);