mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v3] wifi: nl80211: document sta-info nested attribute
@ 2026-08-16 10:12 Hilal Rahmatullah
  2026-08-17 15:47 ` Jakub Kicinski
  2026-08-18 17:07 ` Jakub Kicinski
  0 siblings, 2 replies; 7+ messages in thread
From: Hilal Rahmatullah @ 2026-08-16 10:12 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Donald Hunter, open list:NETWORKING [GENERAL],
	open list
  Cc: Hilal Rahmatullah

Add NL80211_ATTR_STA_INFO nested attributes definition, from
the previous binary type.

Signed-off-by: Hilal Rahmatullah <hilalrahmatullah11@gmail.com>
---
changes in v2:
- Fix nested attribute name
- Add double quote for numeric enum member
changes in v3:
- add missing he-gi attribute on rate-info-attrs
- fix sta-info attributes order

---
 Documentation/netlink/specs/nl80211.yaml | 351 ++++++++++++++++++++++-
 1 file changed, 350 insertions(+), 1 deletion(-)

diff --git a/Documentation/netlink/specs/nl80211.yaml b/Documentation/netlink/specs/nl80211.yaml
index 802097128bda..4ad391db4a76 100644
--- a/Documentation/netlink/specs/nl80211.yaml
+++ b/Documentation/netlink/specs/nl80211.yaml
@@ -211,6 +211,70 @@ definitions:
       - ht20
       - ht40minus
       - ht40plus
+  -
+    name: eht-gi
+    type: enum
+    entries:
+      - 0-8
+      - 1-6
+      - 3-2
+  -
+    name: eht-ru-alloc
+    type: enum
+    entries:
+      - "26"
+      - "52"
+      - 52p26
+      - "106"
+      - 106p26
+      - "242"
+      - "484"
+      - 484p242
+      - "996"
+      - 996p484
+      - 996p484p242
+      - 2x996
+      - 2x996p484
+      - 3x996
+      - 3x996p484
+      - 4x996
+  -
+    name: he-gi
+    type: enum
+    entries:
+      - 0-8
+      - 1-6
+      - 3-2
+  -
+    name: he-ru-alloc
+    type: enum
+    entries:
+      - "26"
+      - "52"
+      - "106"
+      - "242"
+      - "484"
+      - "996"
+      - 2x996
+  -
+    name: mesh-power-mode
+    type: enum
+    entries:
+      - unknown
+      - active
+      - light-sleep
+      - deep-sleep
+  -
+    name: plink-state
+    type: enum
+    entries:
+      - listen
+      - opn-snt
+      - opn-rcvd
+      - cnf-rcvd
+      - estab
+      - holding
+      - blocked
   -
     name: sta-flag-update
     type: struct
@@ -297,7 +361,8 @@ attribute-sets:
         type: u32
       -
         name: sta-info
-        type: binary  # TODO: nest
+        type: nest
+        nested-attributes: sta-info-attrs
       -
         name: wiphy-bands
         type: nest
@@ -1601,6 +1666,102 @@ attribute-sets:
         name: nan
         type: nest
         nested-attributes: frame-type-attrs
+  -
+    name: rate-info-attrs
+    enum-name: nl80211-rate-info
+    name-prefix: nl80211-rate-info-
+    attributes:
+      -
+        name: bitrate
+        type: u16
+      -
+        name: mcs
+        type: u8
+      -
+        name: 40-mhz-width
+        type: flag
+      -
+        name: short-gi
+        type: flag
+      -
+        name: bitrate32
+        type: u32
+      -
+        name: vht-mcs
+        type: u8
+      -
+        name: vht-nss
+        type: u8
+      -
+        name: 80-mhz-width
+        type: flag
+      -
+        name: 80p80-mhz-width
+        type: unused
+      -
+        name: 160-mhz-width
+        type: flag
+      -
+        name: 10-mhz-width
+        type: flag
+      -
+        name: 5-mhz-width
+        type: flag
+      -
+        name: he-mcs
+        type: u8
+      -
+        name: he-nss
+        type: u8
+      -
+        name: he-gi
+        type: u8
+        enum: he-gi
+      -
+        name: he-dcm
+        type: u8
+      -
+        name: he-ru-alloc
+        type: u8
+        enum: he-ru-alloc
+      -
+        name: 320-mhz-width
+        type: flag
+      -
+        name: eht-mcs
+        type: u8
+      -
+        name: eht-nss
+        type: u8
+      -
+        name: eht-gi
+        type: u8
+        enum: eht-gi
+      -
+        name: eht-ru-alloc
+        type: u8
+        enum: eht-ru-alloc
+      -
+        name: s1g-mcs
+        type: u8
+      -
+        name: s1g-nss
+        type: u8
+      -
+        name: 1-mhz-width
+        type: flag
+      -
+        name: 2-mhz-width
+        type: flag
+      -
+        name: 4-mhz-width
+        type: flag
+      -
+        name: 8-mhz-width
+        type: flag
+      -
+        name: 16-mhz-width
+        type: flag
   -
     name: sar-attributes
     enum-name: nl80211-sar-attrs
