* [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements
@ 2018-10-08 10:22 Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 01/12] net: sched: cls_u32: mark root hnode explicitly Jamal Hadi Salim
` (12 more replies)
0 siblings, 13 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Jamal Hadi Salim <hadi@mojatatu.com>
Various improvements from Al.
Changes from version 1: Add missing commit
Al Viro (11):
net: sched: cls_u32: mark root hnode explicitly
net: sched: cls_u32: disallow linking to root hnode
net: sched: cls_u32: make sure that divisor is a power of 2
net: sched: cls_u32: get rid of unused argument of u32_destroy_key()
net: sched: cls_u32: get rid of tc_u_knode ->tp
net: sched: cls_u32: get rid of tc_u_common ->rcu
net: sched: cls_u32: clean tc_u_common hashtable
net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of
tc_u_hnode
net: sched: cls_u32: the tp_c argument of u32_set_parms() is always
tp->data
net: sched: cls_u32: get rid of tp_c
net: sched: cls_u32: keep track of knodes count in tc_u_common
net: sched: cls_u32: simplify the hell out u32_delete() emptiness
check
net/sched/cls_u32.c | 121 +++++++++++++++++-----------------------------------
1 file changed, 38 insertions(+), 83 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH net-next v2 01/12] net: sched: cls_u32: mark root hnode explicitly
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 02/12] net: sched: cls_u32: disallow linking to root hnode Jamal Hadi Salim
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
... and produce consistent error on attempt to delete such.
Existing check in u32_delete() is inconsistent - after
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 100 handle 1: u32 \
divisor 1
tc filter add dev eth0 parent ffff: protocol ip prio 200 handle 2: u32 \
divisor 1
both
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 801: u32
and
tc filter delete dev eth0 parent ffff: protocol ip prio 100 handle 800: u32
will fail (at least with refcounting fixes), but the former will complain
about an attempt to remove a busy table, while the latter will recognize
it as root and yield "Not allowed to delete root node" instead.
The problem with the existing check is that several tcf_proto instances
might share the same tp->data and handle-to-hnode lookup will be the same
for all of them. So comparing an hnode to be deleted with tp->root won't
catch the case when one tp is used to try deleting the root of another.
Solution is trivial - mark the root hnodes explicitly upon allocation and
check for that.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index f218ccf1e2d9..622f4657da94 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -84,6 +84,7 @@ struct tc_u_hnode {
int refcnt;
unsigned int divisor;
struct idr handle_idr;
+ bool is_root;
struct rcu_head rcu;
u32 flags;
/* The 'ht' field MUST be the last field in structure to allow for
@@ -377,6 +378,7 @@ static int u32_init(struct tcf_proto *tp)
root_ht->refcnt++;
root_ht->handle = tp_c ? gen_new_htid(tp_c, root_ht) : 0x80000000;
root_ht->prio = tp->prio;
+ root_ht->is_root = true;
idr_init(&root_ht->handle_idr);
if (tp_c == NULL) {
@@ -692,7 +694,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
goto out;
}
- if (root_ht == ht) {
+ if (ht->is_root) {
NL_SET_ERR_MSG_MOD(extack, "Not allowed to delete root node");
return -EINVAL;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 02/12] net: sched: cls_u32: disallow linking to root hnode
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 01/12] net: sched: cls_u32: mark root hnode explicitly Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 03/12] net: sched: cls_u32: make sure that divisor is a power of 2 Jamal Hadi Salim
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
Operation makes no sense. Nothing will actually break if we do so
(depth limit in u32_classify() will prevent infinite loops), but
according to maintainers it's best prohibited outright.
NOTE: doing so guarantees that u32_destroy() will trigger the call
of u32_destroy_hnode(); we might want to make that unconditional.
Test:
tc qdisc add dev eth0 ingress
tc filter add dev eth0 parent ffff: protocol ip prio 100 u32 \
link 800: offset at 0 mask 0f00 shift 6 plus 0 eat match ip protocol 6 ff
should fail with
Error: cls_u32: Not linking to root node
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 622f4657da94..3357331a80a2 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -797,6 +797,10 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
NL_SET_ERR_MSG_MOD(extack, "Link hash table not found");
return -EINVAL;
}
+ if (ht_down->is_root) {
+ NL_SET_ERR_MSG_MOD(extack, "Not linking to root node");
+ return -EINVAL;
+ }
ht_down->refcnt++;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 03/12] net: sched: cls_u32: make sure that divisor is a power of 2
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 01/12] net: sched: cls_u32: mark root hnode explicitly Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 02/12] net: sched: cls_u32: disallow linking to root hnode Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 04/12] net: sched: cls_u32: get rid of unused argument of u32_destroy_key() Jamal Hadi Salim
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
Tested by modifying iproute2 to allow sending a divisor > 255
Tested-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 3357331a80a2..ce55eea448a0 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -994,7 +994,11 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (tb[TCA_U32_DIVISOR]) {
unsigned int divisor = nla_get_u32(tb[TCA_U32_DIVISOR]);
- if (--divisor > 0x100) {
+ if (!is_power_of_2(divisor)) {
+ NL_SET_ERR_MSG_MOD(extack, "Divisor is not a power of 2");
+ return -EINVAL;
+ }
+ if (divisor-- > 0x100) {
NL_SET_ERR_MSG_MOD(extack, "Exceeded maximum 256 hash buckets");
return -EINVAL;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 04/12] net: sched: cls_u32: get rid of unused argument of u32_destroy_key()
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (2 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 03/12] net: sched: cls_u32: make sure that divisor is a power of 2 Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 05/12] net: sched: cls_u32: get rid of tc_u_knode ->tp Jamal Hadi Salim
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index ce55eea448a0..ef0f2e6ec422 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -405,8 +405,7 @@ static int u32_init(struct tcf_proto *tp)
return 0;
}
-static int u32_destroy_key(struct tcf_proto *tp, struct tc_u_knode *n,
- bool free_pf)
+static int u32_destroy_key(struct tc_u_knode *n, bool free_pf)
{
struct tc_u_hnode *ht = rtnl_dereference(n->ht_down);
@@ -440,7 +439,7 @@ static void u32_delete_key_work(struct work_struct *work)
struct tc_u_knode,
rwork);
rtnl_lock();
- u32_destroy_key(key->tp, key, false);
+ u32_destroy_key(key, false);
rtnl_unlock();
}
@@ -457,7 +456,7 @@ static void u32_delete_key_freepf_work(struct work_struct *work)
struct tc_u_knode,
rwork);
rtnl_lock();
- u32_destroy_key(key->tp, key, true);
+ u32_destroy_key(key, true);
rtnl_unlock();
}
@@ -600,7 +599,7 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
if (tcf_exts_get_net(&n->exts))
tcf_queue_work(&n->rwork, u32_delete_key_freepf_work);
else
- u32_destroy_key(n->tp, n, true);
+ u32_destroy_key(n, true);
}
}
}
@@ -971,13 +970,13 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
tca[TCA_RATE], ovr, extack);
if (err) {
- u32_destroy_key(tp, new, false);
+ u32_destroy_key(new, false);
return err;
}
err = u32_replace_hw_knode(tp, new, flags, extack);
if (err) {
- u32_destroy_key(tp, new, false);
+ u32_destroy_key(new, false);
return err;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 05/12] net: sched: cls_u32: get rid of tc_u_knode ->tp
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (3 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 04/12] net: sched: cls_u32: get rid of unused argument of u32_destroy_key() Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 06/12] net: sched: cls_u32: get rid of tc_u_common ->rcu Jamal Hadi Salim
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
not used anymore
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index ef0f2e6ec422..810c49ac1bbe 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -68,7 +68,6 @@ struct tc_u_knode {
u32 mask;
u32 __percpu *pcpu_success;
#endif
- struct tcf_proto *tp;
struct rcu_work rwork;
/* The 'sel' field MUST be the last field in structure to allow for
* tc_u32_keys allocated at end of structure.
@@ -896,7 +895,6 @@ static struct tc_u_knode *u32_init_knode(struct tcf_proto *tp,
/* Similarly success statistics must be moved as pointers */
new->pcpu_success = n->pcpu_success;
#endif
- new->tp = tp;
memcpy(&new->sel, s, sizeof(*s) + s->nkeys*sizeof(struct tc_u32_key));
if (tcf_exts_init(&new->exts, TCA_U32_ACT, TCA_U32_POLICE)) {
@@ -1112,7 +1110,6 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
n->handle = handle;
n->fshift = s->hmask ? ffs(ntohl(s->hmask)) - 1 : 0;
n->flags = flags;
- n->tp = tp;
err = tcf_exts_init(&n->exts, TCA_U32_ACT, TCA_U32_POLICE);
if (err < 0)
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 06/12] net: sched: cls_u32: get rid of tc_u_common ->rcu
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (4 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 05/12] net: sched: cls_u32: get rid of tc_u_knode ->tp Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 07/12] net: sched: cls_u32: clean tc_u_common hashtable Jamal Hadi Salim
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
unused
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 810c49ac1bbe..c378168f4562 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -98,7 +98,6 @@ struct tc_u_common {
int refcnt;
struct idr handle_idr;
struct hlist_node hnode;
- struct rcu_head rcu;
};
static inline unsigned int u32_hash_fold(__be32 key,
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 07/12] net: sched: cls_u32: clean tc_u_common hashtable
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (5 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 06/12] net: sched: cls_u32: get rid of tc_u_common ->rcu Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 08/12] net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of tc_u_hnode Jamal Hadi Salim
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
* calculate key *once*, not for each hash chain element
* let tc_u_hash() return the pointer to chain head rather than index -
callers are cleaner that way.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index c378168f4562..3f6fba831c57 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -343,19 +343,16 @@ static void *tc_u_common_ptr(const struct tcf_proto *tp)
return block->q;
}
-static unsigned int tc_u_hash(const struct tcf_proto *tp)
+static struct hlist_head *tc_u_hash(void *key)
{
- return hash_ptr(tc_u_common_ptr(tp), U32_HASH_SHIFT);
+ return tc_u_common_hash + hash_ptr(key, U32_HASH_SHIFT);
}
-static struct tc_u_common *tc_u_common_find(const struct tcf_proto *tp)
+static struct tc_u_common *tc_u_common_find(void *key)
{
struct tc_u_common *tc;
- unsigned int h;
-
- h = tc_u_hash(tp);
- hlist_for_each_entry(tc, &tc_u_common_hash[h], hnode) {
- if (tc->ptr == tc_u_common_ptr(tp))
+ hlist_for_each_entry(tc, tc_u_hash(key), hnode) {
+ if (tc->ptr == key)
return tc;
}
return NULL;
@@ -364,10 +361,8 @@ static struct tc_u_common *tc_u_common_find(const struct tcf_proto *tp)
static int u32_init(struct tcf_proto *tp)
{
struct tc_u_hnode *root_ht;
- struct tc_u_common *tp_c;
- unsigned int h;
-
- tp_c = tc_u_common_find(tp);
+ void *key = tc_u_common_ptr(tp);
+ struct tc_u_common *tp_c = tc_u_common_find(key);
root_ht = kzalloc(sizeof(*root_ht), GFP_KERNEL);
if (root_ht == NULL)
@@ -385,12 +380,11 @@ static int u32_init(struct tcf_proto *tp)
kfree(root_ht);
return -ENOBUFS;
}
- tp_c->ptr = tc_u_common_ptr(tp);
+ tp_c->ptr = key;
INIT_HLIST_NODE(&tp_c->hnode);
idr_init(&tp_c->handle_idr);
- h = tc_u_hash(tp);
- hlist_add_head(&tp_c->hnode, &tc_u_common_hash[h]);
+ hlist_add_head(&tp_c->hnode, tc_u_hash(key));
}
tp_c->refcnt++;
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 08/12] net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of tc_u_hnode
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (6 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 07/12] net: sched: cls_u32: clean tc_u_common hashtable Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 09/12] net: sched: cls_u32: the tp_c argument of u32_set_parms() is always tp->data Jamal Hadi Salim
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
the only thing we used ht for was ht->tp_c and callers can get that
without going through ->tp_c at all; start with lifting that into
the callers, next commits will massage those, eventually removing
->tp_c altogether.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 3f6fba831c57..53f34f8cde8b 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -761,7 +761,7 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
};
static int u32_set_parms(struct net *net, struct tcf_proto *tp,
- unsigned long base, struct tc_u_hnode *ht,
+ unsigned long base, struct tc_u_common *tp_c,
struct tc_u_knode *n, struct nlattr **tb,
struct nlattr *est, bool ovr,
struct netlink_ext_ack *extack)
@@ -782,7 +782,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
}
if (handle) {
- ht_down = u32_lookup_ht(ht->tp_c, handle);
+ ht_down = u32_lookup_ht(tp_c, handle);
if (!ht_down) {
NL_SET_ERR_MSG_MOD(extack, "Link hash table not found");
@@ -957,7 +957,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return -ENOMEM;
err = u32_set_parms(net, tp, base,
- rtnl_dereference(n->ht_up), new, tb,
+ rtnl_dereference(n->ht_up)->tp_c, new, tb,
tca[TCA_RATE], ovr, extack);
if (err) {
@@ -1124,7 +1124,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
}
#endif
- err = u32_set_parms(net, tp, base, ht, n, tb, tca[TCA_RATE], ovr,
+ err = u32_set_parms(net, tp, base, ht->tp_c, n, tb, tca[TCA_RATE], ovr,
extack);
if (err == 0) {
struct tc_u_knode __rcu **ins;
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 09/12] net: sched: cls_u32: the tp_c argument of u32_set_parms() is always tp->data
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (7 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 08/12] net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of tc_u_hnode Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 10/12] net: sched: cls_u32: get rid of tp_c Jamal Hadi Salim
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
It must be tc_u_common associated with that tp (i.e. tp->data).
Proof:
* both ->ht_up and ->tp_c are assign-once
* ->tp_c of anything inserted into tp_c->hlist is tp_c
* hnodes never get reinserted into the lists or moved
between those, so anything found by u32_lookup_ht(tp->data, ...)
will have ->tp_c equal to tp->data.
* tp->root->tp_c == tp->data.
* ->ht_up of anything inserted into hnode->ht[...] is
equal to hnode.
* knodes never get reinserted into hash chains or moved
between those, so anything returned by u32_lookup_key(ht, ...)
will have ->ht_up equal to ht.
* any knode returned by u32_get(tp, ...) will have ->ht_up->tp_c
point to tp->data
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 53f34f8cde8b..3ed2c9866b36 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -956,8 +956,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (!new)
return -ENOMEM;
- err = u32_set_parms(net, tp, base,
- rtnl_dereference(n->ht_up)->tp_c, new, tb,
+ err = u32_set_parms(net, tp, base, tp_c, new, tb,
tca[TCA_RATE], ovr, extack);
if (err) {
@@ -1124,7 +1123,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
}
#endif
- err = u32_set_parms(net, tp, base, ht->tp_c, n, tb, tca[TCA_RATE], ovr,
+ err = u32_set_parms(net, tp, base, tp_c, n, tb, tca[TCA_RATE], ovr,
extack);
if (err == 0) {
struct tc_u_knode __rcu **ins;
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 10/12] net: sched: cls_u32: get rid of tp_c
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (8 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 09/12] net: sched: cls_u32: the tp_c argument of u32_set_parms() is always tp->data Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 11/12] net: sched: cls_u32: keep track of knodes count in tc_u_common Jamal Hadi Salim
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
Both hnode ->tp_c and tp_c argument of u32_set_parms()
the latter is redundant, the former - never read...
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 3ed2c9866b36..3d4c360f9b0c 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -79,7 +79,6 @@ struct tc_u_hnode {
struct tc_u_hnode __rcu *next;
u32 handle;
u32 prio;
- struct tc_u_common *tp_c;
int refcnt;
unsigned int divisor;
struct idr handle_idr;
@@ -390,7 +389,6 @@ static int u32_init(struct tcf_proto *tp)
tp_c->refcnt++;
RCU_INIT_POINTER(root_ht->next, tp_c->hlist);
rcu_assign_pointer(tp_c->hlist, root_ht);
- root_ht->tp_c = tp_c;
rcu_assign_pointer(tp->root, root_ht);
tp->data = tp_c;
@@ -761,7 +759,7 @@ static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
};
static int u32_set_parms(struct net *net, struct tcf_proto *tp,
- unsigned long base, struct tc_u_common *tp_c,
+ unsigned long base,
struct tc_u_knode *n, struct nlattr **tb,
struct nlattr *est, bool ovr,
struct netlink_ext_ack *extack)
@@ -782,7 +780,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
}
if (handle) {
- ht_down = u32_lookup_ht(tp_c, handle);
+ ht_down = u32_lookup_ht(tp->data, handle);
if (!ht_down) {
NL_SET_ERR_MSG_MOD(extack, "Link hash table not found");
@@ -956,7 +954,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (!new)
return -ENOMEM;
- err = u32_set_parms(net, tp, base, tp_c, new, tb,
+ err = u32_set_parms(net, tp, base, new, tb,
tca[TCA_RATE], ovr, extack);
if (err) {
@@ -1012,7 +1010,6 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return err;
}
}
- ht->tp_c = tp_c;
ht->refcnt = 1;
ht->divisor = divisor;
ht->handle = handle;
@@ -1123,7 +1120,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
}
#endif
- err = u32_set_parms(net, tp, base, tp_c, n, tb, tca[TCA_RATE], ovr,
+ err = u32_set_parms(net, tp, base, n, tb, tca[TCA_RATE], ovr,
extack);
if (err == 0) {
struct tc_u_knode __rcu **ins;
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 11/12] net: sched: cls_u32: keep track of knodes count in tc_u_common
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (9 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 10/12] net: sched: cls_u32: get rid of tp_c Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 12/12] net: sched: cls_u32: simplify the hell out u32_delete() emptiness check Jamal Hadi Salim
2018-10-08 17:33 ` [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements David Miller
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
allows to simplify u32_delete() considerably
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 3d4c360f9b0c..61593bee08db 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -97,6 +97,7 @@ struct tc_u_common {
int refcnt;
struct idr handle_idr;
struct hlist_node hnode;
+ long knodes;
};
static inline unsigned int u32_hash_fold(__be32 key,
@@ -452,6 +453,7 @@ static void u32_delete_key_freepf_work(struct work_struct *work)
static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
{
+ struct tc_u_common *tp_c = tp->data;
struct tc_u_knode __rcu **kp;
struct tc_u_knode *pkp;
struct tc_u_hnode *ht = rtnl_dereference(key->ht_up);
@@ -462,6 +464,7 @@ static int u32_delete_key(struct tcf_proto *tp, struct tc_u_knode *key)
kp = &pkp->next, pkp = rtnl_dereference(*kp)) {
if (pkp == key) {
RCU_INIT_POINTER(*kp, key->next);
+ tp_c->knodes--;
tcf_unbind_filter(tp, &key->res);
idr_remove(&ht->handle_idr, key->handle);
@@ -576,6 +579,7 @@ static int u32_replace_hw_knode(struct tcf_proto *tp, struct tc_u_knode *n,
static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
struct netlink_ext_ack *extack)
{
+ struct tc_u_common *tp_c = tp->data;
struct tc_u_knode *n;
unsigned int h;
@@ -583,6 +587,7 @@ static void u32_clear_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
while ((n = rtnl_dereference(ht->ht[h])) != NULL) {
RCU_INIT_POINTER(ht->ht[h],
rtnl_dereference(n->next));
+ tp_c->knodes--;
tcf_unbind_filter(tp, &n->res);
u32_remove_hw_knode(tp, n, extack);
idr_remove(&ht->handle_idr, n->handle);
@@ -1141,6 +1146,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
RCU_INIT_POINTER(n->next, pins);
rcu_assign_pointer(*ins, n);
+ tp_c->knodes++;
*arg = n;
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2 12/12] net: sched: cls_u32: simplify the hell out u32_delete() emptiness check
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (10 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 11/12] net: sched: cls_u32: keep track of knodes count in tc_u_common Jamal Hadi Salim
@ 2018-10-08 10:22 ` Jamal Hadi Salim
2018-10-08 17:33 ` [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements David Miller
12 siblings, 0 replies; 14+ messages in thread
From: Jamal Hadi Salim @ 2018-10-08 10:22 UTC (permalink / raw
To: davem; +Cc: xiyou.wangcong, jiri, netdev, viro, Jamal Hadi Salim
From: Al Viro <viro@zeniv.linux.org.uk>
Now that we have the knode count, we can instantly check if
any hnodes are non-empty. And that kills the check for extra
references to root hnode - those could happen only if there was
a knode to carry such a link.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_u32.c | 48 +-----------------------------------------------
1 file changed, 1 insertion(+), 47 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 61593bee08db..ac79a40a0392 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -627,17 +627,6 @@ static int u32_destroy_hnode(struct tcf_proto *tp, struct tc_u_hnode *ht,
return -ENOENT;
}
-static bool ht_empty(struct tc_u_hnode *ht)
-{
- unsigned int h;
-
- for (h = 0; h <= ht->divisor; h++)
- if (rcu_access_pointer(ht->ht[h]))
- return false;
-
- return true;
-}
-
static void u32_destroy(struct tcf_proto *tp, struct netlink_ext_ack *extack)
{
struct tc_u_common *tp_c = tp->data;
@@ -675,13 +664,9 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
struct netlink_ext_ack *extack)
{
struct tc_u_hnode *ht = arg;
- struct tc_u_hnode *root_ht = rtnl_dereference(tp->root);
struct tc_u_common *tp_c = tp->data;
int ret = 0;
- if (ht == NULL)
- goto out;
-
if (TC_U32_KEY(ht->handle)) {
u32_remove_hw_knode(tp, (struct tc_u_knode *)ht, extack);
ret = u32_delete_key(tp, (struct tc_u_knode *)ht);
@@ -702,38 +687,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last,
}
out:
- *last = true;
- if (root_ht) {
- if (root_ht->refcnt > 1) {
- *last = false;
- goto ret;
- }
- if (root_ht->refcnt == 1) {
- if (!ht_empty(root_ht)) {
- *last = false;
- goto ret;
- }
- }
- }
-
- if (tp_c->refcnt > 1) {
- *last = false;
- goto ret;
- }
-
- if (tp_c->refcnt == 1) {
- struct tc_u_hnode *ht;
-
- for (ht = rtnl_dereference(tp_c->hlist);
- ht;
- ht = rtnl_dereference(ht->next))
- if (!ht_empty(ht)) {
- *last = false;
- break;
- }
- }
-
-ret:
+ *last = tp_c->refcnt == 1 && tp_c->knodes == 0;
return ret;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
` (11 preceding siblings ...)
2018-10-08 10:22 ` [PATCH net-next v2 12/12] net: sched: cls_u32: simplify the hell out u32_delete() emptiness check Jamal Hadi Salim
@ 2018-10-08 17:33 ` David Miller
12 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2018-10-08 17:33 UTC (permalink / raw
To: jhs; +Cc: xiyou.wangcong, jiri, netdev, viro, hadi
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Mon, 8 Oct 2018 06:22:32 -0400
> Various improvements from Al.
>
> Changes from version 1: Add missing commit
Series applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-10-09 0:46 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-08 10:22 [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 01/12] net: sched: cls_u32: mark root hnode explicitly Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 02/12] net: sched: cls_u32: disallow linking to root hnode Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 03/12] net: sched: cls_u32: make sure that divisor is a power of 2 Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 04/12] net: sched: cls_u32: get rid of unused argument of u32_destroy_key() Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 05/12] net: sched: cls_u32: get rid of tc_u_knode ->tp Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 06/12] net: sched: cls_u32: get rid of tc_u_common ->rcu Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 07/12] net: sched: cls_u32: clean tc_u_common hashtable Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 08/12] net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of tc_u_hnode Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 09/12] net: sched: cls_u32: the tp_c argument of u32_set_parms() is always tp->data Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 10/12] net: sched: cls_u32: get rid of tp_c Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 11/12] net: sched: cls_u32: keep track of knodes count in tc_u_common Jamal Hadi Salim
2018-10-08 10:22 ` [PATCH net-next v2 12/12] net: sched: cls_u32: simplify the hell out u32_delete() emptiness check Jamal Hadi Salim
2018-10-08 17:33 ` [PATCH net-next v2 00/12] net: sched: cls_u32 Various improvements David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.