[Patch net-next 0/7] net_sched: some more cleanup and improvements

[Patch net-next 0/7] net_sched: some more cleanup and improvements

[Cong Wang]

This patchset collects the previous patches I sent which Jamal doesn't object.
They are still some cleanup and improvements for tc actions and filters.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Cong Wang (7):
  net_sched: act: move idx_gen into struct tcf_hashinfo
  net_sched: act: clean up notification functions
  net_sched: add struct net pointer to tcf_proto_ops->dump
  net_sched: optimize tcf_match_indev()
  net_sched: avoid casting void pointer
  net_sched: cls: move allocation in ->init to generic layer
  net_sched: act: remove struct tcf_act_hdr

 include/net/act_api.h     |  11 ++---
 include/net/pkt_cls.h     |  30 ++++++------
 include/net/sch_generic.h |   3 +-
 net/sched/act_api.c       | 121 ++++++++++++++++++++--------------------------
 net/sched/act_csum.c      |   3 +-
 net/sched/act_gact.c      |   3 +-
 net/sched/act_ipt.c       |   3 +-
 net/sched/act_mirred.c    |   3 +-
 net/sched/act_nat.c       |   3 +-
 net/sched/act_pedit.c     |   3 +-
 net/sched/act_police.c    |   3 +-
 net/sched/act_simple.c    |   3 +-
 net/sched/act_skbedit.c   |   3 +-
 net/sched/cls_api.c       |  18 +++++--
 net/sched/cls_basic.c     |  20 +++-----
 net/sched/cls_bpf.c       |  13 ++---
 net/sched/cls_cgroup.c    |  23 +++------
 net/sched/cls_flow.c      |  10 ++--
 net/sched/cls_fw.c        |  49 +++++++++----------
 net/sched/cls_route.c     |  23 +++------
 net/sched/cls_rsvp.h      |  12 ++---
 net/sched/cls_tcindex.c   |  32 ++++--------
 net/sched/cls_u32.c       |  32 ++++++------
 net/sched/sch_api.c       |   1 +
 24 files changed, 178 insertions(+), 247 deletions(-)

-- 
1.8.3.1

[Patch net-next 1/7] net_sched: act: move idx_gen into struct tcf_hashinfo

[Cong Wang]

There is no need to store the index separatedly
since tcf_hashinfo is allocated statically too.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/act_api.h   |  7 ++++---
 net/sched/act_api.c     | 10 +++++-----
 net/sched/act_csum.c    |  3 +--
 net/sched/act_gact.c    |  3 +--
 net/sched/act_ipt.c     |  3 +--
 net/sched/act_mirred.c  |  3 +--
 net/sched/act_nat.c     |  3 +--
 net/sched/act_pedit.c   |  3 +--
 net/sched/act_police.c  |  3 +--
 net/sched/act_simple.c  |  3 +--
 net/sched/act_skbedit.c |  3 +--
 11 files changed, 18 insertions(+), 26 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index d34e1f4..268f9e6 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -39,6 +39,7 @@ struct tcf_hashinfo {
 	struct hlist_head	*htab;
 	unsigned int		hmask;
 	spinlock_t		lock;
+	u32			index;
 };
 
 static inline unsigned int tcf_hash(u32 index, unsigned int hmask)
@@ -51,6 +52,7 @@ static inline int tcf_hashinfo_init(struct tcf_hashinfo *hf, unsigned int mask)
 	int i;
 
 	spin_lock_init(&hf->lock);
+	hf->index = 0;
 	hf->hmask = mask;
 	hf->htab = kzalloc((mask + 1) * sizeof(struct hlist_head),
 			   GFP_KERNEL);
@@ -105,13 +107,12 @@ struct tcf_common *tcf_hash_lookup(u32 index, struct tcf_hashinfo *hinfo);
 void tcf_hash_destroy(struct tcf_common *p, struct tcf_hashinfo *hinfo);
 int tcf_hash_release(struct tcf_common *p, int bind,
 		     struct tcf_hashinfo *hinfo);
-u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo);
+u32 tcf_hash_new_index(struct tcf_hashinfo *hinfo);
 struct tcf_common *tcf_hash_check(u32 index, struct tc_action *a,
 				  int bind, struct tcf_hashinfo *hinfo);
 struct tcf_common *tcf_hash_create(u32 index, struct nlattr *est,
 				   struct tc_action *a, int size,
-				   int bind, u32 *idx_gen,
-				   struct tcf_hashinfo *hinfo);
+				   int bind, struct tcf_hashinfo *hinfo);
 void tcf_hash_insert(struct tcf_common *p, struct tcf_hashinfo *hinfo);
 
 int tcf_register_action(struct tc_action_ops *a);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f63e146..74ae3ff 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -173,16 +173,16 @@ struct tcf_common *tcf_hash_lookup(u32 index, struct tcf_hashinfo *hinfo)
 }
 EXPORT_SYMBOL(tcf_hash_lookup);
 
-u32 tcf_hash_new_index(u32 *idx_gen, struct tcf_hashinfo *hinfo)
+u32 tcf_hash_new_index(struct tcf_hashinfo *hinfo)
 {
-	u32 val = *idx_gen;
+	u32 val = hinfo->index;
 
 	do {
 		if (++val == 0)
 			val = 1;
 	} while (tcf_hash_lookup(val, hinfo));
 
-	*idx_gen = val;
+	hinfo->index = val;
 	return val;
 }
 EXPORT_SYMBOL(tcf_hash_new_index);