@@ -1631,6 +1792,170 @@ attribute-sets:
       -
         name: end-freq
         type: u32
+  -
+    name: sta-bss-param-attrs
+    enum-name: nl80211-sta-bss-param
+    name-prefix: nl80211-sta-bss-param-
+    attributes:
+      -
+        name: cts-prot
+        type: flag
+      -
+        name: short-preamble
+        type: flag
+      -
+        name: short-slot-time
+        type: flag
+      -
+        name: dtim-period
+        type: u8
+      -
+        name: beacon-interval
+        type: u16
+  -
+    name: sta-info-attrs
+    enum-name: nl80211-sta-info
+    name-prefix: nl80211-sta-info-
+    attributes:
+      -
+        name: inactive-time
+        type: u32
+      -
+        name: rx-bytes
+        type: u32
+      -
+        name: tx-bytes
+        type: u32
+      -
+        name: llid
+        type: u16
+      -
+        name: plid
+        type: u16
+      -
+        name: plink-state
+        type: u8
+        enum: plink-state
+      -
+        name: signal
+        type: u8
+      -
+        name: tx-bitrate
+        type: nest
+        nested-attributes: rate-info-attrs
+      -
+        name: rx-packets
+        type: u32
+      -
+        name: tx-packets
+        type: u32
+      -
+        name: tx-retries
+        type: u32
+      -
+        name: tx-failed
+        type: u32
+      -
+        name: signal-avg
+        type: u8
+      -
+        name: rx-bitrate
+        type: nest
+        nested-attributes: rate-info-attrs
+      -
+        name: bss-param
+        type: nest
+        nested-attributes: sta-bss-param-attrs
+      -
+        name: connected-time
+        type: u32
+      -
+        name: sta-flags
+        type: binary
+        struct: sta-flag-update
+      -
+        name: beacon-loss
+        type: u32
+      -
+        name: t-offset
+        type: s64
+      -
+        name: local-pm
+        type: u32
+        enum: mesh-power-mode
+      -
+        name: peer-pm
+        type: u32
+        enum: mesh-power-mode
+      -
+        name: nonpeer-pm
+        type: u32
+        enum: mesh-power-mode
+      -
+        name: rx-bytes64
+        type: u64
+      -
+        name: tx-bytes64
+        type: u64
+      -
+        name: chain-signal
+        type: u8
+      -
+        name: chain-signal-avg
+        type: u8
+      -
+        name: expected-throughput
+        type: u32
+      -
+        name: rx-drop-misc
+        type: u64
+      -
+        name: beacon-rx
+        type: u64
+      -
+        name: beacon-signal-avg
+        type: u8
+      -
+        name: tid-stats
+        type: indexed-array
+        sub-type: nest
+        nested-attributes: tid-stats-attrs
+      -
+        name: rx-duration
+        type: u64
+      -
+        name: pad
+        type: binary
+      -
+        name: ack-signal
+        type: u8
+      -
+        name: ack-signal-avg
+        type: u8
+      -
+        name: rx-mpdus
+        type: u32
+      -
+        name: fcs-error-count
+        type: u32
+      -
+        name: connected-to-gate
+        type: u8
+      -
+        name: tx-duration
+        type: u64
+      -
+        name: airtime-weight
+        type: u16
+      -
+        name: airtime-link-metric
+        type: u32
+      -
+        name: assoc-at-boottime
+        type: u64
+      -
+        name: connected-to-as
+        type: u8
   -
     name: supported-iftypes
     enum-name: nl80211-iftype
