Hi Asbjørn,

On Tue, Nov 18, 2025 at 12:08:20PM +0000, Asbjørn Sloth Tønnesen wrote:

quoted

I'll need to do my own reading, I guess, but what is going on with this
"legacy" business? Is there some newer genetlink that falls outside of
versioning?

There's a few reasons why the stricter genetlink doesn't fit:
- Less flexible with C naming (breaking UAPI).
- Doesn't allow C struct types.

diff -Naur Documentation/netlink/genetlink{,-legacy}.yaml

Oh, thanks, useful diff. So the protocol didn't change, persay, but the
non-legacy one just firms up some of the floppiness of what was done
before. Makes sense.

quoted

There's lots of control over the C output here. Why can't there also be
a top-level c-function-prefix attribute, so that patch 10/11 is
unnecessary? Stack traces for wireguard all include wg_; why pollute
this with the new "wireguard_" ones?

It could also be just "c-prefix".

Works for me.

quoted

quoted

+      dump:
+        pre: wireguard-nl-get-device-start
+        post: wireguard-nl-get-device-done

Oh, or, the wg_ prefix can be defined here (instead of wireguard_, per
my 10/11 comment above).

The key here is the missing ones, I renamed these for alignment with
doit and dumpit which can't be customized at this time.

Oh, interesting. So actually, the c-prefix thing would let you ditch
this too, and it'd be more consistent.

quoted

On the other hand, maybe that's an implementation detail and doesn't
need to be specified? Or if you think rigidity is important, we should
specify 0 in both directions and then validate it to ensure userspace
sends 0 (all userspaces currently do).

As is, the YNL-based clients are taking advantage of it not being validated.
Changing that would require adding some new YNL type properties.
See this series[1] for my earlier attempt to extend YNL in this area.

[1] https://lore.kernel.org/r/20251022182701.250897-1-ast@fiberby.net/ (local)

The modern way would be to use multi-attrs, but I don't think it's worth it
to transition, you mainly save a few bytes of overhead.

Oh, huh, interesting. In libmnl, this parameter is referred to as "type"
instead of index, so it was natural to stick 0 there. It looks like so
does Go's netlink library, but in wgctrl-go, Matt stuck index there.

So... darn. We're stuck with this being poorly defined. I guess we can
have as the text something like:

    The index/type parameter is unused on SET_DEVICE operations and is zero on GET_DEVICE operations.

WGDEVICE_A_IFINDEX
WGDEVICE_A_PEERS2: NLA_NESTED
   WGPEER_A_PUBLIC_KEY
   [..]
   WGPEER_A_ALLOWEDIPS2: NLA_NESTED
     WGALLOWEDIP_A_FAMILY
     [..]
   WGPEER_A_ALLOWEDIPS2: NLA_NESTED
     WGALLOWEDIP_A_FAMILY
     [..]
WGDEVICE_A_PEERS2: NLA_NESTED
   [..]

Def not worth it. But good to know about for future protocols.

quoted

quoted

+        While this command does accept the other ``WGDEVICE_A_*``
+        attributes, for compatibility reasons, but they are ignored
+        by this command, and should not be used in requests.

Either "While" or ", but" but not both.

However, can we actually just make this strict? No userspaces send
random attributes in a GET. Nothing should break.

I agree that nothing should break, just tried to avoid changing UAPI in the
spec commit, but by moving the split ops conversion patch, then I can eliminate
this before adding the spec.

Okay, great, let's do that.

Thanks a bunch.

Jason