Re: wm_intr may lead wm_start unexpectedly

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Wed, Jun 25, 2014 at 02:43:16PM +0900, Ryota Ozaki wrote:
> Hi,
> 
> I found a strange behavior of if_wm that
> its interrupt handler may call its if_start
> (xmit function) eventually. I don't think
> it's sane. It makes difficult to use mutex for MP.
> 
> Here is a call trace:
> wm_intr => wm_linkintr => wm_linkintr_gmii =>
> mii_pollstat => makphy_service => mii_phy_update =>
> if_link_state_change => in6_if_link_up =>
> nd6_dad_start => nd6_dad_ns_output => ... =>
> => ether_output => ... => wm_start
> 
> The interrupt handler calls mii, mii notifies
> a link state change to inet6, and inet6 tries DAD.
> This IPv6 DAD code (nd6_dad_start and
> nd6_dad_ns_output) is the main issue of my claim.
> nd6_dad_start normally sets up a callout for
> nd6_dad_ns_output, however, it may call
> nd6_dad_ns_output directly at random.
> 
> Looking at (1), in6_if_link_up picks a random delay
> (in tick) ranging from 0 to hz and passes it to
> nd6_dad_start. nd6_dad_start sets up a callout if
> the delay > 0 while calls nd6_dad_ns_output directly
> if the delay == 0 (2). This random behavior makes
> debugging difficult.
> 
> OpenBSD seems to have changed nd6_dad_start to always
> call a callout regardless of the delay (3). I think
> we should do such an approach as that. By doing so,
> we can avoid the above wm_intr => wm_start problem.
> 
> My proposal to fix the problem of nd6_dad_start is to
> allow a negative value of the delay to indicate that
> no delay is required and nd6_dad_ns_output is called
> directly. For any other value including zero,
> a callout is always used.
> 
> Any comments?

why not change in6_if_link_up() to use a delay >= 1 instead of 0 ?

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--