On Tue, Sep 26, 2017 at 6:20 AM, Eric Dumazet <[email protected]> wrote:
> On Mon, Sep 25, 2017 at 10:52 PM, Wei Wang <[email protected]> wrote:
>> On Mon, Sep 25, 2017 at 7:23 PM, Eric Dumazet <[email protected]> wrote:
>>> On Mon, Sep 25, 2017 at 7:07 PM, Martin KaFai Lau <[email protected]> wrote:
>>>
>>>> I am probably still missing something.
>>>>
>>>> Considering the del operation should be under the writer lock,
>>>> if rt->rt6i_node should be NULL (for rt that has already been
>>>> removed from fib6), why this WARN_ON() is triggered?
>>>>
>>>> An example may help.
>>>>
>>>
>>> Look at the stack trace, you'll find the answers...
>>>
>>> ip6_link_failure() -> ip6_del_rt()
>>>
>>> Note that rt might have been deleted from the _tree_ already.
>>
>> Had a brief talk with Martin.
>> He has a valid point.
>> The current WARN_ON() code is as follows:
>> #if RT6_DEBUG >= 2
>>        if (rt->dst.obsolete > 0) {
>>                WARN_ON(fn);
>>                return -ENOENT;
>>        }
>> #endif
>>
>> The WARN_ON() only triggers when fn is not NULL. (I missed it before.)
>> In theory, fib6_del() calls fib6_del_route() which should set
>> rt->rt6i_node to NULL and rt->dst.obsolete to DST_OBSOLETE_DEAD within
>> the same write_lock session.
>> If those 2 values are inconsistent, it indicates something is wrong.
>> Will need more time to root cause the issue.
>>
>> Please ignore this patch. Sorry about the confusion.
>
> Oh well, for some reason I was seeing WARN_ON(1)  here, since this is
> a construct I often add in my tests ...

Just an update on this issue:
This WARNING issue should already be fixed by commit
7483cea79957312e9f8e9cf760a1bc5d6c507113:
Author: Ido Schimmel <[email protected]>
Date:   Thu Aug 3 13:28:22 2017 +0200

    ipv6: fib: Unlink replaced routes from their nodes

    When a route is deleted its node pointer is set to NULL to indicate it's
    no longer linked to its node. Do the same for routes that are replaced.

    This will later allow us to test if a route is still in the FIB by
    checking its node pointer instead of its reference count.

    Signed-off-by: Ido Schimmel <[email protected]>
    Signed-off-by: Jiri Pirko <[email protected]>
    Signed-off-by: David S. Miller <[email protected]>

So no further action is needed on this.

Thanks.
Wei

Reply via email to