Re: [net-next PATCH v2 02/15] net: rcu-ify tcf_proto

[David Miller <davem@davemloft.net>]

From: John Fastabend <john.fastabend@gmail.com>
Date: Mon, 01 Sep 2014 18:39:13 -0700

> On 08/24/2014 10:31 PM, David Miller wrote:
>> From: John Fastabend <john.fastabend@gmail.com>
>> Date: Sun, 24 Aug 2014 17:48:31 -0700
>>
>>> @@ -722,8 +724,9 @@ static void sfq_free(void *addr)
>>>   static void sfq_destroy(struct Qdisc *sch)
>>>   {
>>>   	struct sfq_sched_data *q = qdisc_priv(sch);
>>> +	struct tcf_proto *fl = rtnl_dereference(q->filter_list);
>>>
>>> -	tcf_destroy_chain(&q->filter_list);
>>> +	tcf_destroy_chain(&fl);
> 
> Sorry for the delayed reply...
> 
>>
>> This will cause tcf_destroy_chain() to set the local variable
>> 'fl' to NULL rather than q->filter_list.
>>
>> I don't see how this can be correct at all.
> 
> Right now (without these patches) nothing sets q->filter_list
> to NULL and we only call this when the qdisc is being destroyed.

Yes, it does set it to NULL, by virtue of how the loop iterates.

It iterates by setting the "*fl" to tp->next until that evaluates to
NULL.  Thereby setting *fl to NULL.

So the old code would set ->filter_list to NULL, your code will not.