@@ -1672,6 +1997,30 @@ attribute-sets:
       -
         name: nan
         type: flag
+  -
+    name: tid-stats-attrs
+    enum-name: nl80211-tid-stats
+    name-prefix: nl80211-tid-stats-
+    attributes:
+      -
+        name: rx-msdu
+        type: u64
+      -
+        name: tx-msdu
+        type: u64
+      -
+        name: tx-msdu-retries
+        type: u64
+      -
+        name: tx-msdu-failed
+        type: u64
+      -
+        name: pad
+        type: binary
+      -
+        name: txq-stats
+        type: nest
+        nested-attributes: txq-stats-attrs
   -
     name: txq-stats-attrs
     name-prefix: nl80211-txq-stats-
-- 
2.53.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3] wifi: nl80211: document sta-info nested attribute
  2026-08-16 10:12 [PATCH net v3] wifi: nl80211: document sta-info nested attribute Hilal Rahmatullah
@ 2026-08-17 15:47 ` Jakub Kicinski
  2026-09-05  7:46   ` Johannes Berg
  2026-08-18 17:07 ` Jakub Kicinski
  1 sibling, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2026-08-17 15:47 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Hilal Rahmatullah, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Donald Hunter, open list:NETWORKING [GENERAL],
	open list

On Sun, 16 Aug 2026 17:12:15 +0700 Hilal Rahmatullah wrote:
> Add NL80211_ATTR_STA_INFO nested attributes definition, from
> the previous binary type.
> 
> Signed-off-by: Hilal Rahmatullah <hilalrahmatullah11@gmail.com>
> ---
> changes in v2:
> - Fix nested attribute name
> - Add double quote for numeric enum member
> changes in v3:
> - add missing he-gi attribute on rate-info-attrs
> - fix sta-info attributes order
> 
> ---
>  Documentation/netlink/specs/nl80211.yaml | 351 ++++++++++++++++++++++-

Hi Johannes! Should we add this file to wireless in MAINTAINERS?
Or do you prefer for us to handle the YAML stuff directly?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3] wifi: nl80211: document sta-info nested attribute
  2026-08-16 10:12 [PATCH net v3] wifi: nl80211: document sta-info nested attribute Hilal Rahmatullah
  2026-08-17 15:47 ` Jakub Kicinski
