mbox series

[0/3] iw: scan: ie parsing restructure

Message ID 20240930181145.1043048-1-dylan.eskew@candelatech.com
Headers show
Series iw: scan: ie parsing restructure | expand

Message

Dylan Eskew Sept. 30, 2024, 6:11 p.m. UTC
To prevent double parsing the ie buffer for ies which require
information from other ies, utilize pointer aliasing to simplify
the referencing of other ies.

Dylan Eskew (3):
  iw: scan: add enum for ie ids
  iw: scan: change parsing from in-place to cached
  iw: scan: replace passed ie buffer with alias struct

 ieee80211.h |  82 +++++++++++++
 scan.c      | 326 +++++++++++++++++++++++++++++++---------------------
 2 files changed, 277 insertions(+), 131 deletions(-)

Comments

Johannes Berg Nov. 7, 2024, 2:15 p.m. UTC | #1
On Mon, 2024-09-30 at 11:11 -0700, Dylan Eskew wrote:
> Formerly, ie ids were hardcoded. This change will improve
> readability of logic which may explicitly reference an ie id.

I've applied this, with some changes.

johannes
Johannes Berg Nov. 7, 2024, 2:22 p.m. UTC | #2
On Mon, 2024-09-30 at 11:11 -0700, Dylan Eskew wrote:
> Prevent needing to reparse the ie buffer by passing
> the ieee80211_elems struct containing the alias ie
> pointers.
> 

I think I see why you'd want the previous and this patch, and it's OK
for certain elements (such as the VHT capability or HE capability where
you'd need it), but in general it can't work this way.

We could either make the parsing even more generic to be able to deal
with elements occurring multiple times, or, perhaps more plausibly, have
a context with only what we need (VHT capability seen here, HE
capability for EHT, ...) filled by a pre-parsing step, and then pass
that context around.

In practice for HE/EHT we could also just pass a context around and say
it's an error if e.g. the EHT capabilities element is in the list
_before_ the HE capabilities element, since by spec that's not supposed
to happen. So we could just have the HE capabilities parsing fill the
context with the necessary information about HE (and set a validity
flag) and then check that it was already found when we get to EHT later.

However, that doesn't work for the case with capabilities/VHT ... where
the parsing of the capabilities element is actually referencing the VHT
element _later_ in the frame.

johannes
Dylan Eskew Nov. 7, 2024, 2:43 p.m. UTC | #3
Thanks for the feedback. I didn't necessarily realize that elements
might appear multiple times in a frame, but that information paired
with your implementation advice gives me plenty to figure something
better.

-- Dylan

On 11/7/24 6:22 AM, Johannes Berg wrote:
> On Mon, 2024-09-30 at 11:11 -0700, Dylan Eskew wrote:
>> Prevent needing to reparse the ie buffer by passing
>> the ieee80211_elems struct containing the alias ie
>> pointers.
>>
> I think I see why you'd want the previous and this patch, and it's OK
> for certain elements (such as the VHT capability or HE capability where
> you'd need it), but in general it can't work this way.
>
> We could either make the parsing even more generic to be able to deal
> with elements occurring multiple times, or, perhaps more plausibly, have
> a context with only what we need (VHT capability seen here, HE
> capability for EHT, ...) filled by a pre-parsing step, and then pass
> that context around.
>
> In practice for HE/EHT we could also just pass a context around and say
> it's an error if e.g. the EHT capabilities element is in the list
> _before_ the HE capabilities element, since by spec that's not supposed
> to happen. So we could just have the HE capabilities parsing fill the
> context with the necessary information about HE (and set a validity
> flag) and then check that it was already found when we get to EHT later.
>
> However, that doesn't work for the case with capabilities/VHT ... where
> the parsing of the capabilities element is actually referencing the VHT
> element _later_ in the frame.
>
> johannes