| Message ID | 20240328072916.1164195-1-quic_periyasa@quicinc.com |
|---|---|
| Headers | show |
| Series | wifi: Add multi physical hardware iface combination support | expand |
On 28.03.24 08:29, Karthikeyan Periyasamy wrote: > The prerequisite for MLO support in cfg80211/mac80211 is that all the links > participating in MLO must be from the same wiphy/ieee80211_hw. To meet this > expectation, some drivers may need to group multiple discrete hardware each > acting as a link in MLO under single wiphy. So it required to have some > sort of mapping while describing interface combination capabilities for > each of the underlying physical hardware. This patch set tries to add an > infrastructure to advertise underlying hw specific capabilities like > channel and interface combinations. > > Some of the todos > > - More than one concurrent monitor mode support each operating on > different channels under one ieee80211_hw > - Mechanism for each underlying radio specific configurations like > txpower, channel, etc. > > RFC series Link: https://lore.kernel.org/linux-wireless/20220920100518.19705-1-quic_vthiagar@quicinc.com/ FYI, I made a replacement for the wiphy radio hardware reporting parts of this series with a different design: https://patchwork.kernel.org/project/linux-wireless/list/?series=855042 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 - 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. - Validation happens in mac80211 instead of cfg80211, because that removes a lot of complexity The radio id is tracked per chanctx and only one chanctx per radio is allowed. I think if we ever get a non-mac80211 driver that needs multi-radio support, it should just do its own validation, since that's likely going to be less complex than having cfg80211 do it in the first place. - Felix
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. > - 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. > - Validation happens in mac80211 instead of cfg80211, because that > removes a lot of complexity Sure, that's an internal thing. I don't really _like_ that too much, but I also don't like the approach of building a huge list here. Perhaps a reasonable compromise would be for mac80211 to pass some 'iterate' and 'getinfo' callbacks or something to a validation function, instead of having to pre-build. Then the iteration can be in mac80211, but the validation can be in mac80211, and IMHO that makes the separation and how validation happens also easier to understand. > The radio id is tracked per chanctx and only one chanctx per radio is > allowed. I may be misunderstanding this, but as phrased this seems completely wrong? We absolutely support two channel contexts on a single radio today, with e.g. a regular BSS connection and a P2P-client interface. So not sure what you mean here, but I think it needs to be captured by the driver what it actually supports here, and that's basically interface combinations today for a single radio. johannes
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. :)
The prerequisite for MLO support in cfg80211/mac80211 is that all the links participating in MLO must be from the same wiphy/ieee80211_hw. To meet this expectation, some drivers may need to group multiple discrete hardware each acting as a link in MLO under single wiphy. So it required to have some sort of mapping while describing interface combination capabilities for each of the underlying physical hardware. This patch set tries to add an infrastructure to advertise underlying hw specific capabilities like channel and interface combinations. Some of the todos - More than one concurrent monitor mode support each operating on different channels under one ieee80211_hw - Mechanism for each underlying radio specific configurations like txpower, channel, etc. RFC series Link: https://lore.kernel.org/linux-wireless/20220920100518.19705-1-quic_vthiagar@quicinc.com/ Harshitha Prem (1): wifi: ath12k: Advertise multi hardware iface combination Karthikeyan Periyasamy (1): wifi: ath12k: Introduce iface combination cleanup helper Vasanthakumar Thiagarajan (11): wifi: cfg80211: Add provision to advertise multiple radio in one wiphy wifi: nl80211: send underlying multi-hardware channel capabilities to user space wifi: cfg80211: Refactor the interface combination limit check wifi: cfg80211/mac80211: extend iface comb advertisement for multi-hardware dev wifi: nl80211: Refactor the interface combination limit check wifi: nl80211: send iface combination to user space in multi-hardware wiphy wifi: cfg80211/mac80211: Refactor iface comb iterate callback for multi-hardware dev wifi: cfg80211: Refactor the iface combination iteration helper function wifi: cfg80211: Add multi-hardware iface combination support wifi: mac80211: expose channel context helper function wifi: mac80211: Add multi-hardware support in the iface comb helper drivers/net/wireless/ath/ath12k/mac.c | 147 +++++++++++- include/net/cfg80211.h | 175 +++++++++++++- include/uapi/linux/nl80211.h | 78 ++++++- net/mac80211/chan.c | 35 ++- net/mac80211/ieee80211_i.h | 5 +- net/mac80211/main.c | 46 ++++ net/mac80211/util.c | 196 ++++++++++++++-- net/wireless/core.c | 294 +++++++++++++++++++---- net/wireless/nl80211.c | 154 ++++++++++-- net/wireless/util.c | 321 ++++++++++++++++++++++---- 10 files changed, 1318 insertions(+), 133 deletions(-) base-commit: d69aef8084cc72df7b0f2583096d9b037c647ec8