Re: CVS commit: src/sys/netinet6

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Wed, Dec 27, 2017 at 10:40 AM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> On Wed, Dec 27, 2017 at 10:36 AM, Taylor R Campbell
> <campbell+netbsd-source-changes-d%mumble.net@localhost> wrote:
>>> Date: Wed, 27 Dec 2017 10:31:25 +0900
>>> From: Ryota Ozaki <ozaki-r%netbsd.org@localhost>
>>>
>>> On Wed, Dec 27, 2017 at 10:27 AM, Taylor R Campbell
>>> <campbell+netbsd-source-changes-d%mumble.net@localhost> wrote:
>>> > Can you add this to the collection?
>>> >
>>> > #ifdef NET_MPSAFE
>>> > #define SOFTNET_INTERLOCK_IF_NET_MPSAFE (&softnet_lock)
>>> > #else
>>> > #define SOFTNET_INTERLOCK_IF_MPSAFE     NULL
>>> > #endif
>>>
>>> Oh I meant if not NET_MPSAFE, nd6_dad_stoptimer can be called with or without
>>> softnet_lock. So the macro doesn't work.
>>
>> So push it into the callers: nd6_dad_stop, nd6_dad_duplicated?  That's
>> why I mentioned...
>>
>>> >> > Can you just pass the interlock -- softnet_lock or NULL, depending on
>>> >> > NET_MPSAFE -- as an argument to nd6_dad_stoptimer?  It looks like it
>>> >> > will trickle up into the call stack a little bit, but not all that
>>> >> > far, unless .dom_if_link_state_change and .dom_if_down ever hold
>>> >> > softnet lock.
>>
>> ...the .dom_if_link_state_change and .dom_if_down callbacks, which
>> where as far as I looked back in the call graph.
>
> There are other paths. nd6_dad_stop is called from in6_purgeaddr,
> in6_if_link_down and nd6_ioctl. nd6_dad_duplicated is called nd6_dad_timer,
> nd6_dad_ns_input and nd6_dad_na_input.

I added the below assertion in my local repository and found
arp_dad_stoptimer is the same situation as nd6_dad_stoptimer :-/

  ozaki-r


diff --git a/sys/kern/kern_timeout.c b/sys/kern/kern_timeout.c
index d5b3a519fde..9b5747f08e3 100644
--- a/sys/kern/kern_timeout.c
+++ b/sys/kern/kern_timeout.c
@@ -473,6 +473,7 @@ callout_halt(callout_t *cs, void *interlock)

        KASSERT(c->c_magic == CALLOUT_MAGIC);
        KASSERT(!cpu_intr_p());
+       KASSERT(interlock == NULL || mutex_owned(interlock));

        lock = callout_lock(c);
        relock = NULL;