On Sun, Sep 13, 2020 at 12:23:50PM +0200, Martin Pieuchot wrote:
> Without doing another audit but with the fact that pseudo-device are
> generally run by a thread holding the NET_LOCK() I'd assume it's ok.
Thanks, I'll put it in as its an improvement and comment only (safe);
rest can happen in-tree.
> First we should remove the KRENEL_LOCK() from around pppoeintr(). The
> NET_LOCK() is not an issue right now.
There seem to be some low hanging fruits in pppoe(4) I started with and
am running with already, but I agree that KERNEL_LOCK() is our high
priority problem.
> > Index: if_pppoe.c
> > ===================================================================
> > RCS file: /cvs/src/sys/net/if_pppoe.c,v
> > retrieving revision 1.70
> > diff -u -p -r1.70 if_pppoe.c
> > --- if_pppoe.c 28 Jul 2020 09:52:32 -0000 1.70
> > +++ if_pppoe.c 20 Aug 2020 15:27:09 -0000
> > @@ -114,27 +115,34 @@ struct pppoetag {
> > #define PPPOE_DISC_MAXPADI 4 /* retry PADI four times
> > (quickly) */
> > #define PPPOE_DISC_MAXPADR 2 /* retry PADR twice */
> >
> > +/*
> > + * Locks used to protect struct members and global data
> > + * I immutable after creation
> > + * K kernel lock
>
> I wouldn't bother repeating 'I' and 'K' if they are not used in the
> description below.
`I' is used and `K' completes the list, but omitting unused locks also
indicates which ones are (not) used up front which is nice.
I'll remove `K', I guess.
