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.