@ 2026-08-18 17:07 ` Jakub Kicinski
  1 sibling, 0 replies; 7+ messages in thread
From: Jakub Kicinski @ 2026-08-18 17:07 UTC (permalink / raw)
  To: hilalrahmatullah11
  Cc: Jakub Kicinski, davem, edumazet, pabeni, horms, donald.hunter,
	netdev, linux-kernel

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
wifi: nl80211: document sta-info nested attribute

This converts NL80211_ATTR_STA_INFO in the nl80211 YAML spec from an
opaque binary blob into a real nest, and adds the sta-info-attrs,
rate-info-attrs, sta-bss-param-attrs and tid-stats-attrs sets plus the
supporting enum definitions.  A few questions below about how some of the
new entries line up with what net/wireless/nl80211.c actually puts on the
wire.

> diff --git a/Documentation/netlink/specs/nl80211.yaml b/Documentation/netlink/specs/nl80211.yaml
> index 802097128bdae..4ad391db4a76d 100644
> --- a/Documentation/netlink/specs/nl80211.yaml
> +++ b/Documentation/netlink/specs/nl80211.yaml

[ ... ]

> @@ -297,7 +361,8 @@ attribute-sets:
>          type: u32
>        -
>          name: sta-info
> -        type: binary  # TODO: nest
> +        type: nest
> +        nested-attributes: sta-info-attrs
>        -
>          name: wiphy-bands
>          type: nest

[ ... ]

> @@ -1601,6 +1666,102 @@ attribute-sets:
>          name: nan
>          type: nest
>          nested-attributes: frame-type-attrs
> +  -
> +    name: rate-info-attrs
> +    enum-name: nl80211-rate-info
> +    name-prefix: nl80211-rate-info-
> +    attributes:

[ ... ]

> +      -
> +        name: 16-mhz-width
> +        type: flag

[Severity: Medium]
The set stops at 16-mhz-width, which gets implicit value 29 and lines up
one-for-one with the enum up to that point.  But enum nl80211_rate_info in
include/uapi/linux/nl80211.h in this same tree continues:

	NL80211_RATE_INFO_16_MHZ_WIDTH,
	NL80211_RATE_INFO_UHR_MCS,
	NL80211_RATE_INFO_UHR_ELR,
	NL80211_RATE_INFO_UHR_IM,

and nl80211_put_sta_rate() emits them inside the same nest:

	} else if (info->flags & RATE_INFO_FLAGS_UHR_MCS) {
		if (nla_put_u8(msg, NL80211_RATE_INFO_UHR_MCS, info->mcs))
	...
		if (info->flags & RATE_INFO_FLAGS_UHR_ELR &&
		    nla_put_flag(msg, NL80211_RATE_INFO_UHR_ELR))

With values 30, 31 and 32 missing from the spec, does a get-station or
dump-station on UHR capable hardware abort in the decoder?  In
tools/net/ynl/pyynl/lib/ynl.py the lookup path is:

                attr_spec = attr_space.attrs_by_val[attr.type]
            except (KeyError, UnboundLocalError) as err:
                if not self.process_unknown:
                    raise YnlException(f"Space '{space}' has no attribute "

so unless process_unknown is set the whole reply fails rather than
returning the station data.  Should uhr-mcs, uhr-elr and uhr-im be added
here?

>    -
>      name: sar-attributes
>      enum-name: nl80211-sar-attrs

[ ... ]

> @@ -1631,6 +1792,170 @@ attribute-sets:
>        -
>          name: end-freq
>          type: u32

[ ... ]

> +  -
> +    name: sta-info-attrs
> +    enum-name: nl80211-sta-info
> +    name-prefix: nl80211-sta-info-
> +    attributes:

[ ... ]

> +      -
> +        name: rx-bytes64
> +        type: u64
> +      -
> +        name: tx-bytes64
> +        type: u64
> +      -
> +        name: chain-signal
> +        type: u8
> +      -
> +        name: chain-signal-avg
> +        type: u8

[Severity: Medium]
Are these two really scalars on the wire?  nl80211_send_station() routes
both through nl80211_put_signal():

	if (sinfo->filled & BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL)) {
		if (!nl80211_put_signal(msg, sinfo->chains, sinfo->chain_signal, NL80211_STA_INFO_CHAIN_SIGNAL))

and nl80211_put_signal() builds a nest keyed by chain index:

	attr = nla_nest_start_noflag(msg, id);
	...
		if (nla_put_u8(msg, i, signal[i]))

so the payload is at least nla_total_size(1), never one byte.  The per-link
fill path does the same thing with link_sinfo->chains, and
include/uapi/linux/nl80211.h documents it as:

 * @NL80211_STA_INFO_CHAIN_SIGNAL: per-chain signal strength of last PPDU
 *	Contains a nested array of signal strength attributes (u8, dBm)

With type: u8 the decoder takes the scalar path in
tools/net/ynl/pyynl/lib/ynl.py:

    def as_scalar(self, attr_type, byte_order=None):
        format_ = self.get_format(attr_type, byte_order)
        return format_.unpack(self.raw)[0]

which has no length tolerance, so unpacking an eight byte nest as 'B'
raises struct.error and the reply decode is aborted with "Error decoding
'chain-signal' from 'sta-info-attrs'".  While NL80211_ATTR_STA_INFO was
type: binary the blob was never walked, so does making it a nest turn this
into a live failure for every station report from hardware that sets
sinfo->chains?  Would an indexed array of u8 (sub-attribute type is the
0-based chain index) describe it correctly?

> +      -
> +        name: expected-throughput
> +        type: u32
> +      -
> +        name: rx-drop-misc
> +        type: u64
> +      -
> +        name: beacon-rx
> +        type: u64
> +      -
> +        name: beacon-signal-avg
> +        type: u8
> +      -
> +        name: tid-stats
> +        type: indexed-array
> +        sub-type: nest
> +        nested-attributes: tid-stats-attrs
> +      -
> +        name: rx-duration
> +        type: u64
> +      -
> +        name: pad
> +        type: binary

[Severity: Low]
This isn't a bug, but should this (and the pad entry added to
tid-stats-attrs below) use type: pad instead of type: binary?
NL80211_STA_INFO_PAD is only ever used as the padattr argument:

	    nla_put_u64_64bit(msg, NL80211_STA_INFO_ ## attr,		\
			      sinfo->memb, NL80211_STA_INFO_PAD))

and NL80211_TID_STATS_PAD is used the same way by PUT_TIDVAL_U64.  The
tooling special-cases the dedicated type in
tools/net/ynl/pyynl/lib/ynl.py:

                if attr_spec["type"] == 'pad':
                    continue

so as binary the filler shows up as a decoded pad field and the generated
documentation claims it carries binary content.  Other in-tree specs
(tc.yaml, rt-link.yaml, ethtool.yaml, nftables.yaml) use type: pad for
these.  Note the existing NL80211_ATTR_PAD entry in this file already uses
type: binary, so this matches an existing wart in the family rather than
introducing a functional break.

> +      -
> +        name: ack-signal
> +        type: u8
> +      -
> +        name: ack-signal-avg
> +        type: u8

[Severity: Medium]
Should ack-signal-avg be s8?  The emitter uses the signed helper:

		PUT_SINFO(ACK_SIGNAL, ack_signal, u8);
		PUT_SINFO(ACK_SIGNAL_AVG, avg_ack_signal, s8);

which expands to nla_put_s8(), the per-link path does
PUT_LINK_SINFO(ACK_SIGNAL_AVG, avg_ack_signal, s8), struct station_info
declares avg_ack_signal as s8, and the uapi header says:

 * @NL80211_STA_INFO_ACK_SIGNAL_AVG: avg signal strength of ACK frames (s8, dBm)

The payload is one byte either way, so there is no parse error, just a
value reported as 214 instead of -42.  s8 is accepted by the schema
(the attr-type enum in Documentation/netlink/genetlink-legacy.yaml lists
it).

Related, and weaker: signal, signal-avg, ack-signal and beacon-signal-avg
are put with nla_put_u8() so u8 matches the emitter literally, but the
underlying station_info members are s8 dBm values, so spec-driven decoders
will print them as large positive numbers.  Is that intended here?

> +      -
> +        name: rx-mpdus
> +        type: u32

[ ... ]

> @@ -1672,6 +1997,30 @@ attribute-sets:
>        -
>          name: nan
>          type: flag
> +  -
> +    name: tid-stats-attrs
> +    enum-name: nl80211-tid-stats
> +    name-prefix: nl80211-tid-stats-
> +    attributes:

[ ... ]

> +      -
> +        name: pad
> +        type: binary
> +      -
> +        name: txq-stats
> +        type: nest
> +        nested-attributes: txq-stats-attrs
>    -
>      name: txq-stats-attrs
>      name-prefix: nl80211-txq-stats-

Cross-instance finding from sashiko-gemini (9bc2ddddefa645796d23dda3f200d5d56cc1ca9a67d79f78de96c69e6ca35525):
[Severity: High]
The newly added enums (`eht-gi`, `he-gi`, `eht-ru-alloc`, `he-ru-alloc`, `mesh-power-mode`, `plink-state`) lack the `name-prefix` property, causing YNL to emit incorrect C constants.
-- 
pw-bot: cr

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3] wifi: nl80211: document sta-info nested attribute
  2026-08-17 15:47 ` Jakub Kicinski
