Re: [PATCH 4/5] wifi: brcmfmac: Support bss_info up to v112

From: Arend van Spriel
Date: Tue Nov 07 2023 - 06:51:26 EST


On 11/7/2023 12:11 PM, Hector Martin wrote:
On 20/10/2023 18.59, Arend van Spriel wrote:
On 10/19/2023 3:42 AM, Daniel Berlin wrote:
From: Hector Martin <marcan@xxxxxxxxx>

The structures are compatible and just add fields, so we can just treat
it as always v112. If we start using new fields, that will have to be
gated on the version.

Seems EHT is creeping in here.

Having doubts about compatibility statement (see below)...

Signed-off-by: Hector Martin <marcan@xxxxxxxxx>
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 5 ++-
.../broadcom/brcm80211/brcmfmac/fwil_types.h | 37 +++++++++++++++++--
2 files changed, 36 insertions(+), 6 deletions(-)


[...]

@@ -323,28 +324,56 @@ struct brcmf_bss_info_le {
__le16 capability; /* Capability information */
u8 SSID_len;
u8 SSID[32];
+ u8 bcnflags; /* additional flags w.r.t. beacon */

Ehm. Coming back to your statement "structures are compatible and just
add fields". How are they compatible? You now treat v109 struct as v112
so fields below are shifted because of bcnflags. So you read invalid
information. This does not fly or I am missing something here.

bcmflags was previously an implied padding byte. If you actually check
the offsets of the subsequent fields, you'll see they haven't changed.
In fact this was added at some point in the past and just missing here,
and is a general case of "padding bytes were not explicitly specified"
which is arguably an anti-pattern and should never have been the case.

Yeah. Let's not argue ;-) I did miss something here and leave it with that. What about the EHT stuff? I would prefer to keep it out unless full EHT support is added.

Regards,
Arend

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature