Re: Soft lockup in tc_classify

[Cong Wang <xiyou.wangcong@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

Hello,

On Mon, Dec 19, 2016 at 8:39 AM, Shahar Klein <shahark@mellanox.com> wrote:
>
>
> On 12/13/2016 12:51 AM, Cong Wang wrote:
>>
>> On Mon, Dec 12, 2016 at 1:18 PM, Or Gerlitz <gerlitz.or@gmail.com> wrote:
>>>
>>> On Mon, Dec 12, 2016 at 3:28 PM, Daniel Borkmann <daniel@iogearbox.net>
>>> wrote:
>>>
>>>> Note that there's still the RCU fix missing for the deletion race that
>>>> Cong will still send out, but you say that the only thing you do is to
>>>> add a single rule, but no other operation in involved during that test?
>>>
>>>
>>> What's missing to have the deletion race fixed? making a patch or
>>> testing to a patch which was sent?
>>
>>
>> If you think it would help for this problem, here is my patch rebased
>> on the latest net-next.
>>
>> Again, I don't see how it could help this case yet, especially I don't
>> see how we could have a loop in this singly linked list.
>>
>
> I've applied cong's patch and hit a different lockup(full log attached):


Are you sure this is really different? For me, it is still inside the loop
in tc_classify(), with only a slightly different offset.


>
> Daniel suggested I'll add a print:
>                 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) {
>
> and I couldn't see this print in the output.....

Hmm, that is odd, if this never prints, then my patch should not make any
difference.

There are still two other cases where we could change tp->next, so do you
mind to add two more printk's for debugging?

Attached is the delta patch.

Thanks!

[-- Attachment #2: tc-filter-debug.diff --]
[-- Type: text/plain, Size: 880 bytes --]

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index f9179e0..45bfe9f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -317,6 +317,8 @@ 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);
 
+			printk(KERN_ERR "DEBUGG:SK delete filter by: %pf\n", tp->ops->get);
+
 			RCU_INIT_POINTER(*back, next);
 
 			tfilter_notify(net, skb, n, tp, fh,
@@ -370,6 +372,7 @@ 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) {
+			printk(KERN_ERR "DEBUGG:SK add/change filter by: %pf\n", tp->ops->change);
 			RCU_INIT_POINTER(tp->next, rtnl_dereference(*back));
 			rcu_assign_pointer(*back, tp);
 		}