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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