Re: [PATCH 00/13] wifi: Add multi physical hardware iface combination support

Aditya Kumar Singh
2024-05-27T06:58:28Z

On 5/23/24 22:11, Johannes Berg wrote:
> On Wed, 2024-05-22 at 16:55 +0200, Felix Fietkau wrote:
>>
>> The key differences are:
>> - Only band bitmask and optionally frequency ranges are provided, so no
>> per-radio channel list
>> This is easier to track and vastly reduces the amount of data sent to
>> user space in the wiphy dump
>
> That makes sense, though in your RFC I'd probably remove the band bitmap
> thing, and make the frequency range not be optional. Perhaps in the
> kernel it could be filled in by cfg80211 via a band enum (taking
> lowest/highest frequency in the band's channels that are there), but I
> don't know if I'd want to have to check with this all optional
> throughout the kernel and the userspace advertising API.
>

Agree on that band bitmap thing.

>> - No integration with ifcomb. I don't really see the need for that one
>> at this point. It can easily be added later if it's actually needed.
>
> I mean, sure? But I think that's being lazy, I think everyone else
> thinks it's actually needed. I just got a question about interface
> combinations being broken on iwlwifi because we advertise AP interface
> type in a combination with two channels, which can't be right. I'm
> fixing that, but actually it _would_ be good to know for hardware that
> actually does physically have the capability to operate on two channels,
> and then have the bands etc.
>
> So I do think (some) integration with interface combinations is needed.

Yes! At least for radar detection, some changes are required.
Grouping 5 GHz radio with any other radio, we will need it or else
current interface combination check will fail to register the single
wiphy hardware.

We have a check like this in wiphy_verify_combinations() -

/* DFS only works on one channel. */
if (WARN_ON(c->radar_detect_widths &&
(c->num_different_channels > 1)))
return -EINVAL;

And if the proposal is to keep c->num_different_channels advertised as 1
only from the driver then in [RFC 2/2], this change -

+ if (ieee80211_num_chanctx(local) >= ieee80211_max_num_channels(local))
+ return false;

will never allow to create a channel ctx in any of the other radios
except the one which is brought up first right?

ieee80211_max_num_channels() uses the interface combination advertised
value from driver, so that will be 1 and hence if you bring up on 5 GHz
after 2 GHz, that will lead to 2 >= 1 and hence if condition will be
true and it will return false. Then 5 GHz bring up will fail. So not so
clear on this approach.

Considering above points, feels like under any situation without making
interface combination changes, simply it can not be done. Some or other
issue might pop up later once we try to enable all features in MLO as
well which currently exist in non-MLO case. :)