nlsysevent: don't assume that we can sleep in sysevent_send()
Needs RevisionPublic
Actions

Authored by kevans on Oct 25 2025, 4:12 AM.

Details

Reviewers

bapt
glebius
markj
melifaro

Summary

devctl_notify as written is acceptable in some non-sleepable contexts
(e.g., callouts), but nlsysevent will try to register the system as a
group when sysevent_send() is invoked. Predicate that on whether we
can sleep or not, and simply fail it if it's not a group that we
recognize.

printf may also sleep, so we can't really NL_LOG() in sysevent_send()
or sysevent_write(). Let's just render NL_LOG() harmless entirely if
we can't sleep, and drop some dtrace probes in places we would normally
log.

I have no idea if this is reasonable.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 68113
Build 64996: arc lint + arc unit

Event Timeline

kevans created this revision.Oct 25 2025, 4:12 AM

Herald added a reviewer: melifaro. · View Herald TranscriptOct 25 2025, 4:12 AM

Herald added subscribers: melifaro, imp. · View Herald Transcript

kevans requested review of this revision.Oct 25 2025, 4:12 AM

Harbormaster completed remote builds in B68113: Diff 165021.Oct 25 2025, 4:12 AM

Can you please point me to how reproduce the problem? IMHO, we should just provide a KPI to register new sysevent_groups and pre-register them before the events.

Also, I don't think you really need to care much of NL_LOG(). Many of them are really debugging left overs from when the code was prototyped. Nobody used them except original author. I never used them when working on Netlink.

This revision now requires changes to proceed.Oct 26 2025, 4:48 AM

In D53350#1218507, @glebius wrote:

Can you please point me to how reproduce the problem? IMHO, we should just provide a KPI to register new sysevent_groups and pre-register them before the events.

This one is a local patch that adds devctl notifications to net80211 for assoc/disassoc with an AP- I'd need to look around a bit to see if we have a sample already in-tree that you can look at. It works fine without nlsysevent loaded, and mostly fine with it loaded- the problem is if you load nlsysevent after you've associated, there's a chance you end up panicking when sysevent_new_group attempts to allocate it (but you can't just switch it to M_NOWAIT, because then you get hit by other bits trying to take an sx(9)).

I had thought about pre-registration, but I wans't really sold; the main utility in nlsyslink comes from getting notifications transparently from devctl_notify without requiring anything extra from the bits using it, and we have a limited number of slots (so the lazy-registration method is kind of nice). The current scheme works for 90% of users, it's just this one case that wasn't considered that devctl_notify is expected to work in today.

Also, I don't think you really need to care much of NL_LOG(). Many of them are really debugging left overs from when the code was prototyped. Nobody used them except original author. I never used them when working on Netlink.

I don't mind killing them, it was just worth neutering them in case someone tries to turn on debugging then realizes they hit a case where we can't sleep in this path.