* [PATCH net-next 13/17] net: sched: extend proto ops with 'put' callback
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Add optional tp->ops->put() API to be implemented for filter reference
counting. This new function is called by cls API to release filter
reference for filters returned by tp->ops->change() or tp->ops->get()
functions. Implement tfilter_put() helper to call tp->ops->put() only for
classifiers that implement it.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/sch_generic.h | 1 +
net/sched/cls_api.c | 12 +++++++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 100368594524..24103b7282bd 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -273,6 +273,7 @@ struct tcf_proto_ops {
struct netlink_ext_ack *extack);
void* (*get)(struct tcf_proto*, u32 handle);
+ void (*put)(struct tcf_proto *tp, void *f);
int (*change)(struct net *net, struct sk_buff *,
struct tcf_proto*, unsigned long,
u32 handle, struct nlattr **,
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0949502e31b9..7f65ed84b5e5 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1642,6 +1642,12 @@ static void tfilter_notify_chain(struct net *net, struct sk_buff *oskb,
q, parent, NULL, event, false);
}
+static void tfilter_put(struct tcf_proto *tp, void *fh)
+{
+ if (tp->ops->put && fh)
+ tp->ops->put(tp, fh);
+}
+
static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
struct netlink_ext_ack *extack)
{
@@ -1784,6 +1790,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
goto errout;
}
} else if (n->nlmsg_flags & NLM_F_EXCL) {
+ tfilter_put(tp, fh);
NL_SET_ERR_MSG(extack, "Filter already exists");
err = -EEXIST;
goto errout;
@@ -1798,9 +1805,11 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh,
n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE,
extack);
- if (err == 0)
+ if (err == 0) {
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
RTM_NEWTFILTER, false);
+ tfilter_put(tp, fh);
+ }
errout:
if (err && tp_created)
@@ -2030,6 +2039,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
NL_SET_ERR_MSG(extack, "Failed to send filter notify message");
}
+ tfilter_put(tp, fh);
errout:
if (chain) {
if (tp && !IS_ERR(tp))
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 11/17] net: sched: prevent insertion of new classifiers during chain flush
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Extend tcf_chain with 'flushing' flag. Use the flag to prevent insertion of
new classifier instances when chain flushing is in progress in order to
prevent resource leak when tcf_proto is created by unlocked users
concurrently.
Return EAGAIN error from tcf_chain_tp_insert_unique() to restart
tc_new_tfilter() and lookup the chain/proto again.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/sch_generic.h | 1 +
net/sched/cls_api.c | 35 ++++++++++++++++++++++++++---------
2 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 4809eca41f95..100368594524 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -353,6 +353,7 @@ struct tcf_chain {
unsigned int refcnt;
unsigned int action_refcnt;
bool explicitly_created;
+ bool flushing;
const struct tcf_proto_ops *tmplt_ops;
void *tmplt_priv;
};
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3ce244fbfb4d..d4f763525412 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -483,9 +483,12 @@ static void __tcf_chain_put(struct tcf_chain *chain, bool by_act,
spin_unlock(&block->lock);
/* The last dropped non-action reference will trigger notification. */
- if (is_last && !by_act)
+ if (is_last && !by_act) {
tc_chain_notify_delete(tmplt_ops, tmplt_priv, chain_index,
block, NULL, 0, 0, false);
+ /* Last reference to chain, no need to lock. */
+ chain->flushing = false;
+ }
if (refcnt == 0) {
tc_chain_tmplt_del(tmplt_ops, tmplt_priv);
@@ -517,6 +520,7 @@ static void tcf_chain_flush(struct tcf_chain *chain)
tp = tcf_chain_dereference(chain->filter_chain, chain);
RCU_INIT_POINTER(chain->filter_chain, NULL);
tcf_chain0_head_change(chain, NULL);
+ chain->flushing = true;
spin_unlock(&chain->filter_chain_lock);
while (tp) {
@@ -1382,15 +1386,20 @@ static struct tcf_proto *tcf_chain_tp_prev(struct tcf_chain *chain,
return tcf_chain_dereference(*chain_info->pprev, chain);
}
-static void tcf_chain_tp_insert(struct tcf_chain *chain,
- struct tcf_chain_info *chain_info,
- struct tcf_proto *tp)
+static int tcf_chain_tp_insert(struct tcf_chain *chain,
+ struct tcf_chain_info *chain_info,
+ struct tcf_proto *tp)
{
+ if (chain->flushing)
+ return -EAGAIN;
+
if (*chain_info->pprev == chain->filter_chain)
tcf_chain0_head_change(chain, tp);
tcf_proto_get(tp);
RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain, chain_info));
rcu_assign_pointer(*chain_info->pprev, tp);
+
+ return 0;
}
static void tcf_chain_tp_remove(struct tcf_chain *chain,
@@ -1421,18 +1430,22 @@ static struct tcf_proto *tcf_chain_tp_insert_unique(struct tcf_chain *chain,
{
struct tcf_chain_info chain_info;
struct tcf_proto *tp;
+ int err = 0;
spin_lock(&chain->filter_chain_lock);
tp = tcf_chain_tp_find(chain, &chain_info,
protocol, prio, false);
if (!tp)
- tcf_chain_tp_insert(chain, &chain_info, tp_new);
+ err = tcf_chain_tp_insert(chain, &chain_info, tp_new);
spin_unlock(&chain->filter_chain_lock);
if (tp) {
tcf_proto_destroy(tp_new, NULL);
tp_new = tp;
+ } else if (err) {
+ tcf_proto_destroy(tp_new, NULL);
+ tp_new = ERR_PTR(err);
}
return tp_new;
@@ -1743,11 +1756,15 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
tp = tcf_chain_tp_insert_unique(chain, tp_new, protocol, prio);
- /* tp insert function can return another tp instance, if it was
- * created concurrently.
- */
- if (tp == tp_new)
+ if (IS_ERR(tp)) {
+ err = PTR_ERR(tp);
+ goto errout;
+ } else if (tp == tp_new) {
+ /* tp insert function can return another tp instance, if
+ * it was created concurrently.
+ */
tp_created = 1;
+ }
} else {
spin_unlock(&chain->filter_chain_lock);
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 08/17] net: sched: introduce reference counting for tcf proto
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Add reference counter to tcf proto. Use it to manage tcf proto life cycle
in cls API.
Implement helper get/put functions for tcf proto and use them to modify cls
API to always take reference to tcf proto while using it. This change
allows to concurrently modify proto, instead of relying on rtnl lock for
protection.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/sch_generic.h | 1 +
net/sched/cls_api.c | 44 +++++++++++++++++++++++++++++++++++++-------
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 82fa23da4969..1015e3491187 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -318,6 +318,7 @@ struct tcf_proto {
void *data;
const struct tcf_proto_ops *ops;
struct tcf_chain *chain;
+ refcount_t refcnt;
struct rcu_head rcu;
struct work_struct work;
};
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 8f5dfa3ffb1c..6cbdb28017d3 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -197,6 +197,7 @@ static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
tp->prio = prio;
tp->chain = chain;
INIT_WORK(&tp->work, tcf_proto_destroy_work);
+ refcount_set(&tp->refcnt, 1);
err = tp->ops->init(tp);
if (err) {
@@ -210,12 +211,24 @@ static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
return ERR_PTR(err);
}
+static void tcf_proto_get(struct tcf_proto *tp)
+{
+ refcount_inc(&tp->refcnt);
+}
+
static void tcf_proto_destroy(struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
tc_queue_proto_work(&tp->work);
}
+static void tcf_proto_put(struct tcf_proto *tp,
+ struct netlink_ext_ack *extack)
+{
+ if (refcount_dec_and_test(&tp->refcnt))
+ tcf_proto_destroy(tp, extack);
+}
+
#define ASSERT_BLOCK_LOCKED(block) \
WARN_ONCE(!spin_is_locked(&(block)->lock), \
"BLOCK: assertion failed at %s (%d)\n", __FILE__, __LINE__)
@@ -458,13 +471,13 @@ static void tcf_chain_flush(struct tcf_chain *chain)
spin_lock(&chain->filter_chain_lock);
tp = tcf_chain_dereference(chain->filter_chain, chain);
+ RCU_INIT_POINTER(chain->filter_chain, NULL);
tcf_chain0_head_change(chain, NULL);
spin_unlock(&chain->filter_chain_lock);
while (tp) {
- RCU_INIT_POINTER(chain->filter_chain, tp->next);
- tcf_proto_destroy(tp, NULL);
- tp = rtnl_dereference(chain->filter_chain);
+ tcf_proto_put(tp, NULL);
+ tp = tp->next;
}
}
@@ -1275,9 +1288,9 @@ static void tcf_chain_tp_insert(struct tcf_chain *chain,
{
if (*chain_info->pprev == chain->filter_chain)
tcf_chain0_head_change(chain, tp);
+ tcf_proto_get(tp);
RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain, chain_info));
rcu_assign_pointer(*chain_info->pprev, tp);
- tcf_chain_hold(chain);
}
static void tcf_chain_tp_remove(struct tcf_chain *chain,
@@ -1315,7 +1328,12 @@ static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain,
}
}
chain_info->pprev = pprev;
- chain_info->next = tp ? tp->next : NULL;
+ if (tp) {
+ chain_info->next = tp->next;
+ tcf_proto_get(tp);
+ } else {
+ chain_info->next = NULL;
+ }
return tp;
}
@@ -1590,6 +1608,12 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
errout:
if (chain)
tcf_chain_put(chain);
+ if (chain) {
+ if (tp && !IS_ERR(tp))
+ tcf_proto_put(tp, NULL);
+ if (!tp_created)
+ tcf_chain_put(chain);
+ }
tcf_block_release(q, block);
if (err == -EAGAIN)
/* Replay the request. */
@@ -1720,8 +1744,11 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
}
errout:
- if (chain)
+ if (chain) {
+ if (tp && !IS_ERR(tp))
+ tcf_proto_put(tp, NULL);
tcf_chain_put(chain);
+ }
tcf_block_release(q, block);
return err;
@@ -1812,8 +1839,11 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
}
errout:
- if (chain)
+ if (chain) {
+ if (tp && !IS_ERR(tp))
+ tcf_proto_put(tp, NULL);
tcf_chain_put(chain);
+ }
tcf_block_release(q, block);
return err;
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 15/17] net: sched: add flags to Qdisc class ops struct
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Extend Qdisc_class_ops with flags. Create enum to hold possible class ops
flag values. Add first class ops flags value QDISC_CLASS_OPS_DOIT_UNLOCKED
to indicate that class ops functions can be called without taking rtnl
lock.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/sch_generic.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 7b158d6fae85..c97300f5ea5c 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -175,6 +175,7 @@ static inline int qdisc_avail_bulklimit(const struct netdev_queue *txq)
}
struct Qdisc_class_ops {
+ unsigned int flags;
/* Child qdisc manipulation */
struct netdev_queue * (*select_queue)(struct Qdisc *, struct tcmsg *);
int (*graft)(struct Qdisc *, unsigned long cl,
@@ -206,6 +207,13 @@ struct Qdisc_class_ops {
struct gnet_dump *);
};
+/* Qdisc_class_ops flag values */
+
+/* Implements API that doesn't require rtnl lock */
+enum qdisc_class_ops_flags {
+ QDISC_CLASS_OPS_DOIT_UNLOCKED = 1,
+};
+
struct Qdisc_ops {
struct Qdisc_ops *next;
const struct Qdisc_class_ops *cl_ops;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 04/17] net: sched: protect block->chain0 with block->lock
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
In order to remove dependency on rtnl lock, use block->lock to protect
chain0 struct from concurrent modification. Rearrange code in chain0
callback add and del functions to only access chain0 when block->lock is
held.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 146a02094905..beffcc2ab1fa 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -260,8 +260,11 @@ static void tcf_chain0_head_change(struct tcf_chain *chain,
if (chain->index)
return;
+
+ spin_lock(&block->lock);
list_for_each_entry(item, &block->chain0.filter_chain_list, list)
tcf_chain_head_change_item(item, tp_head);
+ spin_unlock(&block->lock);
}
/* Returns true if block can be safely freed. */
@@ -519,8 +522,8 @@ tcf_chain0_head_change_cb_add(struct tcf_block *block,
struct tcf_block_ext_info *ei,
struct netlink_ext_ack *extack)
{
- struct tcf_chain *chain0 = block->chain0.chain;
struct tcf_filter_chain_list_item *item;
+ struct tcf_chain *chain0;
item = kmalloc(sizeof(*item), GFP_KERNEL);
if (!item) {
@@ -529,9 +532,14 @@ tcf_chain0_head_change_cb_add(struct tcf_block *block,
}
item->chain_head_change = ei->chain_head_change;
item->chain_head_change_priv = ei->chain_head_change_priv;
+
+ spin_lock(&block->lock);
+ chain0 = block->chain0.chain;
if (chain0 && chain0->filter_chain)
tcf_chain_head_change_item(item, chain0->filter_chain);
list_add(&item->list, &block->chain0.filter_chain_list);
+ spin_unlock(&block->lock);
+
return 0;
}
@@ -539,20 +547,23 @@ static void
tcf_chain0_head_change_cb_del(struct tcf_block *block,
struct tcf_block_ext_info *ei)
{
- struct tcf_chain *chain0 = block->chain0.chain;
struct tcf_filter_chain_list_item *item;
+ spin_lock(&block->lock);
list_for_each_entry(item, &block->chain0.filter_chain_list, list) {
if ((!ei->chain_head_change && !ei->chain_head_change_priv) ||
(item->chain_head_change == ei->chain_head_change &&
item->chain_head_change_priv == ei->chain_head_change_priv)) {
- if (chain0)
+ if (block->chain0.chain)
tcf_chain_head_change_item(item, NULL);
list_del(&item->list);
+ spin_unlock(&block->lock);
+
kfree(item);
return;
}
}
+ spin_unlock(&block->lock);
WARN_ON(1);
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 17/17] net: sched: unlock rules update API
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Register netlink protocol handlers for message types RTM_NEWTFILTER,
RTM_DELTFILTER, RTM_GETTFILTER as unlocked. Set rtnl_held variable that
tracks rtnl mutex state to be false by default.
Modify tcf_block_release() to release rtnl lock if it was taken before.
Move code that releases block and qdisc to function __tcf_block_release()
that is used internally by regular block release and by chain update
function, which is not unlocked and doesn't need to release rtnl.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 43 ++++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 848f148f1019..a23aeac8ea4e 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1024,13 +1024,28 @@ static struct tcf_block *tcf_block_find(struct net *net, struct Qdisc **q,
return ERR_PTR(err);
}
-static void tcf_block_release(struct Qdisc *q, struct tcf_block *block)
+static void __tcf_block_release(struct Qdisc *q, struct tcf_block *block,
+ bool rtnl_held)
{
if (!IS_ERR_OR_NULL(block))
tcf_block_refcnt_put(block);
- if (q)
- qdisc_put(q);
+ if (q) {
+ if (rtnl_held)
+ qdisc_put(q);
+ else
+ qdisc_put_unlocked(q);
+ }
+}
+
+static void tcf_block_release(struct Qdisc *q, struct tcf_block *block,
+ bool *rtnl_held)
+{
+ if (*rtnl_held) {
+ rtnl_unlock();
+ *rtnl_held = false;
+ }
+ __tcf_block_release(q, block, false);
}
struct tcf_block_owner_item {
@@ -1706,7 +1721,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
void *fh;
int err;
int tp_created;
- bool rtnl_held = true;
+ bool rtnl_held = false;
if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
return -EPERM;
@@ -1865,7 +1880,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
if (!tp_created)
tcf_chain_put(chain);
}
- tcf_block_release(q, block);
+ tcf_block_release(q, block, &rtnl_held);
if (err == -EAGAIN) {
/* Take rtnl lock in case EAGAIN is caused by concurrent flush
* of target chain.
@@ -1899,7 +1914,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
unsigned long cl = 0;
void *fh = NULL;
int err;
- bool rtnl_held = true;
+ bool rtnl_held = false;
if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
return -EPERM;
@@ -2011,7 +2026,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
tcf_proto_put(tp, NULL);
tcf_chain_put(chain);
}
- tcf_block_release(q, block);
+ tcf_block_release(q, block, &rtnl_held);
return err;
errout_locked:
@@ -2037,7 +2052,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
unsigned long cl = 0;
void *fh = NULL;
int err;
- bool rtnl_held = true;
+ bool rtnl_held = false;
err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, rtm_tca_policy, extack);
if (err < 0)
@@ -2112,7 +2127,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
tcf_proto_put(tp, NULL);
tcf_chain_put(chain);
}
- tcf_block_release(q, block);
+ tcf_block_release(q, block, &rtnl_held);
return err;
}
@@ -2561,7 +2576,7 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
errout:
tcf_chain_put(chain);
errout_block:
- tcf_block_release(q, block);
+ __tcf_block_release(q, block, true);
if (err == -EAGAIN)
/* Replay the request. */
goto replay;
@@ -2899,10 +2914,12 @@ static int __init tc_filter_init(void)
if (err)
goto err_register_pernet_subsys;
- rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_new_tfilter, NULL, 0);
- rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_del_tfilter, NULL, 0);
+ rtnl_register(PF_UNSPEC, RTM_NEWTFILTER, tc_new_tfilter, NULL,
+ RTNL_FLAG_DOIT_UNLOCKED);
+ rtnl_register(PF_UNSPEC, RTM_DELTFILTER, tc_del_tfilter, NULL,
+ RTNL_FLAG_DOIT_UNLOCKED);
rtnl_register(PF_UNSPEC, RTM_GETTFILTER, tc_get_tfilter,
- tc_dump_tfilter, 0);
+ tc_dump_tfilter, RTNL_FLAG_DOIT_UNLOCKED);
rtnl_register(PF_UNSPEC, RTM_NEWCHAIN, tc_ctl_chain, NULL, 0);
rtnl_register(PF_UNSPEC, RTM_DELCHAIN, tc_ctl_chain, NULL, 0);
rtnl_register(PF_UNSPEC, RTM_GETCHAIN, tc_ctl_chain,
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 12/17] net: sched: track rtnl lock status when validating extensions
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Actions API is already updated to not rely on rtnl lock for
synchronization. However, it need to be provided with rtnl status when
called from classifiers API in order to be able to correctly release the
lock when loading kernel module.
Extend extension validation function with 'rtnl_held' flag which is passed
to actions API. Add new 'rtnl_held' parameter to tcf_exts_validate() in cls
API. No classifier is currently updated to support unlocked execution, so
pass hardcoded 'true' flag parameter value.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/pkt_cls.h | 2 +-
net/sched/cls_api.c | 9 +++++----
net/sched/cls_basic.c | 2 +-
net/sched/cls_bpf.c | 3 ++-
net/sched/cls_cgroup.c | 2 +-
net/sched/cls_flow.c | 2 +-
net/sched/cls_flower.c | 3 ++-
net/sched/cls_fw.c | 2 +-
net/sched/cls_matchall.c | 3 ++-
net/sched/cls_route.c | 2 +-
net/sched/cls_rsvp.h | 3 ++-
net/sched/cls_tcindex.c | 2 +-
net/sched/cls_u32.c | 2 +-
13 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 727cb99da5d4..1f1b06311622 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -381,7 +381,7 @@ tcf_exts_exec(struct sk_buff *skb, struct tcf_exts *exts,
int tcf_exts_validate(struct net *net, struct tcf_proto *tp,
struct nlattr **tb, struct nlattr *rate_tlv,
- struct tcf_exts *exts, bool ovr,
+ struct tcf_exts *exts, bool ovr, bool rtnl_held,
struct netlink_ext_ack *extack);
void tcf_exts_destroy(struct tcf_exts *exts);
void tcf_exts_change(struct tcf_exts *dst, struct tcf_exts *src);
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index d4f763525412..0949502e31b9 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2613,7 +2613,7 @@ EXPORT_SYMBOL(tcf_exts_destroy);
int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
struct nlattr *rate_tlv, struct tcf_exts *exts, bool ovr,
- struct netlink_ext_ack *extack)
+ bool rtnl_held, struct netlink_ext_ack *extack)
{
#ifdef CONFIG_NET_CLS_ACT
{
@@ -2623,7 +2623,8 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
if (exts->police && tb[exts->police]) {
act = tcf_action_init_1(net, tp, tb[exts->police],
rate_tlv, "police", ovr,
- TCA_ACT_BIND, true, extack);
+ TCA_ACT_BIND, rtnl_held,
+ extack);
if (IS_ERR(act))
return PTR_ERR(act);
@@ -2635,8 +2636,8 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
err = tcf_action_init(net, tp, tb[exts->action],
rate_tlv, NULL, ovr, TCA_ACT_BIND,
- exts->actions, &attr_size, true,
- extack);
+ exts->actions, &attr_size,
+ rtnl_held, extack);
if (err < 0)
return err;
exts->nr_actions = err;
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 6a5dce8baf19..90e44888f85d 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -148,7 +148,7 @@ static int basic_set_parms(struct net *net, struct tcf_proto *tp,
{
int err;
- err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr, true, extack);
if (err < 0)
return err;
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index fa6fe2fe0f32..9c0ac7c23ad7 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -417,7 +417,8 @@ static int cls_bpf_set_parms(struct net *net, struct tcf_proto *tp,
if ((!is_bpf && !is_ebpf) || (is_bpf && is_ebpf))
return -EINVAL;
- ret = tcf_exts_validate(net, tp, tb, est, &prog->exts, ovr, extack);
+ ret = tcf_exts_validate(net, tp, tb, est, &prog->exts, ovr, true,
+ extack);
if (ret < 0)
return ret;
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 3bc01bdde165..663ee1c6d606 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -110,7 +110,7 @@ static int cls_cgroup_change(struct net *net, struct sk_buff *in_skb,
goto errout;
err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &new->exts, ovr,
- extack);
+ true, extack);
if (err < 0)
goto errout;
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 2bb043cd436b..39a6407d4832 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -445,7 +445,7 @@ static int flow_change(struct net *net, struct sk_buff *in_skb,
goto err2;
err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &fnew->exts, ovr,
- extack);
+ true, extack);
if (err < 0)
goto err2;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 9aada2d0ef06..8bf3d4660fac 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1132,7 +1132,8 @@ static int fl_set_parms(struct net *net, struct tcf_proto *tp,
{
int err;
- err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr, true,
+ extack);
if (err < 0)
return err;
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 29eeeaf3ea44..c8173ebb69f2 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -217,7 +217,7 @@ static int fw_set_parms(struct net *net, struct tcf_proto *tp,
int err;
err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &f->exts, ovr,
- extack);
+ true, extack);
if (err < 0)
return err;
diff --git a/net/sched/cls_matchall.c b/net/sched/cls_matchall.c
index 856fa79d4ffd..5621c38a303a 100644
--- a/net/sched/cls_matchall.c
+++ b/net/sched/cls_matchall.c
@@ -142,7 +142,8 @@ static int mall_set_parms(struct net *net, struct tcf_proto *tp,
{
int err;
- err = tcf_exts_validate(net, tp, tb, est, &head->exts, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, est, &head->exts, ovr, true,
+ extack);
if (err < 0)
return err;
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 0404aa5fa7cb..44b26038c4c4 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -393,7 +393,7 @@ static int route4_set_parms(struct net *net, struct tcf_proto *tp,
struct route4_bucket *b;
int err;
- err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, est, &f->exts, ovr, true, extack);
if (err < 0)
return err;
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index e9ccf7daea7d..9dd9530e6a52 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -502,7 +502,8 @@ static int rsvp_change(struct net *net, struct sk_buff *in_skb,
err = tcf_exts_init(&e, TCA_RSVP_ACT, TCA_RSVP_POLICE);
if (err < 0)
return err;
- err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, tca[TCA_RATE], &e, ovr, true,
+ extack);
if (err < 0)
goto errout2;
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 9ccc93f257db..b7dc667b6ec0 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -314,7 +314,7 @@ tcindex_set_parms(struct net *net, struct tcf_proto *tp, unsigned long base,
err = tcf_exts_init(&e, TCA_TCINDEX_ACT, TCA_TCINDEX_POLICE);
if (err < 0)
return err;
- err = tcf_exts_validate(net, tp, tb, est, &e, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, est, &e, ovr, true, extack);
if (err < 0)
goto errout;
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 4b28fd44576d..cd79ae3b605a 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -725,7 +725,7 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp,
{
int err;
- err = tcf_exts_validate(net, tp, tb, est, &n->exts, ovr, extack);
+ err = tcf_exts_validate(net, tp, tb, est, &n->exts, ovr, true, extack);
if (err < 0)
return err;
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 01/17] net: sched: refactor mini_qdisc_pair_swap() to use workqueue
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
As a part of the effort to remove dependency on rtnl lock, cls API is being
converted to use fine-grained locking mechanisms instead of global rtnl
lock. However, chain_head_change callback for ingress Qdisc is a sleeping
function and cannot be executed while holding a spinlock.
Extend cls API with new workqueue intended to be used for tcf_proto
lifetime management. Modify tcf_proto_destroy() to deallocate proto
asynchronously on workqueue in order to ensure that all chain_head_change
callbacks involving the proto complete before it is freed. Convert
mini_qdisc_pair_swap(), that is used as a chain_head_change callback for
ingress and clsact Qdiscs, to use a workqueue.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Suggested-by: Jiri Pirko <jiri@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/sch_generic.h | 6 ++++++
net/sched/cls_api.c | 48 +++++++++++++++++++++++++++++++++++++++++------
net/sched/sch_generic.c | 37 +++++++++++++++++++++++++++++++++---
net/sched/sch_ingress.c | 4 ++++
4 files changed, 86 insertions(+), 9 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 4d736427a4cb..29118bbd528f 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -319,6 +319,7 @@ struct tcf_proto {
const struct tcf_proto_ops *ops;
struct tcf_chain *chain;
struct rcu_head rcu;
+ struct work_struct work;
};
struct qdisc_skb_cb {
@@ -397,6 +398,8 @@ tc_cls_offload_cnt_update(struct tcf_block *block, u32 *cnt,
}
}
+bool tc_queue_proto_work(struct work_struct *work);
+
static inline void qdisc_cb_private_validate(const struct sk_buff *skb, int sz)
{
struct qdisc_skb_cb *qcb;
@@ -1148,12 +1151,15 @@ struct mini_Qdisc_pair {
struct mini_Qdisc miniq1;
struct mini_Qdisc miniq2;
struct mini_Qdisc __rcu **p_miniq;
+ struct tcf_proto *tp_head;
+ struct work_struct work;
};
void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
struct tcf_proto *tp_head);
void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
struct mini_Qdisc __rcu **p_miniq);
+void mini_qdisc_pair_cleanup(struct mini_Qdisc_pair *miniqp);
static inline void skb_tc_reinsert(struct sk_buff *skb, struct tcf_result *res)
{
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f427a1e00e7e..17c1691bf0c0 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -107,12 +107,14 @@ int register_tcf_proto_ops(struct tcf_proto_ops *ops)
EXPORT_SYMBOL(register_tcf_proto_ops);
static struct workqueue_struct *tc_filter_wq;
+static struct workqueue_struct *tc_proto_wq;
int unregister_tcf_proto_ops(struct tcf_proto_ops *ops)
{
struct tcf_proto_ops *t;
int rc = -ENOENT;
+ flush_workqueue(tc_proto_wq);
/* Wait for outstanding call_rcu()s, if any, from a
* tcf_proto_ops's destroy() handler.
*/
@@ -139,6 +141,12 @@ bool tcf_queue_work(struct rcu_work *rwork, work_func_t func)
}
EXPORT_SYMBOL(tcf_queue_work);
+bool tc_queue_proto_work(struct work_struct *work)
+{
+ return queue_work(tc_proto_wq, work);
+}
+EXPORT_SYMBOL(tc_queue_proto_work);
+
/* Select new prio value from the range, managed by kernel. */
static inline u32 tcf_auto_prio(struct tcf_proto *tp)
@@ -151,6 +159,23 @@ static inline u32 tcf_auto_prio(struct tcf_proto *tp)
return TC_H_MAJ(first);
}
+static void tcf_chain_put(struct tcf_chain *chain);
+
+static void tcf_proto_destroy_work(struct work_struct *work)
+{
+ struct tcf_proto *tp = container_of(work, struct tcf_proto, work);
+ struct tcf_chain *chain = tp->chain;
+
+ rtnl_lock();
+
+ tp->ops->destroy(tp, NULL);
+ module_put(tp->ops->owner);
+ kfree_rcu(tp, rcu);
+ tcf_chain_put(chain);
+
+ rtnl_unlock();
+}
+
static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
u32 prio, struct tcf_chain *chain,
struct netlink_ext_ack *extack)
@@ -171,6 +196,7 @@ static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
tp->protocol = protocol;
tp->prio = prio;
tp->chain = chain;
+ INIT_WORK(&tp->work, tcf_proto_destroy_work);
err = tp->ops->init(tp);
if (err) {
@@ -187,9 +213,7 @@ static struct tcf_proto *tcf_proto_create(const char *kind, u32 protocol,
static void tcf_proto_destroy(struct tcf_proto *tp,
struct netlink_ext_ack *extack)
{
- tp->ops->destroy(tp, extack);
- module_put(tp->ops->owner);
- kfree_rcu(tp, rcu);
+ tc_queue_proto_work(&tp->work);
}
struct tcf_filter_chain_list_item {
@@ -361,7 +385,6 @@ static void tcf_chain_flush(struct tcf_chain *chain)
RCU_INIT_POINTER(chain->filter_chain, tp->next);
tcf_proto_destroy(tp, NULL);
tp = rtnl_dereference(chain->filter_chain);
- tcf_chain_put(chain);
}
}
@@ -1131,7 +1154,6 @@ static void tcf_chain_tp_remove(struct tcf_chain *chain,
if (tp == chain->filter_chain)
tcf_chain0_head_change(chain, next);
RCU_INIT_POINTER(*chain_info->pprev, next);
- tcf_chain_put(chain);
}
static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain,
@@ -1420,8 +1442,14 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
RTM_NEWTFILTER, false);
} else {
- if (tp_created)
+ if (tp_created) {
+ /* tp wasn't inserted to chain tp list. Take reference
+ * to chain manually for tcf_proto_destroy() to
+ * release.
+ */
+ tcf_chain_hold(chain);
tcf_proto_destroy(tp, NULL);
+ }
}
errout:
@@ -2351,6 +2379,12 @@ static int __init tc_filter_init(void)
if (!tc_filter_wq)
return -ENOMEM;
+ tc_proto_wq = alloc_ordered_workqueue("tc_proto_workqueue", 0);
+ if (!tc_proto_wq) {
+ err = -ENOMEM;
+ goto err_proto_wq;
+ }
+
err = register_pernet_subsys(&tcf_net_ops);
if (err)
goto err_register_pernet_subsys;
@@ -2367,6 +2401,8 @@ static int __init tc_filter_init(void)
return 0;
err_register_pernet_subsys:
+ destroy_workqueue(tc_proto_wq);
+err_proto_wq:
destroy_workqueue(tc_filter_wq);
return err;
}
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index de1663f7d3ad..5aa8e485fcb0 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -1363,10 +1363,14 @@ static void mini_qdisc_rcu_func(struct rcu_head *head)
{
}
-void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
- struct tcf_proto *tp_head)
+void __mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
+ struct tcf_proto *tp_head)
{
- struct mini_Qdisc *miniq_old = rtnl_dereference(*miniqp->p_miniq);
+ /* p_miniq is either changed on ordered workqueue or during miniqp
+ * cleanup. In both cases no concurrent modification is possible.
+ */
+ struct mini_Qdisc *miniq_old =
+ rcu_dereference_protected(*miniqp->p_miniq, 1);
struct mini_Qdisc *miniq;
if (!tp_head) {
@@ -1394,6 +1398,25 @@ void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
*/
call_rcu_bh(&miniq_old->rcu, mini_qdisc_rcu_func);
}
+
+void mini_qdisc_pair_swap_work(struct work_struct *work)
+{
+ struct mini_Qdisc_pair *miniqp = container_of(work,
+ struct mini_Qdisc_pair,
+ work);
+ struct tcf_proto *tp_head;
+
+ tp_head = xchg(&miniqp->tp_head, ERR_PTR(-ENOENT));
+ if (!IS_ERR(tp_head))
+ __mini_qdisc_pair_swap(miniqp, tp_head);
+}
+
+void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
+ struct tcf_proto *tp_head)
+{
+ xchg(&miniqp->tp_head, tp_head);
+ tc_queue_proto_work(&miniqp->work);
+}
EXPORT_SYMBOL(mini_qdisc_pair_swap);
void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
@@ -1404,5 +1427,13 @@ void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
miniqp->miniq2.cpu_bstats = qdisc->cpu_bstats;
miniqp->miniq2.cpu_qstats = qdisc->cpu_qstats;
miniqp->p_miniq = p_miniq;
+ INIT_WORK(&miniqp->work, mini_qdisc_pair_swap_work);
}
EXPORT_SYMBOL(mini_qdisc_pair_init);
+
+void mini_qdisc_pair_cleanup(struct mini_Qdisc_pair *miniqp)
+{
+ cancel_work_sync(&miniqp->work);
+ __mini_qdisc_pair_swap(miniqp, NULL);
+}
+EXPORT_SYMBOL(mini_qdisc_pair_cleanup);
diff --git a/net/sched/sch_ingress.c b/net/sched/sch_ingress.c
index ce3f55259d0d..8edc3c46bd9c 100644
--- a/net/sched/sch_ingress.c
+++ b/net/sched/sch_ingress.c
@@ -99,6 +99,7 @@ static void ingress_destroy(struct Qdisc *sch)
struct ingress_sched_data *q = qdisc_priv(sch);
tcf_block_put_ext(q->block, sch, &q->block_info);
+ mini_qdisc_pair_cleanup(&q->miniqp);
net_dec_ingress_queue();
}
@@ -245,6 +246,9 @@ static void clsact_destroy(struct Qdisc *sch)
tcf_block_put_ext(q->egress_block, sch, &q->egress_block_info);
tcf_block_put_ext(q->ingress_block, sch, &q->ingress_block_info);
+ mini_qdisc_pair_cleanup(&q->miniqp_ingress);
+ mini_qdisc_pair_cleanup(&q->miniqp_egress);
+
net_dec_ingress_queue();
net_dec_egress_queue();
}
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 02/17] net: sched: protect block state with spinlock
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
Currently, tcf_block doesn't use any synchronization mechanisms to protect
code that manages lifetime of its chains. block->chain_list and multiple
variables in tcf_chain that control its lifetime assume external
synchronization provided by global rtnl lock. Converting chain reference
counting to atomic reference counters is not possible because cls API uses
multiple counters and flags to control chain lifetime, so all of them must
be synchronized in chain get/put code.
Use single per-block lock to protect block data and manage lifetime of all
chains on the block. Always take block->lock when accessing chain_list.
Chain get and put modify chain lifetime-management data and parent block's
chain_list, so take the lock in these functions. Verify block->lock state
with assertions in functions that expect to be called with the lock taken
and are called from multiple places. Take block->lock when accessing
filter_chain_list.
block->lock is a spinlock which means blocking functions like classifier
ops callbacks cannot be called while holding it. Rearrange chain get and
put functions code to only access protected chain data while holding block
lock and move blocking calls outside critical section:
- Check if chain was explicitly created inside put function while holding
block lock. Add additional argument to __tcf_chain_put() to only put
explicitly created chain.
- Rearrange code to only access chain reference counter and chain action
reference counter while holding block lock.
- Split tcf_chain_destroy() helper to two functions: one that requires
block->lock, and another one that needs to call sleeping functions and
can be executed after lock is released. First helper is used to detach
chain from block and make it inaccessible for concurrent users, second
actually deallocates chain memory (and parent block, if applicable).
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/sch_generic.h | 4 ++
net/sched/cls_api.c | 93 +++++++++++++++++++++++++++++++++++++++--------
2 files changed, 81 insertions(+), 16 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index 29118bbd528f..5f4fc28fc77a 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -350,6 +350,10 @@ struct tcf_chain {
};
struct tcf_block {
+ /* Lock protects tcf_block and lifetime-management data of chains
+ * attached to the block (refcnt, action_refcnt, explicitly_created).
+ */
+ spinlock_t lock;
struct list_head chain_list;
u32 index; /* block index for shared blocks */
refcount_t refcnt;
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 17c1691bf0c0..df3326dd33ef 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -216,6 +216,10 @@ static void tcf_proto_destroy(struct tcf_proto *tp,
tc_queue_proto_work(&tp->work);
}
+#define ASSERT_BLOCK_LOCKED(block) \
+ WARN_ONCE(!spin_is_locked(&(block)->lock), \
+ "BLOCK: assertion failed at %s (%d)\n", __FILE__, __LINE__)
+
struct tcf_filter_chain_list_item {
struct list_head list;
tcf_chain_head_change_t *chain_head_change;
@@ -227,7 +231,9 @@ static struct tcf_chain *tcf_chain_create(struct tcf_block *block,
{
struct tcf_chain *chain;
- chain = kzalloc(sizeof(*chain), GFP_KERNEL);
+ ASSERT_BLOCK_LOCKED(block);
+
+ chain = kzalloc(sizeof(*chain), GFP_ATOMIC);
if (!chain)
return NULL;
list_add_tail(&chain->list, &block->chain_list);
@@ -258,13 +264,29 @@ static void tcf_chain0_head_change(struct tcf_chain *chain,
tcf_chain_head_change_item(item, tp_head);
}
-static void tcf_chain_destroy(struct tcf_chain *chain)
+/* Returns true if block can be safely freed. */
+
+static bool tcf_chain_detach(struct tcf_chain *chain)
{
struct tcf_block *block = chain->block;
+ ASSERT_BLOCK_LOCKED(block);
+
list_del(&chain->list);
if (!chain->index)
block->chain0.chain = NULL;
+
+ if (list_empty(&block->chain_list) &&
+ refcount_read(&block->refcnt) == 0)
+ return true;
+
+ return false;
+}
+
+static void tcf_chain_destroy(struct tcf_chain *chain, bool free_block)
+{
+ struct tcf_block *block = chain->block;
+
kfree(chain);
if (list_empty(&block->chain_list) && !refcount_read(&block->refcnt))
kfree_rcu(block, rcu);
@@ -272,11 +294,15 @@ static void tcf_chain_destroy(struct tcf_chain *chain)
static void tcf_chain_hold(struct tcf_chain *chain)
{
+ ASSERT_BLOCK_LOCKED(chain->block);
+
++chain->refcnt;
}
static bool tcf_chain_held_by_acts_only(struct tcf_chain *chain)
{
+ ASSERT_BLOCK_LOCKED(chain->block);
+
/* In case all the references are action references, this
* chain should not be shown to the user.
*/
@@ -288,6 +314,8 @@ static struct tcf_chain *tcf_chain_lookup(struct tcf_block *block,
{
struct tcf_chain *chain;
+ ASSERT_BLOCK_LOCKED(block);
+
list_for_each_entry(chain, &block->chain_list, list) {
if (chain->index == chain_index)
return chain;
@@ -302,31 +330,40 @@ static struct tcf_chain *__tcf_chain_get(struct tcf_block *block,
u32 chain_index, bool create,
bool by_act)
{
- struct tcf_chain *chain = tcf_chain_lookup(block, chain_index);
+ struct tcf_chain *chain = NULL;
+ bool is_first_reference;
+ spin_lock(&block->lock);
+ chain = tcf_chain_lookup(block, chain_index);
if (chain) {
tcf_chain_hold(chain);
} else {
if (!create)
- return NULL;
+ goto errout;
chain = tcf_chain_create(block, chain_index);
if (!chain)
- return NULL;
+ goto errout;
}
if (by_act)
++chain->action_refcnt;
+ is_first_reference = chain->refcnt - chain->action_refcnt == 1;
+ spin_unlock(&block->lock);
/* Send notification only in case we got the first
* non-action reference. Until then, the chain acts only as
* a placeholder for actions pointing to it and user ought
* not know about them.
*/
- if (chain->refcnt - chain->action_refcnt == 1 && !by_act)
+ if (is_first_reference && !by_act)
tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
RTM_NEWCHAIN, false);
return chain;
+
+errout:
+ spin_unlock(&block->lock);
+ return chain;
}
static struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
@@ -343,37 +380,59 @@ EXPORT_SYMBOL(tcf_chain_get_by_act);
static void tc_chain_tmplt_del(struct tcf_chain *chain);
-static void __tcf_chain_put(struct tcf_chain *chain, bool by_act)
+static void __tcf_chain_put(struct tcf_chain *chain, bool by_act,
+ bool explicitly_created)
{
+ struct tcf_block *block = chain->block;
+ bool is_last, free_block = false;
+ unsigned int refcnt;
+
+ spin_lock(&block->lock);
+ if (explicitly_created) {
+ if (!chain->explicitly_created) {
+ spin_unlock(&block->lock);
+ return;
+ }
+ chain->explicitly_created = false;
+ }
+
if (by_act)
chain->action_refcnt--;
- chain->refcnt--;
+
+ /* tc_chain_notify_delete can't be called while holding block lock.
+ * However, when block is unlocked chain can be changed concurrently, so
+ * save these to temporary variables.
+ */
+ refcnt = --chain->refcnt;
+ is_last = refcnt - chain->action_refcnt == 0;
+ if (refcnt == 0)
+ free_block = tcf_chain_detach(chain);
+ spin_unlock(&block->lock);
/* The last dropped non-action reference will trigger notification. */
- if (chain->refcnt - chain->action_refcnt == 0 && !by_act)
+ if (is_last && !by_act)
tc_chain_notify(chain, NULL, 0, 0, RTM_DELCHAIN, false);
- if (chain->refcnt == 0) {
+ if (refcnt == 0) {
tc_chain_tmplt_del(chain);
- tcf_chain_destroy(chain);
+ tcf_chain_destroy(chain, free_block);
}
}
static void tcf_chain_put(struct tcf_chain *chain)
{
- __tcf_chain_put(chain, false);
+ __tcf_chain_put(chain, false, false);
}
void tcf_chain_put_by_act(struct tcf_chain *chain)
{
- __tcf_chain_put(chain, true);
+ __tcf_chain_put(chain, true, false);
}
EXPORT_SYMBOL(tcf_chain_put_by_act);
static void tcf_chain_put_explicitly_created(struct tcf_chain *chain)
{
- if (chain->explicitly_created)
- tcf_chain_put(chain);
+ __tcf_chain_put(chain, false, true);
}
static void tcf_chain_flush(struct tcf_chain *chain)
@@ -540,6 +599,7 @@ static struct tcf_block *tcf_block_create(struct net *net, struct Qdisc *q,
NL_SET_ERR_MSG(extack, "Memory allocation for block failed");
return ERR_PTR(-ENOMEM);
}
+ spin_lock_init(&block->lock);
INIT_LIST_HEAD(&block->chain_list);
INIT_LIST_HEAD(&block->cb_list);
INIT_LIST_HEAD(&block->owner_list);
@@ -603,7 +663,7 @@ static void tcf_block_put_all_chains(struct tcf_block *block)
static void __tcf_block_put(struct tcf_block *block, struct Qdisc *q,
struct tcf_block_ext_info *ei)
{
- if (refcount_dec_and_test(&block->refcnt)) {
+ if (refcount_dec_and_lock(&block->refcnt, &block->lock)) {
/* Flushing/putting all chains will cause the block to be
* deallocated when last chain is freed. However, if chain_list
* is empty, block has to be manually deallocated. After block
@@ -612,6 +672,7 @@ static void __tcf_block_put(struct tcf_block *block, struct Qdisc *q,
*/
bool free_block = list_empty(&block->chain_list);
+ spin_unlock(&block->lock);
if (tcf_block_shared(block))
tcf_block_remove(block, block->net);
if (!free_block)
--
2.7.5
^ permalink raw reply related
* [PATCH net-next 00/17] Refactor classifier API to work with chain/classifiers without rtnl lock
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
Currently, all netlink protocol handlers for updating rules, actions and
qdiscs are protected with single global rtnl lock which removes any
possibility for parallelism. This patch set is a third step to remove
rtnl lock dependency from TC rules update path.
Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added.
Handlers registered with this flag are called without RTNL taken. End
goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER,
etc.) to be registered with UNLOCKED flag to allow parallel execution.
However, there is no intention to completely remove or split rtnl lock
itself. This patch set addresses specific problems in implementation of
classifiers API that prevent its control path from being executed
concurrently, and completes refactoring of cls API rules update handlers
by removing rtnl lock dependency from code that handles chains and
classifiers. Rules update handlers are registered with
RTNL_FLAG_DOIT_UNLOCKED flag.
This patch set substitutes global rtnl lock dependency on rules update
path in cls API by extending its data structures with following locks:
- tcf_block with 'lock' spinlock. It is used to protect block state, and
life-time management fields of chains on the block (chain->refcnt,
chain->action_refcnt, chain->explicitly crated, etc.).
- tcf_chain with 'filter_chain_lock' spinlock, that is used to protect
list of classifier instances attached to chain.
- tcf_proto with 'lock' spinlock that is intended to be used to
synchronize access to classifiers that support unlocked execution.
Chain0 head change callbacks can be sleeping and cannot be protected by
block spinlock. To solve this issue, sleeping miniqp swap function (used
as head change callback by ingress and clsact Qdiscs) is refactored to
offload sleeping operations to workqueue. New ordered workqueue
'tc_proto_workqueue' is created in cls_api to be used by miniqp and for
tcf_proto deallocation, which is also moved to workqueue to prevent
deallocation of tp's that are still in use by block. Performing both
miniqp swap and tp deallocation on same ordered workqueue ensures that
any pending head change requests involving tp are completed before the
tp is deallocated.
Classifiers are extended with reference counting to accommodate parallel
access by unlocked cls API. Classifier ops structure is extended with
additional 'put' function to allow reference counting of filters and
intended to be used by classifiers that implement rtnl-unlocked API.
Users of classifiers and individual filter instances are modified to
always hold reference while working with them.
Classifiers that support unlocked execution still need to know the
status of rtnl lock, so their API is extended with additional
'rtnl_held' argument that is used to indicate that caller holds rtnl
lock.
Vlad Buslov (17):
net: sched: refactor mini_qdisc_pair_swap() to use workqueue
net: sched: protect block state with spinlock
net: sched: refactor tc_ctl_chain() to use block->lock
net: sched: protect block->chain0 with block->lock
net: sched: traverse chains in block with tcf_get_next_chain()
net: sched: protect chain template accesses with block lock
net: sched: lock the chain when accessing filter_chain list
net: sched: introduce reference counting for tcf proto
net: sched: traverse classifiers in chain with tcf_get_next_proto()
net: sched: refactor tp insert/delete for concurrent execution
net: sched: prevent insertion of new classifiers during chain flush
net: sched: track rtnl lock status when validating extensions
net: sched: extend proto ops with 'put' callback
net: sched: extend proto ops to support unlocked classifiers
net: sched: add flags to Qdisc class ops struct
net: sched: conditionally take rtnl lock on rules update path
net: sched: unlock rules update API
include/net/pkt_cls.h | 6 +-
include/net/sch_generic.h | 73 +++-
net/sched/cls_api.c | 919 +++++++++++++++++++++++++++++++++++++---------
net/sched/cls_basic.c | 14 +-
net/sched/cls_bpf.c | 15 +-
net/sched/cls_cgroup.c | 13 +-
net/sched/cls_flow.c | 15 +-
net/sched/cls_flower.c | 16 +-
net/sched/cls_fw.c | 15 +-
net/sched/cls_matchall.c | 16 +-
net/sched/cls_route.c | 14 +-
net/sched/cls_rsvp.h | 16 +-
net/sched/cls_tcindex.c | 17 +-
net/sched/cls_u32.c | 14 +-
net/sched/sch_api.c | 10 +-
net/sched/sch_generic.c | 37 +-
net/sched/sch_ingress.c | 4 +
17 files changed, 955 insertions(+), 259 deletions(-)
--
2.7.5
^ permalink raw reply
* [PATCH net-next 03/17] net: sched: refactor tc_ctl_chain() to use block->lock
From: Vlad Buslov @ 2018-11-12 7:55 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, ast, daniel, Vlad Buslov
In-Reply-To: <1542009346-23780-1-git-send-email-vladbu@mellanox.com>
In order to remove dependency on rtnl lock, modify chain API to use
block->lock to protect chain from concurrent modification. Rearrange
tc_ctl_chain() code to call tcf_chain_hold() while holding block->lock.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_api.c | 36 +++++++++++++++++++++++++-----------
1 file changed, 25 insertions(+), 11 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index df3326dd33ef..146a02094905 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2047,6 +2047,8 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
err = -EINVAL;
goto errout_block;
}
+
+ spin_lock(&block->lock);
chain = tcf_chain_lookup(block, chain_index);
if (n->nlmsg_type == RTM_NEWCHAIN) {
if (chain) {
@@ -2058,41 +2060,49 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
} else {
NL_SET_ERR_MSG(extack, "Filter chain already exists");
err = -EEXIST;
- goto errout_block;
+ goto errout_block_locked;
}
} else {
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
NL_SET_ERR_MSG(extack, "Need both RTM_NEWCHAIN and NLM_F_CREATE to create a new chain");
err = -ENOENT;
- goto errout_block;
+ goto errout_block_locked;
}
chain = tcf_chain_create(block, chain_index);
if (!chain) {
NL_SET_ERR_MSG(extack, "Failed to create filter chain");
err = -ENOMEM;
- goto errout_block;
+ goto errout_block_locked;
}
}
} else {
if (!chain || tcf_chain_held_by_acts_only(chain)) {
NL_SET_ERR_MSG(extack, "Cannot find specified filter chain");
err = -EINVAL;
- goto errout_block;
+ goto errout_block_locked;
}
tcf_chain_hold(chain);
}
+ if (n->nlmsg_type == RTM_NEWCHAIN) {
+ /* Modifying chain requires holding parent block lock. In case
+ * the chain was successfully added, take a reference to the
+ * chain. This ensures that an empty chain does not disappear at
+ * the end of this function.
+ */
+ tcf_chain_hold(chain);
+ chain->explicitly_created = true;
+ }
+ spin_unlock(&block->lock);
+
switch (n->nlmsg_type) {
case RTM_NEWCHAIN:
err = tc_chain_tmplt_add(chain, net, tca, extack);
- if (err)
+ if (err) {
+ tcf_chain_put_explicitly_created(chain);
goto errout;
- /* In case the chain was successfully added, take a reference
- * to the chain. This ensures that an empty chain
- * does not disappear at the end of this function.
- */
- tcf_chain_hold(chain);
- chain->explicitly_created = true;
+ }
+
tc_chain_notify(chain, NULL, 0, NLM_F_CREATE | NLM_F_EXCL,
RTM_NEWCHAIN, false);
break;
@@ -2127,6 +2137,10 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
/* Replay the request. */
goto replay;
return err;
+
+errout_block_locked:
+ spin_unlock(&block->lock);
+ goto errout_block;
}
/* called with RTNL */
--
2.7.5
^ permalink raw reply related
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Paweł Staszewski @ 2018-11-12 17:06 UTC (permalink / raw)
To: Eric Dumazet, Alexander Duyck
Cc: aaron.lu, linux-mm, LKML, Netdev, Andrew Morton,
Jesper Dangaard Brouer, Tariq Toukan, ilias.apalodimas, yoel,
Mel Gorman, Saeed Mahameed, Michal Hocko, Vlastimil Babka,
dave.hansen
In-Reply-To: <6edcec1a-eefa-7861-1af4-cdf7fa45184c@gmail.com>
W dniu 12.11.2018 o 16:44, Eric Dumazet pisze:
>
> On 11/12/2018 07:30 AM, Alexander Duyck wrote:
>
>> It sounds to me like XDP would probably be your best bet. With that
>> you could probably get away with smaller ring sizes, higher interrupt
>> rates, and get the advantage of it batching the Tx without having to
>> drop packets.
> Add to this that with XDP (or anything lowering per packet processing costs)
> you can reduce number of cpus/queues, get better latencies, and bigger TX batches.
Yes for sure - the best for my use case will be to implement XDP :)
But for real life not test lab use programs like xdp_fwd need to be
extended for minimal information needed from IP router - like counters
and some aditional debug for traffic like sniffing / sampling for ddos
detection.
And that is rly minimum needed - for routing IP traffic with XDP
^ permalink raw reply
* VETH & AF_PACKET problem
From: Anand H. Krishnan @ 2018-11-12 7:12 UTC (permalink / raw)
To: netdev
Hello,
We are seeing a problem with AF_PACKET when used along with the
veth interfaces. SCP complains that message authentication code is
incorrect.
I was browsing the code and I see that veth_xmit calls ____dev_forward_skb
which does a skb_scrub_packet, which in turn calls the skb destructor function.
In the case of packets coming from the AF_PACKET socket, the destructor
function seems to set all the mmap-ed pages to be available for user space to
copy any new packet it wants. Isn't this a problem?
skb_orphan_frags, called by ____dev_forward_skb, seems to do the right thing,
but it probably does not get called for packets from AF_PACKET socket, since the
skb is not a zero copy skb (SKBTX_DEV_ZEROCOPY is not set).
Did I miss something basic here?
(Please cc me, since I am not part of this list)
Thanks,
Anand
^ permalink raw reply
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Paweł Staszewski @ 2018-11-12 17:01 UTC (permalink / raw)
To: Alexander Duyck
Cc: aaron.lu, linux-mm, LKML, Netdev, Andrew Morton,
Jesper Dangaard Brouer, Eric Dumazet, Tariq Toukan,
ilias.apalodimas, yoel, Mel Gorman, Saeed Mahameed, Michal Hocko,
Vlastimil Babka, dave.hansen
In-Reply-To: <CAKgT0UeOBF0yPJLOTBBb3m7nTkmSDxzkCur+iGzJ++Y-jWaw9g@mail.gmail.com>
W dniu 12.11.2018 o 16:30, Alexander Duyck pisze:
> On Sun, Nov 11, 2018 at 4:39 PM Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>>
>> W dniu 12.11.2018 o 00:05, Alexander Duyck pisze:
>>> On Sat, Nov 10, 2018 at 3:54 PM Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>>>>
>>>> W dniu 05.11.2018 o 16:44, Alexander Duyck pisze:
>>>>> On Mon, Nov 5, 2018 at 12:58 AM Aaron Lu <aaron.lu@intel.com> wrote:
>>>>>> page_frag_free() calls __free_pages_ok() to free the page back to
>>>>>> Buddy. This is OK for high order page, but for order-0 pages, it
>>>>>> misses the optimization opportunity of using Per-Cpu-Pages and can
>>>>>> cause zone lock contention when called frequently.
>>>>>>
>>>>>> Paweł Staszewski recently shared his result of 'how Linux kernel
>>>>>> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
>>>>>> found the lock contention comes from page allocator:
>>>>>>
>>>>>> mlx5e_poll_tx_cq
>>>>>> |
>>>>>> --16.34%--napi_consume_skb
>>>>>> |
>>>>>> |--12.65%--__free_pages_ok
>>>>>> | |
>>>>>> | --11.86%--free_one_page
>>>>>> | |
>>>>>> | |--10.10%--queued_spin_lock_slowpath
>>>>>> | |
>>>>>> | --0.65%--_raw_spin_lock
>>>>>> |
>>>>>> |--1.55%--page_frag_free
>>>>>> |
>>>>>> --1.44%--skb_release_data
>>>>>>
>>>>>> Jesper explained how it happened: mlx5 driver RX-page recycle
>>>>>> mechanism is not effective in this workload and pages have to go
>>>>>> through the page allocator. The lock contention happens during
>>>>>> mlx5 DMA TX completion cycle. And the page allocator cannot keep
>>>>>> up at these speeds.[2]
>>>>>>
>>>>>> I thought that __free_pages_ok() are mostly freeing high order
>>>>>> pages and thought this is an lock contention for high order pages
>>>>>> but Jesper explained in detail that __free_pages_ok() here are
>>>>>> actually freeing order-0 pages because mlx5 is using order-0 pages
>>>>>> to satisfy its page pool allocation request.[3]
>>>>>>
>>>>>> The free path as pointed out by Jesper is:
>>>>>> skb_free_head()
>>>>>> -> skb_free_frag()
>>>>>> -> skb_free_frag()
>>>>>> -> page_frag_free()
>>>>>> And the pages being freed on this path are order-0 pages.
>>>>>>
>>>>>> Fix this by doing similar things as in __page_frag_cache_drain() -
>>>>>> send the being freed page to PCP if it's an order-0 page, or
>>>>>> directly to Buddy if it is a high order page.
>>>>>>
>>>>>> With this change, Paweł hasn't noticed lock contention yet in
>>>>>> his workload and Jesper has noticed a 7% performance improvement
>>>>>> using a micro benchmark and lock contention is gone.
>>>>>>
>>>>>> [1]: https://www.spinics.net/lists/netdev/msg531362.html
>>>>>> [2]: https://www.spinics.net/lists/netdev/msg531421.html
>>>>>> [3]: https://www.spinics.net/lists/netdev/msg531556.html
>>>>>> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
>>>>>> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>>>>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
>>>>>> ---
>>>>>> mm/page_alloc.c | 10 ++++++++--
>>>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>>>>> index ae31839874b8..91a9a6af41a2 100644
>>>>>> --- a/mm/page_alloc.c
>>>>>> +++ b/mm/page_alloc.c
>>>>>> @@ -4555,8 +4555,14 @@ void page_frag_free(void *addr)
>>>>>> {
>>>>>> struct page *page = virt_to_head_page(addr);
>>>>>>
>>>>>> - if (unlikely(put_page_testzero(page)))
>>>>>> - __free_pages_ok(page, compound_order(page));
>>>>>> + if (unlikely(put_page_testzero(page))) {
>>>>>> + unsigned int order = compound_order(page);
>>>>>> +
>>>>>> + if (order == 0)
>>>>>> + free_unref_page(page);
>>>>>> + else
>>>>>> + __free_pages_ok(page, order);
>>>>>> + }
>>>>>> }
>>>>>> EXPORT_SYMBOL(page_frag_free);
>>>>>>
>>>>> One thing I would suggest for Pawel to try would be to reduce the Tx
>>>>> qdisc size on his transmitting interfaces, Reduce the Tx ring size,
>>>>> and possibly increase the Tx interrupt rate. Ideally we shouldn't have
>>>>> too many packets in-flight and I suspect that is the issue that Pawel
>>>>> is seeing that is leading to the page pool allocator freeing up the
>>>>> memory. I know we like to try to batch things but the issue is
>>>>> processing too many Tx buffers in one batch leads to us eating up too
>>>>> much memory and causing evictions from the cache. Ideally the Rx and
>>>>> Tx rings and queues should be sized as small as possible while still
>>>>> allowing us to process up to our NAPI budget. Usually I run things
>>>>> with a 128 Rx / 128 Tx setup and then reduce the Tx queue length so we
>>>>> don't have more buffers stored there than we can place in the Tx ring.
>>>>> Then we can avoid the extra thrash of having to pull/push memory into
>>>>> and out of the freelists. Essentially the issue here ends up being
>>>>> another form of buffer bloat.
>>>> Thanks Aleksandar - yes it can be - but in my scenario setting RX buffer
>>>> <4096 producing more interface rx drops - and no_rx_buffer on network
>>>> controller that is receiving more packets
>>>> So i need to stick with 3000-4000 on RX - and yes i was trying to lower
>>>> the TX buff on connectx4 - but that changed nothing before Aaron patch
>>>>
>>>> After Aaron patch - decreasing TX buffer influencing total bandwidth
>>>> that can be handled by the router/server
>>>> Dono why before this patch there was no difference there no matter what
>>>> i set there there was always page_alloc/slowpath on top in perf
>>>>
>>>>
>>>> Currently testing RX4096/TX256 - this helps with bandwidth like +10%
>>>> more bandwidth with less interrupts...
>>> The problem is if you are going for less interrupts you are setting
>>> yourself up for buffer bloat. Basically you are going to use much more
>>> cache and much more memory then you actually need and if things are
>>> properly configured NAPI should take care of the interrupts anyway
>>> since under maximum load you shouldn't stop polling normally.
>> Im trying to balance here - there is problem cause server is forwarding
>> all kingd of protocols packets/different size etc
>>
>> The problem is im trying to go in high interrupt rate - but
>>
>> Setting coalescence to adaptative for rx killing cpu's at 22Gbit/s RX
>> and 22Gbit with rly high interrupt rate
> I wouldn't recommend adaptive just because the behavior would be hard
> to predict.
>
>> So adding a little more latency i can turn off adaptative rx and setup
>> rx-usecs from range 16-64 - and this gives me more or less interrupts -
>> but the problem is - always same bandwidth as maximum
> What about the tx-usecs, is that a functional thing for the adapter
> you are using?
Yes tx-usecs is not used now cause of adaptative mode on tx side:
ethtool -c enp175s0
Coalesce parameters for enp175s0:
Adaptive RX: off TX: on
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0
dmac: 32551
rx-usecs: 64
rx-frames: 128
rx-usecs-irq: 0
rx-frames-irq: 0
tx-usecs: 8
tx-frames: 64
tx-usecs-irq: 0
tx-frames-irq: 0
rx-usecs-low: 0
rx-frame-low: 0
tx-usecs-low: 0
tx-frame-low: 0
rx-usecs-high: 0
rx-frame-high: 0
tx-usecs-high: 0
tx-frame-high: 0
>
> The Rx side logic should be pretty easy to figure out. Essentially you
> want to keep the Rx ring size as small as possible while at the same
> time avoiding storming the system with interrupts. I know for 10Gb/s I
> have used a value of 25us in the past. What you want to watch for is
> if you are dropping packets on the Rx side or not. Ideally you want
> enough buffers that you can capture any burst while you wait for the
> interrupt routine to catch up.
>
>>> One issue I have seen is people delay interrupts for as long as
>>> possible which isn't really a good thing since most network
>>> controllers will use NAPI which will disable the interrupts and leave
>>> them disabled whenever the system is under heavy stress so you should
>>> be able to get the maximum performance by configuring an adapter with
>>> small ring sizes and for high interrupt rates.
>> Sure this is bad to setup rx-usec for high values - cause at some point
>> this will add high latency for packet traversing both sides - and start
>> to hurt buffers
>>
>> But my problem is a little different now i have no problems with RX side
>> - cause i can setup anything like:
>>
>> coalescence from 16 to 64
>>
>> rx ring from 3000 to max 8192
>>
>> And it does not change my max bw - only produces less or more interrupts.
> Right so the issue itself isn't Rx, you aren't throttled there. We are
> probably looking at an issue of PCIe bandwidth or Tx slowing things
> down. The fact that you are still filing interrupts is a bit
> surprising though. Are the Tx and Rx interrupts linked for the device
> you are using or are they firing them seperately? Normally Rx traffic
> won't generate many interrupts under a stress test as NAPI will leave
> the interrupts disabled unless it can keep up. Anyway, my suggestion
> would be to look at tuning things for as small a ring size as
> possible.
PCIe bw was eliminated - previously there was one 2 port 100G card
installed in one pciex16 (max bw for pcie x16 gen3 is 32GB/s 16/16GB
bidirectional)
Currently there are two separate nic's installed in two separate x16
slots - so can't be problem with pcie bandwidth
But i think I reach memory bandwidth limit now for 70Gbit/70Gbit :)
But wondering if there is any counter that can help me to diagnose
problems with memory bandwidth ?
stream app tests gives me results like:
./stream_c.exe
-------------------------------------------------------------
STREAM version $Revision: 5.10 $
-------------------------------------------------------------
This system uses 8 bytes per array element.
-------------------------------------------------------------
Array size = 10000000 (elements), Offset = 0 (elements)
Memory per array = 76.3 MiB (= 0.1 GiB).
Total memory required = 228.9 MiB (= 0.2 GiB).
Each kernel will be executed 10 times.
The *best* time for each kernel (excluding the first iteration)
will be used to compute the reported bandwidth.
-------------------------------------------------------------
Number of Threads requested = 56
Number of Threads counted = 56
-------------------------------------------------------------
Your clock granularity/precision appears to be 1 microseconds.
Each test below will take on the order of 4081 microseconds.
(= 4081 clock ticks)
Increase the size of the arrays if this shows that
you are not getting at least 20 clock ticks per test.
-------------------------------------------------------------
WARNING -- The above is only a rough guideline.
For best results, please be sure you know the
precision of your system timer.
-------------------------------------------------------------
Function Best Rate MB/s Avg time Min time Max time
Copy: 29907.2 0.005382 0.005350 0.005405
Scale: 28787.3 0.005611 0.005558 0.005650
Add: 34153.3 0.007037 0.007027 0.007055
Triad: 34944.0 0.006880 0.006868 0.006887
-------------------------------------------------------------
Solution Validates: avg error less than 1.000000e-13 on all three arrays
But this is for node 0+1
When limiting test to one node and cores used by network controllers:
-------------------------------------------------------------
STREAM version $Revision: 5.10 $
-------------------------------------------------------------
This system uses 8 bytes per array element.
-------------------------------------------------------------
Array size = 10000000 (elements), Offset = 0 (elements)
Memory per array = 76.3 MiB (= 0.1 GiB).
Total memory required = 228.9 MiB (= 0.2 GiB).
Each kernel will be executed 10 times.
The *best* time for each kernel (excluding the first iteration)
will be used to compute the reported bandwidth.
-------------------------------------------------------------
Number of Threads requested = 28
Number of Threads counted = 28
-------------------------------------------------------------
Your clock granularity/precision appears to be 1 microseconds.
Each test below will take on the order of 6107 microseconds.
(= 6107 clock ticks)
Increase the size of the arrays if this shows that
you are not getting at least 20 clock ticks per test.
-------------------------------------------------------------
WARNING -- The above is only a rough guideline.
For best results, please be sure you know the
precision of your system timer.
-------------------------------------------------------------
Function Best Rate MB/s Avg time Min time Max time
Copy: 20156.4 0.007946 0.007938 0.007958
Scale: 19436.1 0.008237 0.008232 0.008243
Add: 20184.7 0.011896 0.011890 0.011904
Triad: 20687.9 0.011607 0.011601 0.011613
-------------------------------------------------------------
Solution Validates: avg error less than 1.000000e-13 on all three arrays
-------------------------------------------------------------
Close to the limit but still some place - there can be some doubled
operations like for RX/TX side and network controllers can use more
bandwidth or just can't do this more optimally - cause of
bulking/buffers etc.
So currently there are only four from six channels used - i will upgrade
also memory and populate all six channels left/right side for two memory
controllers that cpu have.
>> So I start to change params for TX side - and for now i know that the
>> best for me is
>>
>> coalescence adaptative on
>>
>> TX buffer 128
>>
>> This helps with max BW that for now is close to 70Gbit/s RX and 70Gbit
>> TX but after this change i have increasing DROPS on TX side for vlan
>> interfaces.
> So this sounds like you are likely bottlenecked due to either PCIe
> bandwidth or latency. When you start putting back-pressure on the Tx
> like you have described it starts pushing packets onto the Qdisc
> layer. One thing that happens when packets are on the qdisc layer is
> that they can start to perform a bulk dequeue. The side effect of this
> is that you write multiple packets to the descriptor ring and then
> update the hardware doorbell only once for the entire group of packets
> instead of once per packet.
yes the problem is i just can't find any place where counters will shows
me why nic's start to drop packets
it does not reflect in cpu load or any other counter besides rx_phy
drops and tx_vlan drop packets
>> And only 50% cpu (max was 50% for 70Gbit/s)
>>
>>
>>> It is easiest to think of it this way. Your total packet rate is equal
>>> to your interrupt rate times the number of buffers you will store in
>>> the ring. So if you have some fixed rate "X" for packets and an
>>> interrupt rate of "i" then your optimal ring size should be "X/i". So
>>> if you lower the interrupt rate you end up hurting the throughput
>>> unless you increase the buffer size. However at a certain point the
>>> buffer size starts becoming an issue. For example with UDP flows I
>>> often see massive packet drops if you tune the interrupt rate too low
>>> and then put the system under heavy stress.
>> Yes - in normal life traffic - most of ddos'es are like this many pps
>> with small frames.
> It sounds to me like XDP would probably be your best bet. With that
> you could probably get away with smaller ring sizes, higher interrupt
> rates, and get the advantage of it batching the Tx without having to
> drop packets.
Yes im testing in lab xdp_fwd currently - but have some problems with
random drops that occuring randomly where server forwards only 1/10
packet and after some time it starts to work normally.
Currently trying to eliminate nic's offloading that can cause this - so
turning off one by one and running tests.
^ permalink raw reply
* Re: [PATCH net-next 1/6] net: sched: register callbacks for indirect tc block binds
From: Jiri Pirko @ 2018-11-12 6:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: davem, oss-drivers, netdev, xiyou.wangcong, jhs, gerlitz.or, ozsh,
vladbu, John Hurley
In-Reply-To: <20181110052131.3306-2-jakub.kicinski@netronome.com>
Sat, Nov 10, 2018 at 06:21:26AM CET, jakub.kicinski@netronome.com wrote:
>From: John Hurley <john.hurley@netronome.com>
>
>Currently drivers can register to receive TC block bind/unbind callbacks
>by implementing the setup_tc ndo in any of their given netdevs. However,
>drivers may also be interested in binds to higher level devices (e.g.
>tunnel drivers) to potentially offload filters applied to them.
>
>Introduce indirect block devs which allows drivers to register callbacks
>for block binds on other devices. The callback is triggered when the
>device is bound to a block, allowing the driver to register for rules
>applied to that block using already available functions.
>
>Freeing an indirect block callback will trigger an unbind event (if
>necessary) to direct the driver to remove any offloaded rules and unreg
>any block rule callbacks. It is the responsibility of the implementing
>driver to clean any registered indirect block callbacks before exiting,
>if the block it still active at such a time.
>
>Allow registering an indirect block dev callback for a device that is
>already bound to a block. In this case (if it is an ingress block),
>register and also trigger the callback meaning that any already installed
>rules can be replayed to the calling driver.
>
>Signed-off-by: John Hurley <john.hurley@netronome.com>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
^ permalink raw reply
* Re: [PATCH 2/2] dt-binding: mediatek-dwmac: add binding document for MediaTek MT2712 DWMAC
From: Rob Herring @ 2018-11-12 16:22 UTC (permalink / raw)
To: Biao Huang
Cc: mark.rutland, devicetree, nelson.chang, andrew, netdev, sean.wang,
liguo.zhang, linux-kernel, matthias.bgg, joabreu, linux-mediatek,
honghui.zhang, yt.shen, davem, linux-arm-kernel
In-Reply-To: <1540782294-27721-3-git-send-email-biao.huang@mediatek.com>
On Mon, Oct 29, 2018 at 11:04:54AM +0800, Biao Huang wrote:
> The commit adds the device tree binding documentation for the MediaTek DWMAC
> found on MediaTek MT2712.
>
> Signed-off-by: Biao Huang <biao.huang@mediatek.com>
> ---
> .../devicetree/bindings/net/mediatek-dwmac.txt | 49 ++++++++++++++++++++
> 1 file changed, 49 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/mediatek-dwmac.txt
>
> diff --git a/Documentation/devicetree/bindings/net/mediatek-dwmac.txt b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> new file mode 100644
> index 0000000..8baf0e4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/mediatek-dwmac.txt
> @@ -0,0 +1,49 @@
> +MediaTek DWMAC glue layer controller
> +
> +This file documents platform glue layer for stmmac.
> +Please see stmmac.txt for the other unchanged properties.
> +
> +The device node has following properties.
> +
> +Required properties:
> +- compatible: Should be "mediatek,mt2712-gmac" for MT2712 SoC
> +- reg: Address and length of the register set for the device
> +- interrupts: Should contain the MAC interrupts
> +- interrupt-names: Should contain a list of interrupt names corresponding to
> + the interrupts in the interrupts property, if available.
> + Should be "macirq" for the main MAC IRQ
> +- clocks: Must contain a phandle for each entry in clock-names.
> +- clock-names: the name of the clock listed in the clocks property. These are
> + "axi", "apb", "mac_ext", "mac_parent", "ptp_ref", "ptp_parent", "ptp_top"
> + for MT2712 SoC
Clocks should represent the physical clocks connected to a block. Parent
clocks are not in that category.
> +- mac-address: See ethernet.txt in the same directory
> +- phy-mode: See ethernet.txt in the same directory
> +
> +Example:
> +
> + eth: ethernet@1101c000 {
> + compatible = "mediatek,mt2712-gmac";
> + reg = <0 0x1101c000 0 0x1300>;
> + interrupts = <GIC_SPI 237 IRQ_TYPE_LEVEL_LOW>;
> + interrupt-names = "macirq";
> + phy-mode ="rgmii-id";
> + mac-address = [00 55 7b b5 7d f7];
> + clock-names = "axi",
> + "apb",
> + "mac_ext",
> + "mac_parent",
> + "ptp_ref",
> + "ptp_parent",
> + "ptp_top";
> + clocks = <&pericfg CLK_PERI_GMAC>,
> + <&pericfg CLK_PERI_GMAC_PCLK>,
> + <&topckgen CLK_TOP_ETHER_125M_SEL>,
> + <&topckgen CLK_TOP_ETHERPLL_125M>,
> + <&topckgen CLK_TOP_ETHER_50M_SEL>,
> + <&topckgen CLK_TOP_APLL1_D3>,
> + <&topckgen CLK_TOP_APLL1>;
> + snps,txpbl = <32>;
> + snps,rxpbl = <32>;
> + snps,reset-gpio = <&pio 87 GPIO_ACTIVE_LOW>;
> + snps,reset-active-low;
> + };
> --
> 1.7.9.5
>
^ permalink raw reply
* 答复: [PATCH][RFC] udp: cache sock to avoid searching it twice
From: Li,Rongqing @ 2018-11-12 5:46 UTC (permalink / raw)
To: Paolo Abeni, netdev@vger.kernel.org; +Cc: Willem de Bruijn
In-Reply-To: <2b32d67c09bbdec46b97a3cf2898145ce60a6836.camel@redhat.com>
> > return pp;
> > }
>
> What if 'pp' is NULL?
>
> Aside from that, this replace a lookup with 2 atomic ops, and only when
> such lookup is amortized on multiple aggregated packets: I'm unsure if
> it's worthy and I don't understand how that improves RR tests (where
> the socket can't see multiple, consecutive skbs, AFAIK).
>
> Cheers,
>
> Paolo
>
If we not release the socket in udp_gro_complete , we can reduce a udp socket
Lookup when do ip demux again, it maybe more worthy.
I test UDP_STREAM, find no difference, both can reach NIC's limit, 10G;
so Test RR, I will do more tests
-RongQing
^ permalink raw reply
* 答复: [PATCH][RFC] udp: cache sock to avoid searching it twice
From: Li,Rongqing @ 2018-11-12 5:39 UTC (permalink / raw)
To: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <0c6a070f-d2ac-2ffc-00d5-097fe84ae42c@gmail.com>
On Sat, Nov 10, 2018 at 1:29 AM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 11/08/2018 10:21 PM, Li RongQing wrote:
> > GRO for UDP needs to lookup socket twice, first is in gro receive,
> > second is gro complete, so if store sock to skb to avoid looking up
> > twice, this can give small performance boost
> >
> > netperf -t UDP_RR -l 10
> >
> > Before:
> > Rate per sec: 28746.01
> > After:
> > Rate per sec: 29401.67
> >
> > Signed-off-by: Li RongQing <lirongqing@baidu.com>
> > ---
> > net/ipv4/udp_offload.c | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 0646d61f4fa8..429570112a33 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -408,6 +408,11 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> >
> > if (udp_sk(sk)->gro_enabled) {
> > pp = call_gro_receive(udp_gro_receive_segment, head, skb);
> > +
> > + if (!IS_ERR(pp) && NAPI_GRO_CB(pp)->count > 1) {
> > + sock_hold(sk);
> > + pp->sk = sk;
>
>
> You also have to set pp->destructor to sock_edemux
>
> flush_gro_hash -> kfree_skb()
>
> If there is no destructor, the reference on pp->sk will never be released.
>
>
Ok, thanks,
does it need to reset sk in udp_gro_complete, ip early demuxing will lookup udp socket again, if we can keep it, we can avoid to lookup socket again
-RongQing
>
>
> > + }
> > rcu_read_unlock();
> > return pp;
> > }
> > @@ -444,6 +449,10 @@ struct sk_buff *udp_gro_receive(struct list_head *head, struct sk_buff *skb,
> > skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
> > pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
> >
> > + if (!IS_ERR(pp) && NAPI_GRO_CB(pp)->count > 1) {
> > + sock_hold(sk);
> > + pp->sk = sk;
> > + }
> > out_unlock:
> > rcu_read_unlock();
> > skb_gro_flush_final(skb, pp, flush);
> > @@ -502,7 +511,9 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
> > uh->len = newlen;
> >
> > rcu_read_lock();
> > - sk = (*lookup)(skb, uh->source, uh->dest);
> > + sk = skb->sk;
> > + if (!sk)
> > + sk = (*lookup)(skb, uh->source, uh->dest);
> > if (sk && udp_sk(sk)->gro_enabled) {
> > err = udp_gro_complete_segment(skb);
> > } else if (sk && udp_sk(sk)->gro_complete) {
> > @@ -516,6 +527,11 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff,
> > err = udp_sk(sk)->gro_complete(sk, skb,
> > nhoff + sizeof(struct udphdr));
> > }
> > +
> > + if (skb->sk) {
> > + sock_put(skb->sk);
> > + skb->sk = NULL;
> > + }
> > rcu_read_unlock();
> >
> > if (skb->remcsum_offload)
> >
^ permalink raw reply
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Eric Dumazet @ 2018-11-12 15:44 UTC (permalink / raw)
To: Alexander Duyck, Paweł Staszewski
Cc: aaron.lu, linux-mm, LKML, Netdev, Andrew Morton,
Jesper Dangaard Brouer, Eric Dumazet, Tariq Toukan,
ilias.apalodimas, yoel, Mel Gorman, Saeed Mahameed, Michal Hocko,
Vlastimil Babka, dave.hansen
In-Reply-To: <CAKgT0UeOBF0yPJLOTBBb3m7nTkmSDxzkCur+iGzJ++Y-jWaw9g@mail.gmail.com>
On 11/12/2018 07:30 AM, Alexander Duyck wrote:
> It sounds to me like XDP would probably be your best bet. With that
> you could probably get away with smaller ring sizes, higher interrupt
> rates, and get the advantage of it batching the Tx without having to
> drop packets.
Add to this that with XDP (or anything lowering per packet processing costs)
you can reduce number of cpus/queues, get better latencies, and bigger TX batches.
^ permalink raw reply
* KASAN: use-after-free Read in nbp_vlan_rcu_free
From: syzbot @ 2018-11-12 5:51 UTC (permalink / raw)
To: bridge, davem, linux-kernel, netdev, nikolay, roopa,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: e12e00e388de Merge tag 'kbuild-fixes-v4.20' of git://git.k..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14cdb6f5400000
kernel config: https://syzkaller.appspot.com/x/.config?x=8f215f21f041a0d7
dashboard link: https://syzkaller.appspot.com/bug?extid=04681da557a0e49a52e5
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
Unfortunately, I don't have any reproducer for this crash yet.
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+04681da557a0e49a52e5@syzkaller.appspotmail.com
device bridge_slave_1 left promiscuous mode
bridge0: port 2(bridge_slave_1) entered disabled state
device bridge_slave_0 left promiscuous mode
bridge0: port 1(bridge_slave_0) entered disabled state
==================================================================
BUG: KASAN: use-after-free in nbp_vlan_rcu_free+0x152/0x160
net/bridge/br_vlan.c:200
Read of size 8 at addr ffff8881bbc44a18 by task swapper/1/0
CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.20.0-rc1+ #332
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
<IRQ>
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x244/0x39d lib/dump_stack.c:113
print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
nbp_vlan_rcu_free+0x152/0x160 net/bridge/br_vlan.c:200
__rcu_reclaim kernel/rcu/rcu.h:240 [inline]
rcu_do_batch kernel/rcu/tree.c:2437 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2716 [inline]
rcu_process_callbacks+0x100a/0x1ac0 kernel/rcu/tree.c:2697
__do_softirq+0x308/0xb7e kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x17f/0x1c0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:536 [inline]
smp_apic_timer_interrupt+0x1cb/0x760 arch/x86/kernel/apic/apic.c:1061
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:804
</IRQ>
RIP: 0010:native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:57
Code: e9 2c ff ff ff 48 89 c7 48 89 45 d8 e8 d3 43 f3 f9 48 8b 45 d8 e9 ca
fe ff ff 48 89 df e8 c2 43 f3 f9 eb 82 55 48 89 e5 fb f4 <5d> c3 0f 1f 84
00 00 00 00 00 55 48 89 e5 f4 5d c3 90 90 90 90 90
RSP: 0018:ffff8881d9b27cb8 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
RAX: dffffc0000000000 RBX: 1ffff1103b364f9b RCX: 0000000000000000
RDX: 1ffffffff12a3f71 RSI: 0000000000000001 RDI: ffffffff8951fb88
RBP: ffff8881d9b27cb8 R08: ffff8881d9b14340 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: ffff8881d9b27d78
R13: ffffffff8a14e1e0 R14: 0000000000000000 R15: 0000000000000001
arch_safe_halt arch/x86/include/asm/paravirt.h:151 [inline]
default_idle+0xbf/0x490 arch/x86/kernel/process.c:498
arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:489
default_idle_call+0x6d/0x90 kernel/sched/idle.c:93
cpuidle_idle_call kernel/sched/idle.c:153 [inline]
do_idle+0x49b/0x5c0 kernel/sched/idle.c:262
cpu_startup_entry+0x18/0x20 kernel/sched/idle.c:353
start_secondary+0x487/0x5f0 arch/x86/kernel/smpboot.c:271
secondary_startup_64+0xa4/0xb0 arch/x86/kernel/head_64.S:243
Allocated by task 13858:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620
kmalloc include/linux/slab.h:546 [inline]
kzalloc include/linux/slab.h:741 [inline]
br_vlan_add+0x6e5/0x1340 net/bridge/br_vlan.c:650
br_vlan_init+0x339/0x3e0 net/bridge/br_vlan.c:1047
br_dev_init+0x10d/0x2a0 net/bridge/br_device.c:134
register_netdevice+0x332/0x11d0 net/core/dev.c:8459
br_dev_newlink+0x2a/0x130 net/bridge/br_netlink.c:1309
rtnl_newlink+0xf08/0x1da0 net/core/rtnetlink.c:3175
rtnetlink_rcv_msg+0x46a/0xc20 net/core/rtnetlink.c:4947
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2477
rtnetlink_rcv+0x1c/0x20 net/core/rtnetlink.c:4965
netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
netlink_unicast+0x5a5/0x760 net/netlink/af_netlink.c:1336
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1917
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x7fd/0x930 net/socket.c:2116
__sys_sendmsg+0x11d/0x280 net/socket.c:2154
__do_sys_sendmsg net/socket.c:2163 [inline]
__se_sys_sendmsg net/socket.c:2161 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2161
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 9:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3817
br_master_vlan_rcu_free+0xa8/0xe0 net/bridge/br_vlan.c:174
__rcu_reclaim kernel/rcu/rcu.h:240 [inline]
rcu_do_batch kernel/rcu/tree.c:2437 [inline]
invoke_rcu_callbacks kernel/rcu/tree.c:2716 [inline]
rcu_process_callbacks+0x100a/0x1ac0 kernel/rcu/tree.c:2697
__do_softirq+0x308/0xb7e kernel/softirq.c:292
The buggy address belongs to the object at ffff8881bbc44a00
which belongs to the cache kmalloc-96 of size 96
The buggy address is located 24 bytes inside of
96-byte region [ffff8881bbc44a00, ffff8881bbc44a60)
The buggy address belongs to the page:
page:ffffea0006ef1100 count:1 mapcount:0 mapping:ffff8881da8004c0 index:0x0
flags: 0x2fffc0000000200(slab)
raw: 02fffc0000000200 ffffea0007471a08 ffffea000757b908 ffff8881da8004c0
raw: 0000000000000000 ffff8881bbc44000 0000000100000020 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8881bbc44900: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
ffff8881bbc44980: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
> ffff8881bbc44a00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
^
ffff8881bbc44a80: 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc fc
ffff8881bbc44b00: fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
^ permalink raw reply
* Re: [PATCH] mt76: fix uninitialized mutex access setting rts threshold
From: Kalle Valo @ 2018-11-12 5:50 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: nbd, sgruszka, linux-wireless, netdev
In-Reply-To: <1cfa6dca73f1d81595f061ded6275293c6e96b75.1541847014.git.lorenzo.bianconi@redhat.com>
Lorenzo Bianconi <lorenzo.bianconi@redhat.com> writes:
> Fix following crash due to a leftover uninitialized mutex access
> in mt76x2_set_rts_threshold routine.
>
> [ 31.018059] Call Trace:
> [ 31.018341] register_lock_class+0x51f/0x530
> [ 31.018828] __lock_acquire+0x6c/0x1580
> [ 31.019247] lock_acquire+0x88/0x120
> [ 31.021089] __mutex_lock+0x4a/0x4f0
> [ 31.023343] mt76x2_set_rts_threshold+0x28/0x50
> [ 31.023831] ieee80211_set_wiphy_params+0x16d/0x4e0
> [ 31.024344] nl80211_set_wiphy+0x72b/0xbc0
> [ 31.024781] genl_family_rcv_msg+0x192/0x3a0
> [ 31.025233] genl_rcv_msg+0x42/0x89
> [ 31.026079] netlink_rcv_skb+0x38/0x100
> [ 31.026475] genl_rcv+0x1f/0x30
> [ 31.026804] netlink_unicast+0x19c/0x250
> [ 31.027212] netlink_sendmsg+0x1ed/0x390
> [ 31.027615] sock_sendmsg+0x31/0x40
> [ 31.027973] ___sys_sendmsg+0x23c/0x280
> [ 31.030414] __sys_sendmsg+0x42/0x80
> [ 31.030783] do_syscall_64+0x4a/0x170
> [ 31.031160] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 31.031677] RIP: 0033:0x7f3498b39ba7
> [ 31.033953] RSP: 002b:00007fffe19675b8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> [ 31.034883] RAX: ffffffffffffffda RBX: 00000000012d5350 RCX: 00007f3498b39ba7
> [ 31.035756] RDX: 0000000000000000 RSI: 00007fffe19675f0 RDI: 0000000000000003
> [ 31.036587] RBP: 00000000012da740 R08: 0000000000000002 R09: 0000000000000000
> [ 31.037422] R10: 0000000000000006 R11: 0000000000000246 R12: 00000000012da880
> [ 31.038252] R13: 00007fffe19675f0 R14: 00007fffe19678c0 R15: 00000000012da880
>
> Fixes: 108a4861ef19 ("mt76: create new mt76x02-lib module for common
> mt76x{0,2} code")
> Reported-by: lorenzo.trisolini@fluidmesh.com
> Reported-by: luca.bisti@fluidmesh.com
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
This should be v2, but no need to resend.
> This patch is for 4.20
Ok, I'll queue this for that release.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 1/2] mm/page_alloc: free order-0 pages through PCP in page_frag_free()
From: Alexander Duyck @ 2018-11-12 15:30 UTC (permalink / raw)
To: Paweł Staszewski
Cc: aaron.lu, linux-mm, LKML, Netdev, Andrew Morton,
Jesper Dangaard Brouer, Eric Dumazet, Tariq Toukan,
ilias.apalodimas, yoel, Mel Gorman, Saeed Mahameed, Michal Hocko,
Vlastimil Babka, dave.hansen
In-Reply-To: <bd33633b-2f6c-0034-a130-38a8468531db@itcare.pl>
On Sun, Nov 11, 2018 at 4:39 PM Paweł Staszewski <pstaszewski@itcare.pl> wrote:
>
>
> W dniu 12.11.2018 o 00:05, Alexander Duyck pisze:
> > On Sat, Nov 10, 2018 at 3:54 PM Paweł Staszewski <pstaszewski@itcare.pl> wrote:
> >>
> >>
> >> W dniu 05.11.2018 o 16:44, Alexander Duyck pisze:
> >>> On Mon, Nov 5, 2018 at 12:58 AM Aaron Lu <aaron.lu@intel.com> wrote:
> >>>> page_frag_free() calls __free_pages_ok() to free the page back to
> >>>> Buddy. This is OK for high order page, but for order-0 pages, it
> >>>> misses the optimization opportunity of using Per-Cpu-Pages and can
> >>>> cause zone lock contention when called frequently.
> >>>>
> >>>> Paweł Staszewski recently shared his result of 'how Linux kernel
> >>>> handles normal traffic'[1] and from perf data, Jesper Dangaard Brouer
> >>>> found the lock contention comes from page allocator:
> >>>>
> >>>> mlx5e_poll_tx_cq
> >>>> |
> >>>> --16.34%--napi_consume_skb
> >>>> |
> >>>> |--12.65%--__free_pages_ok
> >>>> | |
> >>>> | --11.86%--free_one_page
> >>>> | |
> >>>> | |--10.10%--queued_spin_lock_slowpath
> >>>> | |
> >>>> | --0.65%--_raw_spin_lock
> >>>> |
> >>>> |--1.55%--page_frag_free
> >>>> |
> >>>> --1.44%--skb_release_data
> >>>>
> >>>> Jesper explained how it happened: mlx5 driver RX-page recycle
> >>>> mechanism is not effective in this workload and pages have to go
> >>>> through the page allocator. The lock contention happens during
> >>>> mlx5 DMA TX completion cycle. And the page allocator cannot keep
> >>>> up at these speeds.[2]
> >>>>
> >>>> I thought that __free_pages_ok() are mostly freeing high order
> >>>> pages and thought this is an lock contention for high order pages
> >>>> but Jesper explained in detail that __free_pages_ok() here are
> >>>> actually freeing order-0 pages because mlx5 is using order-0 pages
> >>>> to satisfy its page pool allocation request.[3]
> >>>>
> >>>> The free path as pointed out by Jesper is:
> >>>> skb_free_head()
> >>>> -> skb_free_frag()
> >>>> -> skb_free_frag()
> >>>> -> page_frag_free()
> >>>> And the pages being freed on this path are order-0 pages.
> >>>>
> >>>> Fix this by doing similar things as in __page_frag_cache_drain() -
> >>>> send the being freed page to PCP if it's an order-0 page, or
> >>>> directly to Buddy if it is a high order page.
> >>>>
> >>>> With this change, Paweł hasn't noticed lock contention yet in
> >>>> his workload and Jesper has noticed a 7% performance improvement
> >>>> using a micro benchmark and lock contention is gone.
> >>>>
> >>>> [1]: https://www.spinics.net/lists/netdev/msg531362.html
> >>>> [2]: https://www.spinics.net/lists/netdev/msg531421.html
> >>>> [3]: https://www.spinics.net/lists/netdev/msg531556.html
> >>>> Reported-by: Paweł Staszewski <pstaszewski@itcare.pl>
> >>>> Analysed-by: Jesper Dangaard Brouer <brouer@redhat.com>
> >>>> Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> >>>> ---
> >>>> mm/page_alloc.c | 10 ++++++++--
> >>>> 1 file changed, 8 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >>>> index ae31839874b8..91a9a6af41a2 100644
> >>>> --- a/mm/page_alloc.c
> >>>> +++ b/mm/page_alloc.c
> >>>> @@ -4555,8 +4555,14 @@ void page_frag_free(void *addr)
> >>>> {
> >>>> struct page *page = virt_to_head_page(addr);
> >>>>
> >>>> - if (unlikely(put_page_testzero(page)))
> >>>> - __free_pages_ok(page, compound_order(page));
> >>>> + if (unlikely(put_page_testzero(page))) {
> >>>> + unsigned int order = compound_order(page);
> >>>> +
> >>>> + if (order == 0)
> >>>> + free_unref_page(page);
> >>>> + else
> >>>> + __free_pages_ok(page, order);
> >>>> + }
> >>>> }
> >>>> EXPORT_SYMBOL(page_frag_free);
> >>>>
> >>> One thing I would suggest for Pawel to try would be to reduce the Tx
> >>> qdisc size on his transmitting interfaces, Reduce the Tx ring size,
> >>> and possibly increase the Tx interrupt rate. Ideally we shouldn't have
> >>> too many packets in-flight and I suspect that is the issue that Pawel
> >>> is seeing that is leading to the page pool allocator freeing up the
> >>> memory. I know we like to try to batch things but the issue is
> >>> processing too many Tx buffers in one batch leads to us eating up too
> >>> much memory and causing evictions from the cache. Ideally the Rx and
> >>> Tx rings and queues should be sized as small as possible while still
> >>> allowing us to process up to our NAPI budget. Usually I run things
> >>> with a 128 Rx / 128 Tx setup and then reduce the Tx queue length so we
> >>> don't have more buffers stored there than we can place in the Tx ring.
> >>> Then we can avoid the extra thrash of having to pull/push memory into
> >>> and out of the freelists. Essentially the issue here ends up being
> >>> another form of buffer bloat.
> >> Thanks Aleksandar - yes it can be - but in my scenario setting RX buffer
> >> <4096 producing more interface rx drops - and no_rx_buffer on network
> >> controller that is receiving more packets
> >> So i need to stick with 3000-4000 on RX - and yes i was trying to lower
> >> the TX buff on connectx4 - but that changed nothing before Aaron patch
> >>
> >> After Aaron patch - decreasing TX buffer influencing total bandwidth
> >> that can be handled by the router/server
> >> Dono why before this patch there was no difference there no matter what
> >> i set there there was always page_alloc/slowpath on top in perf
> >>
> >>
> >> Currently testing RX4096/TX256 - this helps with bandwidth like +10%
> >> more bandwidth with less interrupts...
> > The problem is if you are going for less interrupts you are setting
> > yourself up for buffer bloat. Basically you are going to use much more
> > cache and much more memory then you actually need and if things are
> > properly configured NAPI should take care of the interrupts anyway
> > since under maximum load you shouldn't stop polling normally.
>
> Im trying to balance here - there is problem cause server is forwarding
> all kingd of protocols packets/different size etc
>
> The problem is im trying to go in high interrupt rate - but
>
> Setting coalescence to adaptative for rx killing cpu's at 22Gbit/s RX
> and 22Gbit with rly high interrupt rate
I wouldn't recommend adaptive just because the behavior would be hard
to predict.
> So adding a little more latency i can turn off adaptative rx and setup
> rx-usecs from range 16-64 - and this gives me more or less interrupts -
> but the problem is - always same bandwidth as maximum
What about the tx-usecs, is that a functional thing for the adapter
you are using?
The Rx side logic should be pretty easy to figure out. Essentially you
want to keep the Rx ring size as small as possible while at the same
time avoiding storming the system with interrupts. I know for 10Gb/s I
have used a value of 25us in the past. What you want to watch for is
if you are dropping packets on the Rx side or not. Ideally you want
enough buffers that you can capture any burst while you wait for the
interrupt routine to catch up.
> >
> > One issue I have seen is people delay interrupts for as long as
> > possible which isn't really a good thing since most network
> > controllers will use NAPI which will disable the interrupts and leave
> > them disabled whenever the system is under heavy stress so you should
> > be able to get the maximum performance by configuring an adapter with
> > small ring sizes and for high interrupt rates.
>
> Sure this is bad to setup rx-usec for high values - cause at some point
> this will add high latency for packet traversing both sides - and start
> to hurt buffers
>
> But my problem is a little different now i have no problems with RX side
> - cause i can setup anything like:
>
> coalescence from 16 to 64
>
> rx ring from 3000 to max 8192
>
> And it does not change my max bw - only produces less or more interrupts.
Right so the issue itself isn't Rx, you aren't throttled there. We are
probably looking at an issue of PCIe bandwidth or Tx slowing things
down. The fact that you are still filing interrupts is a bit
surprising though. Are the Tx and Rx interrupts linked for the device
you are using or are they firing them seperately? Normally Rx traffic
won't generate many interrupts under a stress test as NAPI will leave
the interrupts disabled unless it can keep up. Anyway, my suggestion
would be to look at tuning things for as small a ring size as
possible.
> So I start to change params for TX side - and for now i know that the
> best for me is
>
> coalescence adaptative on
>
> TX buffer 128
>
> This helps with max BW that for now is close to 70Gbit/s RX and 70Gbit
> TX but after this change i have increasing DROPS on TX side for vlan
> interfaces.
So this sounds like you are likely bottlenecked due to either PCIe
bandwidth or latency. When you start putting back-pressure on the Tx
like you have described it starts pushing packets onto the Qdisc
layer. One thing that happens when packets are on the qdisc layer is
that they can start to perform a bulk dequeue. The side effect of this
is that you write multiple packets to the descriptor ring and then
update the hardware doorbell only once for the entire group of packets
instead of once per packet.
> And only 50% cpu (max was 50% for 70Gbit/s)
>
>
> > It is easiest to think of it this way. Your total packet rate is equal
> > to your interrupt rate times the number of buffers you will store in
> > the ring. So if you have some fixed rate "X" for packets and an
> > interrupt rate of "i" then your optimal ring size should be "X/i". So
> > if you lower the interrupt rate you end up hurting the throughput
> > unless you increase the buffer size. However at a certain point the
> > buffer size starts becoming an issue. For example with UDP flows I
> > often see massive packet drops if you tune the interrupt rate too low
> > and then put the system under heavy stress.
>
> Yes - in normal life traffic - most of ddos'es are like this many pps
> with small frames.
It sounds to me like XDP would probably be your best bet. With that
you could probably get away with smaller ring sizes, higher interrupt
rates, and get the advantage of it batching the Tx without having to
drop packets.
^ permalink raw reply
* Re: [RFC] mm: Replace all open encodings for NUMA_NO_NODE
From: Anshuman Khandual @ 2018-11-12 4:14 UTC (permalink / raw)
To: Joseph Qi, linux-mm, linux-kernel
Cc: ocfs2-devel, linux-fbdev, dri-devel, netdev, linux-media, iommu,
linux-rdma, dmaengine, linux-block, sparclinux, linuxppc-dev,
linux-ia64, linux-alpha
In-Reply-To: <b92e3275-7a04-a148-bb5b-38658c270583@arm.com>
On 11/12/2018 09:40 AM, Anshuman Khandual wrote:
>
>
> On 11/12/2018 09:27 AM, Joseph Qi wrote:
>> For ocfs2 part, node means host in the cluster, not NUMA node.
>>
>
> Does not -1 indicate an invalid node which can never be present ?
>
My bad, got it wrong. Seems like this is nothing to do with NUMA node
at all. Will drop the changes from ocfs2.
^ permalink raw reply
* Re: [PATCH net-next 0/6] net: sched: indirect tc block cb registration
From: Jakub Kicinski @ 2018-11-12 4:13 UTC (permalink / raw)
To: David Miller, gerlitz.or
Cc: oss-drivers, netdev, jiri, xiyou.wangcong, jhs, ozsh, vladbu
In-Reply-To: <20181111.095535.170428974627804709.davem@davemloft.net>
On Sun, 11 Nov 2018 09:55:35 -0800 (PST), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Fri, 9 Nov 2018 21:21:25 -0800
>
> > John says:
> >
> > This patchset introduces an alternative to egdev offload by allowing a
> > driver to register for block updates when an external device (e.g. tunnel
> > netdev) is bound to a TC block. Drivers can track new netdevs or register
> > to existing ones to receive information on such events. Based on this,
> > they may register for block offload rules using already existing
> > functions.
> >
> > The patchset also implements this new indirect block registration in the
> > NFP driver to allow the offloading of tunnel rules. The use of egdev
> > offload (which is currently only used for tunnel offload) is subsequently
> > removed.
>
> Really nice. Series applied.
>
> Can the Mellanox folks use this too so that we can remove egdev altogether?
> mlx5 is the only remaining user.
I believe Or and Oz are working on mlx5 counterpart, hopefully to land
in this cycle? :)
^ permalink raw reply
* [PATCH net-next 4/4] net: ethernet: ti: cpsw: allow vlan tagged packets to be timestamped
From: Ivan Khoronzhuk @ 2018-11-12 14:00 UTC (permalink / raw)
To: grygorii.strashko, davem
Cc: linux-omap, netdev, linux-kernel, Ivan Khoronzhuk
In-Reply-To: <20181112140023.12407-1-ivan.khoronzhuk@linaro.org>
Allow vlan tagged packets to be timestamped, as no any restrictions
for this.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
---
drivers/net/ethernet/ti/cpsw.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 48ea21e7e0cb..e4aa030f1726 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -283,7 +283,7 @@ struct cpsw_ss_regs {
#define CTRL_V2_TS_BITS \
(TS_320 | TS_319 | TS_132 | TS_131 | TS_130 | TS_129 |\
- TS_TTL_NONZERO | TS_ANNEX_D_EN | TS_LTYPE1_EN)
+ TS_TTL_NONZERO | TS_ANNEX_D_EN | TS_LTYPE1_EN | VLAN_LTYPE1_EN)
#define CTRL_V2_ALL_TS_MASK (CTRL_V2_TS_BITS | TS_TX_EN | TS_RX_EN)
#define CTRL_V2_TX_TS_BITS (CTRL_V2_TS_BITS | TS_TX_EN)
@@ -293,7 +293,7 @@ struct cpsw_ss_regs {
#define CTRL_V3_TS_BITS \
(TS_107 | TS_320 | TS_319 | TS_132 | TS_131 | TS_130 | TS_129 |\
TS_TTL_NONZERO | TS_ANNEX_F_EN | TS_ANNEX_D_EN |\
- TS_LTYPE1_EN)
+ TS_LTYPE1_EN | VLAN_LTYPE1_EN)
#define CTRL_V3_ALL_TS_MASK (CTRL_V3_TS_BITS | TS_TX_EN | TS_RX_EN)
#define CTRL_V3_TX_TS_BITS (CTRL_V3_TS_BITS | TS_TX_EN)
@@ -2235,6 +2235,7 @@ static void cpsw_hwtstamp_v2(struct cpsw_priv *priv)
slave_write(slave, mtype, CPSW2_TS_SEQ_MTYPE);
slave_write(slave, ctrl, CPSW2_CONTROL);
writel_relaxed(ETH_P_1588, &cpsw->regs->ts_ltype);
+ writel_relaxed(ETH_P_8021Q, &cpsw->regs->vlan_ltype);
}
static int cpsw_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
--
2.17.1
^ permalink raw reply related
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