Re: ath12k: handling of HE and EHT capabilities

[Alexander Wilhelm <alexander.wilhelm@westermo.com>]

On Thu, Mar 12, 2026 at 10:37:46AM +0100, Johannes Berg wrote:
> Hi,
> > For example, I use the `iw` tool to display the capabilities and their
> > descriptions. The code for that has the following function prototypes:
> > 
> >     * void print_ht_capability(__u16 cap);
> >     * void print_vht_info(__u32 capa, const __u8 *mcs);
> >     * static void __print_he_capa(const __u16 *mac_cap,
> >                                   const __u16 *phy_cap,
> >                                   const __u16 *mcs_set, size_t mcs_len,
> >                                   const __u8 *ppet, int ppet_len,
> >                                   bool indent);
> >     * static void __print_eht_capa(int band,
> >                                    const __u8 *mac_cap,
> >                                    const __u32 *phy_cap,
> >                                    const __u8 *mcs_set, size_t mcs_len,
> >                                    const __u8 *ppet, size_t ppet_len,
> >                                    const __u16 *he_phy_cap,
> >                                    bool indent);
> 
> This is perhaps a bit unfortunate, but note that the HE and EHT __u16
> and __u32 here are really little endian pointers, and the functions do
> byte-order conversion.

I don’t see this in the function. For example, the MAC capabilities are a
`u16 *` in CPU endianness, which is simply memcpy’d from the parsed
`NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC`. Later, they are treated as `u16 *`,
as shown in the following code:

    printf("%s\t\tHE MAC Capabilities (0x", pre);
    for (i = 0; i < 3; i++)
        printf("%04x", mac_cap[i]);
    printf("):\n");

Here is the result on little‑ vs. big‑endian platforms:

    Little endian:
    HE MAC Capabilities (0x081a010d030f):

    Big endian:
    HE MAC Capabilities (0x0b00189a4010):

For the PHY capabilities, they are also a `u16 *`, but they are treated as
a `u8 *`. However, later in the description printing, `PRINT_HE_CAP` does
not take endianness into account. This prints the correct hex values, but
the interpretation/description is wrong:

    Little endian:
    HE PHY Capabilities: (0x1c604c89ffdb839c110c00):
        HE40/HE80/5GHz
        HE160/5GHz
        HE160/HE80+80/5GHz
        LDPC Coding in Payload
        HE SU PPDU with 1x HE-LTF and 0.8us GI
        STBC Tx <= 80MHz
        STBC Rx <= 80MHz
        Full Bandwidth UL MU-MIMO
        DCM Max Constellation: 1
        DCM Max Constellation Rx: 1
        SU Beamformer
        SU Beamformee
        MU Beamformer
        Beamformee STS <= 80Mhz: 7
        Beamformee STS > 80Mhz: 7
        Sounding Dimensions <= 80Mhz: 3
        Sounding Dimensions > 80Mhz: 3
        Ng = 16 SU Feedback
        Ng = 16 MU Feedback
        Codebook Size SU Feedback
        Codebook Size MU Feedback
        PPE Threshold Present
        HE SU PPDU & HE PPDU 4x HE-LTF 0.8us GI
        Max NC: 3
        STBC Rx > 80MHz
        HE ER SU PPDU 4x HE-LTF 0.8us GI
        HE ER SU PPDU 1x HE-LTF 0.8us GI
        TX 1024-QAM
        RX 1024-QAM

    Big endian:
    HE PHY Capabilities: (0x1c604c89ffda839c110c00):
        Punctured Preamble RX: 12
        HE SU PPDU with 1x HE-LTF and 0.8us GI
        Doppler Rx
        Full Bandwidth UL MU-MIMO
        DCM Max Constellation: 3
        DCM Max NSS Tx: 1
        DCM Max Constellation Rx: 3
        DCM Max NSS Rx: 1
        Rx HE MU PPDU from Non-AP STA
        SU Beamformer
        SU Beamformee
        Beamformee STS <= 80Mhz: 2
        Beamformee STS > 80Mhz: 4
        Sounding Dimensions <= 80Mhz: 3
        Ng = 16 MU Feedback
        Codebook Size MU Feedback
        Triggered MU Beamforming Feedback
        Triggered CQI Feedback
        Partial Bandwidth DL MU-MIMO
        PPE Threshold Present
        SRP-based SR
        Max NC: 2
        20MHz in 160/80+80MHz HE PPDU
        80MHz in 160/80+80MHz HE PPDU
        HE ER SU PPDU 1x HE-LTF 0.8us GI
        DCM Max BW: 2

