* [Patch net-next 7/7] net_sched: act: remove struct tcf_act_hdr
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 6/7] net_sched: cls: move allocation in ->init to generic layer
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Thomas Graf, David S. Miller, Jamal Hadi Salim
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 5/7] net_sched: avoid casting void pointer
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 4/7] net_sched: optimize tcf_match_indev()
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 3/7] net_sched: add struct net pointer to tcf_proto_ops->dump
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 2/7] net_sched: act: clean up notification functions
From: Cong Wang @ 2014-01-10 0:14 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 1/7] net_sched: act: move idx_gen into struct tcf_hashinfo
From: Cong Wang @ 2014-01-10 0:13 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
In-Reply-To: <1389312845-10304-1-git-send-email-xiyou.wangcong@gmail.com>
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
* [Patch net-next 0/7] net_sched: some more cleanup and improvements
From: Cong Wang @ 2014-01-10 0:13 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Jamal Hadi Salim, David S. Miller
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
^ permalink raw reply
* [PATCH] netdevice.7: document SIOCGIFCONF case ifc_req==NULL
From: Tilman Schmidt @ 2014-01-09 23:30 UTC (permalink / raw)
To: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-man-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
Add the missing description of the possibility to call SIOCGIFCONF
with ifc_req==NULL to determine the needed buffer size, as described
in http://lkml.indiana.edu/hypermail/linux/kernel/0110.1/0506.html
and verified against source files net/core/dev_ioctl.c and
net/ipv4/devinet.c in the current kernel git tree.
This functionality has been present since the beginning of the 2.6
series. It's about time it gets documented.
While I'm at it, also generally clarify the section on SIOCGIFCONF.
Patch prepared against man-pages version 3.55.
Signed-off-by: Tilman Schmidt <tilman-ZTO5kqT2PaM@public.gmane.org>
---
man7/netdevice.7 | 83 +++++++++++++++++++++++++++++++++++++------------------
1 file changed, 57 insertions(+), 26 deletions(-)
--- a/man7/netdevice.7 2013-12-12 08:42:55.000000000 +0100
+++ b/man7/netdevice.7 2014-01-09 23:43:23.000000000 +0100
@@ -15,7 +15,7 @@
.\" Modified, 2011-11-02, <bidulock-B8ReZeU34LJAfugRpC6u6w@public.gmane.org>, added many basic
.\" but missing ioctls, such as SIOCGIFADDR.
.\"
-.TH NETDEVICE 7 2012-04-26 "Linux" "Linux Programmer's Manual"
+.TH NETDEVICE 7 2014-01-09 "Linux" "Linux Programmer's Manual"
.SH NAME
netdevice \- low-level access to Linux network devices
.SH SYNOPSIS
@@ -29,7 +29,7 @@
Linux supports some standard ioctls to configure network devices.
They can be used on any socket's file descriptor regardless of the
family or type.
-They pass an
+Most of them pass an
.I ifreq
structure:
@@ -53,14 +53,6 @@
char *ifr_data;
};
};
-
-struct ifconf {
- int ifc_len; /* size of buffer */
- union {
- char *ifc_buf; /* buffer address */
- struct ifreq *ifc_req; /* array of structures */
- };
-};
.fi
.in
@@ -265,30 +257,69 @@
means only addresses of the
.B AF_INET
(IPv4) family for compatibility.
-The user passes a
+Unlike the others, this ioctl passes an
.I ifconf
-structure as argument to the ioctl.
-It contains a pointer to an array of
-.I ifreq
-structures in
+structure:
+
+.in +4n
+.nf
+struct ifconf {
+ int ifc_len; /* size of buffer */
+ union {
+ char *ifc_buf; /* buffer address */
+ struct ifreq *ifc_req; /* array of structures */
+ };
+};
+.fi
+.in
+
+If
.I ifc_req
-and its length in bytes in
+is NULL,
+.B SIOCGIFCONF
+returns the necessary buffer size in bytes
+for receiving all available addresses in
.IR ifc_len .
-The kernel fills the ifreqs with all current L3 interface addresses that
-are running:
+Otherwise
+.I ifc_req
+contains a pointer to an array of
+.I ifreq
+structures to be filled with all currently active L3 interface addresses.
+.I ifc_len
+contains the size of the array in bytes.
+Within each
+.I ifreq
+structure,
.I ifr_name
-contains the interface name (eth0:1 etc.),
+will receive the interface name, and
.I ifr_addr
the address.
-The kernel returns with the actual length in
+The actual number of bytes transferred is returned in
.IR ifc_len .
-If
+
+If the size specified by
+.I ifc_len
+is insufficient to store all the addresses,
+the kernel will skip the exceeding ones and return success.
+There is no reliable way of detecting this condition once it has occurred.
+It is therefore recommended to either determine the necessary buffer size
+beforehand by calling
+.B SIOCGIFCONF
+with
+.I ifc_req
+set to NULL, or to retry the call with a bigger buffer whenever
.I ifc_len
-is equal to the original length the buffer probably has overflowed
-and you should retry with a bigger buffer to get all addresses.
-When no error occurs the ioctl returns 0;
-otherwise \-1.
-Overflow is not an error.
+upon return differs by less than sizeof(struct
+.IR ifreq )
+from its original value.
+
+If an error occurs accessing the
+.I ifconf
+or
+.I ifreq
+structures,
+.B EFAULT
+will be returned.
.\" Slaving isn't supported in 2.2
.\" .
.\" .TP
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next] net: xfrm6: silence sparse warning
From: Stephen Hemminger @ 2014-01-09 23:04 UTC (permalink / raw)
To: David Miller; +Cc: ying.xue, steffen.klassert, dborkman, netdev
In-Reply-To: <20140108.005656.1582987448512057112.davem@davemloft.net>
On Wed, 08 Jan 2014 00:56:56 -0500 (EST)
David Miller <davem@davemloft.net> wrote:
> From: Ying Xue <ying.xue@windriver.com>
> Date: Wed, 8 Jan 2014 13:55:09 +0800
>
> > 3. Just drop the patch
>
> This is the only suitable thing to do.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
There was some patches floating around to eliminate uses of variable
length array in the kernel since they aren't supported by CLANG.
^ permalink raw reply
* Re: [PATCH net-next] net/mlx4_en: call gro handler for encapsulated frames
From: Eric Dumazet @ 2014-01-09 22:57 UTC (permalink / raw)
To: Or Gerlitz; +Cc: David Miller, netdev, Amir Vadai, Or Gerlitz, Jerry Chu
In-Reply-To: <CAJZOPZ+U=q==kyYzqgObrmPwj_qYXvon2o40FxkU6AmsprG2og@mail.gmail.com>
On Fri, 2014-01-10 at 00:24 +0200, Or Gerlitz wrote:
> On Thu, Jan 9, 2014 at 8:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > From: Eric Dumazet <edumazet@google.com>
> >
> > In order to use the native GRO handling of encapsulated protocols on
> > mlx4, we need to call napi_gro_receive() instead of netif_receive_skb()
> > unless busy polling is in action.
> >
> > While we are at it, rename mlx4_en_cq_ll_polling() to
> > mlx4_en_cq_busy_polling()
> >
> > Tested with GRE tunnel : GRO aggregation is now performed on the
> > ethernet device instead of being done later on gre device.
>
> Hi Eric,
>
> Just to make clear, this patch serves (very nicely!) encapsulated
> frames for which the HW is not able to do checksum check. For
> non-encapsulated packets or (e.g) vxlan encapsulated packets which are
> checksummed by the HW we already go to the GRO path of the ethernet
> device.
Yes : mlx4 has two different paths, one calling napi_gro_frags(), and
one calling netif_receive_skb()
Obviously, the path calling napi_gro_frags() is already GRO enabled ;)
So if you are saying vxlan hits napi_gro_frags(), thats good to know,
but doesn't seem to be the case for GRE, IPIP, SIT tunnels.
^ permalink raw reply
* [PATCH net-next] hyperv: Add support for physically discontinuous receive buffer
From: Haiyang Zhang @ 2014-01-09 22:24 UTC (permalink / raw)
To: davem, netdev
Cc: haiyangz, kys, olaf, jasowang, linux-kernel, driverdev-devel
This will allow us to use bigger receive buffer, and prevent allocation failure
due to fragmented memory.
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
Reviewed-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel.c | 14 ++++++++------
drivers/net/hyperv/hyperv_net.h | 2 +-
drivers/net/hyperv/netvsc.c | 7 ++-----
3 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index cea623c..69ea36f 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -209,7 +209,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
{
int i;
int pagecount;
- unsigned long long pfn;
struct vmbus_channel_gpadl_header *gpadl_header;
struct vmbus_channel_gpadl_body *gpadl_body;
struct vmbus_channel_msginfo *msgheader;
@@ -219,7 +218,6 @@ static int create_gpadl_header(void *kbuffer, u32 size,
int pfnsum, pfncount, pfnleft, pfncurr, pfnsize;
pagecount = size >> PAGE_SHIFT;
- pfn = virt_to_phys(kbuffer) >> PAGE_SHIFT;
/* do we need a gpadl body msg */
pfnsize = MAX_SIZE_CHANNEL_MESSAGE -
@@ -248,7 +246,8 @@ static int create_gpadl_header(void *kbuffer, u32 size,
gpadl_header->range[0].byte_offset = 0;
gpadl_header->range[0].byte_count = size;
for (i = 0; i < pfncount; i++)
- gpadl_header->range[0].pfn_array[i] = pfn+i;
+ gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(
+ kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
*msginfo = msgheader;
*messagecount = 1;
@@ -301,7 +300,9 @@ static int create_gpadl_header(void *kbuffer, u32 size,
* so the hypervisor gurantees that this is ok.
*/
for (i = 0; i < pfncurr; i++)
- gpadl_body->pfn[i] = pfn + pfnsum + i;
+ gpadl_body->pfn[i] = slow_virt_to_phys(
+ kbuffer + PAGE_SIZE * (pfnsum + i)) >>
+ PAGE_SHIFT;
/* add to msg header */
list_add_tail(&msgbody->msglistentry,
@@ -327,7 +328,8 @@ static int create_gpadl_header(void *kbuffer, u32 size,
gpadl_header->range[0].byte_offset = 0;
gpadl_header->range[0].byte_count = size;
for (i = 0; i < pagecount; i++)
- gpadl_header->range[0].pfn_array[i] = pfn+i;
+ gpadl_header->range[0].pfn_array[i] = slow_virt_to_phys(
+ kbuffer + PAGE_SIZE * i) >> PAGE_SHIFT;
*msginfo = msgheader;
*messagecount = 1;
@@ -344,7 +346,7 @@ nomem:
* vmbus_establish_gpadl - Estabish a GPADL for the specified buffer
*
* @channel: a channel
- * @kbuffer: from kmalloc
+ * @kbuffer: from kmalloc or vmalloc
* @size: page-size multiple
* @gpadl_handle: some funky thing
*/
diff --git a/drivers/net/hyperv/hyperv_net.h b/drivers/net/hyperv/hyperv_net.h
index a26eecb..7b594ce 100644
--- a/drivers/net/hyperv/hyperv_net.h
+++ b/drivers/net/hyperv/hyperv_net.h
@@ -462,7 +462,7 @@ struct nvsp_message {
#define NETVSC_MTU 65536
-#define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*2) /* 2MB */
+#define NETVSC_RECEIVE_BUFFER_SIZE (1024*1024*16) /* 16MB */
#define NETVSC_RECEIVE_BUFFER_ID 0xcafe
diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 93b485b..03a2c6e 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -136,8 +136,7 @@ static int netvsc_destroy_recv_buf(struct netvsc_device *net_device)
if (net_device->recv_buf) {
/* Free up the receive buffer */
- free_pages((unsigned long)net_device->recv_buf,
- get_order(net_device->recv_buf_size));
+ vfree(net_device->recv_buf);
net_device->recv_buf = NULL;
}
@@ -163,9 +162,7 @@ static int netvsc_init_recv_buf(struct hv_device *device)
return -ENODEV;
ndev = net_device->ndev;
- net_device->recv_buf =
- (void *)__get_free_pages(GFP_KERNEL|__GFP_ZERO,
- get_order(net_device->recv_buf_size));
+ net_device->recv_buf = vzalloc(net_device->recv_buf_size);
if (!net_device->recv_buf) {
netdev_err(ndev, "unable to allocate receive "
"buffer of size %d\n", net_device->recv_buf_size);
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH net-next] net/mlx4_en: call gro handler for encapsulated frames
From: Or Gerlitz @ 2014-01-09 22:24 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Amir Vadai, Or Gerlitz, Jerry Chu
On Thu, Jan 9, 2014 at 8:30 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> In order to use the native GRO handling of encapsulated protocols on
> mlx4, we need to call napi_gro_receive() instead of netif_receive_skb()
> unless busy polling is in action.
>
> While we are at it, rename mlx4_en_cq_ll_polling() to
> mlx4_en_cq_busy_polling()
>
> Tested with GRE tunnel : GRO aggregation is now performed on the
> ethernet device instead of being done later on gre device.
Hi Eric,
Just to make clear, this patch serves (very nicely!) encapsulated
frames for which the HW is not able to do checksum check. For
non-encapsulated packets or (e.g) vxlan encapsulated packets which are
checksummed by the HW we already go to the GRO path of the ethernet
device.
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Amir Vadai <amirv@mellanox.com>
> Cc: Jerry Chu <hkchu@google.com>
> Cc: Or Gerlitz <ogerlitz@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx4/en_rx.c | 8 +++++---
> drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 4 ++--
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index 3b66f26ba049..890922c1c8ee 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -724,7 +724,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
> * - not an IP fragment
> * - no LLS polling in progress
> */
> - if (!mlx4_en_cq_ll_polling(cq) &&
> + if (!mlx4_en_cq_busy_polling(cq) &&
> (dev->features & NETIF_F_GRO)) {
> struct sk_buff *gro_skb = napi_get_frags(&cq->napi);
> if (!gro_skb)
> @@ -816,8 +816,10 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
>
> skb_mark_napi_id(skb, &cq->napi);
>
> - /* Push it up the stack */
> - netif_receive_skb(skb);
> + if (!mlx4_en_cq_busy_polling(cq))
> + napi_gro_receive(&cq->napi, skb);
> + else
> + netif_receive_skb(skb);
>
> next:
> for (nr = 0; nr < priv->num_frags; nr++)
> diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> index 2f1e200f2e4c..fe7bdfebf353 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
> @@ -661,7 +661,7 @@ static inline bool mlx4_en_cq_unlock_poll(struct mlx4_en_cq *cq)
> }
>
> /* true if a socket is polling, even if it did not get the lock */
> -static inline bool mlx4_en_cq_ll_polling(struct mlx4_en_cq *cq)
> +static inline bool mlx4_en_cq_busy_polling(struct mlx4_en_cq *cq)
> {
> WARN_ON(!(cq->state & MLX4_CQ_LOCKED));
> return cq->state & CQ_USER_PEND;
> @@ -691,7 +691,7 @@ static inline bool mlx4_en_cq_unlock_poll(struct mlx4_en_cq *cq)
> return false;
> }
>
> -static inline bool mlx4_en_cq_ll_polling(struct mlx4_en_cq *cq)
> +static inline bool mlx4_en_cq_busy_polling(struct mlx4_en_cq *cq)
> {
> return false;
> }
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: Stephen Hemminger @ 2014-01-09 22:20 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Jason Wang, John Fastabend, John Fastabend, Neil Horman, davem,
netdev, linux-kernel, Vlad Yasevich
In-Reply-To: <20140109220323.GC9385@redhat.com>
On Fri, 10 Jan 2014 00:03:23 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Thu, Jan 09, 2014 at 01:39:08PM -0800, Stephen Hemminger wrote:
> > On Thu, 09 Jan 2014 16:55:07 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> >
> > > What if use do want a qdisc and want to change the its queue length for
> > > tun/macvlan? And the the name tx_queue_length is misleading. For tun it
> > > may make sense since it was used in transmission path. For macvtap it
> > > was not. So maybe what we need is just a new ioctl for both tun/macvtap
> > > and a new feature flag. If user create the device with new feature flag,
> > > the socket receive queue length could be changed by ioctl instead of
> > > dev->tx_queue_length. If not, the old behaviour could be kept.
> >
> > The overloading of tx_queue_len in macvtap was the original design mistake.
> > Can't this just be undone and expose rx_queue_len as sysfs attribute?
>
> Yes but we need to avoid breaking user-visible ABI.
I think in this case, it was a mistake and hasn't been around long enough
to cause serious damage.
> So I think we'll need to catch any access attempts and redirect them to
> the new rx_queue_len. I posted a patch like this using new
> ndo_set_tx_queue_len/ndo_get_tx_queue_len. Have you seen it? What do
> you think?
It encourages others to do/make the same mistake so I don't like it.
^ permalink raw reply
* Re: [net-next 05/15] i40e: add a comment on barrier and fix panic on reset
From: Scott Feldman @ 2014-01-09 22:16 UTC (permalink / raw)
To: Jeff Kirsher
Cc: David Miller, Greg Rose, Netdev, gospo, sassmann, Mitch Williams,
Jesse Brandeburg
In-Reply-To: <1389271944-26227-6-git-send-email-jeffrey.t.kirsher@intel.com>
On Jan 9, 2014, at 4:52 AM, Jeff Kirsher <jeffrey.t.kirsher@intel.com> wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
>
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
> index ea76134..5cdc67c 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
> @@ -422,7 +422,7 @@ void i40e_vsi_reset_stats(struct i40e_vsi *vsi)
> memset(&vsi->net_stats_offsets, 0, sizeof(vsi->net_stats_offsets));
> memset(&vsi->eth_stats, 0, sizeof(vsi->eth_stats));
> memset(&vsi->eth_stats_offsets, 0, sizeof(vsi->eth_stats_offsets));
> - if (vsi->rx_rings)
> + if (vsi->rx_rings && vsi->rx_rings[0]) {
Any reason why just [0] is checked for !NULL here...
> for (i = 0; i < vsi->num_queue_pairs; i++) {
> memset(&vsi->rx_rings[i]->stats, 0 ,
> sizeof(vsi->rx_rings[i]->stats));
> @@ -433,6 +433,7 @@ void i40e_vsi_reset_stats(struct i40e_vsi *vsi)
> memset(&vsi->tx_rings[i]->tx_stats, 0,
> sizeof(vsi->tx_rings[i]->tx_stats));
> }
> + }
> vsi->stat_offsets_loaded = false;
> }
>
> @@ -2101,8 +2105,11 @@ static void i40e_vsi_free_rx_resources(struct i40e_vsi *vsi)
> {
> int i;
>
> + if (!vsi->rx_rings)
> + return;
> +
> for (i = 0; i < vsi->num_queue_pairs; i++)
> - if (vsi->rx_rings[i]->desc)
> + if (vsi->rx_rings[i] && vsi->rx_rings[i]->desc)
but here every [i] is checked for !NULL here?
> i40e_free_rx_resources(vsi->rx_rings[i]);
> }
>
If [0] check is sufficient to know if array members are allocated, maybe an wrapper func would help document intent:
static bool i40e_vsi_rings_allocated(struct i40e_ring *ring)
{
return (ring && ring[0]);
}
-scott
^ permalink raw reply
* [PATCH net-next] net: gro: change GRO overflow strategy
From: Eric Dumazet @ 2014-01-09 22:12 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Jerry Chu
From: Eric Dumazet <edumazet@google.com>
GRO layer has a limit of 8 flows being held in GRO list,
for performance reason.
When a packet comes for a flow not yet in the list,
and list is full, we immediately give it to upper
stacks, lowering aggregation performance.
With TSO auto sizing and FQ packet scheduler, this situation
happens more often.
This patch changes strategy to simply evict the oldest flow of
the list. This works better because of the nature of packet
trains for which GRO is efficient. This also has the effect
of lowering the GRO latency if many flows are competing.
Tested :
Used a 40Gbps NIC, with 4 RX queues, and 200 concurrent TCP_STREAM
netperf.
Before patch, aggregate rate is 11Gbps (while a single flow can reach
30Gbps)
After patch, line rate is reached.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Jerry Chu <hkchu@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
net/core/dev.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index ce01847793c0..a8280154c42a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3882,10 +3882,23 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
if (same_flow)
goto ok;
- if (NAPI_GRO_CB(skb)->flush || napi->gro_count >= MAX_GRO_SKBS)
+ if (NAPI_GRO_CB(skb)->flush)
goto normal;
- napi->gro_count++;
+ if (unlikely(napi->gro_count >= MAX_GRO_SKBS)) {
+ struct sk_buff *nskb = napi->gro_list;
+
+ /* locate the end of the list to select the 'oldest' flow */
+ while (nskb->next) {
+ pp = &nskb->next;
+ nskb = *pp;
+ }
+ *pp = NULL;
+ nskb->next = NULL;
+ napi_gro_complete(nskb);
+ } else {
+ napi->gro_count++;
+ }
NAPI_GRO_CB(skb)->count = 1;
NAPI_GRO_CB(skb)->age = jiffies;
skb_shinfo(skb)->gso_size = skb_gro_len(skb);
^ permalink raw reply related
* Re: [PATCH net-next 1/2] include/uapi/linux/xfrm.h: Pack struct xfrm_userpolicy_info
From: Sergei Shtylyov @ 2014-01-09 23:07 UTC (permalink / raw)
To: Fan Du; +Cc: steffen.klassert, davem, stephen, dev, netdev
In-Reply-To: <52CF29B1.1000905@cogentembedded.com>
On 01/10/2014 01:58 AM, Sergei Shtylyov wrote:
>>>> Otherwise 64bits kernel has sizeof(struct xfrm_userpolicy_info) 168 bytes,
>>>> while 32bits compiled iproute2 see the same structure as 164 bytes, which
>>>> leading deficit xfrm policy, in turn broken IPsec connectivity.
>>>> Fix this by packing the structure.
>>> This will force byte-by-byte access to all members on some arches like
>>> ARM...
>>>> Signed-off-by: Fan Du <fan.du@windriver.com>
>>>> ---
>>>> include/uapi/linux/xfrm.h | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
>>>> index a8cd6a4..470bfae 100644
>>>> --- a/include/uapi/linux/xfrm.h
>>>> +++ b/include/uapi/linux/xfrm.h
>>>> @@ -405,7 +405,7 @@ struct xfrm_userpolicy_info {
>>>> /* Automatically expand selector to include matching ICMP payloads. */
>>>> #define XFRM_POLICY_ICMP 2
>>>> __u8 share;
>>>> -};
>>>> +} __attribute__((packed));
>>> Please use the __packed macro instead. I guess you haven't run
>>> checkpatch.pl?
>> Lucky me, I run checkpatch every time before sending patch out.
> Ah, this time it didn't have the *struct* start in the context, so that's
> why there was no complaint (probably). Usually, it suggests using __packed...
Ah, no. Looking at the script, it just doesn't WARN about this in the
files under include/uapi/. Probably the macro is undefined in this context.
WBR, Sergei
^ permalink raw reply
* Re: [PATCH net 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: Michael S. Tsirkin @ 2014-01-09 22:03 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jason Wang, John Fastabend, John Fastabend, Neil Horman, davem,
netdev, linux-kernel, Vlad Yasevich
In-Reply-To: <20140109133908.354a4d73@nehalam.linuxnetplumber.net>
On Thu, Jan 09, 2014 at 01:39:08PM -0800, Stephen Hemminger wrote:
> On Thu, 09 Jan 2014 16:55:07 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
> > What if use do want a qdisc and want to change the its queue length for
> > tun/macvlan? And the the name tx_queue_length is misleading. For tun it
> > may make sense since it was used in transmission path. For macvtap it
> > was not. So maybe what we need is just a new ioctl for both tun/macvtap
> > and a new feature flag. If user create the device with new feature flag,
> > the socket receive queue length could be changed by ioctl instead of
> > dev->tx_queue_length. If not, the old behaviour could be kept.
>
> The overloading of tx_queue_len in macvtap was the original design mistake.
> Can't this just be undone and expose rx_queue_len as sysfs attribute?
Yes but we need to avoid breaking user-visible ABI.
So I think we'll need to catch any access attempts and redirect them to
the new rx_queue_len. I posted a patch like this using new
ndo_set_tx_queue_len/ndo_get_tx_queue_len. Have you seen it? What do
you think?
^ permalink raw reply
* Re: [PATCH net-next 1/2] include/uapi/linux/xfrm.h: Pack struct xfrm_userpolicy_info
From: Sergei Shtylyov @ 2014-01-09 22:58 UTC (permalink / raw)
To: Fan Du; +Cc: steffen.klassert, davem, stephen, dev, netdev
In-Reply-To: <52CE6052.5030905@windriver.com>
On 01/09/2014 11:39 AM, Fan Du wrote:
>>> Otherwise 64bits kernel has sizeof(struct xfrm_userpolicy_info) 168 bytes,
>>> while 32bits compiled iproute2 see the same structure as 164 bytes, which
>>> leading deficit xfrm policy, in turn broken IPsec connectivity.
>>> Fix this by packing the structure.
>> This will force byte-by-byte access to all members on some arches like
>> ARM...
>>> Signed-off-by: Fan Du <fan.du@windriver.com>
>>> ---
>>> include/uapi/linux/xfrm.h | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
>>> index a8cd6a4..470bfae 100644
>>> --- a/include/uapi/linux/xfrm.h
>>> +++ b/include/uapi/linux/xfrm.h
>>> @@ -405,7 +405,7 @@ struct xfrm_userpolicy_info {
>>> /* Automatically expand selector to include matching ICMP payloads. */
>>> #define XFRM_POLICY_ICMP 2
>>> __u8 share;
>>> -};
>>> +} __attribute__((packed));
>> Please use the __packed macro instead. I guess you haven't run
>> checkpatch.pl?
> Lucky me, I run checkpatch every time before sending patch out.
Ah, this time it didn't have the *struct* start in the context, so that's
why there was no complaint (probably). Usually, it suggests using __packed...
WBR, Sergei
^ permalink raw reply
* Re: [net-next 05/15] i40e: add a comment on barrier and fix panic on reset
From: David Miller @ 2014-01-09 21:56 UTC (permalink / raw)
To: bhutchings
Cc: gregory.v.rose, sergei.shtylyov, jeffrey.t.kirsher, netdev, gospo,
sassmann, mitch.a.williams, jesse.brandeburg
In-Reply-To: <1389302590.2025.55.camel@bwh-desktop.uk.level5networks.com>
From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 9 Jan 2014 21:23:10 +0000
> On Thu, 2014-01-09 at 21:21 +0000, Ben Hutchings wrote:
>> On Thu, 2014-01-09 at 15:12 -0500, David Miller wrote:
>> > From: Greg Rose <gregory.v.rose@intel.com>
>> > Date: Thu, 9 Jan 2014 09:17:35 -0800
>> >
>> > > Yes, it's apparent that two different internal patches were compressed
>> > > together. If its unacceptable to do this then I'll speak to Jeff about
>> > > splitting them.
>> >
>> > I won't reject the entire pull request on account of this, but please
>> > don't do this in the future.
>>
>> I don't understand this. You don't want to see patches that are known
>> to introduce regressions, and you expect people to squash together known
>> buggy patches with their subsequent fix-ups before submitting.
> [...]
>
> Hmm, looking at this again, I suppose it is just this particular
> combination of two unrelated changes you're objecting to?
Yes.
^ permalink raw reply
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Andrey Wagin @ 2014-01-09 21:46 UTC (permalink / raw)
To: Eric Dumazet
Cc: Florian Westphal, netfilter-devel, netfilter, coreteam, netdev,
LKML, vvs, Pablo Neira Ayuso, Patrick McHardy, Jozsef Kadlecsik,
David S. Miller, Cyrill Gorcunov
In-Reply-To: <1389281025.31367.35.camel@edumazet-glaptop2.roam.corp.google.com>
2014/1/9 Eric Dumazet <eric.dumazet@gmail.com>:
> On Thu, 2014-01-09 at 09:24 +0400, Andrew Vagin wrote:
>
>> I have one more question. Looks like I found another problem.
>>
>> init_conntrack:
>> hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode,
>> &net->ct.unconfirmed);
>>
>> __nf_conntrack_hash_insert:
>> hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode,
>> &net->ct.hash[hash]);
>>
>> We use one hlist_nulls_node to add a conntrack into two different lists.
>> As far as I understand, it's unacceptable in case of
>> SLAB_DESTROY_BY_RCU.
>
> I guess you missed :
>
> net/netfilter/nf_conntrack_core.c:1598: INIT_HLIST_NULLS_HEAD(&net->ct.unconfirmed, UNCONFIRMED_NULLS_VAL);
but we can look up something suitable and return this value, but it
will be unconfirmed. Ok, I see. This situation is fixed by this patch
too.
I don't understand the result of your discussion with Florian. Here
are a few states of conntracts: it can be used and it's initialized.
The sign of the first state is non-zero refcnt and the sign of the
second state is the confirmed bit.
In the first state conntrack is attached to skb and inserted in the
unconfirmed list. In this state the use count can be incremented in
ctnetlink_dump_list() and skb_clone().
In the second state conntrack may be attached to a few skb-s and
inserted to net->ct.hash.
I have read all emails again and I can't understand when this patch
doesn't work. Maybe you could give a sequence of actions? Thanks.
>
>
>>
>> Lets imagine that we have two threads. The first one enumerates objects
>> from a first list, the second one recreates an object and insert it in a
>> second list. If a first thread in this moment stays on the object, it
>> can read "next", when it's in the second list, so it will continue
>> to enumerate objects from the second list. It is not what we want, isn't
>> it?
>>
>> cpu1 cpu2
>> hlist_nulls_for_each_entry_rcu(ct)
>> destroy_conntrack
>> kmem_cache_free
>>
>> init_conntrack
>> hlist_nulls_add_head_rcu
>> ct = ct->next
>>
>
> This will be fine.
>
> I think we even have a counter to count number of occurrence of this
> rare event. (I personally never read a non null search_restart value)
>
> NF_CT_STAT_INC(net, search_restart);
Thank you for explanation.
^ permalink raw reply
* Re: [PATCH net 1/2] macvlan: forbid L2 fowarding offload for macvtap
From: Stephen Hemminger @ 2014-01-09 21:39 UTC (permalink / raw)
To: Jason Wang
Cc: Michael S. Tsirkin, John Fastabend, John Fastabend, Neil Horman,
davem, netdev, linux-kernel, Vlad Yasevich
In-Reply-To: <52CE63EB.5000109@redhat.com>
On Thu, 09 Jan 2014 16:55:07 +0800
Jason Wang <jasowang@redhat.com> wrote:
> What if use do want a qdisc and want to change the its queue length for
> tun/macvlan? And the the name tx_queue_length is misleading. For tun it
> may make sense since it was used in transmission path. For macvtap it
> was not. So maybe what we need is just a new ioctl for both tun/macvtap
> and a new feature flag. If user create the device with new feature flag,
> the socket receive queue length could be changed by ioctl instead of
> dev->tx_queue_length. If not, the old behaviour could be kept.
The overloading of tx_queue_len in macvtap was the original design mistake.
Can't this just be undone and expose rx_queue_len as sysfs attribute?
^ permalink raw reply
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Florian Westphal @ 2014-01-09 21:26 UTC (permalink / raw)
To: Andrew Vagin
Cc: Florian Westphal, Eric Dumazet, Andrey Vagin, netfilter-devel,
netfilter, coreteam, netdev, linux-kernel, vvs, Pablo Neira Ayuso,
Patrick McHardy, Jozsef Kadlecsik, David S. Miller,
Cyrill Gorcunov
In-Reply-To: <20140109210749.GA29440@paralelels.com>
Andrew Vagin <avagin@parallels.com> wrote:
> On Thu, Jan 09, 2014 at 09:56:22PM +0100, Florian Westphal wrote:
> > Andrew Vagin <avagin@parallels.com> wrote:
> > > Can we allocate conntrack with zero ct_general.use and increment it at
> > > the first time before inserting the conntrack into the hash table?
> > > When conntrack is allocated it is attached exclusively to one skb.
> > > It must be destroyed with skb, if it has not been confirmed, so we
> > > don't need refcnt on this stage.
> > >
> > > I found only one place, where a reference counter of unconfirmed
> > > conntract can incremented. It's ctnetlink_dump_table().
> >
> > What about skb_clone, etc? They will also increment the refcnt
> > if a conntrack entry is attached to the skb.
>
> We can not attach an unconfirmed conntrack to a few skb, because
s/few/new/?
> nf_nat_setup_info can be executed concurrently for the same conntrack.
>
> How do we avoid this race condition for cloned skb-s?
Simple, the assumption is that only one cpu owns the nfct, so it does
not matter if the skb is cloned in between, as there are no parallel
users.
The only possibility (that I know of) to violate this is to create
a bridge, enable call-iptables sysctl, add -j NFQUEUE rules and then
wait for packets that need to be forwarded to several recipients,
e.g. multicast traffic.
see
http://marc.info/?l=netfilter-devel&m=131471083501656&w=2
or search 'netfilter: nat: work around shared nfct struct in bridge
case'
^ permalink raw reply
* Re: [net-next 05/15] i40e: add a comment on barrier and fix panic on reset
From: Ben Hutchings @ 2014-01-09 21:23 UTC (permalink / raw)
To: David Miller
Cc: gregory.v.rose, sergei.shtylyov, jeffrey.t.kirsher, netdev, gospo,
sassmann, mitch.a.williams, jesse.brandeburg
In-Reply-To: <1389302517.2025.54.camel@bwh-desktop.uk.level5networks.com>
On Thu, 2014-01-09 at 21:21 +0000, Ben Hutchings wrote:
> On Thu, 2014-01-09 at 15:12 -0500, David Miller wrote:
> > From: Greg Rose <gregory.v.rose@intel.com>
> > Date: Thu, 9 Jan 2014 09:17:35 -0800
> >
> > > Yes, it's apparent that two different internal patches were compressed
> > > together. If its unacceptable to do this then I'll speak to Jeff about
> > > splitting them.
> >
> > I won't reject the entire pull request on account of this, but please
> > don't do this in the future.
>
> I don't understand this. You don't want to see patches that are known
> to introduce regressions, and you expect people to squash together known
> buggy patches with their subsequent fix-ups before submitting.
[...]
Hmm, looking at this again, I suppose it is just this particular
combination of two unrelated changes you're objecting to?
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [net-next 05/15] i40e: add a comment on barrier and fix panic on reset
From: Ben Hutchings @ 2014-01-09 21:21 UTC (permalink / raw)
To: David Miller
Cc: gregory.v.rose, sergei.shtylyov, jeffrey.t.kirsher, netdev, gospo,
sassmann, mitch.a.williams, jesse.brandeburg
In-Reply-To: <20140109.151224.1539252523028219271.davem@davemloft.net>
On Thu, 2014-01-09 at 15:12 -0500, David Miller wrote:
> From: Greg Rose <gregory.v.rose@intel.com>
> Date: Thu, 9 Jan 2014 09:17:35 -0800
>
> > Yes, it's apparent that two different internal patches were compressed
> > together. If its unacceptable to do this then I'll speak to Jeff about
> > splitting them.
>
> I won't reject the entire pull request on account of this, but please
> don't do this in the future.
I don't understand this. You don't want to see patches that are known
to introduce regressions, and you expect people to squash together known
buggy patches with their subsequent fix-ups before submitting.
So, if a Change-Id is supposed to refer back to an 'original' version of
a patch/commit, shouldn't there sometimes be more than one of them in
the version that goes upstream? You do something very similar yourself
sometimes, with upstream references in stable backports.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox