Message ID | 1462728392-28489-6-git-send-email-jhs@emojatatu.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Sun, May 8, 2016 at 10:26 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > From: Jamal Hadi Salim <jhs@mojatatu.com> > > The process below was broken and is fixed with this patch. > > //add a skbedit action and give it an instance id of 1 > sudo tc actions add action skbedit mark 10 index 1 > //create a filter which binds to skbedit action id 1 > sudo tc filter add dev $DEV parent ffff: protocol ip prio 1 u32\ > match ip dst 17.0.0.1/32 flowid 1:10 action skbedit index 1 > > Message before fix was: > RTNETLINK answers: Invalid argument > We have an error talking to the kernel > > Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com> > --- > net/sched/act_skbedit.c | 22 +++++++++++++++------- > 1 file changed, 15 insertions(+), 7 deletions(-) > > diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c > index 51b2499..784b478 100644 > --- a/net/sched/act_skbedit.c > +++ b/net/sched/act_skbedit.c > @@ -69,7 +69,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, > struct tcf_skbedit *d; > u32 flags = 0, *priority = NULL, *mark = NULL; > u16 *queue_mapping = NULL; > - int ret = 0, err; > + int ret = 0, err, aexists = 0; > > if (nla == NULL) > return -EINVAL; > @@ -96,12 +96,22 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, > mark = nla_data(tb[TCA_SKBEDIT_MARK]); > } > > - if (!flags) > - return -EINVAL; > - > parm = nla_data(tb[TCA_SKBEDIT_PARMS]); > > - if (!tcf_hash_check(tn, parm->index, a, bind)) { > + aexists = tcf_hash_check(tn, parm->index, a, bind); > + > + /* if action exists and this is a late filter bind, no need > + * to continue processing > + */ This comment looks useless, at least for me, because the code is already clear. > + if (aexists && bind) > + return 0; One extra tab? > + > + if (!flags) { > + tcf_hash_release(a, bind); > + return -EINVAL; > + } > + > + if (!aexists) { > ret = tcf_hash_create(tn, parm->index, est, a, > sizeof(*d), bind, false); > if (ret) > @@ -111,8 +121,6 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, > ret = ACT_P_CREATED; > } else { > d = to_skbedit(a); > - if (bind) > - return 0; > tcf_hash_release(a, bind); > if (!ovr) > return -EEXIST; > -- > 1.9.1 >
On 16-05-08 11:28 PM, Cong Wang wrote: >> + /* if action exists and this is a late filter bind, no need >> + * to continue processing >> + */ > > This comment looks useless, at least for me, because the code > is already clear. > I will get rid of it. Note: I added it to one patch only incase the feature of late binding was foreign to some people. > >> + if (aexists && bind) >> + return 0; > > > One extra tab? > thanks for catching it. cheers, jamal
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index 51b2499..784b478 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -69,7 +69,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, struct tcf_skbedit *d; u32 flags = 0, *priority = NULL, *mark = NULL; u16 *queue_mapping = NULL; - int ret = 0, err; + int ret = 0, err, aexists = 0; if (nla == NULL) return -EINVAL; @@ -96,12 +96,22 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, mark = nla_data(tb[TCA_SKBEDIT_MARK]); } - if (!flags) - return -EINVAL; - parm = nla_data(tb[TCA_SKBEDIT_PARMS]); - if (!tcf_hash_check(tn, parm->index, a, bind)) { + aexists = tcf_hash_check(tn, parm->index, a, bind); + + /* if action exists and this is a late filter bind, no need + * to continue processing + */ + if (aexists && bind) + return 0; + + if (!flags) { + tcf_hash_release(a, bind); + return -EINVAL; + } + + if (!aexists) { ret = tcf_hash_create(tn, parm->index, est, a, sizeof(*d), bind, false); if (ret) @@ -111,8 +121,6 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, ret = ACT_P_CREATED; } else { d = to_skbedit(a); - if (bind) - return 0; tcf_hash_release(a, bind); if (!ovr) return -EEXIST;