| Message ID | 52B6DC9B.9060309@mojatatu.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Jamal Hadi Salim <jhs@mojatatu.com> Date: Sun, 22 Dec 2013 07:35:39 -0500 > This bug seems to have been around for some time, nothing to do with > recent changes; i am just doing tests i havent run in a while and > saw it. It is against net-next. If you think it is important enough, > I could also create one against linus tree.. Jamal, I've tried to be patient with you over the years, but I really need you to start submitting your changes more cleanly. Your SUBJECT line MUST be the first line of the commit log message, optionally with a prefix of the form "[PATCH xxx]" where "xxx" can be "net", "net-next" and also contain versioning information such as "v2" or "v3" etc. Then your email message body must contain the commit message, with no indentation whatsoever. Then you may optionally provide a line with "---" then supplementary information and text which you don't wish to end up in the final commit message. Then you provide the patch. And this all must be inline and not as an attachment. What I get from you is the output of "git show" as an attachment and I absolutely cannot use that as-is. It idents the commit message, amongst other things. You can use "git format-patch" instead and munge it into a proper email by hand. Unlike "git show", "git format-patch" won't indent the commit message. Please take a few moments to read other netdev patch postings and see how regulars do it. They do what works for me. Thank you. -- 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 12/22/13 17:50, David Miller wrote: > From: Jamal Hadi Salim <jhs@mojatatu.com> > Date: Sun, 22 Dec 2013 07:35:39 -0500 > >> This bug seems to have been around for some time, nothing to do with >> recent changes; i am just doing tests i havent run in a while and >> saw it. It is against net-next. If you think it is important enough, >> I could also create one against linus tree.. > > Jamal, I've tried to be patient with you over the years, but I really > need you to start submitting your changes more cleanly. > Ok, let me know how this one went ;-> I think i know how to send this way if you find it fits your workflow. 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
From: Jamal Hadi Salim <jhs@mojatatu.com> Date: Mon, 23 Dec 2013 08:07:12 -0500 > Ok, let me know how this one went ;-> > I think i know how to send this way if you find it fits your workflow. Looks better, for sure. But somehow got lost on the way to patchwork. -- 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/net/sched/act_csum.c b/net/sched/act_csum.c index 9cc6717..8b1d657 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -70,16 +70,16 @@ static int tcf_csum_init(struct net *n, struct nlattr *nla, struct nlattr *est, &csum_idx_gen, &csum_hash_info); if (IS_ERR(pc)) return PTR_ERR(pc); - p = to_tcf_csum(pc); ret = ACT_P_CREATED; } else { - p = to_tcf_csum(pc); - if (!ovr) { - tcf_hash_release(pc, bind, &csum_hash_info); + if (bind)/* dont override defaults */ + return 0; + tcf_hash_release(pc, bind, &csum_hash_info); + if (!ovr) return -EEXIST; - } } + p = to_tcf_csum(pc); spin_lock_bh(&p->tcf_lock); p->tcf_action = parm->action; p->update_flags = parm->update_flags; diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index dea9273..af5641c 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -95,10 +95,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla, return PTR_ERR(pc); ret = ACT_P_CREATED; } else { - if (!ovr) { - tcf_hash_release(pc, bind, &gact_hash_info); + if (bind)/* dont override defaults */ + return 0; + tcf_hash_release(pc, bind, &gact_hash_info); + if (!ovr) return -EEXIST; - } } gact = to_gact(pc); diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c index e13ecbb..2426369 100644 --- a/net/sched/act_ipt.c +++ b/net/sched/act_ipt.c @@ -134,10 +134,12 @@ static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est, return PTR_ERR(pc); ret = ACT_P_CREATED; } else { - if (!ovr) { - tcf_ipt_release(to_ipt(pc), bind); + if (bind)/* dont override defaults */ + return 0; + tcf_ipt_release(to_ipt(pc), bind); + + if (!ovr) return -EEXIST; - } } ipt = to_ipt(pc); diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c index 921fea4..584e655 100644 --- a/net/sched/act_nat.c +++ b/net/sched/act_nat.c @@ -64,15 +64,15 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est, &nat_idx_gen, &nat_hash_info); if (IS_ERR(pc)) return PTR_ERR(pc); - p = to_tcf_nat(pc); ret = ACT_P_CREATED; } else { - p = to_tcf_nat(pc); - if (!ovr) { - tcf_hash_release(pc, bind, &nat_hash_info); + if (bind) + return 0; + tcf_hash_release(pc, bind, &nat_hash_info); + if (!ovr) return -EEXIST; - } } + p = to_tcf_nat(pc); spin_lock_bh(&p->tcf_lock); p->old_addr = parm->old_addr; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index e2520e9..7291893 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -78,10 +78,12 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla, ret = ACT_P_CREATED; } else { p = to_pedit(pc); - if (!ovr) { - tcf_hash_release(pc, bind, &pedit_hash_info); + tcf_hash_release(pc, bind, &pedit_hash_info); + if (bind) + return 0; + if (!ovr) return -EEXIST; - } + if (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys) { keys = kmalloc(ksize, GFP_KERNEL); if (keys == NULL) diff --git a/net/sched/act_police.c b/net/sched/act_police.c index 819a9a4..9295b86 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -162,10 +162,12 @@ static int tcf_act_police_locate(struct net *net, struct nlattr *nla, if (bind) { police->tcf_bindcnt += 1; police->tcf_refcnt += 1; + return 0; } if (ovr) goto override; - return ret; + /* not replacing */ + return -EEXIST; } } diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 81aebc1..b44491e 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -135,10 +135,13 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla, ret = ACT_P_CREATED; } else { d = to_defact(pc); - if (!ovr) { - tcf_simp_release(d, bind); + + if (bind) + return 0; + tcf_simp_release(d, bind); + if (!ovr) return -EEXIST; - } + reset_policy(d, defdata, parm); } diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index aa0a4c0..0fa1aad 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -112,10 +112,11 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla, ret = ACT_P_CREATED; } else { d = to_skbedit(pc); - if (!ovr) { - tcf_hash_release(pc, bind, &skbedit_hash_info); + if (bind) + return 0; + tcf_hash_release(pc, bind, &skbedit_hash_info); + if (!ovr) return -EEXIST; - } } spin_lock_bh(&d->tcf_lock);
Dave, This bug seems to have been around for some time, nothing to do with recent changes; i am just doing tests i havent run in a while and saw it. It is against net-next. If you think it is important enough, I could also create one against linus tree.. cheers, jamal commit c845c05bed4834616deec4b3c504946326ecfc44 Author: Jamal Hadi Salim <hadi@mojatatu.com> Date: Sun Dec 22 07:15:57 2013 -0500 This is a bug fix. The existing code tries to kill many birds with one stone: Handling binding of actions to filters, new actions and replacing of action attributes. A simple test case to illustrate: ---- moja@fe1:~$ sudo tc actions add action drop index 12 moja@fe1:~$ actions get action gact index 12 action order 1: gact action drop random type none pass val 0 index 12 ref 1 bind 0 moja@fe1:~$ sudo tc actions replace action ok index 12 moja@fe1:~$ actions get action gact index 12 action order 1: gact action pass random type none pass val 0 index 12 ref 2 bind 0 ---- The above shows the refcounf being wrongly incremented on replace. There are more complex scenarios with binding of actions to filters that i am leaving out that didnt work as well... Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>