@ 2026-09-05  7:46   ` Johannes Berg
  2026-10-02 17:34     ` Johannes Berg
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2026-09-05  7:46 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Hilal Rahmatullah, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Donald Hunter, open list:NETWORKING [GENERAL],
	open list

On Mon, 2026-08-17 at 08:47 -0700, Jakub Kicinski wrote:
> > 
> > ---
> >  Documentation/netlink/specs/nl80211.yaml | 351 ++++++++++++++++++++++-
> 
> Hi Johannes! Should we add this file to wireless in MAINTAINERS?
> Or do you prefer for us to handle the YAML stuff directly?

Not sure, we don't really seem to be using it for all I know, and so
it's just been getting some random updates like this, I guess.

I think I tend to prefer the status quo, but I guess I could also just
apply patches to it if you prefer.

johannes

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3] wifi: nl80211: document sta-info nested attribute
  2026-09-05  7:46   ` Johannes Berg
@ 2026-10-02 17:34     ` Johannes Berg
  2026-10-02 20:25       ` Jakub Kicinski
  0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2026-10-02 17:34 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Hilal Rahmatullah, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Donald Hunter, open list:NETWORKING [GENERAL],
	open list

On Sat, 2026-09-05 at 09:46 +0200, Johannes Berg wrote:
> On Mon, 2026-08-17 at 08:47 -0700, Jakub Kicinski wrote:
> > > 
> > > ---
> > >  Documentation/netlink/specs/nl80211.yaml | 351 ++++++++++++++++++++++-
> > 
> > Hi Johannes! Should we add this file to wireless in MAINTAINERS?
> > Or do you prefer for us to handle the YAML stuff directly?
> 
> Not sure, we don't really seem to be using it for all I know, and so
> it's just been getting some random updates like this, I guess.
> 
> I think I tend to prefer the status quo, but I guess I could also just
> apply patches to it if you prefer.

So that's what I said a month ago, but because Jouni prodded me about
our inconsistent documentation today, I actually looked at converting
*all* of nl80211.h and our hand-rolled policies to be generated.

It ... takes some work, and ends up being 12k (!) lines of yaml, vs.
today's 7.2k of nl80211.h and ~1k policy in nl80211.c.

It also requires a bunch of changes to the tooling:

 - kdoc comments for attribute-set enums
 - exact-len-warn check
 - validate-fn check support and creating their prototypes
 - combining min-len/max-len for binary attributes
 - strict-start-type support
 - flag to generate the whole attrs without linking them from ops
   (otherwise we get even many many more lines of yaml)
 - NLA_NESTED/indexed arrays support
 - allow nesting the global policy into itself (yeah ... I want some of
   what we were smoking ;-) )
 - allowing inheriting different members for kernel policy side
 - drop nesting when subset overrides type ... not even sure how to
   explain this right now
 - option to define only policy, not ops
 - allow taking ops IDs from an enum, because our ops don't match the
   ynl ops too well
 - enum-max-name for definitions to adjust naming

There's a license issue - we have ISC license on nl80211.h now, but
Donald and Hilal picked GPL/syscall/BSD-3 for the yaml and of course
that carries over to the generated .h file, not sure if that's an issue
for downstream projects but might be with advertising clause? Could we
change it if I'm adding >10k new lines from nl80211.h? :)

I can post it but ... is it worth it? Not even sure it would
significantly simplify our lives, although the generated header file
with documentation is much more regular in that it always has the type,
etc. (which is what Jouni was asking about)

johannes

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3] wifi: nl80211: document sta-info nested attribute
  2026-10-02 17:34     ` Johannes Berg
