Re: separate L3 output KERNEL_LOCK

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi,

On 2016/06/14 23:14, Taylor R Campbell wrote:
>    Date: Tue, 14 Jun 2016 13:33:08 +0900
>    From: Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>
> 
>    The design to implement this concept is consisted of below two parts.
>        1. at L3 output processing call L2 output routine
>           - remove KERNEL_LOCK from caller(L3)
>           - wrap callee(L2) ifp->if_output with KERNEL_LOCK if necessary
>        2. at L2 output processing call device driver output routine
>           - check device driver MP-safe flags (*)
>           - if device driver has MP-safe flag, call without KERNEL_LOCK and
>             vice versa
> 
>          (*) Unfortunately, struct ifnet->if_flags is already used all bit.
>              So, I change if__pad1 to if_extflags and use it as flags.
> 
> I was going to ask `Why two different mechanisms for if_output and
> if_start -- why not use the MPSAFE flag for both?', until the answer
> occurred to me:  An ifnet has a split identity between the
> device-specific parts, whose code lives in sys/dev, and the
> protocol-specific parts, whose code lives in sys/net; and the two
> identities may be independently made MP-safe.

Oh, I forget to explain the reasons. The reason is the flag checking
codes seems racy. Therefore, I want to use KERNEL_LOCK wrapper not only
if_output but also if_start. However, there are many device drivers
much more than L2 component, so I give up to apply KERNEL_LOCK wrapper
to if_start.
# When I was implementing wm(4) TX multiqueue support, I had a hard time
# to debug the similar flag checking codes...

Practically, MPSAFE flag must be written at initialization, and then be
read only at other run time. So, the flag checking codes would not cause
race.

> That said, why not not use two flags, say IFEF_OUTPUT_MPSAFE and
> IFEF_START_MPSAFE?  I never much liked the WRAP_FOO device -- is there
> a particular reason it's better for if_output?
> 
> It seems to me that it is easier to audit changes for the flag than to
> audit changes for the wrapper: for the flag, it is only necessary to
> make sure all callers of ifp->if_start instead use if_start_lock; for
> WRAP_FOO, it's not as easy to make sure you adjusted everything.  But
> maybe I'm missing something about the motivation for WRAP_FOO here.

Fair enough. I will rewrite my code to use that two flags.


Thanks,

-- 
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>