@@ -215,7 +215,7 @@ EXPORT_SYMBOL(tcf_hash_check);
 
 struct tcf_common *tcf_hash_create(u32 index, struct nlattr *est,
 				   struct tc_action *a, int size, int bind,
-				   u32 *idx_gen, struct tcf_hashinfo *hinfo)
+				   struct tcf_hashinfo *hinfo)
 {
 	struct tcf_common *p = kzalloc(size, GFP_KERNEL);
 
@@ -227,7 +227,7 @@ struct tcf_common *tcf_hash_create(u32 index, struct nlattr *est,
 
 	spin_lock_init(&p->tcfc_lock);
 	INIT_HLIST_NODE(&p->tcfc_head);
-	p->tcfc_index = index ? index : tcf_hash_new_index(idx_gen, hinfo);
+	p->tcfc_index = index ? index : tcf_hash_new_index(hinfo);
 	p->tcfc_tm.install = jiffies;
 	p->tcfc_tm.lastuse = jiffies;
 	if (est) {
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 8b1d657..ee28e1c 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -37,7 +37,6 @@
 #include <net/tc_act/tc_csum.h>
 
 #define CSUM_TAB_MASK 15
-static u32 csum_idx_gen;
 static struct tcf_hashinfo csum_hash_info;
 
 static const struct nla_policy csum_policy[TCA_CSUM_MAX + 1] = {
@@ -67,7 +66,7 @@ static int tcf_csum_init(struct net *n, struct nlattr *nla, struct nlattr *est,
 	pc = tcf_hash_check(parm->index, a, bind, &csum_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
-				     &csum_idx_gen, &csum_hash_info);
+				     &csum_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index af5641c..f26e6b8 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -24,7 +24,6 @@
 #include <net/tc_act/tc_gact.h>
 
 #define GACT_TAB_MASK	15
-static u32 gact_idx_gen;
 static struct tcf_hashinfo gact_hash_info;
 
 #ifdef CONFIG_GACT_PROB
@@ -90,7 +89,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
 	pc = tcf_hash_check(parm->index, a, bind, &gact_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*gact),
-				     bind, &gact_idx_gen, &gact_hash_info);
+				     bind, &gact_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 2426369..484bd19 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -29,7 +29,6 @@
 
 
 #define IPT_TAB_MASK     15
-static u32 ipt_idx_gen;
 static struct tcf_hashinfo ipt_hash_info;
 
 static int ipt_init_target(struct xt_entry_target *t, char *table, unsigned int hook)
@@ -129,7 +128,7 @@ static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est,
 	pc = tcf_hash_check(index, a, bind, &ipt_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(index, est, a, sizeof(*ipt), bind,
-				     &ipt_idx_gen, &ipt_hash_info);
+				     &ipt_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 9dbb8cd..5d05b57 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -30,7 +30,6 @@
 #include <linux/if_arp.h>
 
 #define MIRRED_TAB_MASK     7
-static u32 mirred_idx_gen;
 static LIST_HEAD(mirred_list);
 static struct tcf_hashinfo mirred_hash_info;
 
@@ -107,7 +106,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
 		if (dev == NULL)
 			return -EINVAL;
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*m), bind,
-				     &mirred_idx_gen, &mirred_hash_info);
+				     &mirred_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 584e655..a49fa23 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -30,7 +30,6 @@
 
 
 #define NAT_TAB_MASK	15
-static u32 nat_idx_gen;
 
 static struct tcf_hashinfo nat_hash_info;
 
@@ -61,7 +60,7 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
 	pc = tcf_hash_check(parm->index, a, bind, &nat_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
-				     &nat_idx_gen, &nat_hash_info);
+				     &nat_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		ret = ACT_P_CREATED;
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index 7291893..f361e4e 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -24,7 +24,6 @@
 #include <net/tc_act/tc_pedit.h>
 
 #define PEDIT_TAB_MASK	15
-static u32 pedit_idx_gen;
 
 static struct tcf_hashinfo pedit_hash_info;
 
@@ -63,7 +62,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 		if (!parm->nkeys)
 			return -EINVAL;
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*p), bind,
-				     &pedit_idx_gen, &pedit_hash_info);
+				     &pedit_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 		p = to_pedit(pc);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 9295b86..a719fdf 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -41,7 +41,6 @@ struct tcf_police {
 	container_of(pc, struct tcf_police, common)
 
 #define POL_TAB_MASK     15
-static u32 police_idx_gen;
 static struct tcf_hashinfo police_hash_info;
 
 /* old policer structure from before tc actions */
@@ -251,7 +250,7 @@ override:
 
 	police->tcfp_t_c = ktime_to_ns(ktime_get());
 	police->tcf_index = parm->index ? parm->index :
-		tcf_hash_new_index(&police_idx_gen, &police_hash_info);
+		tcf_hash_new_index(&police_hash_info);
 	h = tcf_hash(police->tcf_index, POL_TAB_MASK);
 	spin_lock_bh(&police_hash_info.lock);
 	hlist_add_head(&police->tcf_head, &police_hash_info.htab[h]);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index b44491e..f7d5406 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -25,7 +25,6 @@
 #include <net/tc_act/tc_defact.h>
 
 #define SIMP_TAB_MASK     7
-static u32 simp_idx_gen;
 static struct tcf_hashinfo simp_hash_info;
 
 #define SIMP_MAX_DATA	32
@@ -118,7 +117,7 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
 	pc = tcf_hash_check(parm->index, a, bind, &simp_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*d), bind,
-				     &simp_idx_gen, &simp_hash_info);
+				     &simp_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 0fa1aad..74af461 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -28,7 +28,6 @@
 #include <net/tc_act/tc_skbedit.h>
 
 #define SKBEDIT_TAB_MASK     15
-static u32 skbedit_idx_gen;
 static struct tcf_hashinfo skbedit_hash_info;
 
 static int tcf_skbedit(struct sk_buff *skb, const struct tc_action *a,
@@ -104,7 +103,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
 	pc = tcf_hash_check(parm->index, a, bind, &skbedit_hash_info);
 	if (!pc) {
 		pc = tcf_hash_create(parm->index, est, a, sizeof(*d), bind,
-				     &skbedit_idx_gen, &skbedit_hash_info);
+				     &skbedit_hash_info);
 		if (IS_ERR(pc))
 			return PTR_ERR(pc);
 
-- 
1.8.3.1

[Patch net-next 2/7] net_sched: act: clean up notification functions

[Cong Wang]

Refactor tcf_add_notify() and factor out tcf_del_notify().

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/act_api.c | 95 ++++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 55 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 74ae3ff..178bf2e 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -789,6 +789,33 @@ noflush_out:
 }
 
 static int
+tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+	       u32 portid)
+{
+	int ret;
+	struct sk_buff *skb;
+
+	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
+	if (!skb)
+		return -ENOBUFS;
+
+	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
+			 0, 1) <= 0) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
+
+	/* now do the delete */
+	tcf_action_destroy(actions, 0);
+
+	ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+			     n->nlmsg_flags & NLM_F_ECHO);
+	if (ret > 0)
+		return 0;
+	return ret;
+}
+
+static int
 tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	      u32 portid, int event)
 {
@@ -821,27 +848,9 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	if (event == RTM_GETACTION)
 		ret = act_get_notify(net, portid, n, &actions, event);
 	else { /* delete */
-		struct sk_buff *skb;
-
-		skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
-		if (!skb) {
-			ret = -ENOBUFS;
-			goto err;
-		}
-
-		if (tca_get_fill(skb, &actions, portid, n->nlmsg_seq, 0, event,
-				 0, 1) <= 0) {
-			kfree_skb(skb);
-			ret = -EINVAL;
+		ret = tcf_del_notify(net, n, &actions, portid);
+		if (ret)
 			goto err;
-		}
-
-		/* now do the delete */
-		tcf_action_destroy(&actions, 0);
-		ret = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
-				     n->nlmsg_flags & NLM_F_ECHO);
-		if (ret > 0)
-			return 0;
 		return ret;
 	}
 err:
@@ -849,60 +858,36 @@ err:
 	return ret;
 }
 
-static int tcf_add_notify(struct net *net, struct list_head *actions,
-			  u32 portid, u32 seq, int event, u16 flags)
+static int
+tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+	       u32 portid)
 {
-	struct tcamsg *t;
-	struct nlmsghdr *nlh;
 	struct sk_buff *skb;
-	struct nlattr *nest;
-	unsigned char *b;
 	int err = 0;
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb)
 		return -ENOBUFS;
 
-	b = skb_tail_pointer(skb);
-
-	nlh = nlmsg_put(skb, portid, seq, event, sizeof(*t), flags);
-	if (!nlh)
-		goto out_kfree_skb;
-	t = nlmsg_data(nlh);
-	t->tca_family = AF_UNSPEC;
-	t->tca__pad1 = 0;
-	t->tca__pad2 = 0;
-
-	nest = nla_nest_start(skb, TCA_ACT_TAB);
-	if (nest == NULL)
-		goto out_kfree_skb;
-
-	if (tcf_action_dump(skb, actions, 0, 0) < 0)
-		goto out_kfree_skb;
-
-	nla_nest_end(skb, nest);
-
-	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
-	NETLINK_CB(skb).dst_group = RTNLGRP_TC;
+	if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, n->nlmsg_flags,
+			 RTM_NEWACTION, 0, 0) <= 0) {
+		kfree_skb(skb);
+		return -EINVAL;
+	}
 
-	err = rtnetlink_send(skb, net, portid, RTNLGRP_TC, flags & NLM_F_ECHO);
+	err = rtnetlink_send(skb, net, portid, RTNLGRP_TC,
+			     n->nlmsg_flags & NLM_F_ECHO);
 	if (err > 0)
 		err = 0;
 	return err;
-
-out_kfree_skb:
-	kfree_skb(skb);
-	return -1;
 }
 
