Hooked to devctl_notify, this allows consumers to received events
by subscribing to a system over a generic netlink protocol
- rG FreeBSD src repository
No Test Coverage
- Build Status
Buildable 48583 Build 45469: arc lint + arc unit
note that it requires: https://reviews.freebsd.org/D37573
Also independant from this, it show inconsistencies in the devctl_notify "systems"
Userland example of usage will follow later
Please see some minor comments inline.
That's an internal header, which shouldn't be used here.
I'd suggest using NL_LOG() for consistency in all reporting. It's easy to setup:
#define DEBUG_MOD_NAME nl_sysevent #define DEBUG_MAX_LEVEL LOG_DEBUG3 #include <netlink/netlink_debug.h> _DECLARE_DEBUG(LOG_DEBUG3);
Also, given it's not a hot path, I'd probably skip predictors to improve readability.
Maybe it's worth splitting the function in two, with the first part getting the proper group, setting vnet & calling the second?
nlmsg_type should be equal to the family id allocated by the module (e.g. ctrl_family_id).
I'd still allocate some CMD (NLS_NEWEVENT? ) to make it non-zero.
I'd probably consider renaming CTRL_ prefix to something like NLEVT_ or similar (or even remove it),
Thank you for addressing the comments!
Added a couple of nitpicks that can be addressed at the commit time.
Q: what do you think of providing these notifications to each VNET? This can be addressed later, especially given the support should be implemented in the core netlink code. For now, I'm more curious about the concept.
I'd suggest either removing it or converting it to NL_LOG().
Nit: the message will be prefixed by [nl_sysevent] sysevent_send_to_group , so probably worth skipping 'nlsysevent: '.
Also: each memory allocation failure will be reported by the underlying nlmsg_refill_buffer(). Currently, it's not verbose, but this will change.
Nit: no prefix needed, '\n' is redundant
Nit: maybe NLSE_ATTR_MAX ?
I don't know, so far I don't see any message going through devctl_notify which should be vnet aware, but let's see how this get used in the future
Oh, sorry, let me be a bit more specific.
I'm talking about the case when a user wants to put some notification consumer to jail (for example, as a part of jail-service-by-default) with VNET. Currently, such a service won't be able to receive any notifications as it's not in the default VNET anymore.
So the questions is that if this behaviour is desired or not. If not, how should we proceed (have per-jail tunable to allow events passing etc) ?
I don't think putting some notification per jail (if not in the default vnet) is a desired behaviour.
Hard no on this table.
If you want to make this an 'enum', I'd do it old-school X11 way with
that said, I'd be OK on the table if there were some plan to have a way to register all the names and call devctl_notify with a token from such a registration rather than just a bare string.