@ 2026-10-02 20:25       ` Jakub Kicinski
  2026-10-02 20:30         ` Johannes Berg
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Kicinski @ 2026-10-02 20:25 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Hilal Rahmatullah, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Donald Hunter, open list:NETWORKING [GENERAL],
	open list

On Fri, 02 Oct 2026 19:34:19 +0200 Johannes Berg wrote:
> On Sat, 2026-09-05 at 09:46 +0200, Johannes Berg wrote:
> > On Mon, 2026-08-17 at 08:47 -0700, Jakub Kicinski wrote:  
>  [...]  
> > > 
> > > Hi Johannes! Should we add this file to wireless in MAINTAINERS?
> > > Or do you prefer for us to handle the YAML stuff directly?  
> > 
> > Not sure, we don't really seem to be using it for all I know, and so
> > it's just been getting some random updates like this, I guess.
> > 
> > I think I tend to prefer the status quo, but I guess I could also just
> > apply patches to it if you prefer.  
> 
> So that's what I said a month ago, but because Jouni prodded me about
> our inconsistent documentation today, I actually looked at converting
> *all* of nl80211.h and our hand-rolled policies to be generated.
> 
> It ... takes some work, and ends up being 12k (!) lines of yaml, vs.
> today's 7.2k of nl80211.h and ~1k policy in nl80211.c.
> 
> It also requires a bunch of changes to the tooling:
> 
>  - kdoc comments for attribute-set enums
>  - exact-len-warn check
>  - validate-fn check support and creating their prototypes
>  - combining min-len/max-len for binary attributes
>  - strict-start-type support
>  - flag to generate the whole attrs without linking them from ops
>    (otherwise we get even many many more lines of yaml)
>  - NLA_NESTED/indexed arrays support
>  - allow nesting the global policy into itself (yeah ... I want some of
>    what we were smoking ;-) )
>  - allowing inheriting different members for kernel policy side
>  - drop nesting when subset overrides type ... not even sure how to
>    explain this right now
>  - option to define only policy, not ops
>  - allow taking ops IDs from an enum, because our ops don't match the
>    ynl ops too well
>  - enum-max-name for definitions to adjust naming
> 
> There's a license issue - we have ISC license on nl80211.h now, but
> Donald and Hilal picked GPL/syscall/BSD-3 for the yaml and of course

