> On 23 May 2020, at 12:54, Martin Pieuchot <m...@openbsd.org> wrote: > > On 22/05/20(Fri) 13:25, Vitaliy Makkoveev wrote: >> On Fri, May 22, 2020 at 07:57:13AM +1000, David Gwynne wrote: >>> [...] >>> can you try the following diff? >>> >> >> I tested this diff and it works for me. But the problem I pointed is >> about pipex(4) locking. >> >> pipex(4) requires NET_LOCK() be grabbed not only for underlaying >> ip{,6}_output() but for itself too. But since pppac_start() has >> unpredictable behavior I suggested to make it predictable [1]. > > What needs the NET_LOCK() in their? We're talking about > pipex_ppp_output(), right? Does it really need the NET_LOCK() or > the KERNEL_LOCK() is what protects those data structures?
Yes, about pipex_ppp_output() and pipex_output(). Except ip{,6}_output() nothing requires NET_LOCK(). As David Gwynne pointed, they can be replaced by ip{,6}_send(). > >> With net/ifq.c rev 1.37 and rev 1.39 pppx_if_start() routine will be >> allways called under NET_LOCK(), but pppac_start() will not. It depends >> on packet count stored in `if_snd' (look at net/ifq.c:125). > > Ideally the *_start() routine should not require the NET_LOCK(). > Ideally the pseudo-drivers should not require the NET_LOCK(). That's > what we should aim for. NET_LOCK() is not required. It’s locked while corresponding start routines were called directly from pppx_if_output() and pppac_output(). In case of pppac_start() you can't predict NET_LOCK() status. > > In case of pipex(4) is isn't clear that the NET_LOCK() is necessary. I guess, pipex(4) was wrapped by NET_LOCK() to protect it while it’s accessed through `pr_input’. Is NET_LOCK() required for this case?