Re: CVS commit: src/sys/net

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

On Mon, Feb 15, 2016 at 8:06 PM, Roy Marples <roy%marples.name@localhost> wrote:
> On 15/02/2016 10:32, Ryota Ozaki wrote:
>> On Mon, Feb 15, 2016 at 6:35 PM, Roy Marples <roy%marples.name@localhost> wrote:
>>> On 15/02/2016 08:08, Ryota Ozaki wrote:
>>>> Module Name:  src
>>>> Committed By: ozaki-r
>>>> Date:         Mon Feb 15 08:08:04 UTC 2016
>>>>
>>>> Modified Files:
>>>>       src/sys/net: if.c if.h
>>>>
>>>> Log Message:
>>>> Run if_link_state_change in softint
>>>>
>>>> if_link_state_change can execute the network stack that is expected to
>>>> not run in hardware interrupt (at least now), however network drivers
>>>> may call it in hardware interrupt. Avoid that by introducing a new
>>>> softint for if_link_state_change.
>>>
>>> I don't know anything about softints, so this could be a silly question,
>>> but this change looks racey to me. Is there a guarantee anywhere that
>>> if_link_state_change() won't be called again before
>>> if_link_state_change_si() has been called?
>>
>> Not guaranteed. Hmm, you're right. There is a race condition;
>> ifp->if_link_state can be overwritten before calling
>> if_link_state_change_si and it does matter if ifp->if_link_state
>> has been back to ifp->if_old_link_state. (If ifp->if_link_state
>> is different from ifp->if_old_link_state, it doesn't matter
>> because it's just that the later state wins.)
>>
>> I think we can fix by returning from if_link_state_change_si
>> if ifp->if_link_state == ifp->if_old_link_state. Is it correct?
>
> Then we're not doing what we should because the protocol actions may not
> fire.

Oh, I see. We shouldn't drop any events of link state changes.

>
> Can we not extend softint_schedule to take some user data? That way,
> each call (which I'm assuming would be sequential) can carry the link
> state change in the user data which can then be applied to the interface.

I sometimes want that feature but I don't think changing the API
is a good idea.

  ozaki-r