> >     struct ieee80211_sta_ht_cap {
> >         u16 cap; /* use IEEE80211_HT_CAP_ */
> >         bool ht_supported;
> >         u8 ampdu_factor;
> >         u8 ampdu_density;
> >         struct ieee80211_mcs_info mcs;
> >     };
> > 
> >     struct ieee80211_sta_vht_cap {
> >         bool vht_supported;
> >         u32 cap; /* use IEEE80211_VHT_CAP_ */
> >         struct ieee80211_vht_mcs_info vht_mcs;
> >     };
> > 
> > The structs for HT and VHT use `u16` and `u32` data types for the `cap`
> > variable, matching what `iw` does. That part is consistent.
> 
> Careful. There are different structs used in different places, notably
> HT/VHT and HE/EHT differ.
> 
> For HT and VHT, look at the start of nl80211_send_band_rateinfo(), which
> sends themas individual attributes, defined in enum nl80211_band_attr,
> and the values that are u16 (NL80211_BAND_ATTR_HT_CAPA) or u32
> (NL80211_BAND_ATTR_VHT_CAPA) are in host byte order, though both are
> actually documented misleadingly ("as in [V]HT information IE" is just
> all around wrong.)
> 
> For HE/EHT, you have it in nl80211_send_iftype_data() since it's per
> interface type, and all the individual values are just as they appear in
> the spec, regardless of their size.

Okay, I think I understand so far. I had also initially wondered why HE/EHT
capabilities were treated separately from the HT/VHT capabilities.

> Note that spec is generally in little endian, but sometimes has strange
> field lengths like MAC capabilities being 6 bytes in HE:
> 
> >     struct ieee80211_he_cap_elem {
> >         u8 mac_cap_info[6];
> >         u8 phy_cap_info[11];
> >     } __packed;
> > 
> >     struct ieee80211_he_6ghz_capa {
> >         /* uses IEEE80211_HE_6GHZ_CAP_* below */
> >         __le16 capa; }
> >     __packed;
> > 
> > However, for HE the types differ from the `iw` implementation. Here, `u8`
> > arrays are used instead of `u16` for MAC and PHY capabilities. The 6 GHz
> > capabilities use `u16`, which is also different.
> 
> That doesn't really matter, they're just a set of 6 or 11 bytes, and
> e.g. the HE MAC capabilities are treated by the kernel as a set of 6
> bytes, but by iw as a set of 3 __le16, which results in the same
> interpretation, or at least should.

Sure. I agree with you, it makes no difference whether I use `u8[6]` or
`__le16[3]`, as long as I use `memcpy` and don’t perform any CPU‑endian
swapping at this point.

> >     struct ieee80211_eht_cap_elem_fixed {
> >         u8 mac_cap_info[2];
> >         u8 phy_cap_info[9];
> >     } __packed;
> > 
> > For EHT, `u8` arrays are also used for both MAC and PHY caps, instead of
> > `u32` for the PHY caps as in the `iw` implementation.
> 
> Same thing here.
> 
> > The current `ath12k` implementation always uses `u32` values, which does
> > not work on big‑endian platforms:
> 
> Yeah, that seems problematic and not really fitting for something that's
> 6, 11, 2 or 9 bytes long?
> 
> > I want to address and fix this issue. However, I cannot apply the “never
> > break the userspace” rule here, as it seems, it is already broken.
> 
> I don't think it's broken, why do you say so?

Well, if `ath12k` uses `u32` in CPU‑native order, that’s a bug, and I can’t
get `ieee80211_hw` registered. If I use `__le32` in little-endian order
instead, I end up with incorrect capabilities and mismatched descriptions
shown by the `iw` tool (but I can get the driver running). So neither
approach seems to be a 100% solution at first glance. Did I misinterpret
the rule?

> What's (clearly) broken is how ath12k puts the data into the HE/EHT
> structs that the kernel expects, but per your dmesg:
> 
> >     ath12k_pci 0001:01:00.0: ieee80211 registration failed: -22
> >     ath12k_pci 0001:01:00.0: failed register the radio with mac80211: -22
> 
> it seems that even mac80211 doesn't like the capabilities, so the byte
> order issue already exists there.
> 
> It seems to me the issue is that ath12k_band_cap is in u32, converted,
> but then memcpy()d.

The `ath12k` driver uses `u32` arrays in CPU‑native order for this, so the
swap is effectively happening. Later, in `ieee80211_register_hw`, the
values are compared at the bit level, and that’s where it fails. I
understand that technically `__le32` could be used in `ath12k`, meaning no
swap, but since `u8` arrays are used in `cfg80211`, that might actually be
the better approach. I just wanted to provide a solution that is clean and
acceptable. The `iw` tool still needs to be updated accordingly.


Best regards
Alexander Wilhelm