| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <52FB6C81.2010302@mojatatu.com>
Date: Wed, 12 Feb 2014 07:43:45 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: Cong Wang <xiyou.wangcong@...il.com>, netdev@...r.kernel.org
CC: "David S. Miller" <davem@...emloft.net>
Subject: Re: [Patch net-next v3 2/5] net_sched: act: refactor cleanup ops
On 02/11/14 20:07, Cong Wang wrote:
> For bindcnt and refcnt etc., they are common for all actions,
> not need to repeat such operations for their own, they can be unified
> now. Actions just need to do its specific cleanup if needed.
>
> Cc: Jamal Hadi Salim <jhs@...atatu.com>
> Cc: David S. Miller <davem@...emloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> ---
> include/net/act_api.h | 2 +-
> net/sched/act_api.c | 8 +++++---
> net/sched/act_csum.c | 1 -
> net/sched/act_gact.c | 1 -
> net/sched/act_ipt.c | 21 +++++----------------
> net/sched/act_mirred.c | 20 +++++---------------
> net/sched/act_nat.c | 1 -
> net/sched/act_pedit.c | 13 +++----------
> net/sched/act_police.c | 1 -
> net/sched/act_simple.c | 17 +++--------------
> net/sched/act_skbedit.c | 1 -
> 11 files changed, 22 insertions(+), 64 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 24ae910..3d22f42 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -89,7 +89,7 @@ struct tc_action_ops {
> struct module *owner;
> int (*act)(struct sk_buff *, const struct tc_action *, struct tcf_result *);
> int (*dump)(struct sk_buff *, struct tc_action *, int, int);
> - int (*cleanup)(struct tc_action *, int bind);
> + void (*cleanup)(struct tc_action *, int bind);
> int (*lookup)(struct tc_action *, u32);
> int (*init)(struct net *net, struct nlattr *nla,
> struct nlattr *est, struct tc_action *act, int ovr,
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 4f2b807..a5bf935 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -56,6 +56,8 @@ int tcf_hash_release(struct tc_action *a, int bind)
>
> p->tcfc_refcnt--;
> if (p->tcfc_bindcnt <= 0 && p->tcfc_refcnt <= 0) {
> + if (a->ops->cleanup)
> + a->ops->cleanup(a, bind);
> tcf_hash_destroy(a);
> ret = 1;
> }
> @@ -277,8 +279,8 @@ int tcf_register_action(struct tc_action_ops *act)
> {
> struct tc_action_ops *a;
>
> - /* Must supply act, dump, cleanup and init */
> - if (!act->act || !act->dump || !act->cleanup || !act->init)
> + /* Must supply act, dump and init */
> + if (!act->act || !act->dump || !act->init)
> return -EINVAL;
>
> /* Supply defaults */
> @@ -390,7 +392,7 @@ void tcf_action_destroy(struct list_head *actions, int bind)
> struct tc_action *a, *tmp;
>
> list_for_each_entry_safe(a, tmp, actions, list) {
> - if (a->ops->cleanup(a, bind) == ACT_P_DELETED)
> + if (tcf_hash_release(a, bind) == ACT_P_DELETED)
> module_put(a->ops->owner);
> list_del(&a->list);
> kfree(a);
> diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
> index f0f6e7a..8df3060 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -566,7 +566,6 @@ static struct tc_action_ops act_csum_ops = {
> .owner = THIS_MODULE,
> .act = tcf_csum,
> .dump = tcf_csum_dump,
> - .cleanup = tcf_hash_release,
> .init = tcf_csum_init,
> };
>
> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
> index af6c0ac..094a1b5 100644
> --- a/net/sched/act_gact.c
> +++ b/net/sched/act_gact.c
> @@ -185,7 +185,6 @@ static struct tc_action_ops act_gact_ops = {
> .owner = THIS_MODULE,
> .act = tcf_gact,
> .dump = tcf_gact_dump,
> - .cleanup = tcf_hash_release,
> .init = tcf_gact_init,
> };
>
> diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
> index f5e6978..71f29f1 100644
> --- a/net/sched/act_ipt.c
> +++ b/net/sched/act_ipt.c
> @@ -69,23 +69,12 @@ static void ipt_destroy_target(struct xt_entry_target *t)
> module_put(par.target->me);
> }
>
> -static int tcf_ipt_release(struct tc_action *a, int bind)
> +static void tcf_ipt_release(struct tc_action *a, int bind)
> {
> struct tcf_ipt *ipt = to_ipt(a);
> - int ret = 0;
> - if (ipt) {
> - if (bind)
> - ipt->tcf_bindcnt--;
> - ipt->tcf_refcnt--;
> - if (ipt->tcf_bindcnt <= 0 && ipt->tcf_refcnt <= 0) {
> - ipt_destroy_target(ipt->tcfi_t);
> - kfree(ipt->tcfi_tname);
> - kfree(ipt->tcfi_t);
> - tcf_hash_destroy(a);
> - ret = ACT_P_DELETED;
> - }
> - }
> - return ret;
> + ipt_destroy_target(ipt->tcfi_t);
> + kfree(ipt->tcfi_tname);
> + kfree(ipt->tcfi_t);
> }
>
> static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = {
> @@ -133,7 +122,7 @@ static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est,
> } else {
> if (bind)/* dont override defaults */
> return 0;
> - tcf_ipt_release(a, bind);
> + tcf_hash_release(a, bind);
>
> if (!ovr)
> return -EEXIST;
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 3edeeca..0f00eb9 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -33,22 +33,12 @@
> static LIST_HEAD(mirred_list);
> static struct tcf_hashinfo mirred_hash_info;
>
> -static int tcf_mirred_release(struct tc_action *a, int bind)
> +static void tcf_mirred_release(struct tc_action *a, int bind)
> {
> struct tcf_mirred *m = to_mirred(a);
> - if (m) {
> - if (bind)
> - m->tcf_bindcnt--;
> - m->tcf_refcnt--;
> - if (!m->tcf_bindcnt && m->tcf_refcnt <= 0) {
> - list_del(&m->tcfm_list);
> - if (m->tcfm_dev)
> - dev_put(m->tcfm_dev);
> - tcf_hash_destroy(a);
> - return 1;
> - }
> - }
> - return 0;
> + list_del(&m->tcfm_list);
> + if (m->tcfm_dev)
> + dev_put(m->tcfm_dev);
> }
>
> static const struct nla_policy mirred_policy[TCA_MIRRED_MAX + 1] = {
> @@ -110,7 +100,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
> ret = ACT_P_CREATED;
> } else {
> if (!ovr) {
> - tcf_mirred_release(a, bind);
> + tcf_hash_release(a, bind);
> return -EEXIST;
> }
> }
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index ce9a391..9a3cb1d 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -289,7 +289,6 @@ static struct tc_action_ops act_nat_ops = {
> .owner = THIS_MODULE,
> .act = tcf_nat,
> .dump = tcf_nat_dump,
> - .cleanup = tcf_hash_release,
> .init = tcf_nat_init,
> };
>
> diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> index 091ced3..8aa795b 100644
> --- a/net/sched/act_pedit.c
> +++ b/net/sched/act_pedit.c
> @@ -99,18 +99,11 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
> return ret;
> }
>
> -static int tcf_pedit_cleanup(struct tc_action *a, int bind)
> +static void tcf_pedit_cleanup(struct tc_action *a, int bind)
> {
> struct tcf_pedit *p = a->priv;
> -
> - if (p) {
> - struct tc_pedit_key *keys = p->tcfp_keys;
> - if (tcf_hash_release(a, bind)) {
> - kfree(keys);
> - return 1;
> - }
> - }
> - return 0;
> + struct tc_pedit_key *keys = p->tcfp_keys;
> + kfree(keys);
> }
>
> static int tcf_pedit(struct sk_buff *skb, const struct tc_action *a,
> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index 4695d02..7ff7bef 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -354,7 +354,6 @@ static struct tc_action_ops act_police_ops = {
> .owner = THIS_MODULE,
> .act = tcf_act_police,
> .dump = tcf_act_police_dump,
> - .cleanup = tcf_hash_release,
> .init = tcf_act_police_locate,
> .walk = tcf_act_police_walker
> };
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index 11c2922..14b5e36 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -47,21 +47,10 @@ static int tcf_simp(struct sk_buff *skb, const struct tc_action *a,
> return d->tcf_action;
> }
>
> -static int tcf_simp_release(struct tc_action *a, int bind)
> +static void tcf_simp_release(struct tc_action *a, int bind)
> {
> struct tcf_defact *d = to_defact(a);
> - int ret = 0;
> - if (d) {
> - if (bind)
> - d->tcf_bindcnt--;
> - d->tcf_refcnt--;
> - if (d->tcf_bindcnt <= 0 && d->tcf_refcnt <= 0) {
> - kfree(d->tcfd_defdata);
> - tcf_hash_destroy(a);
> - ret = 1;
> - }
> - }
> - return ret;
> + kfree(d->tcfd_defdata);
> }
>
> static int alloc_defdata(struct tcf_defact *d, char *defdata)
> @@ -132,7 +121,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
>
> if (bind)
> return 0;
> - tcf_simp_release(a, bind);
> + tcf_hash_release(a, bind);
> if (!ovr)
> return -EEXIST;
>
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index 71fd2d4..9f91928 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -180,7 +180,6 @@ static struct tc_action_ops act_skbedit_ops = {
> .owner = THIS_MODULE,
> .act = tcf_skbedit,
> .dump = tcf_skbedit_dump,
> - .cleanup = tcf_hash_release,
> .init = tcf_skbedit_init,
> };
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists