From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 98093105A583 for ; Thu, 12 Mar 2026 11:05:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:MIME-Version: Content-Transfer-Encoding:Content-Type:References:In-Reply-To:Date:Cc:To:From :Subject:Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=ZMGToIvcX5txTRmJV0EqdHQwkIhD6yRYOwJqrPBV2SA=; b=UIpWLDYPSVbHOfG8/xFxm6v4JT Pahp9rhjhAniTmScomyElk8PHQUHxgCIYQRR1qNr3+IL2vdTs8/LwvxiQrOH6az65lNNWlEP9bUmI bkkmu8+dcs7osOaoREMcOQq2AkFqgIx6Tl5YTvvwFNi36pdy4vBZOC3r4mqNYuuojgQwXxfWCjHVW JkiXB8DwDHkAxxasxpm63AhYcEkm/JFXRSuGHT5KDD6MGfND0wcth+ZKI3mj0AW9WOXoRpQBqqfc/ mMGRTwVxV19KzlaFZM0P67JVIPTnDOFp2NI5FyxzIrdSSJhdQbWvfFeolLoIqhHNsawmdMs3v19hA MqeA55jg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1w0drF-0000000DuZS-1WjU; Thu, 12 Mar 2026 11:05:37 +0000 Received: from s3.sipsolutions.net ([2a01:4f8:242:246e::2] helo=sipsolutions.net) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1w0dr5-0000000DuYV-1Cnq for ath12k@lists.infradead.org; Thu, 12 Mar 2026 11:05:33 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sipsolutions.net; s=mail; h=MIME-Version:Content-Transfer-Encoding: Content-Type:References:In-Reply-To:Date:Cc:To:From:Subject:Message-ID:Sender :Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-To: Resent-Cc:Resent-Message-ID; bh=ZMGToIvcX5txTRmJV0EqdHQwkIhD6yRYOwJqrPBV2SA=; t=1773313527; x=1774523127; b=C0Ft6EbNhIHUktU4izxtPwIe0Br+5URBvncg7QzmkClKekJ bOWinrRrAxaX40AF3y2Nw1lVxMIzsMvaDF4ztbY2MKzZS0Q6ctPtD9uG5tTTNlyAXA1xNEvXmgdww oFL2/qGz+J0pnxTohhJkwW3/LCQK2VG2xaxlijVOHEhmtHgUvxU75IIrRkKlbBvh3nBHmBnwYzpF/ ed7qcNx+jFijJMuErxeB0tK7H88cVrGzjpdmLjYc5QifOWYi+3EQjUONgQ3JoaC7OTiLnkCqoOLPa ARR0lV9tHmxzi+xESjOHp3vWSN6Ql4tSQ8ApYXQJhDhtzv1ts1ZL2WvFHEieniCQ==; Received: by sipsolutions.net with esmtpsa (TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_256_GCM:256) (Exim 4.98.2) (envelope-from ) id 1w0dr1-0000000FoB4-1Y3Y; Thu, 12 Mar 2026 12:05:23 +0100 Message-ID: <40b4b959b2ea5afd55381e6ae2d0c1908456734c.camel@sipsolutions.net> Subject: Re: ath12k: handling of HE and EHT capabilities From: Johannes Berg To: Alexander Wilhelm Cc: Jeff Johnson , ath12k@lists.infradead.org, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org Date: Thu, 12 Mar 2026 12:05:22 +0100 In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.58.3 (3.58.3-1.fc43) MIME-Version: 1.0 X-malware-bazaar: not-scanned X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260312_040527_774864_DF1F1BFB X-CRM114-Status: GOOD ( 34.80 ) X-BeenThere: ath12k@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "ath12k" Errors-To: ath12k-bounces+ath12k=archiver.kernel.org@lists.infradead.org On Thu, 2026-03-12 at 11:53 +0100, Alexander Wilhelm wrote: > 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 thei= r > > > descriptions. The code for that has the following function prototypes= : > > >=20 > > > * 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_le= n, > > > 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_le= n, > > > const __u8 *ppet, size_t ppet_len, > > > const __u16 *he_phy_cap, > > > bool indent); > >=20 > > 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. >=20 > I don=E2=80=99t see this in the function. For example, the MAC capabiliti= es are a > `u16 *` in CPU endianness, which is simply memcpy=E2=80=99d from the pars= ed > `NL80211_BAND_IFTYPE_ATTR_HE_CAP_MAC`. Later, they are treated as `u16 *`= , > as shown in the following code: >=20 > printf("%s\t\tHE MAC Capabilities (0x", pre); > for (i =3D 0; i < 3; i++) > printf("%04x", mac_cap[i]); > printf("):\n"); >=20 > Here is the result on little=E2=80=91 vs. big=E2=80=91endian platforms: >=20 > Little endian: > HE MAC Capabilities (0x081a010d030f): >=20 > Big endian: > HE MAC Capabilities (0x0b00189a4010): Oh, OK, so _that_ print is definitely wrong in iw. But the individual prints are converted: #define PRINT_HE_CAP(_var, _idx, _bit, _str) \ do { \ if (le16toh(_var[_idx]) & BIT(_bit)) \ printf("%s\t\t\t" _str "\n", pre); \ } while (0) > For the PHY capabilities, they are also a `u16 *`, but they are treated a= s > a `u8 *`. However, later in the description printing, `PRINT_HE_CAP` does > not take endianness into account. PRINT_HE_CAP *does* convert, there's le16toh() there. Same in PRINT_HE_CAP_MASK. It should convert, because it's from a u8[6] kernel API that just carries the values as they are in the spec. > > > I want to address and fix this issue. However, I cannot apply the =E2= =80=9Cnever > > > break the userspace=E2=80=9D rule here, as it seems, it is already br= oken. > >=20 > > I don't think it's broken, why do you say so? Well, I see now that I missed the printf("%s\t\tHE MAC Capabilities (0x", pre); for (i =3D 0; i < 3; i++) printf("%04x", le16toh(mac_cap[i])); and printf("%s\t\tEHT MAC Capabilities (0x", pre); for (i =3D 0; i < 2; i++) printf("%02x", mac_cap[i]); parts, those are definitely broken in iw on big endian platforms. We should fix those in iw. The actual individual prints seem fine though. > Well, if `ath12k` uses `u32` in CPU=E2=80=91native order, that=E2=80=99s = a bug, and I can=E2=80=99t > 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? The *descriptions* should be fine I think? Just the first line with the hex would be messed up. > > What's (clearly) broken is how ath12k puts the data into the HE/EHT > > structs that the kernel expects, but per your dmesg: > >=20 > > > ath12k_pci 0001:01:00.0: ieee80211 registration failed: -22 > > > ath12k_pci 0001:01:00.0: failed register the radio with mac80211:= -22 > >=20 > > it seems that even mac80211 doesn't like the capabilities, so the byte > > order issue already exists there. > >=20 > > It seems to me the issue is that ath12k_band_cap is in u32, converted, > > but then memcpy()d. >=20 > The `ath12k` driver uses `u32` arrays in CPU=E2=80=91native order for thi= s, so the > swap is effectively happening. Yeah but the swap is wrong, since HE/EHT capabilities are just byte arrays in spec byte order in cfg80211/nl80211. > Later, in `ieee80211_register_hw`, the > values are compared at the bit level, and that=E2=80=99s where it fails. = I > understand that technically `__le32` could be used in `ath12k`, meaning n= o > swap, but since `u8` arrays are used in `cfg80211`, that might actually b= e > the better approach. Sure, could use u8 in ath12k too, dunno, up to the maintainer. At least if it was __le32 you could still memcpy() from it since no swap happened, and wouldn't change the code structure that much. johannes