| Message ID | db7d83ea6d97e118a14029727e9e18d6e47b753d.1717696995.git-series.nbd@nbd.name |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cfg80211/mac80211: support defining multiple radios per wiphy | expand |
On Thu, 2024-06-06 at 20:07 +0200, Felix Fietkau 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. This is of course the motivation now, but I do wonder if this wouldn't potentially also apply to a single device that's full dual-band capable in some way? But doesn't really matter now. But the thing is that it would let userspace differentiate between what we mostly have today in a single device (multiple channels can be used, but you have to go to powersave etc.), vs. a fully concurrent device. IOW, it feels like this could be used to advertise fully concurrent capabilities? > + * struct wiphy_radio - This structure describes a physical radio belonging > + * to a wiphy. It is used to describe concurrent-channel capabilities of the > + * phy. Only one channel can be active on the radio described by struct > + * wiphy_radio. that's a bit long for the 'short description' :P maybe just say "struct wiphy_radio - physical radio of a wiphy" and move the full description down. > + * > + * @radio: radios belonging to this wiphy > + * @n_radio: number of radios Somewhere - either here or above - we should probably say that it's assumed you only have a single radio (with the properties covered by the interface combinations in the wiphy itself) if this isn't given at all. (Which is what we assume today, more or less.) > +++ b/include/uapi/linux/nl80211.h > @@ -3401,6 +3401,8 @@ enum nl80211_attrs { > > NL80211_ATTR_ASSOC_SPP_AMSDU, > > + NL80211_ATTR_RADIOS, missing docs > +/** > + * enum nl80211_wiphy_radio_attrs - wiphy radio attributes > + * > + * @__NL80211_WIPHY_RADIO_ATTR_INVALID: Invalid maybe if this is WIPHY_RADIO also call it NL80211_ATTR_WIPHY_RADIOS above? > + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges > + * supported by this radio. Do we really want this complexity? We only have a single range now, do we expect that to change? Non-contiguous ranges, where a hole in the middle is supported by another radio? Not sure I see the value vs. just having min/max freq directly here? > + freqs = nla_nest_start_noflag(msg, NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES); Please don't add new _noflag code. > + nl_combis = nla_nest_start_noflag(msg, > + NL80211_WIPHY_RADIO_ATTR_INTERFACE_COMBINATIONS); same here (and yes maybe userspace wants to unify the parsing of this with the existing interface combinations attribute and pass the attribute ID or something, but then it can fix the nested flag too.) johannes
On 07.06.24 11:24, Johannes Berg wrote: > On Thu, 2024-06-06 at 20:07 +0200, Felix Fietkau 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. > > This is of course the motivation now, but I do wonder if this wouldn't > potentially also apply to a single device that's full dual-band capable > in some way? But doesn't really matter now. > > But the thing is that it would let userspace differentiate between what > we mostly have today in a single device (multiple channels can be used, > but you have to go to powersave etc.), vs. a fully concurrent device. > > IOW, it feels like this could be used to advertise fully concurrent > capabilities? Yes, that's my intention with the patches as well. I will update the description. >> + * struct wiphy_radio - This structure describes a physical radio belonging >> + * to a wiphy. It is used to describe concurrent-channel capabilities of the >> + * phy. Only one channel can be active on the radio described by struct >> + * wiphy_radio. > > that's a bit long for the 'short description' :P > > maybe just say "struct wiphy_radio - physical radio of a wiphy" and move > the full description down. Sure >> + * >> + * @radio: radios belonging to this wiphy >> + * @n_radio: number of radios > > Somewhere - either here or above - we should probably say that it's > assumed you only have a single radio (with the properties covered by the > interface combinations in the wiphy itself) if this isn't given at all. > > (Which is what we assume today, more or less.) > >> +++ b/include/uapi/linux/nl80211.h >> @@ -3401,6 +3401,8 @@ enum nl80211_attrs { >> >> NL80211_ATTR_ASSOC_SPP_AMSDU, >> >> + NL80211_ATTR_RADIOS, > > missing docs > >> +/** >> + * enum nl80211_wiphy_radio_attrs - wiphy radio attributes >> + * >> + * @__NL80211_WIPHY_RADIO_ATTR_INVALID: Invalid > > maybe if this is WIPHY_RADIO also call it NL80211_ATTR_WIPHY_RADIOS > above? Will do >> + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges >> + * supported by this radio. > > Do we really want this complexity? We only have a single range now, do > we expect that to change? Non-contiguous ranges, where a hole in the > middle is supported by another radio? > > Not sure I see the value vs. just having min/max freq directly here? I think there's hardware where one of the radios can be dual-band (non-concurrent). >> + freqs = nla_nest_start_noflag(msg, NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES); > > Please don't add new _noflag code. > >> + nl_combis = nla_nest_start_noflag(msg, >> + NL80211_WIPHY_RADIO_ATTR_INTERFACE_COMBINATIONS); > > same here > > (and yes maybe userspace wants to unify the parsing of this with the > existing interface combinations attribute and pass the attribute ID or > something, but then it can fix the nested flag too.) Will remove _noflag. - Felix
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 5da9bb0ac6a4..27355e08ae52 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -5403,6 +5403,38 @@ struct wiphy_iftype_akm_suites { int n_akm_suites; }; +/** + * struct wiphy_radio_freq_range - wiphy frequency range + * @start_freq: start range edge frequency (kHz) + * @end_freq: end range edge frequency (kHz) + */ +struct wiphy_radio_freq_range { + u32 start_freq; + u32 end_freq; +}; + + +/** + * struct wiphy_radio - This structure describes a physical radio belonging + * to a wiphy. It is used to describe concurrent-channel capabilities of the + * phy. Only one channel can be active on the radio described by struct + * wiphy_radio. + * + * @freq_range: frequency range that the radio can operate on. + * @n_freq_range: number of elements in @freq_range + * + * @iface_combinations: Valid interface combinations array, should not + * list single interface types. + * @n_iface_combinations: number of entries in @iface_combinations array. + */ +struct wiphy_radio { + const struct wiphy_radio_freq_range *freq_range; + int n_freq_range; + + const struct ieee80211_iface_combination *iface_combinations; + int n_iface_combinations; +}; + #define CFG80211_HW_TIMESTAMP_ALL_PEERS 0xffff /** @@ -5621,6 +5653,9 @@ struct wiphy_iftype_akm_suites { * A value of %CFG80211_HW_TIMESTAMP_ALL_PEERS indicates the driver * supports enabling HW timestamping for all peers (i.e. no need to * specify a mac address). + * + * @radio: radios belonging to this wiphy + * @n_radio: number of radios */ struct wiphy { struct mutex mtx; @@ -5771,6 +5806,9 @@ struct wiphy { u16 hw_timestamp_max_peers; + const struct wiphy_radio *radio; + int n_radio; + char priv[] __aligned(NETDEV_ALIGN); }; diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index f917bc6c9b6f..784bf7501d97 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -3401,6 +3401,8 @@ enum nl80211_attrs { NL80211_ATTR_ASSOC_SPP_AMSDU, + NL80211_ATTR_RADIOS, + /* add attributes here, update the policy in nl80211.c */ __NL80211_ATTR_AFTER_LAST, @@ -7999,4 +8001,50 @@ enum nl80211_ap_settings_flags { NL80211_AP_SETTINGS_SA_QUERY_OFFLOAD_SUPPORT = 1 << 1, }; +/** + * enum nl80211_wiphy_radio_attrs - wiphy radio attributes + * + * @__NL80211_WIPHY_RADIO_ATTR_INVALID: Invalid + * + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges + * supported by this radio. + * @NL80211_WIPHY_RADIO_ATTR_INTERFACE_COMBINATIONS: Nested attribute listing + * the supported interface combinations for this radio. In each nested item, + * it contains attributes defined in &enum nl80211_if_combination_attrs. + * + * @__NL80211_WIPHY_RADIO_ATTR_LAST: Internal + * @NL80211_WIPHY_RADIO_ATTR_MAX: Highest attribute + */ +enum nl80211_wiphy_radio_attrs { + __NL80211_WIPHY_RADIO_ATTR_INVALID, + + NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES, + NL80211_WIPHY_RADIO_ATTR_INTERFACE_COMBINATIONS, + + /* keep last */ + __NL80211_WIPHY_RADIO_ATTR_LAST, + NL80211_WIPHY_RADIO_ATTR_MAX = __NL80211_WIPHY_RADIO_ATTR_LAST - 1, +}; + +/** + * enum nl80211_wiphy_radio_freq_range - wiphy radio frequency range + * + * @__NL80211_WIPHY_RADIO_FREQ_ATTR_INVALID: Invalid + * + * @NL80211_WIPHY_RADIO_FREQ_ATTR_START: Frequency range start + * @NL80211_WIPHY_RADIO_FREQ_ATTR_END: Frequency range end + * + * @__NL80211_WIPHY_RADIO_FREQ_ATTR_LAST: Internal + * @NL80211_WIPHY_RADIO_FREQ_ATTR_MAX: Highest attribute + */ +enum nl80211_wiphy_radio_freq_range { + __NL80211_WIPHY_RADIO_FREQ_ATTR_INVALID, + + NL80211_WIPHY_RADIO_FREQ_ATTR_START, + NL80211_WIPHY_RADIO_FREQ_ATTR_END, + + __NL80211_WIPHY_RADIO_FREQ_ATTR_LAST, + NL80211_WIPHY_RADIO_FREQ_ATTR_MAX = __NL80211_WIPHY_RADIO_FREQ_ATTR_LAST - 1, +}; + #endif /* __LINUX_NL80211_H */ diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 7b0ba0fff082..a8e3a08e908d 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -2399,6 +2399,79 @@ static int nl80211_put_mbssid_support(struct wiphy *wiphy, struct sk_buff *msg) return -ENOBUFS; } +static int nl80211_put_radio(struct wiphy *wiphy, struct sk_buff *msg, int idx) +{ + const struct wiphy_radio *r = &wiphy->radio[idx]; + struct nlattr *radio, *freqs, *freq, *nl_combis; + int i; + + radio = nla_nest_start(msg, idx); + if (!radio) + return -ENOBUFS; + + freqs = nla_nest_start_noflag(msg, NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES); + if (!freqs) + goto nla_put_failure; + + for (i = 0; i < r->n_freq_range; i++) { + const struct wiphy_radio_freq_range *range = &r->freq_range[i]; + int ret; + + freq = nla_nest_start(msg, i); + ret = nla_put_u32(msg, NL80211_WIPHY_RADIO_FREQ_ATTR_START, + range->start_freq) || + nla_put_u32(msg, NL80211_WIPHY_RADIO_FREQ_ATTR_END, + range->end_freq); + nla_nest_end(msg, freq); + + if (ret) + goto nla_put_failure; + } + + nla_nest_end(msg, freqs); + + nl_combis = nla_nest_start_noflag(msg, + NL80211_WIPHY_RADIO_ATTR_INTERFACE_COMBINATIONS); + if (!nl_combis) + goto nla_put_failure; + + for (i = 0; i < r->n_iface_combinations; i++) + if (nl80211_put_ifcomb_data(msg, true, i + 1, + &r->iface_combinations[i])) + goto nla_put_failure; + + nla_nest_end(msg, nl_combis); + nla_nest_end(msg, radio); + return 0; + +nla_put_failure: + return -ENOBUFS; +} + +static int nl80211_put_radios(struct wiphy *wiphy, struct sk_buff *msg) +{ + struct nlattr *radios; + int i; + + if (!wiphy->n_radio) + return 0; + + radios = nla_nest_start(msg, NL80211_ATTR_RADIOS); + if (!radios) + return -ENOBUFS; + + for (i = 0; i < wiphy->n_radio; i++) + if (nl80211_put_radio(wiphy, msg, i)) + goto fail; + + nla_nest_end(msg, radios); + return 0; + +fail: + nla_nest_cancel(msg, radios); + return -ENOBUFS; +} + struct nl80211_dump_wiphy_state { s64 filter_wiphy; long start; @@ -3008,6 +3081,12 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, rdev->wiphy.hw_timestamp_max_peers)) goto nla_put_failure; + state->split_start++; + break; + case 17: + if (nl80211_put_radios(&rdev->wiphy, msg)) + goto nla_put_failure; + /* done */ state->split_start = 0; break;
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. With this change, supported frequencies and interface combinations of each individual radio are reported to user space. This allows user space to figure out the limitations of what combination of channels can be used concurrently. Signed-off-by: Felix Fietkau <nbd@nbd.name> --- include/net/cfg80211.h | 38 ++++++++++++++++++- include/uapi/linux/nl80211.h | 48 ++++++++++++++++++++++- net/wireless/nl80211.c | 79 +++++++++++++++++++++++++++++++++++++- 3 files changed, 165 insertions(+)