That license is required for all netlink specs, FWIW
It's totally my fault if it's wrong, I just didn't want it to be
_just_ GPL since that would propagate to use space code which
we want to be permissive about. The spec is effectively a uAPI
description it should be public domain in my non-layer mind.

> that carries over to the generated .h file, not sure if that's an issue
> for downstream projects but might be with advertising clause? Could we
> change it if I'm adding >10k new lines from nl80211.h? :)
> 
> I can post it but ... is it worth it? Not even sure it would
> significantly simplify our lives, although the generated header file
> with documentation is much more regular in that it always has the type,
> etc. (which is what Jouni was asking about)

Well, I certainly don't care so my choice would be not to have to review
the code gen changes (:

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net v3] wifi: nl80211: document sta-info nested attribute
  2026-10-02 20:25       ` Jakub Kicinski
@ 2026-10-02 20:30         ` Johannes Berg
  0 siblings, 0 replies; 7+ messages in thread
From: Johannes Berg @ 2026-10-02 20:30 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Hilal Rahmatullah, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Donald Hunter, open list:NETWORKING [GENERAL],
	open list

On Fri, 2026-10-02 at 13:25 -0700, Jakub Kicinski wrote:
> > 
> > There's a license issue - we have ISC license on nl80211.h now, but
> > Donald and Hilal picked GPL/syscall/BSD-3 for the yaml and of course
> 
> That license is required for all netlink specs, FWIW
> It's totally my fault if it's wrong, I just didn't want it to be
> _just_ GPL since that would propagate to use space code which
> we want to be permissive about. The spec is effectively a uAPI
> description it should be public domain in my non-layer mind.

Oh, sure. But BSD-3 isn't (advertising clause), and people copy at least
the nl80211.h file around to other projects. And the syscall exception
doesn't really talk about distributing that code. So presumably going
from ISC to this would require people to update even their binary
distribution's docs (where it's built in) ...

> > that carries over to the generated .h file, not sure if that's an issue
> > for downstream projects but might be with advertising clause? Could we
> > change it if I'm adding >10k new lines from nl80211.h? :)
> > 
> > I can post it but ... is it worth it? Not even sure it would
> > significantly simplify our lives, although the generated header file
> > with documentation is much more regular in that it always has the type,
> > etc. (which is what Jouni was asking about)
> 
> Well, I certainly don't care so my choice would be not to have to review
> the code gen changes (:

Fair :)

I'm not really sure it gives _that_ much benefit ... I guess I'll mull
it over and maybe run it by some wifi people.

Thanks :)

johannes

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-02 20:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-16 10:12 [PATCH net v3] wifi: nl80211: document sta-info nested attribute Hilal Rahmatullah
2026-08-17 15:47 ` Jakub Kicinski
2026-09-05  7:46   ` Johannes Berg
2026-10-02 17:34     ` Johannes Berg
2026-10-02 20:25       ` Jakub Kicinski
2026-10-02 20:30         ` Johannes Berg
2026-08-18 17:07 ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®