* [Patch net-next 1/7] net_sched: act: move idx_gen into struct tcf_hashinfo
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
@ 2014-01-10 0:13 ` Cong Wang
2014-01-12 12:34 ` Jamal Hadi Salim
2014-01-10 0:14 ` [Patch net-next 2/7] net_sched: act: clean up notification functions Cong Wang
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:13 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Patch net-next 2/7] net_sched: act: clean up notification functions
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
2014-01-10 0:13 ` [Patch net-next 1/7] net_sched: act: move idx_gen into struct tcf_hashinfo Cong Wang
@ 2014-01-10 0:14 ` Cong Wang
2014-01-12 12:38 ` Jamal Hadi Salim
2014-01-10 0:14 ` [Patch net-next 3/7] net_sched: add struct net pointer to tcf_proto_ops->dump Cong Wang
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Patch net-next 3/7] net_sched: add struct net pointer to tcf_proto_ops->dump
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
2014-01-10 0:13 ` [Patch net-next 1/7] net_sched: act: move idx_gen into struct tcf_hashinfo Cong Wang
2014-01-10 0:14 ` [Patch net-next 2/7] net_sched: act: clean up notification functions Cong Wang
@ 2014-01-10 0:14 ` Cong Wang
2014-01-12 12:46 ` Jamal Hadi Salim
2014-01-10 0:14 ` [Patch net-next 4/7] net_sched: optimize tcf_match_indev() Cong Wang
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Patch net-next 4/7] net_sched: optimize tcf_match_indev()
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
` (2 preceding siblings ...)
2014-01-10 0:14 ` [Patch net-next 3/7] net_sched: add struct net pointer to tcf_proto_ops->dump Cong Wang
@ 2014-01-10 0:14 ` Cong Wang
2014-01-12 12:50 ` Jamal Hadi Salim
2014-01-10 0:14 ` [Patch net-next 5/7] net_sched: avoid casting void pointer Cong Wang
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Patch net-next 4/7] net_sched: optimize tcf_match_indev()
2014-01-10 0:14 ` [Patch net-next 4/7] net_sched: optimize tcf_match_indev() Cong Wang
@ 2014-01-12 12:50 ` Jamal Hadi Salim
0 siblings, 0 replies; 20+ messages in thread
From: Jamal Hadi Salim @ 2014-01-12 12:50 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: David S. Miller
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
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Patch net-next 5/7] net_sched: avoid casting void pointer
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
` (3 preceding siblings ...)
2014-01-10 0:14 ` [Patch net-next 4/7] net_sched: optimize tcf_match_indev() Cong Wang
@ 2014-01-10 0:14 ` Cong Wang
2014-01-12 12:55 ` Jamal Hadi Salim
2014-01-10 0:14 ` [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer Cong Wang
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
` (4 preceding siblings ...)
2014-01-10 0:14 ` [Patch net-next 5/7] net_sched: avoid casting void pointer Cong Wang
@ 2014-01-10 0:14 ` Cong Wang
2014-01-12 13:07 ` Jamal Hadi Salim
2014-01-13 19:49 ` David Miller
2014-01-10 0:14 ` [Patch net-next 7/7] net_sched: act: remove struct tcf_act_hdr Cong Wang
2014-01-12 12:32 ` [Patch net-next 0/7] net_sched: some more cleanup and improvements Jamal Hadi Salim
7 siblings, 2 replies; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Thomas Graf, David S. Miller, Jamal Hadi Salim
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer
2014-01-10 0:14 ` [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer Cong Wang
@ 2014-01-12 13:07 ` Jamal Hadi Salim
2014-01-14 18:56 ` Cong Wang
2014-01-13 19:49 ` David Miller
1 sibling, 1 reply; 20+ messages in thread
From: Jamal Hadi Salim @ 2014-01-12 13:07 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: Thomas Graf, David S. Miller
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);
> }
>
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer
2014-01-12 13:07 ` Jamal Hadi Salim
@ 2014-01-14 18:56 ` Cong Wang
0 siblings, 0 replies; 20+ messages in thread
From: Cong Wang @ 2014-01-14 18:56 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Linux Kernel Network Developers, Thomas Graf, David S. Miller
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer
2014-01-10 0:14 ` [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer Cong Wang
2014-01-12 13:07 ` Jamal Hadi Salim
@ 2014-01-13 19:49 ` David Miller
2014-01-14 18:57 ` Cong Wang
1 sibling, 1 reply; 20+ messages in thread
From: David Miller @ 2014-01-13 19:49 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, tgraf, jhs
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.
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Patch net-next 7/7] net_sched: act: remove struct tcf_act_hdr
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
` (5 preceding siblings ...)
2014-01-10 0:14 ` [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer Cong Wang
@ 2014-01-10 0:14 ` Cong Wang
2014-01-12 13:13 ` Jamal Hadi Salim
2014-01-12 12:32 ` [Patch net-next 0/7] net_sched: some more cleanup and improvements Jamal Hadi Salim
7 siblings, 1 reply; 20+ messages in thread
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [Patch net-next 0/7] net_sched: some more cleanup and improvements
2014-01-10 0:13 [Patch net-next 0/7] net_sched: some more cleanup and improvements Cong Wang
` (6 preceding siblings ...)
2014-01-10 0:14 ` [Patch net-next 7/7] net_sched: act: remove struct tcf_act_hdr Cong Wang
@ 2014-01-12 12:32 ` Jamal Hadi Salim
2014-01-14 18:54 ` Cong Wang
7 siblings, 1 reply; 20+ messages in thread
From: Jamal Hadi Salim @ 2014-01-12 12:32 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: David S. Miller
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
^ permalink raw reply [flat|nested] 20+ messages in thread