-
 static int
 tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	       u32 portid, int ovr)
 {
 	int ret = 0;
 	LIST_HEAD(actions);
-	u32 seq = n->nlmsg_seq;
 
 	ret = tcf_action_init(net, nla, NULL, NULL, ovr, 0, &actions);
 	if (ret)
@@ -911,7 +896,7 @@ tcf_action_add(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
 	/* dump then free all the actions after update; inserted policy
 	 * stays intact
 	 */
-	ret = tcf_add_notify(net, &actions, portid, seq, RTM_NEWACTION, n->nlmsg_flags);
+	ret = tcf_add_notify(net, n, &actions, portid);
 	cleanup_a(&actions);
 done:
 	return ret;
-- 
1.8.3.1

[Patch net-next 3/7] net_sched: add struct net pointer to tcf_proto_ops->dump

[Cong Wang]

It will be needed by the next patch.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/sch_generic.h |  2 +-
 net/sched/cls_api.c       | 11 ++++++-----
 net/sched/cls_basic.c     |  2 +-
 net/sched/cls_bpf.c       |  2 +-
 net/sched/cls_cgroup.c    |  2 +-
 net/sched/cls_flow.c      |  2 +-
 net/sched/cls_fw.c        |  2 +-
 net/sched/cls_route.c     |  2 +-
 net/sched/cls_rsvp.h      |  2 +-
 net/sched/cls_tcindex.c   |  2 +-
 net/sched/cls_u32.c       |  2 +-
 11 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 013d96d..d062f81 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -204,7 +204,7 @@ struct tcf_proto_ops {
 	void			(*walk)(struct tcf_proto*, struct tcf_walker *arg);
 
 	/* rtnetlink specific */
-	int			(*dump)(struct tcf_proto*, unsigned long,
+	int			(*dump)(struct net*, struct tcf_proto*, unsigned long,
 					struct sk_buff *skb, struct tcmsg*);
 
 	struct module		*owner;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d8c42b1..29a30a1 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -340,7 +340,7 @@ errout:
 	return err;
 }
 
-static int tcf_fill_node(struct sk_buff *skb, struct tcf_proto *tp,
+static int tcf_fill_node(struct net *net, struct sk_buff *skb, struct tcf_proto *tp,
 			 unsigned long fh, u32 portid, u32 seq, u16 flags, int event)
 {
 	struct tcmsg *tcm;
@@ -362,7 +362,7 @@ static int tcf_fill_node(struct sk_buff *skb, struct tcf_proto *tp,
 	tcm->tcm_handle = fh;
 	if (RTM_DELTFILTER != event) {
 		tcm->tcm_handle = 0;
-		if (tp->ops->dump && tp->ops->dump(tp, fh, skb, tcm) < 0)
+		if (tp->ops->dump && tp->ops->dump(net, tp, fh, skb, tcm) < 0)
 			goto nla_put_failure;
 	}
 	nlh->nlmsg_len = skb_tail_pointer(skb) - b;
@@ -385,7 +385,7 @@ static int tfilter_notify(struct net *net, struct sk_buff *oskb,
 	if (!skb)
 		return -ENOBUFS;
 
-	if (tcf_fill_node(skb, tp, fh, portid, n->nlmsg_seq, 0, event) <= 0) {
+	if (tcf_fill_node(net, skb, tp, fh, portid, n->nlmsg_seq, 0, event) <= 0) {
 		kfree_skb(skb);
 		return -EINVAL;
 	}
@@ -404,8 +404,9 @@ static int tcf_node_dump(struct tcf_proto *tp, unsigned long n,
 			 struct tcf_walker *arg)
 {
 	struct tcf_dump_args *a = (void *)arg;
+	struct net *net = sock_net(a->skb->sk);
 
-	return tcf_fill_node(a->skb, tp, n, NETLINK_CB(a->cb->skb).portid,
+	return tcf_fill_node(net, a->skb, tp, n, NETLINK_CB(a->cb->skb).portid,
 			     a->cb->nlh->nlmsg_seq, NLM_F_MULTI, RTM_NEWTFILTER);
 }
 
@@ -463,7 +464,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
 		if (t > s_t)
 			memset(&cb->args[1], 0, sizeof(cb->args)-sizeof(cb->args[0]));
 		if (cb->args[1] == 0) {
-			if (tcf_fill_node(skb, tp, 0, NETLINK_CB(cb->skb).portid,
+			if (tcf_fill_node(net, skb, tp, 0, NETLINK_CB(cb->skb).portid,
 					  cb->nlh->nlmsg_seq, NLM_F_MULTI,
 					  RTM_NEWTFILTER) <= 0)
 				break;
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index b655203..af6bea1 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -241,7 +241,7 @@ skip:
 	}
 }
 
-static int basic_dump(struct tcf_proto *tp, unsigned long fh,
+static int basic_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		      struct sk_buff *skb, struct tcmsg *t)
 {
 	struct basic_filter *f = (struct basic_filter *) fh;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 00a5a58..8e3cf49 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -295,7 +295,7 @@ errout:
 	return ret;
 }
 
-static int cls_bpf_dump(struct tcf_proto *tp, unsigned long fh,
+static int cls_bpf_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 			struct sk_buff *skb, struct tcmsg *tm)
 {
 	struct cls_bpf_prog *prog = (struct cls_bpf_prog *) fh;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 8349fcd..8e2158a 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -164,7 +164,7 @@ skip:
 	arg->count++;
 }
 
-static int cls_cgroup_dump(struct tcf_proto *tp, unsigned long fh,
+static int cls_cgroup_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 			   struct sk_buff *skb, struct tcmsg *t)
 {
 	struct cls_cgroup_head *head = tp->root;
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index dfd18a5..257029c 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -563,7 +563,7 @@ static void flow_put(struct tcf_proto *tp, unsigned long f)
 {
 }
 
-static int flow_dump(struct tcf_proto *tp, unsigned long fh,
+static int flow_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		     struct sk_buff *skb, struct tcmsg *t)
 {
 	struct flow_filter *f = (struct flow_filter *)fh;
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 3f9cece..d605285 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -324,7 +324,7 @@ static void fw_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 	}
 }
 
-static int fw_dump(struct tcf_proto *tp, unsigned long fh,
+static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		   struct sk_buff *skb, struct tcmsg *t)
 {
 	struct fw_head *head = (struct fw_head *)tp->root;
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 2473953..c913613 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -551,7 +551,7 @@ static void route4_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 	}
 }
 
-static int route4_dump(struct tcf_proto *tp, unsigned long fh,
+static int route4_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		       struct sk_buff *skb, struct tcmsg *t)
 {
 	struct route4_filter *f = (struct route4_filter *)fh;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 4f25c2a..19f8e5d 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -594,7 +594,7 @@ static void rsvp_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 	}
 }
 
-static int rsvp_dump(struct tcf_proto *tp, unsigned long fh,
+static int rsvp_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		     struct sk_buff *skb, struct tcmsg *t)
 {
 	struct rsvp_filter *f = (struct rsvp_filter *)fh;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index ffad187..f575353 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -422,7 +422,7 @@ static void tcindex_destroy(struct tcf_proto *tp)
 }
 
 
-static int tcindex_dump(struct tcf_proto *tp, unsigned long fh,
+static int tcindex_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
     struct sk_buff *skb, struct tcmsg *t)
 {
 	struct tcindex_data *p = PRIV(tp);
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 20f2fb7..e25411a 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -712,7 +712,7 @@ static void u32_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 	}
 }
 
-static int u32_dump(struct tcf_proto *tp, unsigned long fh,
+static int u32_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		     struct sk_buff *skb, struct tcmsg *t)
 {
 	struct tc_u_knode *n = (struct tc_u_knode *)fh;
-- 
1.8.3.1

[Patch net-next 4/7] net_sched: optimize tcf_match_indev()

[Cong Wang]

tcf_match_indev() is called in fast path, it is not wise to
search for a netdev by ifindex and then compare by its name,
just compare the ifindex.

Also, dev->name could be changed by user-space, therefore
the match would be always fail, but dev->ifindex could
be consistent.

BTW, this will also save some bytes from the core struct of u32.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/pkt_cls.h | 30 +++++++++++++++---------------
 net/sched/cls_fw.c    | 19 ++++++++++++-------
 net/sched/cls_u32.c   | 19 ++++++++++++-------
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 50ea079..a2441fb 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -338,27 +338,27 @@ static inline int tcf_valid_offset(const struct sk_buff *skb,
 #include <net/net_namespace.h>
 
 static inline int
-tcf_change_indev(struct tcf_proto *tp, char *indev, struct nlattr *indev_tlv)
+tcf_change_indev(struct net *net, struct nlattr *indev_tlv)
 {
+	char indev[IFNAMSIZ];
+	struct net_device *dev;
+
 	if (nla_strlcpy(indev, indev_tlv, IFNAMSIZ) >= IFNAMSIZ)
 		return -EINVAL;
-	return 0;
+	dev = __dev_get_by_name(net, indev);
+	if (!dev)
+		return -ENODEV;
+	return dev->ifindex;
 }
 
-static inline int
-tcf_match_indev(struct sk_buff *skb, char *indev)
+static inline bool
+tcf_match_indev(struct sk_buff *skb, int ifindex)
 {
-	struct net_device *dev;
-
-	if (indev[0]) {
-		if  (!skb->skb_iif)
-			return 0;
-		dev = __dev_get_by_index(dev_net(skb->dev), skb->skb_iif);
-		if (!dev || strcmp(indev, dev->name))
-			return 0;
-	}
-
-	return 1;
+	if (!ifindex)
+		return true;
+	if  (!skb->skb_iif)
+		return false;
+	return ifindex == skb->skb_iif;
 }
 #endif /* CONFIG_NET_CLS_IND */
 
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index d605285..ca662aa 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -41,7 +41,7 @@ struct fw_filter {
 	u32			id;
 	struct tcf_result	res;
 #ifdef CONFIG_NET_CLS_IND
-	char			indev[IFNAMSIZ];
+	int			ifindex;
 #endif /* CONFIG_NET_CLS_IND */
 	struct tcf_exts		exts;
 };
@@ -86,7 +86,7 @@ static int fw_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			if (f->id == id) {
 				*res = f->res;
 #ifdef CONFIG_NET_CLS_IND
-				if (!tcf_match_indev(skb, f->indev))
+				if (!tcf_match_indev(skb, f->ifindex))
 					continue;
 #endif /* CONFIG_NET_CLS_IND */
 				r = tcf_exts_exec(skb, &f->exts, res);
@@ -207,9 +207,11 @@ fw_change_attrs(struct net *net, struct tcf_proto *tp, struct fw_filter *f,
 
 #ifdef CONFIG_NET_CLS_IND
 	if (tb[TCA_FW_INDEV]) {
-		err = tcf_change_indev(tp, f->indev, tb[TCA_FW_INDEV]);
-		if (err < 0)
+		int ret;
+		ret = tcf_change_indev(net, tb[TCA_FW_INDEV]);
+		if (ret < 0)
 			goto errout;
+		f->ifindex = ret;
 	}
 #endif /* CONFIG_NET_CLS_IND */
 
@@ -348,9 +350,12 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 	    nla_put_u32(skb, TCA_FW_CLASSID, f->res.classid))
 		goto nla_put_failure;
 #ifdef CONFIG_NET_CLS_IND
-	if (strlen(f->indev) &&
-	    nla_put_string(skb, TCA_FW_INDEV, f->indev))
-		goto nla_put_failure;
+	if (f->ifindex) {
+		struct net_device *dev;
+		dev = __dev_get_by_index(net, f->ifindex);
+		if (dev && nla_put_string(skb, TCA_FW_INDEV, dev->name))
+			goto nla_put_failure;
+	}
 #endif /* CONFIG_NET_CLS_IND */
 	if (head->mask != 0xFFFFFFFF &&
 	    nla_put_u32(skb, TCA_FW_MASK, head->mask))
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e25411a..f509b79 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -48,7 +48,7 @@ struct tc_u_knode {
 	struct tc_u_hnode	*ht_up;
 	struct tcf_exts		exts;
 #ifdef CONFIG_NET_CLS_IND
-	char                     indev[IFNAMSIZ];
+	int			ifindex;
 #endif
 	u8			fshift;
 	struct tcf_result	res;
@@ -152,7 +152,7 @@ check_terminal:
 
 				*res = n->res;
 #ifdef CONFIG_NET_CLS_IND
-				if (!tcf_match_indev(skb, n->indev)) {
+				if (!tcf_match_indev(skb, n->ifindex)) {
 					n = n->next;
 					goto next_knode;
 				}
@@ -527,9 +527,11 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
 
 #ifdef CONFIG_NET_CLS_IND
 	if (tb[TCA_U32_INDEV]) {
-		err = tcf_change_indev(tp, n->indev, tb[TCA_U32_INDEV]);
-		if (err < 0)
+		int ret;
+		ret = tcf_change_indev(net, tb[TCA_U32_INDEV]);
+		if (ret < 0)
 			goto errout;
+		n->ifindex = ret;
 	}
 #endif
 	tcf_exts_change(tp, &n->exts, &e);
@@ -760,9 +762,12 @@ static int u32_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 			goto nla_put_failure;
 
 #ifdef CONFIG_NET_CLS_IND
-		if (strlen(n->indev) &&
-		    nla_put_string(skb, TCA_U32_INDEV, n->indev))
-			goto nla_put_failure;
+		if (n->ifindex) {
+			struct net_device *dev;
+			dev = __dev_get_by_index(net, n->ifindex);
+			if (dev && nla_put_string(skb, TCA_U32_INDEV, dev->name))
+				goto nla_put_failure;
+		}
 #endif
 #ifdef CONFIG_CLS_U32_PERF
 		if (nla_put(skb, TCA_U32_PCNT,
-- 
1.8.3.1

[Patch net-next 5/7] net_sched: avoid casting void pointer

[Cong Wang]

tp->root is a void* pointer, no need to cast it.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 net/sched/cls_basic.c   | 10 +++++-----
 net/sched/cls_fw.c      | 14 +++++++-------
 net/sched/cls_route.c   |  6 +++---
 net/sched/cls_tcindex.c | 17 +++++++----------
 net/sched/cls_u32.c     |  2 +-
 5 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index af6bea1..e98ca99 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -38,7 +38,7 @@ static int basic_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			  struct tcf_result *res)
 {
 	int r;
-	struct basic_head *head = (struct basic_head *) tp->root;
+	struct basic_head *head = tp->root;
 	struct basic_filter *f;
 
 	list_for_each_entry(f, &head->flist, link) {
@@ -56,7 +56,7 @@ static int basic_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 static unsigned long basic_get(struct tcf_proto *tp, u32 handle)
 {
 	unsigned long l = 0UL;
-	struct basic_head *head = (struct basic_head *) tp->root;
+	struct basic_head *head = tp->root;
 	struct basic_filter *f;
 
 	if (head == NULL)
@@ -107,7 +107,7 @@ static void basic_destroy(struct tcf_proto *tp)
 
 static int basic_delete(struct tcf_proto *tp, unsigned long arg)
 {
-	struct basic_head *head = (struct basic_head *) tp->root;
+	struct basic_head *head = tp->root;
 	struct basic_filter *t, *f = (struct basic_filter *) arg;
 
 	list_for_each_entry(t, &head->flist, link)
@@ -164,7 +164,7 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
 			struct nlattr **tca, unsigned long *arg)
 {
 	int err;
-	struct basic_head *head = (struct basic_head *) tp->root;
+	struct basic_head *head = tp->root;
 	struct nlattr *tb[TCA_BASIC_MAX + 1];
 	struct basic_filter *f = (struct basic_filter *) *arg;
 
@@ -225,7 +225,7 @@ errout:
 
 static void basic_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 {
-	struct basic_head *head = (struct basic_head *) tp->root;
+	struct basic_head *head = tp->root;
 	struct basic_filter *f;
 
 	list_for_each_entry(f, &head->flist, link) {
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index ca662aa..ed00e8c 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -75,7 +75,7 @@ static inline int fw_hash(u32 handle)
 static int fw_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			  struct tcf_result *res)
 {
-	struct fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	struct fw_filter *f;
 	int r;
 	u32 id = skb->mark;
@@ -111,7 +111,7 @@ static int fw_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 
 static unsigned long fw_get(struct tcf_proto *tp, u32 handle)
 {
-	struct fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	struct fw_filter *f;
 
 	if (head == NULL)
@@ -160,7 +160,7 @@ static void fw_destroy(struct tcf_proto *tp)
 
 static int fw_delete(struct tcf_proto *tp, unsigned long arg)
 {
-	struct fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	struct fw_filter *f = (struct fw_filter *)arg;
 	struct fw_filter **fp;
 
@@ -190,7 +190,7 @@ 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 fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	struct tcf_exts e;
 	u32 mask;
 	int err;
@@ -237,7 +237,7 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 		     struct nlattr **tca,
 		     unsigned long *arg)
 {
-	struct fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	struct fw_filter *f = (struct fw_filter *) *arg;
 	struct nlattr *opt = tca[TCA_OPTIONS];
 	struct nlattr *tb[TCA_FW_MAX + 1];
@@ -300,7 +300,7 @@ errout:
 
 static void fw_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 {
-	struct fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	int h;
 
 	if (head == NULL)
@@ -329,7 +329,7 @@ static void fw_walk(struct tcf_proto *tp, struct tcf_walker *arg)
 static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
 		   struct sk_buff *skb, struct tcmsg *t)
 {
-	struct fw_head *head = (struct fw_head *)tp->root;
+	struct fw_head *head = tp->root;
 	struct fw_filter *f = (struct fw_filter *)fh;
 	unsigned char *b = skb_tail_pointer(skb);
 	struct nlattr *nest;
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index c913613..1ad3068 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -123,7 +123,7 @@ static inline int route4_hash_wild(void)
 static int route4_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			   struct tcf_result *res)
 {
-	struct route4_head *head = (struct route4_head *)tp->root;
+	struct route4_head *head = tp->root;
 	struct dst_entry *dst;
 	struct route4_bucket *b;
 	struct route4_filter *f;
@@ -213,7 +213,7 @@ static inline u32 from_hash(u32 id)
 
 static unsigned long route4_get(struct tcf_proto *tp, u32 handle)
 {
-	struct route4_head *head = (struct route4_head *)tp->root;
+	struct route4_head *head = tp->root;
 	struct route4_bucket *b;
 	struct route4_filter *f;
 	unsigned int h1, h2;
@@ -284,7 +284,7 @@ static void route4_destroy(struct tcf_proto *tp)
 
 static int route4_delete(struct tcf_proto *tp, unsigned long arg)
 {
-	struct route4_head *head = (struct route4_head *)tp->root;
+	struct route4_head *head = tp->root;
 	struct route4_filter **fp, *f = (struct route4_filter *)arg;
 	unsigned int h = 0;
 	struct route4_bucket *b;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index f575353..eed8404 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -24,9 +24,6 @@
 #define DEFAULT_HASH_SIZE	64	/* optimized for diffserv */
 
 
-#define	PRIV(tp)	((struct tcindex_data *) (tp)->root)
-
-
 struct tcindex_filter_result {
 	struct tcf_exts		exts;
 	struct tcf_result	res;
@@ -77,7 +74,7 @@ tcindex_lookup(struct tcindex_data *p, u16 key)
 static int tcindex_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 			    struct tcf_result *res)
 {
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcindex_filter_result *f;
 	int key = (skb->tc_index & p->mask) >> p->shift;
 
@@ -102,7 +99,7 @@ static int tcindex_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 
 static unsigned long tcindex_get(struct tcf_proto *tp, u32 handle)
 {
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcindex_filter_result *r;
 
 	pr_debug("tcindex_get(tp %p,handle 0x%08x)\n", tp, handle);
@@ -140,7 +137,7 @@ static int tcindex_init(struct tcf_proto *tp)
 static int
 __tcindex_delete(struct tcf_proto *tp, unsigned long arg, int lock)
 {
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcindex_filter_result *r = (struct tcindex_filter_result *) arg;
 	struct tcindex_filter *f = NULL;
 
@@ -338,7 +335,7 @@ tcindex_change(struct net *net, struct sk_buff *in_skb,
 {
 	struct nlattr *opt = tca[TCA_OPTIONS];
 	struct nlattr *tb[TCA_TCINDEX_MAX + 1];
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcindex_filter_result *r = (struct tcindex_filter_result *) *arg;
 	int err;
 
@@ -360,7 +357,7 @@ tcindex_change(struct net *net, struct sk_buff *in_skb,
 
 static void tcindex_walk(struct tcf_proto *tp, struct tcf_walker *walker)
 {
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcindex_filter *f, *next;
 	int i;
 
@@ -407,7 +404,7 @@ static int tcindex_destroy_element(struct tcf_proto *tp,
 
 static void tcindex_destroy(struct tcf_proto *tp)
 {
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcf_walker walker;
 
 	pr_debug("tcindex_destroy(tp %p),p %p\n", tp, p);
@@ -425,7 +422,7 @@ static void tcindex_destroy(struct tcf_proto *tp)
 static int tcindex_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
     struct sk_buff *skb, struct tcmsg *t)
 {
-	struct tcindex_data *p = PRIV(tp);
+	struct tcindex_data *p = tp->root;
 	struct tcindex_filter_result *r = (struct tcindex_filter_result *) fh;
 	unsigned char *b = skb_tail_pointer(skb);
 	struct nlattr *nest;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index f509b79..84c28da 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -95,7 +95,7 @@ static int u32_classify(struct sk_buff *skb, const struct tcf_proto *tp, struct
 		unsigned int	  off;
 	} stack[TC_U32_MAXDEPTH];
 
-	struct tc_u_hnode *ht = (struct tc_u_hnode *)tp->root;
+	struct tc_u_hnode *ht = tp->root;
 	unsigned int off = skb_network_offset(skb);
 	struct tc_u_knode *n;
 	int sdepth = 0;
-- 
1.8.3.1

[Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer

[Cong Wang]

Most of the filters need allocation of tp->root in ->init()
and free it in ->destroy(), make this generic.

Also we could reduce the use of tcf_tree_lock a bit.

Cc: Thomas Graf <tgraf@suug.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/sch_generic.h |  1 +
 net/sched/cls_api.c       |  7 +++++++
 net/sched/cls_basic.c     |  8 ++------
 net/sched/cls_bpf.c       | 11 ++---------
 net/sched/cls_cgroup.c    | 21 +++++++--------------
 net/sched/cls_flow.c      |  8 ++------
 net/sched/cls_fw.c        | 14 ++++----------
 net/sched/cls_route.c     | 15 ++-------------
 net/sched/cls_rsvp.h      | 10 ++--------
 net/sched/cls_tcindex.c   | 13 ++-----------
 net/sched/cls_u32.c       |  9 ++-------
 net/sched/sch_api.c       |  1 +
 12 files changed, 34 insertions(+), 84 deletions(-)

diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index d062f81..819dc1d 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -208,6 +208,7 @@ struct tcf_proto_ops {
 					struct sk_buff *skb, struct tcmsg*);
 
 	struct module		*owner;
+	size_t			root_size;
 };
 
 struct tcf_proto {
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 29a30a1..8460c75 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -262,6 +262,13 @@ replay:
 		tp->q = q;
 		tp->classify = tp_ops->classify;
 		tp->classid = parent;
+		tp->root = kzalloc(tp_ops->root_size, GFP_KERNEL);
+		if (!tp->root) {
+			err = -ENOBUFS;
+			module_put(tp_ops->owner);
+			kfree(tp);
+			goto errout;
+		}
 
 		err = tp_ops->init(tp);
 		if (err != 0) {
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index e98ca99..318f672 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -75,13 +75,9 @@ static void basic_put(struct tcf_proto *tp, unsigned long f)
 
 static int basic_init(struct tcf_proto *tp)
 {
-	struct basic_head *head;
+	struct basic_head *head = tp->root;
 
-	head = kzalloc(sizeof(*head), GFP_KERNEL);
-	if (head == NULL)
-		return -ENOBUFS;
 	INIT_LIST_HEAD(&head->flist);
-	tp->root = head;
 	return 0;
 }
 
@@ -102,7 +98,6 @@ static void basic_destroy(struct tcf_proto *tp)
 		list_del(&f->link);
 		basic_delete_filter(tp, f);
 	}
-	kfree(head);
 }
 
 static int basic_delete(struct tcf_proto *tp, unsigned long arg)
@@ -288,6 +283,7 @@ static struct tcf_proto_ops cls_basic_ops __read_mostly = {
 	.walk		=	basic_walk,
 	.dump		=	basic_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	=	sizeof(struct basic_head),
 };
 
 static int __init init_basic(void)
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 8e3cf49..eedd296 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -75,15 +75,9 @@ static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
 
 static int cls_bpf_init(struct tcf_proto *tp)
 {
-	struct cls_bpf_head *head;
-
-	head = kzalloc(sizeof(*head), GFP_KERNEL);
-	if (head == NULL)
-		return -ENOBUFS;
+	struct cls_bpf_head *head = tp->root;
 
 	INIT_LIST_HEAD(&head->plist);
-	tp->root = head;
-
 	return 0;
 }
 
@@ -126,8 +120,6 @@ static void cls_bpf_destroy(struct tcf_proto *tp)
 		list_del(&prog->link);
 		cls_bpf_delete_prog(tp, prog);
 	}
-
-	kfree(head);
 }
 
 static unsigned long cls_bpf_get(struct tcf_proto *tp, u32 handle)
@@ -366,6 +358,7 @@ static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
 	.delete		=	cls_bpf_delete,
 	.walk		=	cls_bpf_walk,
 	.dump		=	cls_bpf_dump,
+	.root_size	=	sizeof(struct cls_bpf_head),
 };
 
 static int __init cls_bpf_init_mod(void)
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 8e2158a..4b7e083 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -22,6 +22,7 @@ struct cls_cgroup_head {
 	u32			handle;
 	struct tcf_exts		exts;
 	struct tcf_ematch_tree	ematches;
+	bool			init;
 };
 
 static int cls_cgroup_classify(struct sk_buff *skb, const struct tcf_proto *tp,
@@ -73,6 +74,9 @@ static void cls_cgroup_put(struct tcf_proto *tp, unsigned long f)
 
 static int cls_cgroup_init(struct tcf_proto *tp)
 {
+	struct cls_cgroup_head *head = tp->root;
+
+	tcf_exts_init(&head->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
 	return 0;
 }
 
@@ -94,20 +98,9 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
 	if (!tca[TCA_OPTIONS])
 		return -EINVAL;
 
-	if (head == NULL) {
-		if (!handle)
-			return -EINVAL;
-
-		head = kzalloc(sizeof(*head), GFP_KERNEL);
-		if (head == NULL)
-			return -ENOBUFS;
-
-		tcf_exts_init(&head->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
+	if (!head->init) {
 		head->handle = handle;
-
-		tcf_tree_lock(tp);
-		tp->root = head;
-		tcf_tree_unlock(tp);
+		head->init = true;
 	}
 
 	if (handle != head->handle)
@@ -140,7 +133,6 @@ static void cls_cgroup_destroy(struct tcf_proto *tp)
 	if (head) {
 		tcf_exts_destroy(tp, &head->exts);
 		tcf_em_tree_destroy(tp, &head->ematches);
-		kfree(head);
 	}
 }
 
@@ -205,6 +197,7 @@ static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
 	.walk		=	cls_cgroup_walk,
 	.dump		=	cls_cgroup_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	=	sizeof(struct cls_cgroup_head),
 };
 
 static int __init init_cgroup_cls(void)
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 257029c..b39080a 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -526,13 +526,9 @@ static int flow_delete(struct tcf_proto *tp, unsigned long arg)
 
 static int flow_init(struct tcf_proto *tp)
 {
-	struct flow_head *head;
+	struct flow_head *head = tp->root;
 
-	head = kzalloc(sizeof(*head), GFP_KERNEL);
-	if (head == NULL)
-		return -ENOBUFS;
 	INIT_LIST_HEAD(&head->filters);
-	tp->root = head;
 	return 0;
 }
 
@@ -545,7 +541,6 @@ static void flow_destroy(struct tcf_proto *tp)
 		list_del(&f->list);
 		flow_destroy_filter(tp, f);
 	}
-	kfree(head);
 }
 
 static unsigned long flow_get(struct tcf_proto *tp, u32 handle)
@@ -653,6 +648,7 @@ static struct tcf_proto_ops cls_flow_ops __read_mostly = {
 	.dump		= flow_dump,
 	.walk		= flow_walk,
 	.owner		= THIS_MODULE,
+	.root_size	= sizeof(struct flow_head),
 };
 
 static int __init cls_flow_init(void)
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index ed00e8c..73cd277 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -34,6 +34,7 @@
 struct fw_head {
 	struct fw_filter *ht[HTSIZE];
 	u32 mask;
+	bool init;
 };
 
 struct fw_filter {
@@ -155,7 +156,6 @@ static void fw_destroy(struct tcf_proto *tp)
 			fw_delete_filter(tp, f);
 		}
 	}
-	kfree(head);
 }
 
 static int fw_delete(struct tcf_proto *tp, unsigned long arg)
@@ -259,19 +259,12 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
 	if (!handle)
 		return -EINVAL;
 
-	if (head == NULL) {
+	if (!head->init) {
 		u32 mask = 0xFFFFFFFF;
 		if (tb[TCA_FW_MASK])
 			mask = nla_get_u32(tb[TCA_FW_MASK]);
-
-		head = kzalloc(sizeof(struct fw_head), GFP_KERNEL);
-		if (head == NULL)
-			return -ENOBUFS;
 		head->mask = mask;
-
-		tcf_tree_lock(tp);
-		tp->root = head;
-		tcf_tree_unlock(tp);
+		head->init = true;
 	}
 
 	f = kzalloc(sizeof(struct fw_filter), GFP_KERNEL);
@@ -388,6 +381,7 @@ static struct tcf_proto_ops cls_fw_ops __read_mostly = {
 	.walk		=	fw_walk,
 	.dump		=	fw_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	= 	sizeof(struct fw_head),
 };
 
 static int __init init_fw(void)
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 1ad3068..038f35f 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -279,7 +279,6 @@ static void route4_destroy(struct tcf_proto *tp)
 			kfree(b);
 		}
 	}
-	kfree(head);
 }
 
 static int route4_delete(struct tcf_proto *tp, unsigned long arg)
@@ -462,20 +461,9 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
 		goto reinsert;
 	}
 
-	err = -ENOBUFS;
-	if (head == NULL) {
-		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
-		if (head == NULL)
-			goto errout;
-
-		tcf_tree_lock(tp);
-		tp->root = head;
-		tcf_tree_unlock(tp);
-	}
-
 	f = kzalloc(sizeof(struct route4_filter), GFP_KERNEL);
 	if (f == NULL)
-		goto errout;
+		return -ENOBUFS;
 
 	tcf_exts_init(&f->exts, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
 	err = route4_set_parms(net, tp, base, f, handle, head, tb,
@@ -613,6 +601,7 @@ static struct tcf_proto_ops cls_route4_ops __read_mostly = {
 	.walk		=	route4_walk,
 	.dump		=	route4_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	=	sizeof(struct route4_head),
 };
 
 static int __init init_route4(void)
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index 19f8e5d..47930bc 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -242,14 +242,7 @@ static void rsvp_put(struct tcf_proto *tp, unsigned long f)
 
 static int rsvp_init(struct tcf_proto *tp)
 {
-	struct rsvp_head *data;
-
-	data = kzalloc(sizeof(struct rsvp_head), GFP_KERNEL);
-	if (data) {
-		tp->root = data;
-		return 0;
-	}
-	return -ENOBUFS;
+	return 0;
 }
 
 static void
@@ -656,6 +649,7 @@ static struct tcf_proto_ops RSVP_OPS __read_mostly = {
 	.walk		=	rsvp_walk,
 	.dump		=	rsvp_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	=	sizeof(struct rsvp_head),
 };
 
 static int __init init_rsvp(void)
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index eed8404..6454158 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -118,18 +118,11 @@ static void tcindex_put(struct tcf_proto *tp, unsigned long f)
 
 static int tcindex_init(struct tcf_proto *tp)
 {
-	struct tcindex_data *p;
-
-	pr_debug("tcindex_init(tp %p)\n", tp);
-	p = kzalloc(sizeof(struct tcindex_data), GFP_KERNEL);
-	if (!p)
-		return -ENOMEM;
+	struct tcindex_data *p = tp->root;
 
 	p->mask = 0xffff;
 	p->hash = DEFAULT_HASH_SIZE;
 	p->fall_through = 1;
-
-	tp->root = p;
 	return 0;
 }
 
@@ -407,15 +400,12 @@ static void tcindex_destroy(struct tcf_proto *tp)
 	struct tcindex_data *p = tp->root;
 	struct tcf_walker walker;
 
-	pr_debug("tcindex_destroy(tp %p),p %p\n", tp, p);
 	walker.count = 0;
 	walker.skip = 0;
 	walker.fn = &tcindex_destroy_element;
 	tcindex_walk(tp, &walker);
 	kfree(p->perfect);
 	kfree(p->h);
-	kfree(p);
-	tp->root = NULL;
 }
 
 
@@ -491,6 +481,7 @@ static struct tcf_proto_ops cls_tcindex_ops __read_mostly = {
 	.walk		=	tcindex_walk,
 	.dump		=	tcindex_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	=	sizeof(struct tcindex_data),
 };
 
 static int __init init_tcindex(void)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 84c28da..678c2d72 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -300,15 +300,11 @@ static u32 gen_new_htid(struct tc_u_common *tp_c)
 
 static int u32_init(struct tcf_proto *tp)
 {
-	struct tc_u_hnode *root_ht;
+	struct tc_u_hnode *root_ht = tp->root;
 	struct tc_u_common *tp_c;
 
 	tp_c = tp->q->u32_node;
 
-	root_ht = kzalloc(sizeof(*root_ht), GFP_KERNEL);
-	if (root_ht == NULL)
-		return -ENOBUFS;
-
 	root_ht->divisor = 0;
 	root_ht->refcnt++;
 	root_ht->handle = tp_c ? gen_new_htid(tp_c) : 0x80000000;
@@ -329,7 +325,6 @@ static int u32_init(struct tcf_proto *tp)
 	tp_c->hlist = root_ht;
 	root_ht->tp_c = tp_c;
 
-	tp->root = root_ht;
 	tp->data = tp_c;
 	return 0;
 }
@@ -394,7 +389,6 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
 	for (hn = &tp_c->hlist; *hn; hn = &(*hn)->next) {
 		if (*hn == ht) {
 			*hn = ht->next;
-			kfree(ht);
 			return 0;
 		}
 	}
@@ -801,6 +795,7 @@ static struct tcf_proto_ops cls_u32_ops __read_mostly = {
 	.walk		=	u32_walk,
 	.dump		=	u32_dump,
 	.owner		=	THIS_MODULE,
+	.root_size	=	sizeof(struct tc_u_hnode),
 };
 
 static int __init init_u32(void)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index 1313145..5fef7f4 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1829,6 +1829,7 @@ EXPORT_SYMBOL(tc_classify);
 void tcf_destroy(struct tcf_proto *tp)
 {
 	tp->ops->destroy(tp);
+	kfree(tp->root);
 	module_put(tp->ops->owner);
 	kfree(tp);
 }
-- 
1.8.3.1

[Patch net-next 7/7] net_sched: act: remove struct tcf_act_hdr

[Cong Wang]

It is not necessary at all.

Cc: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: David S. Miller <davem@davemloft.net>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
 include/net/act_api.h |  4 ----
 net/sched/act_api.c   | 16 ++++++++--------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 268f9e6..e171387 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -73,10 +73,6 @@ static inline void tcf_hashinfo_destroy(struct tcf_hashinfo *hf)
 #define ACT_P_CREATED 1
 #define ACT_P_DELETED 1
 
-struct tcf_act_hdr {
-	struct tcf_common	common;
-};
-
 struct tc_action {
 	void			*priv;
 	const struct tc_action_ops	*ops;
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 178bf2e..35f89e9 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -556,9 +556,9 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *a,
 {
 	int err = 0;
 	struct gnet_dump d;
-	struct tcf_act_hdr *h = a->priv;
+	struct tcf_common *p = a->priv;
 
-	if (h == NULL)
+	if (p == NULL)
 		goto errout;
 
 	/* compat_mode being true specifies a call that is supposed
@@ -567,20 +567,20 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *a,
 	if (compat_mode) {
 		if (a->type == TCA_OLD_COMPAT)
 			err = gnet_stats_start_copy_compat(skb, 0,
-				TCA_STATS, TCA_XSTATS, &h->tcf_lock, &d);
+				TCA_STATS, TCA_XSTATS, &p->tcfc_lock, &d);
 		else
 			return 0;
 	} else
 		err = gnet_stats_start_copy(skb, TCA_ACT_STATS,
-					    &h->tcf_lock, &d);
+					    &p->tcfc_lock, &d);
 
 	if (err < 0)
 		goto errout;
 
-	if (gnet_stats_copy_basic(&d, &h->tcf_bstats) < 0 ||
-	    gnet_stats_copy_rate_est(&d, &h->tcf_bstats,
-				     &h->tcf_rate_est) < 0 ||
-	    gnet_stats_copy_queue(&d, &h->tcf_qstats) < 0)
+	if (gnet_stats_copy_basic(&d, &p->tcfc_bstats) < 0 ||
+	    gnet_stats_copy_rate_est(&d, &p->tcfc_bstats,
+				     &p->tcfc_rate_est) < 0 ||
+	    gnet_stats_copy_queue(&d, &p->tcfc_qstats) < 0)
 		goto errout;
 
 	if (gnet_stats_finish_copy(&d) < 0)
-- 
1.8.3.1

Re: [Patch net-next 0/7] net_sched: some more cleanup and improvements

[Jamal Hadi Salim]

On 01/09/14 19:13, Cong Wang wrote:
> This patchset collects the previous patches I sent which Jamal doesn't object.
> They are still some cleanup and improvements for tc actions and filters.
>
>

Cong - I dont have time to test at the moment and like you said
they dont look controversial; i will review them, please try to test
them to the best you can.

cheers,
jamal

Re: [Patch net-next 1/7] net_sched: act: move idx_gen into struct tcf_hashinfo

[Jamal Hadi Salim]

On 01/09/14 19:13, Cong Wang wrote:
> There is no need to store the index separatedly
> since tcf_hashinfo is allocated statically too.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

An improvement for sure. Thanks Cong!

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Re: [Patch net-next 2/7] net_sched: act: clean up notification functions

[Jamal Hadi Salim]

On 01/09/14 19:14, Cong Wang wrote:
> Refactor tcf_add_notify() and factor out tcf_del_notify().
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>


Thanks Cong!

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Re: [Patch net-next 3/7] net_sched: add struct net pointer to tcf_proto_ops->dump

[Jamal Hadi Salim]

On 01/09/14 19:14, Cong Wang wrote:
> It will be needed by the next patch.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>


Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [Patch net-next 4/7] net_sched: optimize tcf_match_indev()

[Jamal Hadi Salim]

On 01/09/14 19:14, Cong Wang wrote:
> tcf_match_indev() is called in fast path, it is not wise to
> search for a netdev by ifindex and then compare by its name,
> just compare the ifindex.
>
> Also, dev->name could be changed by user-space, therefore
> the match would be always fail, but dev->ifindex could
> be consistent.
>
> BTW, this will also save some bytes from the core struct of u32.
>

excellent.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [Patch net-next 5/7] net_sched: avoid casting void pointer

[Jamal Hadi Salim]

On 01/09/14 19:14, Cong Wang wrote:
> tp->root is a void* pointer, no need to cast it.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>


Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer

[Jamal Hadi Salim]

On 01/09/14 19:14, Cong Wang wrote:
> Most of the filters need allocation of tp->root in ->init()
> and free it in ->destroy(), make this generic.
>
> Also we could reduce the use of tcf_tree_lock a bit.
>
> Cc: Thomas Graf <tgraf@suug.ch>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>


Hrm. This one worries me a little.
I dont see how just pre-allocing the private head of the classifier
magically allows you to get rid of locks. Have you tested against those
classifiers you changed?
If those locks are useless - then that is a separate patch to kill
them (sorry, dont have time to test myself right now).

cheers,
jamal

> ---
>   include/net/sch_generic.h |  1 +
>   net/sched/cls_api.c       |  7 +++++++
>   net/sched/cls_basic.c     |  8 ++------
>   net/sched/cls_bpf.c       | 11 ++---------
>   net/sched/cls_cgroup.c    | 21 +++++++--------------
>   net/sched/cls_flow.c      |  8 ++------
>   net/sched/cls_fw.c        | 14 ++++----------
>   net/sched/cls_route.c     | 15 ++-------------
>   net/sched/cls_rsvp.h      | 10 ++--------
>   net/sched/cls_tcindex.c   | 13 ++-----------
>   net/sched/cls_u32.c       |  9 ++-------
>   net/sched/sch_api.c       |  1 +
>   12 files changed, 34 insertions(+), 84 deletions(-)
>
> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
> index d062f81..819dc1d 100644
> --- a/include/net/sch_generic.h
> +++ b/include/net/sch_generic.h
> @@ -208,6 +208,7 @@ struct tcf_proto_ops {
>   					struct sk_buff *skb, struct tcmsg*);
>
>   	struct module		*owner;
> +	size_t			root_size;
>   };
>
>   struct tcf_proto {
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 29a30a1..8460c75 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -262,6 +262,13 @@ replay:
>   		tp->q = q;
>   		tp->classify = tp_ops->classify;
>   		tp->classid = parent;
> +		tp->root = kzalloc(tp_ops->root_size, GFP_KERNEL);
> +		if (!tp->root) {
> +			err = -ENOBUFS;
> +			module_put(tp_ops->owner);
> +			kfree(tp);
> +			goto errout;
> +		}
>
>   		err = tp_ops->init(tp);
>   		if (err != 0) {
> diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
> index e98ca99..318f672 100644
> --- a/net/sched/cls_basic.c
> +++ b/net/sched/cls_basic.c
> @@ -75,13 +75,9 @@ static void basic_put(struct tcf_proto *tp, unsigned long f)
>
>   static int basic_init(struct tcf_proto *tp)
>   {
> -	struct basic_head *head;
> +	struct basic_head *head = tp->root;
>
> -	head = kzalloc(sizeof(*head), GFP_KERNEL);
> -	if (head == NULL)
> -		return -ENOBUFS;
>   	INIT_LIST_HEAD(&head->flist);
> -	tp->root = head;
>   	return 0;
>   }
>
> @@ -102,7 +98,6 @@ static void basic_destroy(struct tcf_proto *tp)
>   		list_del(&f->link);
>   		basic_delete_filter(tp, f);
>   	}
> -	kfree(head);
>   }
>
>   static int basic_delete(struct tcf_proto *tp, unsigned long arg)
> @@ -288,6 +283,7 @@ static struct tcf_proto_ops cls_basic_ops __read_mostly = {
>   	.walk		=	basic_walk,
>   	.dump		=	basic_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	=	sizeof(struct basic_head),
>   };
>
>   static int __init init_basic(void)
> diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
> index 8e3cf49..eedd296 100644
> --- a/net/sched/cls_bpf.c
> +++ b/net/sched/cls_bpf.c
> @@ -75,15 +75,9 @@ static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
>
>   static int cls_bpf_init(struct tcf_proto *tp)
>   {
> -	struct cls_bpf_head *head;
> -
> -	head = kzalloc(sizeof(*head), GFP_KERNEL);
> -	if (head == NULL)
> -		return -ENOBUFS;
> +	struct cls_bpf_head *head = tp->root;
>
>   	INIT_LIST_HEAD(&head->plist);
> -	tp->root = head;
> -
>   	return 0;
>   }
>
> @@ -126,8 +120,6 @@ static void cls_bpf_destroy(struct tcf_proto *tp)
>   		list_del(&prog->link);
>   		cls_bpf_delete_prog(tp, prog);
>   	}
> -
> -	kfree(head);
>   }
>
>   static unsigned long cls_bpf_get(struct tcf_proto *tp, u32 handle)
> @@ -366,6 +358,7 @@ static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
>   	.delete		=	cls_bpf_delete,
>   	.walk		=	cls_bpf_walk,
>   	.dump		=	cls_bpf_dump,
> +	.root_size	=	sizeof(struct cls_bpf_head),
>   };
>
>   static int __init cls_bpf_init_mod(void)
> diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
> index 8e2158a..4b7e083 100644
> --- a/net/sched/cls_cgroup.c
> +++ b/net/sched/cls_cgroup.c
> @@ -22,6 +22,7 @@ struct cls_cgroup_head {
>   	u32			handle;
>   	struct tcf_exts		exts;
>   	struct tcf_ematch_tree	ematches;
> +	bool			init;
>   };
>
>   static int cls_cgroup_classify(struct sk_buff *skb, const struct tcf_proto *tp,
> @@ -73,6 +74,9 @@ static void cls_cgroup_put(struct tcf_proto *tp, unsigned long f)
>
>   static int cls_cgroup_init(struct tcf_proto *tp)
>   {
> +	struct cls_cgroup_head *head = tp->root;
> +
> +	tcf_exts_init(&head->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
>   	return 0;
>   }
>
> @@ -94,20 +98,9 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
>   	if (!tca[TCA_OPTIONS])
>   		return -EINVAL;
>
> -	if (head == NULL) {
> -		if (!handle)
> -			return -EINVAL;
> -
> -		head = kzalloc(sizeof(*head), GFP_KERNEL);
> -		if (head == NULL)
> -			return -ENOBUFS;
> -
> -		tcf_exts_init(&head->exts, TCA_CGROUP_ACT, TCA_CGROUP_POLICE);
> +	if (!head->init) {
>   		head->handle = handle;
> -
> -		tcf_tree_lock(tp);
> -		tp->root = head;
> -		tcf_tree_unlock(tp);
> +		head->init = true;
>   	}
>
>   	if (handle != head->handle)
> @@ -140,7 +133,6 @@ static void cls_cgroup_destroy(struct tcf_proto *tp)
>   	if (head) {
>   		tcf_exts_destroy(tp, &head->exts);
>   		tcf_em_tree_destroy(tp, &head->ematches);
> -		kfree(head);
>   	}
>   }
>
> @@ -205,6 +197,7 @@ static struct tcf_proto_ops cls_cgroup_ops __read_mostly = {
>   	.walk		=	cls_cgroup_walk,
>   	.dump		=	cls_cgroup_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	=	sizeof(struct cls_cgroup_head),
>   };
>
>   static int __init init_cgroup_cls(void)
> diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
> index 257029c..b39080a 100644
> --- a/net/sched/cls_flow.c
> +++ b/net/sched/cls_flow.c
> @@ -526,13 +526,9 @@ static int flow_delete(struct tcf_proto *tp, unsigned long arg)
>
>   static int flow_init(struct tcf_proto *tp)
>   {
> -	struct flow_head *head;
> +	struct flow_head *head = tp->root;
>
> -	head = kzalloc(sizeof(*head), GFP_KERNEL);
> -	if (head == NULL)
> -		return -ENOBUFS;
>   	INIT_LIST_HEAD(&head->filters);
> -	tp->root = head;
>   	return 0;
>   }
>
> @@ -545,7 +541,6 @@ static void flow_destroy(struct tcf_proto *tp)
>   		list_del(&f->list);
>   		flow_destroy_filter(tp, f);
>   	}
> -	kfree(head);
>   }
>
>   static unsigned long flow_get(struct tcf_proto *tp, u32 handle)
> @@ -653,6 +648,7 @@ static struct tcf_proto_ops cls_flow_ops __read_mostly = {
>   	.dump		= flow_dump,
>   	.walk		= flow_walk,
>   	.owner		= THIS_MODULE,
> +	.root_size	= sizeof(struct flow_head),
>   };
>
>   static int __init cls_flow_init(void)
> diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
> index ed00e8c..73cd277 100644
> --- a/net/sched/cls_fw.c
> +++ b/net/sched/cls_fw.c
> @@ -34,6 +34,7 @@
>   struct fw_head {
>   	struct fw_filter *ht[HTSIZE];
>   	u32 mask;
> +	bool init;
>   };
>
>   struct fw_filter {
> @@ -155,7 +156,6 @@ static void fw_destroy(struct tcf_proto *tp)
>   			fw_delete_filter(tp, f);
>   		}
>   	}
> -	kfree(head);
>   }
>
>   static int fw_delete(struct tcf_proto *tp, unsigned long arg)
> @@ -259,19 +259,12 @@ static int fw_change(struct net *net, struct sk_buff *in_skb,
>   	if (!handle)
>   		return -EINVAL;
>
> -	if (head == NULL) {
> +	if (!head->init) {
>   		u32 mask = 0xFFFFFFFF;
>   		if (tb[TCA_FW_MASK])
>   			mask = nla_get_u32(tb[TCA_FW_MASK]);
> -
> -		head = kzalloc(sizeof(struct fw_head), GFP_KERNEL);
> -		if (head == NULL)
> -			return -ENOBUFS;
>   		head->mask = mask;
> -
> -		tcf_tree_lock(tp);
> -		tp->root = head;
> -		tcf_tree_unlock(tp);
> +		head->init = true;
>   	}
>
>   	f = kzalloc(sizeof(struct fw_filter), GFP_KERNEL);
> @@ -388,6 +381,7 @@ static struct tcf_proto_ops cls_fw_ops __read_mostly = {
>   	.walk		=	fw_walk,
>   	.dump		=	fw_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	= 	sizeof(struct fw_head),
>   };
>
>   static int __init init_fw(void)
> diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
> index 1ad3068..038f35f 100644
> --- a/net/sched/cls_route.c
> +++ b/net/sched/cls_route.c
> @@ -279,7 +279,6 @@ static void route4_destroy(struct tcf_proto *tp)
>   			kfree(b);
>   		}
>   	}
> -	kfree(head);
>   }
>
>   static int route4_delete(struct tcf_proto *tp, unsigned long arg)
> @@ -462,20 +461,9 @@ static int route4_change(struct net *net, struct sk_buff *in_skb,
>   		goto reinsert;
>   	}
>
> -	err = -ENOBUFS;
> -	if (head == NULL) {
> -		head = kzalloc(sizeof(struct route4_head), GFP_KERNEL);
> -		if (head == NULL)
> -			goto errout;
> -
> -		tcf_tree_lock(tp);
> -		tp->root = head;
> -		tcf_tree_unlock(tp);
> -	}
> -
>   	f = kzalloc(sizeof(struct route4_filter), GFP_KERNEL);
>   	if (f == NULL)
> -		goto errout;
> +		return -ENOBUFS;
>
>   	tcf_exts_init(&f->exts, TCA_ROUTE4_ACT, TCA_ROUTE4_POLICE);
>   	err = route4_set_parms(net, tp, base, f, handle, head, tb,
> @@ -613,6 +601,7 @@ static struct tcf_proto_ops cls_route4_ops __read_mostly = {
>   	.walk		=	route4_walk,
>   	.dump		=	route4_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	=	sizeof(struct route4_head),
>   };
>
>   static int __init init_route4(void)
> diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
> index 19f8e5d..47930bc 100644
> --- a/net/sched/cls_rsvp.h
> +++ b/net/sched/cls_rsvp.h
> @@ -242,14 +242,7 @@ static void rsvp_put(struct tcf_proto *tp, unsigned long f)
>
>   static int rsvp_init(struct tcf_proto *tp)
>   {
> -	struct rsvp_head *data;
> -
> -	data = kzalloc(sizeof(struct rsvp_head), GFP_KERNEL);
> -	if (data) {
> -		tp->root = data;
> -		return 0;
> -	}
> -	return -ENOBUFS;
> +	return 0;
>   }
>
>   static void
> @@ -656,6 +649,7 @@ static struct tcf_proto_ops RSVP_OPS __read_mostly = {
>   	.walk		=	rsvp_walk,
>   	.dump		=	rsvp_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	=	sizeof(struct rsvp_head),
>   };
>
>   static int __init init_rsvp(void)
> diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
> index eed8404..6454158 100644
> --- a/net/sched/cls_tcindex.c
> +++ b/net/sched/cls_tcindex.c
> @@ -118,18 +118,11 @@ static void tcindex_put(struct tcf_proto *tp, unsigned long f)
>
>   static int tcindex_init(struct tcf_proto *tp)
>   {
> -	struct tcindex_data *p;
> -
> -	pr_debug("tcindex_init(tp %p)\n", tp);
> -	p = kzalloc(sizeof(struct tcindex_data), GFP_KERNEL);
> -	if (!p)
> -		return -ENOMEM;
> +	struct tcindex_data *p = tp->root;
>
>   	p->mask = 0xffff;
>   	p->hash = DEFAULT_HASH_SIZE;
>   	p->fall_through = 1;
> -
> -	tp->root = p;
>   	return 0;
>   }
>
> @@ -407,15 +400,12 @@ static void tcindex_destroy(struct tcf_proto *tp)
>   	struct tcindex_data *p = tp->root;
>   	struct tcf_walker walker;
>
> -	pr_debug("tcindex_destroy(tp %p),p %p\n", tp, p);
>   	walker.count = 0;
>   	walker.skip = 0;
>   	walker.fn = &tcindex_destroy_element;
>   	tcindex_walk(tp, &walker);
>   	kfree(p->perfect);
>   	kfree(p->h);
> -	kfree(p);
> -	tp->root = NULL;
>   }
>
>
> @@ -491,6 +481,7 @@ static struct tcf_proto_ops cls_tcindex_ops __read_mostly = {
>   	.walk		=	tcindex_walk,
>   	.dump		=	tcindex_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	=	sizeof(struct tcindex_data),
>   };
>
>   static int __init init_tcindex(void)
> diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
> index 84c28da..678c2d72 100644
> --- a/net/sched/cls_u32.c
> +++ b/net/sched/cls_u32.c
> @@ -300,15 +300,11 @@ static u32 gen_new_htid(struct tc_u_common *tp_c)
>
>   static int u32_init(struct tcf_proto *tp)
>   {
> -	struct tc_u_hnode *root_ht;
> +	struct tc_u_hnode *root_ht = tp->root;
>   	struct tc_u_common *tp_c;
>
>   	tp_c = tp->q->u32_node;
>
> -	root_ht = kzalloc(sizeof(*root_ht), GFP_KERNEL);
> -	if (root_ht == NULL)
> -		return -ENOBUFS;
> -
>   	root_ht->divisor = 0;
>   	root_ht->refcnt++;
>   	root_ht->handle = tp_c ? gen_new_htid(tp_c) : 0x80000000;
> @@ -329,7 +325,6 @@ static int u32_init(struct tcf_proto *tp)
>   	tp_c->hlist = root_ht;
>   	root_ht->tp_c = tp_c;
>
> -	tp->root = root_ht;
>   	tp->data = tp_c;
>   	return 0;
>   }
> @@ -394,7 +389,6 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht)
>   	for (hn = &tp_c->hlist; *hn; hn = &(*hn)->next) {
>   		if (*hn == ht) {
>   			*hn = ht->next;
> -			kfree(ht);
>   			return 0;
>   		}
>   	}
> @@ -801,6 +795,7 @@ static struct tcf_proto_ops cls_u32_ops __read_mostly = {
>   	.walk		=	u32_walk,
>   	.dump		=	u32_dump,
>   	.owner		=	THIS_MODULE,
> +	.root_size	=	sizeof(struct tc_u_hnode),
>   };
>
>   static int __init init_u32(void)
> diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
> index 1313145..5fef7f4 100644
> --- a/net/sched/sch_api.c
> +++ b/net/sched/sch_api.c
> @@ -1829,6 +1829,7 @@ EXPORT_SYMBOL(tc_classify);
>   void tcf_destroy(struct tcf_proto *tp)
>   {
>   	tp->ops->destroy(tp);
> +	kfree(tp->root);
>   	module_put(tp->ops->owner);
>   	kfree(tp);
>   }
>

Re: [Patch net-next 7/7] net_sched: act: remove struct tcf_act_hdr

[Jamal Hadi Salim]

On 01/09/14 19:14, Cong Wang wrote:
> It is not necessary at all.
>
> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Interestingly enough this is how things were originally ;->

Thanks Cong for all the effort.

Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal

Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer

[David Miller]

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu,  9 Jan 2014 16:14:04 -0800

> +	if (!head->init) {
>  		head->handle = handle;
> -
> -		tcf_tree_lock(tp);
> -		tp->root = head;
> -		tcf_tree_unlock(tp);
> +		head->init = true;
 ...
>  		head->mask = mask;
> -
> -		tcf_tree_lock(tp);
> -		tp->root = head;
> -		tcf_tree_unlock(tp);
> +		head->init = true;

Like Jamal, I don't think these transformations are valid.

You can't make the root visible in the hierarchy until the
->handle and ->mask (respectively) members are initialized
in these two classifiers.

What I'm going to do for now is apply patches 1-5 and 7.

Thanks.

Re: [Patch net-next 0/7] net_sched: some more cleanup and improvements

[Cong Wang]

On Sun, Jan 12, 2014 at 4:32 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 01/09/14 19:13, Cong Wang wrote:
>>
>> This patchset collects the previous patches I sent which Jamal doesn't
>> object.
>> They are still some cleanup and improvements for tc actions and filters.
>>
>>
>
> Cong - I dont have time to test at the moment and like you said
> they dont look controversial; i will review them, please try to test
> them to the best you can.
>

I did and I will continue to test them in net-next.

Thanks for reviews!

Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer

[Cong Wang]

On Sun, Jan 12, 2014 at 5:07 AM, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On 01/09/14 19:14, Cong Wang wrote:
>>
>> Most of the filters need allocation of tp->root in ->init()
>> and free it in ->destroy(), make this generic.
>>
>> Also we could reduce the use of tcf_tree_lock a bit.
>>
>> Cc: Thomas Graf <tgraf@suug.ch>
>> Cc: David S. Miller <davem@davemloft.net>
>> Cc: Jamal Hadi Salim <jhs@mojatatu.com>
>> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>
>
>
> Hrm. This one worries me a little.
> I dont see how just pre-allocing the private head of the classifier
> magically allows you to get rid of locks. Have you tested against those
> classifiers you changed?
> If those locks are useless - then that is a separate patch to kill
> them (sorry, dont have time to test myself right now).


Yeah, I am not sure either. I will re-think about this locking stuff.

Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer

[Cong Wang]

On Mon, Jan 13, 2014 at 11:49 AM, David Miller <davem@davemloft.net> wrote:
>
> What I'm going to do for now is apply patches 1-5 and 7.
>

Sounds good to me. I will re-work on it.
Thanks!