| Message ID | 1397605563-29756-1-git-send-email-xiyou.wangcong@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
On 04/15/14 19:46, Cong Wang wrote: > From: Cong Wang <cwang@twopensource.com> > > When actions are attached to a filter, they are a part of the filter > itself, so when changing a filter we should allow to overwrite the actions > inside as well. > > In my specific case, when I tried to _append_ a new action to an existing > filter which already has an action, I got EEXIST since kernel refused > to overwrite the existing one in kernel. > > This patch checks if we are changing the filter checking NLM_F_CREATE flag > (Sigh, filters don't use NLM_F_REPLACE...) and then passes the boolean down > to actions. This fixes the problem above. What are you trying to achieve? These are general netlink rules (which can be streamlined by the object). Append means "I dont care if this exists, add it to the end" In that case, you would specify the an existing filter rule but in order to resolve ambiguity tc classifiers provide priorities (i.e just specify a different priority) and the rule will be added before or after the conflicting rule. If you dont do that then you will get back EEXIST to tell you there is a conflict. You cant replace an existing filter in particular when it has a graph of actions attached to it. You can replace the paremetrization of an existing bound action - but i am not sure that is what you are trying to do here. For that address the specific action directly. i.e tc action .... If otoh you wanted to replace the filter + action graph with a backup rule, then just add it lower in the priority list and delete the existing one etc. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 16, 2014 at 5:30 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > On 04/15/14 19:46, Cong Wang wrote: >> >> From: Cong Wang <cwang@twopensource.com> >> >> When actions are attached to a filter, they are a part of the filter >> itself, so when changing a filter we should allow to overwrite the actions >> inside as well. >> >> In my specific case, when I tried to _append_ a new action to an existing >> filter which already has an action, I got EEXIST since kernel refused >> to overwrite the existing one in kernel. >> >> This patch checks if we are changing the filter checking NLM_F_CREATE flag >> (Sigh, filters don't use NLM_F_REPLACE...) and then passes the boolean >> down >> to actions. This fixes the problem above. > > > > What are you trying to achieve? I thought it's clear: I have a filter F1 contains an action A1, they are already in kernel (therefore I can read them from libnl cache), then I want to append A2 to F1 so that F1 would have two actions A1 and A2. In short: change from F1 -> A1 to F1 -> A1 -> A2 atomically. > These are general netlink rules (which can be streamlined by > the object). > Append means "I dont care if this exists, add it to the end" Exactly what I meant. > In that case, you would specify the an existing filter rule but > in order to resolve ambiguity tc classifiers provide priorities > (i.e just specify a different priority) and the rule will be added > before or after the conflicting rule. > If you dont do that then you will get back EEXIST to tell you > there is a conflict. But I already told kernel I am changing a filter, so why it complains? I should change an existing one, shouldn't I? > You cant replace an existing filter in particular when it has > a graph of actions attached to it. You can replace the paremetrization > of an existing bound action - but i am not sure that is what you > are trying to do here. For that address the specific action directly. > i.e tc action .... Why? Since actions are inside it, I should be able to change any part of it, right? > If otoh you wanted to replace the filter + action graph with a backup > rule, then just add it lower in the priority list and delete the > existing one etc. > This is not atomic, is it? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/16/14 17:10, Cong Wang wrote: > On Wed, Apr 16, 2014 at 5:30 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > I thought it's clear: I have a filter F1 contains an action A1, > they are already in kernel (therefore I can read them from libnl cache), > then I want to append A2 to F1 so that F1 would have two actions > A1 and A2. > > In short: change from F1 -> A1 to F1 -> A1 -> A2 atomically. > Ok, got it. >> In that case, you would specify the an existing filter rule but >> in order to resolve ambiguity tc classifiers provide priorities >> (i.e just specify a different priority) and the rule will be added >> before or after the conflicting rule. >> If you dont do that then you will get back EEXIST to tell you >> there is a conflict. > > But I already told kernel I am changing a filter, so why it > complains? I should change an existing one, shouldn't I? > [..] > Why? Since actions are inside it, I should be able to change any > part of it, right? > The challenge is in the semantics. You are making change to the graph _not_ to the filter. There are dependencies in a graph. IOW, I doubt what you are doing could be made generic and safe without it being a two step operation since we have multiple tables to deal with. Example - what would you do if you wanted to change the graph so that you add something in the middle or remove something at the end? >> If otoh you wanted to replace the filter + action graph with a backup >> rule, then just add it lower in the priority list and delete the >> existing one etc. >> > > This is not atomic, is it? > It avoids the need for atomicity. Backup rule will never be used as long as the active is still in use. If you can do the two steps in the kernel as i described, then you can achieve your purpose but i worry it will complicate code for a corner use case (which has a work around already). cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 17, 2014 at 4:50 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > On 04/16/14 17:10, Cong Wang wrote: >> >> Why? Since actions are inside it, I should be able to change any >> part of it, right? >> > > The challenge is in the semantics. You are making change to the > graph _not_ to the filter. There are dependencies in a graph. IOW, > I doubt what you are doing could be made generic and safe without > it being a two step operation since we have multiple tables to deal > with. > Example - what would you do if you wanted to change the graph > so that you add something in the middle or remove something at > the end? Actions attached to a filter are at the end of the graph, they should be able to be added/removed together. > > >>> If otoh you wanted to replace the filter + action graph with a backup >>> rule, then just add it lower in the priority list and delete the >>> existing one etc. >>> >> >> This is not atomic, is it? >> > > It avoids the need for atomicity. Backup rule will never be used > as long as the active is still in use. > If you can do the two steps in the kernel as i described, then > you can achieve your purpose but i worry it will complicate code > for a corner use case (which has a work around already). > This is a workaround, not a fix. What if I have multiple threads trying to append an action at the same time? This workaround can't guarantee the correctness. My case is a perfectly valid use, we have to fix it, maybe in another way. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/17/14 16:48, Cong Wang wrote: > Actions attached to a filter are at the end of the graph, they should > be able to be added/removed together. I will try to be clear: you are changing policy which is a higher level semantic than any one of filter or actions. Consider: a) A simple policy in a linear graph if (icmp) --> A-->B-->C-->D-->E With the following individual change operations: i)change to delete C ii)change to add G after B iii) add F as branch from D iv) add H after E and maintain linear status v) after #iv make the graph at node D selectively branch between E/H Ok, let provide a slightly complex graph, sorry it is hard to do ascii diagram for this, so i will illustrate programmatically: if (icmp) { mark with tag 11 //mark index 7 if (rate < 10kbs) { //policer index 1 return } else { mark with tag 12 //mark index 3 if (rate2 < 20kbs) {//policer index 2 return } else { mirror to dummy0 //mirred index 8 } } } What does adding/deleting even mean in this case? > > This is a workaround, not a fix. What if I have multiple threads > trying to append an action at the same time? This workaround > can't guarantee the correctness. > Sorry Cong, your approach is NOT a fix. There is _no way_ you can achieve correctness without a two phase operation. Talk to the netfilter guys about such experiences (in 2.x kernels?). The multiple threads issue is taken care of by locking, you just want to reduce the amount of lock time needed; that is what that (proven) 2 phase approach is doing. I called it workaround because you dont need to make any changes in the kernel. There is a tiny window of inconsistency possible in presence of multiple contenders but you already get that with things like RCU (grace period) that you added. > My case is a perfectly valid use, I am not questioning your use case - just it shouldnt come as a hack at the expense of all other use cases (of which the most important are not replacing/changing a graph but creating and deleting one). >we have to fix it, maybe in another way. If you really really really insist i has to be done in the kernel; then i would hope youd do it in some form of pseudo transacational approach as i described is done today in user space. I hope avoidance of a fat approach like 2PC as well is in order. Again as i said I am extremely leary of optimizing for such corner case since you can achieve all that in user space. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 18, 2014 at 7:26 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > Ok, let provide a slightly complex graph, sorry it is hard to do ascii > diagram for this, so i will illustrate programmatically: > > if (icmp) { > mark with tag 11 //mark index 7 > if (rate < 10kbs) { //policer index 1 > return > } else { > mark with tag 12 //mark index 3 > if (rate2 < 20kbs) {//policer index 2 > return > } else { > mirror to dummy0 //mirred index 8 > } > } > } > > What does adding/deleting even mean in this case? In this case, all the statements inside if (icmp) {} are actions, right? Sorry, I still fail to see why not allowing to change them *together*? IOW, what's wrong with changing if (icmp) { A } to if (icmp) { B } ? where A and B could be any complex combination of actions. RTNL lock guarantees this is transactional. Users are responsible to ensure the logic of A or B is correct, not the kernel. Kernel should allow even a wrong combination, since there is no way to check the correctness in kernel. I never mean to only add or remove one of them inside, although my specific case is just for appending, my patch should allow to overwrite all the actions together. [...] > > If you really really really insist i has to be done in the kernel; > then i would hope youd do it in some form of pseudo transacational > approach as i described is done today in user space. > I hope avoidance of a fat approach like 2PC as well is in order. > Again as i said I am extremely leary of optimizing for such > corner case since you can achieve all that in user space. > It is not a corner case, it is a very basic functionality we need: We mirror icmp packets to every vethX device, when one of them is gone, we just remove the action; when a new one comes up, we append an action. So simple... -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/18/14 13:18, Cong Wang wrote: > In this case, all the statements inside if (icmp) {} are actions, right? > Sorry, I still fail to see why not allowing to change them *together*? > Yes, thats what i am saying as well. Maybe we were agreeing all along. > IOW, what's wrong with changing if (icmp) { A } to if (icmp) { B } ? > where A and B could be any complex combination of actions. > RTNL lock guarantees this is transactional. > RTNL is one dimension. The other is the datapath processing. You need to make sure that packets still flow correctly during the change over. My suggestion was you add the new rule first with a lower priority so that it is never used as long as the current one is in place. You then do a delete of the old one. RCU grace period passes where current packets are processed then the new rule takes effect. You dont have to follow that suggestion as long as you achieve the goal. > Users are responsible to ensure the logic of A or B is correct, not > the kernel. Kernel should allow even a wrong combination, > since there is no way to check the correctness in kernel. > I'd be happy with that. > I never mean to only add or remove one of them inside, although > my specific case is just for appending, my patch should allow to > overwrite all the actions together. > Well - then go nuts and put out a patch. Replace _all or none_ is a reasonable approach. > It is not a corner case, it is a very basic functionality we need: > > We mirror icmp packets to every vethX device, when one of them > is gone, we just remove the action; when a new one comes up, > we append an action. So simple... > This is where my problem was Cong - you have a simple use case and i was hoping that you dont base that for a generic solution. If you only have one action, then no problem in deleting/adding it. But if you have a group of actions then you delete/add the whole group. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Apr 19, 2014 at 4:10 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > On 04/18/14 13:18, Cong Wang wrote: >> IOW, what's wrong with changing if (icmp) { A } to if (icmp) { B } ? >> where A and B could be any complex combination of actions. >> RTNL lock guarantees this is transactional. >> > > RTNL is one dimension. The other is the datapath processing. > You need to make sure that packets still flow correctly during the > change over. Sure, since we grab tcf_tree_lock() before changing actions in tcf_exts_change(), I think this is guaranteed too. > >> I never mean to only add or remove one of them inside, although >> my specific case is just for appending, my patch should allow to >> overwrite all the actions together. >> > > Well - then go nuts and put out a patch. > Replace _all or none_ is a reasonable approach. > > Great! We both agree on this. Looking at the current code, we first initialize a list of actions and then replace them as a whole by splicing the lists with tcf_tree_lock held, so this is already done. IOW, this patch is enough. Or am I missing anything? Thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/21/14 19:28, Cong Wang wrote: > On Sat, Apr 19, 2014 at 4:10 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: >> On 04/18/14 13:18, Cong Wang wrote: >> RTNL is one dimension. The other is the datapath processing. >> You need to make sure that packets still flow correctly during the >> change over. > > Sure, since we grab tcf_tree_lock() before changing actions in > tcf_exts_change(), I think this is guaranteed too. > Maybe - I thought there were recent changes to tcf_tree_lock that eased the restriction. Youd need to make sure. >> Well - then go nuts and put out a patch. >> Replace _all or none_ is a reasonable approach. >> >> > > Great! We both agree on this. > > Looking at the current code, we first initialize a list of actions > and then replace them as a whole by splicing the lists with > tcf_tree_lock held, so this is already done. IOW, this patch > is enough. > > Or am I missing anything? > I dont see what you are seeing. However, some quick tests will prove it. Example go from 1->3 actions and backwards 3->1 actions. I.e create a policy with (handwave) 3 actions and replace it with another that has only one action. Do this while you are sending a ping exercising this policy. I can see how yours with 1->0 and 0->1 will work. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 23, 2014 at 4:30 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > > I dont see what you are seeing. However, some quick tests will prove it. > Example go from 1->3 actions and backwards 3->1 actions. > I.e create a policy with (handwave) 3 actions and replace it with > another that has only one action. Do this while you are sending a > ping exercising this policy. > I can see how yours with 1->0 and 0->1 will work. See below (with this patch applied): # tc filter show dev lo parent ffff: filter protocol arp pref 49152 basic filter protocol arp pref 49152 basic handle 0x1 action order 1: mirred (Egress Mirror to device dummy1) pipe index 1 ref 1 bind 1 action order 2: mirred (Egress Mirror to device dummy2) pipe index 2 ref 1 bind 1 action order 3: mirred (Egress Mirror to device dummy3) pipe index 3 ref 1 bind 1 # tc filter change dev lo parent ffff: protocol arp pref 49152 handle 1 basic action mirred egress mirror dev dummy1 index 1 # tc filter show dev lo parent ffff: filter protocol arp pref 49152 basic filter protocol arp pref 49152 basic handle 0x1 action order 1: mirred (Egress Mirror to device dummy1) pipe index 1 ref 1 bind 1 # tc action ls action mirred action order 0: mirred (Egress Mirror to device dummy1) pipe index 1 ref 1 bind 1 -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/24/14 17:12, Cong Wang wrote: > On Wed, Apr 23, 2014 at 4:30 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: >> >> I dont see what you are seeing. However, some quick tests will prove it. >> Example go from 1->3 actions and backwards 3->1 actions. >> I.e create a policy with (handwave) 3 actions and replace it with >> another that has only one action. Do this while you are sending a >> ping exercising this policy. >> I can see how yours with 1->0 and 0->1 will work. > > See below (with this patch applied): > Ok, thanks Cong. Since you are using this in your system already, I am assuming it is tested with running traffic at the same time. Please resubmit the patch with my signed off as well.. cheers, jamal -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 25, 2014 at 8:48 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote: > On 04/24/14 17:12, Cong Wang wrote: >> >> On Wed, Apr 23, 2014 at 4:30 AM, Jamal Hadi Salim <jhs@mojatatu.com> >> wrote: >>> >>> >>> I dont see what you are seeing. However, some quick tests will prove it. >>> Example go from 1->3 actions and backwards 3->1 actions. >>> I.e create a policy with (handwave) 3 actions and replace it with >>> another that has only one action. Do this while you are sending a >>> ping exercising this policy. >>> I can see how yours with 1->0 and 0->1 will work. >> >> >> See below (with this patch applied): >> > > Ok, thanks Cong. Since you are using this in your system already, > I am assuming it is tested with running traffic at the same time. Yes I did. > > Please resubmit the patch with my signed off as well.. > OK, and will rebase it on net-next since it is not an urgent issue. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h index a2441fb..6da46dc 100644 --- a/include/net/pkt_cls.h +++ b/include/net/pkt_cls.h @@ -136,7 +136,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts, int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, struct nlattr *rate_tlv, - struct tcf_exts *exts); + struct tcf_exts *exts, bool ovr); void tcf_exts_destroy(struct tcf_proto *tp, struct tcf_exts *exts); void tcf_exts_change(struct tcf_proto *tp, struct tcf_exts *dst, struct tcf_exts *src); diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h index d062f81..624f985 100644 --- a/include/net/sch_generic.h +++ b/include/net/sch_generic.h @@ -199,7 +199,7 @@ struct tcf_proto_ops { int (*change)(struct net *net, struct sk_buff *, struct tcf_proto*, unsigned long, u32 handle, struct nlattr **, - unsigned long *); + unsigned long *, bool); int (*delete)(struct tcf_proto*, unsigned long); void (*walk)(struct tcf_proto*, struct tcf_walker *arg); diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c index 29a30a1..6786130 100644 --- a/net/sched/cls_api.c +++ b/net/sched/cls_api.c @@ -317,7 +317,8 @@ replay: } } - err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh); + err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh, + n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE); if (err == 0) { if (tp_created) { spin_lock_bh(root_lock); @@ -504,7 +505,7 @@ void tcf_exts_destroy(struct tcf_proto *tp, struct tcf_exts *exts) EXPORT_SYMBOL(tcf_exts_destroy); int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, - struct nlattr *rate_tlv, struct tcf_exts *exts) + struct nlattr *rate_tlv, struct tcf_exts *exts, bool ovr) { #ifdef CONFIG_NET_CLS_ACT { @@ -513,7 +514,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, INIT_LIST_HEAD(&exts->actions); if (exts->police && tb[exts->police]) { act = tcf_action_init_1(net, tb[exts->police], rate_tlv, - "police", TCA_ACT_NOREPLACE, + "police", ovr, TCA_ACT_BIND); if (IS_ERR(act)) return PTR_ERR(act); @@ -523,7 +524,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb, } else if (exts->action && tb[exts->action]) { int err; err = tcf_action_init(net, tb[exts->action], rate_tlv, - NULL, TCA_ACT_NOREPLACE, + NULL, ovr, TCA_ACT_BIND, &exts->actions); if (err) return err; diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c index e98ca99..0ae1813 100644 --- a/net/sched/cls_basic.c +++ b/net/sched/cls_basic.c @@ -130,14 +130,14 @@ static const struct nla_policy basic_policy[TCA_BASIC_MAX + 1] = { static int basic_set_parms(struct net *net, struct tcf_proto *tp, struct basic_filter *f, unsigned long base, struct nlattr **tb, - struct nlattr *est) + struct nlattr *est, bool ovr) { int err; struct tcf_exts e; struct tcf_ematch_tree t; tcf_exts_init(&e, TCA_BASIC_ACT, TCA_BASIC_POLICE); - err = tcf_exts_validate(net, tp, tb, est, &e); + err = tcf_exts_validate(net, tp, tb, est, &e, ovr); if (err < 0) return err; @@ -161,7 +161,7 @@ errout: static int basic_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, - struct nlattr **tca, unsigned long *arg) + struct nlattr **tca, unsigned long *arg, bool ovr) { int err; struct basic_head *head = tp->root; @@ -179,7 +179,7 @@ static int basic_change(struct net *net, struct sk_buff *in_skb, if (f != NULL) { if (handle && f->handle != handle) return -EINVAL; - return basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE]); + return basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE], ovr); } err = -ENOBUFS; @@ -206,7 +206,7 @@ static int basic_change(struct net *net, struct sk_buff *in_skb, f->handle = head->hgenerator; } - err = basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE]); + err = basic_set_parms(net, tp, f, base, tb, tca[TCA_RATE], ovr); if (err < 0) goto errout; diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c index 8e3cf49..1618696 100644 --- a/net/sched/cls_bpf.c +++ b/net/sched/cls_bpf.c @@ -156,7 +156,7 @@ static void cls_bpf_put(struct tcf_proto *tp, unsigned long f) static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp, struct cls_bpf_prog *prog, unsigned long base, struct nlattr **tb, - struct nlattr *est) + struct nlattr *est, bool ovr) { struct sock_filter *bpf_ops, *bpf_old; struct tcf_exts exts; @@ -170,7 +170,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp, return -EINVAL; tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE); - ret = tcf_exts_validate(net, tp, tb, est, &exts); + ret = tcf_exts_validate(net, tp, tb, est, &exts, ovr); if (ret < 0) return ret; @@ -242,7 +242,7 @@ static u32 cls_bpf_grab_new_handle(struct tcf_proto *tp, static int cls_bpf_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct cls_bpf_head *head = tp->root; struct cls_bpf_prog *prog = (struct cls_bpf_prog *) *arg; @@ -260,7 +260,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb, if (handle && prog->handle != handle) return -EINVAL; return cls_bpf_modify_existing(net, tp, prog, base, tb, - tca[TCA_RATE]); + tca[TCA_RATE], ovr); } prog = kzalloc(sizeof(*prog), GFP_KERNEL); @@ -277,7 +277,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb, goto errout; } - ret = cls_bpf_modify_existing(net, tp, prog, base, tb, tca[TCA_RATE]); + ret = cls_bpf_modify_existing(net, tp, prog, base, tb, tca[TCA_RATE], ovr); if (ret < 0) goto errout; diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c index 8e2158a..cacf01b 100644 --- a/net/sched/cls_cgroup.c +++ b/net/sched/cls_cgroup.c @@ -83,7 +83,7 @@ static const struct nla_policy cgroup_policy[TCA_CGROUP_MAX + 1] = { static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct nlattr *tb[TCA_CGROUP_MAX + 1]; struct cls_cgroup_head *head = tp->root; @@ -119,7 +119,7 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb, return err; tcf_exts_init(&e, TCA_CGROUP_ACT, TCA_CGROUP_POLICE); - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); if (err < 0) return err; diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c index 257029c..35be16f 100644 --- a/net/sched/cls_flow.c +++ b/net/sched/cls_flow.c @@ -349,7 +349,7 @@ static const struct nla_policy flow_policy[TCA_FLOW_MAX + 1] = { static int flow_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct flow_head *head = tp->root; struct flow_filter *f; @@ -393,7 +393,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb, } tcf_exts_init(&e, TCA_FLOW_ACT, TCA_FLOW_POLICE); - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); if (err < 0) return err; diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c index 63a3ce7..861b03c 100644 --- a/net/sched/cls_fw.c +++ b/net/sched/cls_fw.c @@ -169,7 +169,7 @@ static const struct nla_policy fw_policy[TCA_FW_MAX + 1] = { static int fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f, - struct nlattr **tb, struct nlattr **tca, unsigned long base) + struct nlattr **tb, struct nlattr **tca, unsigned long base, bool ovr) { struct fw_head *head = tp->root; struct tcf_exts e; @@ -177,7 +177,7 @@ fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f, int err; tcf_exts_init(&e, TCA_FW_ACT, TCA_FW_POLICE); - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); if (err < 0) return err; @@ -218,7 +218,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct fw_head *head = tp->root; struct fw_filter *f = (struct fw_filter *) *arg; @@ -236,7 +236,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, if (f != NULL) { if (f->id != handle && handle) return -EINVAL; - return fw_change_attrs(net, tp, f, tb, tca, base); + return fw_change_attrs(net, tp, f, tb, tca, base, ovr); } if (!handle) @@ -264,7 +264,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb, tcf_exts_init(&f->exts, TCA_FW_ACT, TCA_FW_POLICE); f->id = handle; - err = fw_change_attrs(net, tp, f, tb, tca, base); + err = fw_change_attrs(net, tp, f, tb, tca, base, ovr); if (err < 0) goto errout; diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c index 1ad3068..dd9fc25 100644 --- a/net/sched/cls_route.c +++ b/net/sched/cls_route.c @@ -333,7 +333,8 @@ static const struct nla_policy route4_policy[TCA_ROUTE4_MAX + 1] = { static int route4_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, struct route4_filter *f, u32 handle, struct route4_head *head, - struct nlattr **tb, struct nlattr *est, int new) + struct nlattr **tb, struct nlattr *est, int new, + bool ovr) { int err; u32 id = 0, to = 0, nhandle = 0x8000; @@ -343,7 +344,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp, struct tcf_exts e; tcf_exts_init(&e, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE); - err = tcf_exts_validate(net, tp, tb, est, &e); + err = tcf_exts_validate(net, tp, tb, est, &e, ovr); if (err < 0) return err; @@ -428,7 +429,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct route4_head *head = tp->root; struct route4_filter *f, *f1, **fp; @@ -455,7 +456,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, old_handle = f->handle; err = route4_set_parms(net, tp, base, f, handle, head, tb, - tca[TCA_RATE], 0); + tca[TCA_RATE], 0, ovr); if (err < 0) return err; @@ -479,7 +480,7 @@ static int route4_change(struct net *net, struct sk_buff *in_skb, tcf_exts_init(&f->exts, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE); err = route4_set_parms(net, tp, base, f, handle, head, tb, - tca[TCA_RATE], 1); + tca[TCA_RATE], 1, ovr); if (err < 0) goto errout; diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h index 19f8e5d..1020e23 100644 --- a/net/sched/cls_rsvp.h +++ b/net/sched/cls_rsvp.h @@ -415,7 +415,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct rsvp_head *data = tp->root; struct rsvp_filter *f, **fp; @@ -436,7 +436,7 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb, return err; tcf_exts_init(&e, TCA_RSVP_ACT, TCA_RSVP_POLICE); - err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e); + err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr); if (err < 0) return err; diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c index eed8404..d11d0a4 100644 --- a/net/sched/cls_tcindex.c +++ b/net/sched/cls_tcindex.c @@ -192,7 +192,7 @@ static int tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, u32 handle, struct tcindex_data *p, struct tcindex_filter_result *r, struct nlattr **tb, - struct nlattr *est) + struct nlattr *est, bool ovr) { int err, balloc = 0; struct tcindex_filter_result new_filter_result, *old_r = r; @@ -202,7 +202,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, struct tcf_exts e; tcf_exts_init(&e, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE); - err = tcf_exts_validate(net, tp, tb, est, &e); + err = tcf_exts_validate(net, tp, tb, est, &e, ovr); if (err < 0) return err; @@ -331,7 +331,7 @@ errout: static int tcindex_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, - struct nlattr **tca, unsigned long *arg) + struct nlattr **tca, unsigned long *arg, bool ovr) { struct nlattr *opt = tca[TCA_OPTIONS]; struct nlattr *tb[TCA_TCINDEX_MAX + 1]; @@ -351,7 +351,7 @@ tcindex_change(struct net *net, struct sk_buff *in_skb, return err; return tcindex_set_parms(net, tp, base, handle, p, r, tb, - tca[TCA_RATE]); + tca[TCA_RATE], ovr); } diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 84c28da..c39b583 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -486,13 +486,13 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = { static int u32_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base, struct tc_u_hnode *ht, struct tc_u_knode *n, struct nlattr **tb, - struct nlattr *est) + struct nlattr *est, bool ovr) { int err; struct tcf_exts e; tcf_exts_init(&e, TCA_U32_ACT, TCA_U32_POLICE); - err = tcf_exts_validate(net, tp, tb, est, &e); + err = tcf_exts_validate(net, tp, tb, est, &e, ovr); if (err < 0) return err; @@ -545,7 +545,7 @@ errout: static int u32_change(struct net *net, struct sk_buff *in_skb, struct tcf_proto *tp, unsigned long base, u32 handle, struct nlattr **tca, - unsigned long *arg) + unsigned long *arg, bool ovr) { struct tc_u_common *tp_c = tp->data; struct tc_u_hnode *ht; @@ -569,7 +569,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb, return -EINVAL; return u32_set_parms(net, tp, base, n->ht_up, n, tb, - tca[TCA_RATE]); + tca[TCA_RATE], ovr); } if (tb[TCA_U32_DIVISOR]) { @@ -656,7 +656,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb, } #endif - err = u32_set_parms(net, tp, base, ht, n, tb, tca[TCA_RATE]); + err = u32_set_parms(net, tp, base, ht, n, tb, tca[TCA_RATE], ovr); if (err == 0) { struct tc_u_knode **ins; for (ins = &ht->ht[TC_U32_HASH(handle)]; *ins; ins = &(*ins)->next)