Re: Making bpf MPSAFE (was Re: struct ifnet and ifaddr handling ...)

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

On Sat, Aug 9, 2014 at 8:26 PM, Darren Reed <darrenr%netbsd.org@localhost> wrote:
> On 7/08/2014 7:49 PM, Ryota Ozaki wrote:
>> Hi,
>>
>> I thought I need more experience of pserialize
>> (and lock primitives) to tackle ifnet work.
>> So I suspended the work and now I am trying
>> another easier task, bpf.
>>
>> http://www.netbsd.org/~ozaki-r/mpsafe-bpf.diff
>>
>
> This code has race conditions built into it where locks are
> dropped for others to be acquiredwith no knowledge stored
> anywhere that a critical section of code is being executed:

Right. So I use a global lock and a reference counting mechanism
in the new patch(*) as I said in the mail I just sent.

(*) http://www.netbsd.org/~ozaki-r/mpsafe-bpf-v2.diff

I also modified such codes to not depend on that a target
object isn't changed after releasing a lock by, for example,
moving a pointer reference of the object to inside a critical
section.

>
>
> +        mutex_exit(d->bd_lock);
> +        KERNEL_LOCK(1, NULL);
>
> What you need here is a perimeter mechanism that doesn't
> hold any locks over the inner section of code.

I may misunderstand what you mean though, KERNEL_LOCK (splnet)
is intended to replicate lock conditions for non-MP-safe
functions outside (ifpromisc in this case). Once we make
entire networking code MP-safe, we can get rid of KERNEL_LOCK.

>
> This needs to be fixed:
>
>         error = tsleep(d, PRINET|PCATCH, "bpf", d->bd_rtout);
>
> - it should actually be a cv_timedwait_sig() on bd_lock.

Sure. I changed to use it.

Thanks,
  ozaki-r

>
> Darren
>