On 12/21/2016 9:03 AM, Cong Wang wrote:
On Tue, Dec 20, 2016 at 10:44 PM, Shahar Klein <shah...@mellanox.com> wrote:
Tried it with same results
This piece is pretty interesting:
[ 408.554689] DEBUGG:SK thread-2853[cpu-1] setting tp_created to 1
tp=ffff94b5b02805a0 back=ffff94b9ea932060
[ 408.574258] DEBUGG:SK thread-2853[cpu-1] add/change filter by:
fl_get [cls_flower] tp=ffff94b5b02805a0 tp->next=ffff94b9ea932060
[ 408.587849] DEBUGG:SK destroy ffff94b5b0280780 tcf_destroy:1905
[ 408.595862] DEBUGG:SK thread-2845[cpu-1] add/change filter by:
fl_get [cls_flower] tp=ffff94b5b02805a0 tp->next=ffff94b5b02805a0
Looks like you added a debug printk inside tcf_destroy() too,
which seems racy with filter creation, it should not happen since
in both cases we take RTNL lock.
Don't know if changing all RCU_INIT_POINTER in that file to
rcu_assign_pointer could help anything or not. Mind to try?
Tried it with same results
Thanks for debugging!
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 3fbba79..b8a66d8 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -305,7 +305,7 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct
nlmsghdr *n)
kfree(tp);
goto errout;
}
-
+ printk(KERN_ERR "DEBUGG:SK thread-%d[cpu-%d] setting tp_created
to 1 tp=%p back=%p\n", current->pid, current->on_cpu, tp,
rtnl_dereference(*back));
tp_created = 1;
} else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind))
@@ -317,11 +317,13 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct
nlmsghdr *n)
if (n->nlmsg_type == RTM_DELTFILTER && t->tcm_handle == 0) {
struct tcf_proto *next = rtnl_dereference(tp->next);
- RCU_INIT_POINTER(*back, next);
+ printk(KERN_ERR "DEBUGG:SK delete filter by: %pf\n",
tp->ops->get);
+
+ rcu_assign_pointer(*back, next);
tfilter_notify(net, skb, n, tp, fh,
RTM_DELTFILTER, false);
- tcf_destroy(tp, true);
+ tcf_destroy(tp);
err = 0;
goto errout;
}
@@ -331,25 +333,30 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct
nlmsghdr *n)
!(n->nlmsg_flags & NLM_F_CREATE))
goto errout;
} else {
+ bool last;
+
switch (n->nlmsg_type) {
case RTM_NEWTFILTER:
err = -EEXIST;
if (n->nlmsg_flags & NLM_F_EXCL) {
if (tp_created)
- tcf_destroy(tp, true);
+ tcf_destroy(tp);
goto errout;
}
break;
case RTM_DELTFILTER:
- err = tp->ops->delete(tp, fh);
+ printk(KERN_ERR "DEBUGG:SK %s:%d\n", __func__,
__LINE__);
+ err = tp->ops->delete(tp, fh, &last);
if (err == 0) {
- struct tcf_proto *next =
rtnl_dereference(tp->next);
-
tfilter_notify(net, skb, n, tp,
t->tcm_handle,
RTM_DELTFILTER, false);
- if (tcf_destroy(tp, false))
- RCU_INIT_POINTER(*back, next);
+ if (last) {
+ struct tcf_proto *next =
rtnl_dereference(tp->next);
+
+ rcu_assign_pointer(*back, next);
+ tcf_destroy(tp);
+ }
}
goto errout;
case RTM_GETTFILTER:
@@ -366,13 +373,14 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct
nlmsghdr *n)
n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE
: TCA_ACT_REPLACE);
if (err == 0) {
if (tp_created) {
- RCU_INIT_POINTER(tp->next, rtnl_dereference(*back));
+ rcu_assign_pointer(tp->next, rtnl_dereference(*back));
rcu_assign_pointer(*back, tp);
+ printk(KERN_ERR "DEBUGG:SK thread-%d[cpu-%d] add/change
filter by: %pf tp=%p tp->next=%p\n", current->pid, current->on_cpu,
tp->ops->get, tp, tp->next);
}
tfilter_notify(net, skb, n, tp, fh, RTM_NEWTFILTER, false);
} else {
if (tp_created)
- tcf_destroy(tp, true);
+ tcf_destroy(tp);
}
errout:
