Re: [PATCH 02/13] wifi: nl80211: send underlying multi-hardware channel capabilities to user space

Jakub Kicinski
2024-03-28T18:49:13Z

On Thu, 28 Mar 2024 13:01:55 +0100 Johannes Berg wrote:
> If we do that, including NL80211_MULTI_HW_ATTR_IDX for illustrative
> purposes though I think it should be removed, we'd end up with:
>
> NL80211_ATTR_MULTI_HW
> - NL80211_MULTI_HW_ATTR_IDX: 0
> - NL80211_MULTI_HW_ATTR_FREQ: 2412
> - NL80211_MULTI_HW_ATTR_FREQ: 2417
> ...
> NL80211_ATTR_MULTI_HW
> - NL80211_MULTI_HW_ATTR_IDX: 1
> - NL80211_MULTI_HW_ATTR_FREQ: 5180
> - NL80211_MULTI_HW_ATTR_FREQ: 5200
> ...
>
> which _is_ a lot more compact, and removes all the uninteresting mid-
> level indexing.
>
> So in that sense, I prefer that, but I'm truly not sure how the (hand-
> written) userspace code would deal with that.

I think the best way today would be two walks:

for_each_attr() {
switch (type):
case THE_A_ARRAY_1:
cnt1++;
break;
case THE_A_ARRAY_2:
cnt2++;
break;
}

if (cnt1)
array_1 = calloc();
cnt1 = 0; /* we'll use it as index in second loop */
if (cnt2)
array_2 = calloc();
cnt2 = 0;

for_each_attr() {
/* [ normal parsing, populating array_1[cnt1++] etc. ] */
}

Compared to "indexed array" the only practical difference I think is
the fact that all attrs are walked. I think you have to count them
either way before parsing.

I was wondering at some point whether we should require that all
multi-attr attributes are grouped together. Or add an explicit "count"
attribute. But couldn't convince myself that such extra rules will
pay off sufficiently with perf and/or ease of use...