Re: struct ifnet and ifaddr handling [was: Re: Making global variables of if.c MPSAFE]

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

Hi,

I'm back to the work.

Here is a new patch: http://www.netbsd.org/~ozaki-r/psz-ifnet.diff

I think the patch reflects rmind's suggestions:
- Use pserialize for IFNET_FOREACH
  - but use a lock for blockable/sleepable critical sections
- cpu_intr_p workaround for HW interrupt

Any comments?

Thanks,
  ozaki-r

On Tue, Aug 26, 2014 at 4:28 AM, Mindaugas Rasiukevicius
<rmind%netbsd.org@localhost> wrote:
> Ryota Ozaki <ozaki-r%NetBSD.org@localhost> wrote:
>> > I generally agree with Dennis that is not the way we want to take in
>> > the long-term.  The cost of read-write lock is very high.  The plan
>> > is to use passive serialisation to protect the interfaces and their
>> > addresses.  Also, the ultimate goal would also be to use a better
>> > data structure (linked lists are not really efficient) and change the
>> > way interfaces are referenced i.e. instead of referencing ifnet_t,
>> > the network stack should use a unique ID.
>>
>> I have no objection to the direction. My concern is an intermediate
>> solution.
>
> Right, but I think we can still avoid read-write lock for the intermediate
> solution.  ID based interface referencing requires a greater revamp.
>
>> > Note that the code paths
>> > looking up the interface or its address(es) should not block (if they
>> > do, the code can be rearranged).
>>
>> Some codes under sys/compat can be blocked during the iterations,
>> for example linux_getifconf at [1] that may block due to copyout.
>>
>> [1]
>> http://nxr.netbsd.org/xref/src/sys/compat/linux/common/linux_socket.c#1134
>
> Yes, but it is not performance sensitive path.  You can acquire the lock
> used on the pserialize(9) writer side.  Basically, we need to optimise the
> IP input/output paths; the rest can be heavy-locked for now.
>
>> > Also, in the long run, ifnet list
>> > should not be accessed from the hard interrupt context -- all users
>> > ought to be running in the softintr(9) context.
>>
>> The ifnet list is accessed in m_reclaim that may be called from
>> hardware interrupt context via say MCL_GET.
>
> Yes, but my point was that we should eliminate the access of ifnet list,
> as well as other network stack structures, from the hard interrupt.  This
> means converting the existing code to use softintr(9).  In the long term,
> the drivers should always trigger softintr(9) and L2 never be called from
> the hard interrupt context.  As for m_reclaim(9), we might workaround it
> with a test on cpu_intr_p() for now.
>
>> > We may need to take an intermediate solution, but I think we can
>> > already switch to pserialize(9) + reference counting on ifnet_t for
>> > the ip_input/ip_output() paths.  I need to resume my work on the
>> > routing subsystem patch-up, though.
>>
>> I think we need to get rid of blockable operations mentioned the above.
>
> Or just treat them as writers.
>
> --
> Mindaugas