From: Stuart Henderson Subject: Re: wg(4) logging enhancement - revised patch To: Lloyd Cc: Vitaliy Makkoveev , "tech@openbsd.org" Date: Mon, 30 Dec 2024 18:33:31 +0000 On 2024/12/30 18:10, Lloyd wrote: > On Monday, December 30th, 2024, Stuart Henderson wrote: > > > Rather than sprinkling ISSET(...IFF_DEBUG) all over the place and adding > > extra indented blocks, using something analogous to the DPRINTF macro > > would be easier to read, and probably reduce the number of times it > > overflows to another line. > > IIRC I did this because some of the messages utilized addlog to build the > messages and I thought using a macro made it more cryptic and confusing. If > concise > verbose then I can rework it, no problem. FWIW I wasn't familiar > with the code standard until Vitaliy pointed it out. > > > I don't know wg(4) particularly well, but are there any concerns > > regarding locking with things like this - i.e. 1, accessing information > > from peer->p_endpoint, and 2. building up the log entry using addlog? > > Some of the other network drivers are using addlog - ppp(4) and pf(4). In > particular the IP address printing code was taken from pf and modified. ppp makes a lot of use of NET_LOCK and KERNEL_LOCK, afaik not much relating to ppp is allowed to run in parallel on multiple CPUs. wg generally has finer grained locking, there's some NET_LOCK but not all that much, and no KERNEL_LOCK, much more of the code can run in parallel (and so access to structs which might be modified concurrently by another CPU needs to take care of locking). I don't know for sure that it will be a problem, and if it _is_ a problem then it will only manifest under certain conditions (e.g. you might see problems when doing things like adding/removing peers whilst triggering the debug related code). But when I was trying to understand why such logging wasn't _already_ being done, that came into my mind as a possible reason.