2024-05-22 12:53:08

by Felix Fietkau

[permalink] [raw]
Subject: [RFC 1/2] cfg80211: add support for advertising multiple radios belonging to a wiphy

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, the bands and supported frequencies 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 <[email protected]>
---
include/net/cfg80211.h | 34 ++++++++++++++++
include/uapi/linux/nl80211.h | 48 +++++++++++++++++++++++
net/wireless/nl80211.c | 75 ++++++++++++++++++++++++++++++++++++
3 files changed, 157 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f2ca495ac9f8..58d8375ffa11 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -5407,6 +5407,34 @@ 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.
+ *
+ * @band_mask: Mask of enum nl80211_band describing the bands that the radio
+ * can operate on.
+ * @num_freq_range: number of elements in @freq_range
+ * @freq_range: frequency range that the radio can operate on (optional)
+ */
+struct wiphy_radio {
+ u16 band_mask;
+ u16 n_freq_range;
+ const struct wiphy_radio_freq_range *freq_range;
+};
+
#define CFG80211_HW_TIMESTAMP_ALL_PEERS 0xffff

/**
@@ -5625,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;
@@ -5775,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..dfb01d673094 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_BAND_MASK: Bitmask of bands supported by this
+ * radio.
+ *
+ * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges
+ * supported by this radio.
+ *
+ * @__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_BAND_MASK,
+ NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES,
+
+ /* 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 3c0bca4238d3..c07abdf104ec 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2392,6 +2392,78 @@ 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;
+ int i;
+
+ radio = nla_nest_start(msg, idx);
+ if (!radio)
+ return -ENOBUFS;
+
+ if (nla_put_u32(msg, NL80211_WIPHY_RADIO_ATTR_BAND_MASK, r->band_mask))
+ goto fail;
+
+ if (!r->n_freq_range)
+ goto out;
+
+ freqs = nla_nest_start(msg, NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES);
+ if (!freqs)
+ goto fail;
+
+ 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 fail_freq;
+ }
+
+ nla_nest_end(msg, freqs);
+
+out:
+ nla_nest_end(msg, radio);
+ return 0;
+
+fail_freq:
+ nla_nest_cancel(msg, freqs);
+fail:
+ nla_nest_cancel(msg, radio);
+ 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;
@@ -3001,6 +3073,9 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
rdev->wiphy.hw_timestamp_max_peers))
goto nla_put_failure;

+ if (nl80211_put_radios(&rdev->wiphy, msg))
+ goto nla_put_failure;
+
/* done */
state->split_start = 0;
break;
--
2.44.0



2024-05-22 12:53:13

by Felix Fietkau

[permalink] [raw]
Subject: [RFC 2/2] mac80211: ensure that only one channel context is active per radio

With multi-radio hardware, we need to ensure that a radio is not assigned to
multiple channels at the same time.

Signed-off-by: Felix Fietkau <[email protected]>
---
include/net/mac80211.h | 2 +
net/mac80211/chan.c | 96 +++++++++++++++++++++++++++++++++++++-----
2 files changed, 87 insertions(+), 11 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2bb24aba84fd..0e9b61726067 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -250,6 +250,7 @@ struct ieee80211_chan_req {
* @min_def: the minimum channel definition currently required.
* @ap: the channel definition the AP actually is operating as,
* for use with (wider bandwidth) OFDMA
+ * @radio_idx: index of the wiphy radio used used for this channel
* @rx_chains_static: The number of RX chains that must always be
* active on the channel to receive MIMO transmissions
* @rx_chains_dynamic: The number of RX chains that must be enabled
@@ -264,6 +265,7 @@ struct ieee80211_chanctx_conf {
struct cfg80211_chan_def min_def;
struct cfg80211_chan_def ap;

+ int radio_idx;
u8 rx_chains_static, rx_chains_dynamic;

bool radar_enabled;
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 380695fdc32f..589041de99c6 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -60,11 +60,24 @@ static int ieee80211_num_chanctx(struct ieee80211_local *local)
return num;
}

-static bool ieee80211_can_create_new_chanctx(struct ieee80211_local *local)
+static bool ieee80211_can_create_new_chanctx(struct ieee80211_local *local,
+ int radio_idx)
{
+ struct ieee80211_chanctx *ctx;
+
lockdep_assert_wiphy(local->hw.wiphy);

- return ieee80211_num_chanctx(local) < ieee80211_max_num_channels(local);
+ if (ieee80211_num_chanctx(local) >= ieee80211_max_num_channels(local))
+ return false;
+
+ if (radio_idx < 0)
+ return true;
+
+ list_for_each_entry(ctx, &local->chanctx_list, list)
+ if (ctx->conf.radio_idx == radio_idx)
+ return false;
+
+ return true;
}

static struct ieee80211_chanctx *
@@ -638,7 +651,8 @@ ieee80211_chanctx_radar_required(struct ieee80211_local *local,
static struct ieee80211_chanctx *
ieee80211_alloc_chanctx(struct ieee80211_local *local,
const struct ieee80211_chan_req *chanreq,
- enum ieee80211_chanctx_mode mode)
+ enum ieee80211_chanctx_mode mode,
+ int radio_idx)
{
struct ieee80211_chanctx *ctx;

@@ -656,6 +670,7 @@ ieee80211_alloc_chanctx(struct ieee80211_local *local,
ctx->conf.rx_chains_dynamic = 1;
ctx->mode = mode;
ctx->conf.radar_enabled = false;
+ ctx->conf.radio_idx = radio_idx;
_ieee80211_recalc_chanctx_min_def(local, ctx, NULL);

return ctx;
@@ -689,14 +704,14 @@ static struct ieee80211_chanctx *
ieee80211_new_chanctx(struct ieee80211_local *local,
const struct ieee80211_chan_req *chanreq,
enum ieee80211_chanctx_mode mode,
- bool assign_on_failure)
+ bool assign_on_failure, int radio_idx)
{
struct ieee80211_chanctx *ctx;
int err;

lockdep_assert_wiphy(local->hw.wiphy);

- ctx = ieee80211_alloc_chanctx(local, chanreq, mode);
+ ctx = ieee80211_alloc_chanctx(local, chanreq, mode, radio_idx);
if (!ctx)
return ERR_PTR(-ENOMEM);

@@ -1053,6 +1068,51 @@ int ieee80211_link_unreserve_chanctx(struct ieee80211_link_data *link)
return 0;
}

+static bool
+ieee80211_get_radio_freq_match(const struct wiphy_radio *radio,
+ const struct ieee80211_chan_req *chanreq)
+{
+ const struct wiphy_radio_freq_range *r;
+ u32 freq;
+ int i;
+
+ if (!radio->n_freq_range)
+ return true;
+
+ freq = ieee80211_channel_to_khz(chanreq->oper.chan);
+ for (i = 0; i < radio->n_freq_range; i++) {
+ r = &radio->freq_range[i];
+ if (freq >= r->start_freq && freq <= r->end_freq)
+ return true;
+ }
+
+ return false;
+}
+
+static int
+ieee80211_get_chanreq_radio(struct ieee80211_local *local,
+ const struct ieee80211_chan_req *chanreq)
+{
+ struct wiphy *wiphy = local->hw.wiphy;
+ const struct wiphy_radio *radio;
+ u32 band_mask;
+ int i;
+
+ band_mask = BIT(chanreq->oper.chan->band);
+ for (i = 0; i < wiphy->n_radio; i++) {
+ radio = &wiphy->radio[i];
+ if (!(radio->band_mask & band_mask))
+ continue;
+
+ if (!ieee80211_get_radio_freq_match(radio, chanreq))
+ continue;
+
+ return i;
+ }
+
+ return -1;
+}
+
int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
const struct ieee80211_chan_req *chanreq,
enum ieee80211_chanctx_mode mode,
@@ -1061,6 +1121,7 @@ int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
struct ieee80211_sub_if_data *sdata = link->sdata;
struct ieee80211_local *local = sdata->local;
struct ieee80211_chanctx *new_ctx, *curr_ctx, *ctx;
+ int radio_idx;

lockdep_assert_wiphy(local->hw.wiphy);

@@ -1070,9 +1131,10 @@ int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,

new_ctx = ieee80211_find_reservation_chanctx(local, chanreq, mode);
if (!new_ctx) {
- if (ieee80211_can_create_new_chanctx(local)) {
+ radio_idx = ieee80211_get_chanreq_radio(local, chanreq);
+ if (ieee80211_can_create_new_chanctx(local, radio_idx)) {
new_ctx = ieee80211_new_chanctx(local, chanreq, mode,
- false);
+ false, radio_idx);
if (IS_ERR(new_ctx))
return PTR_ERR(new_ctx);
} else {
@@ -1111,6 +1173,9 @@ int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
if (!list_empty(&ctx->reserved_links))
continue;

+ if (ctx->conf.radio_idx != radio_idx)
+ continue;
+
curr_ctx = ctx;
break;
}
@@ -1126,7 +1191,8 @@ int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
!list_empty(&curr_ctx->reserved_links))
return -EBUSY;

- new_ctx = ieee80211_alloc_chanctx(local, chanreq, mode);
+ new_ctx = ieee80211_alloc_chanctx(local, chanreq, mode,
+ radio_idx);
if (!new_ctx)
return -ENOMEM;

@@ -1745,6 +1811,7 @@ int _ieee80211_link_use_channel(struct ieee80211_link_data *link,
struct ieee80211_chanctx *ctx;
u8 radar_detect_width = 0;
bool reserved = false;
+ int radio_idx;
int ret;

lockdep_assert_wiphy(local->hw.wiphy);
@@ -1773,11 +1840,18 @@ int _ieee80211_link_use_channel(struct ieee80211_link_data *link,

ctx = ieee80211_find_chanctx(local, link, chanreq, mode);
/* Note: context is now reserved */
- if (ctx)
+ if (ctx) {
reserved = true;
- else
+ } else {
+ radio_idx = ieee80211_get_chanreq_radio(local, chanreq);
+ if (ieee80211_can_create_new_chanctx(local, radio_idx)) {
+ ret = -EBUSY;
+ goto out;
+ }
+
ctx = ieee80211_new_chanctx(local, chanreq, mode,
- assign_on_failure);
+ assign_on_failure, radio_idx);
+ }
if (IS_ERR(ctx)) {
ret = PTR_ERR(ctx);
goto out;
--
2.44.0


2024-05-22 14:43:56

by Ben Greear

[permalink] [raw]
Subject: Re: [RFC 1/2] cfg80211: add support for advertising multiple radios belonging to a wiphy

On 5/22/24 05:30, 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.
> With this change, the bands and supported frequencies 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 <[email protected]>
> ---
> include/net/cfg80211.h | 34 ++++++++++++++++
> include/uapi/linux/nl80211.h | 48 +++++++++++++++++++++++
> net/wireless/nl80211.c | 75 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 157 insertions(+)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index f2ca495ac9f8..58d8375ffa11 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -5407,6 +5407,34 @@ 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.
> + *
> + * @band_mask: Mask of enum nl80211_band describing the bands that the radio
> + * can operate on.
> + * @num_freq_range: number of elements in @freq_range
> + * @freq_range: frequency range that the radio can operate on (optional)
> + */
> +struct wiphy_radio {
> + u16 band_mask;
> + u16 n_freq_range;
> + const struct wiphy_radio_freq_range *freq_range;
> +};

Do you think we might should add the radio_idx in here so that we don't
depend on position in the array in case we need to add/remove radios
for some reason?

Or radio MAC addr or some other more persistent way to detect exactly
what this is in user-space?


> +
> #define CFG80211_HW_TIMESTAMP_ALL_PEERS 0xffff
>
> /**
> @@ -5625,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;
> @@ -5775,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..dfb01d673094 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_BAND_MASK: Bitmask of bands supported by this
> + * radio.
> + *
> + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges
> + * supported by this radio.
> + *
> + * @__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_BAND_MASK,
> + NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES,
> +
> + /* 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 3c0bca4238d3..c07abdf104ec 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -2392,6 +2392,78 @@ 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];

Maybe allow radio[] array to be sparse (ie, idx 0 is there, idx 2 is there, idx 1 is NULL
because user wants to remove that radio from MLO consideration for whatever reason?

For what it's worth, I like the general approach.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2024-05-22 14:46:21

by Felix Fietkau

[permalink] [raw]
Subject: Re: [RFC 1/2] cfg80211: add support for advertising multiple radios belonging to a wiphy

On 22.05.24 16:23, Ben Greear wrote:
> On 5/22/24 05:30, 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.
>> With this change, the bands and supported frequencies 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 <[email protected]>
>> ---
>> include/net/cfg80211.h | 34 ++++++++++++++++
>> include/uapi/linux/nl80211.h | 48 +++++++++++++++++++++++
>> net/wireless/nl80211.c | 75 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 157 insertions(+)
>>
>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> index f2ca495ac9f8..58d8375ffa11 100644
>> --- a/include/net/cfg80211.h
>> +++ b/include/net/cfg80211.h
>> @@ -5407,6 +5407,34 @@ 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.
>> + *
>> + * @band_mask: Mask of enum nl80211_band describing the bands that the radio
>> + * can operate on.
>> + * @num_freq_range: number of elements in @freq_range
>> + * @freq_range: frequency range that the radio can operate on (optional)
>> + */
>> +struct wiphy_radio {
>> + u16 band_mask;
>> + u16 n_freq_range;
>> + const struct wiphy_radio_freq_range *freq_range;
>> +};
>
> Do you think we might should add the radio_idx in here so that we don't
> depend on position in the array in case we need to add/remove radios
> for some reason?
>
> Or radio MAC addr or some other more persistent way to detect exactly
> what this is in user-space?

Why would radios be added/removed at run time?

>> #define CFG80211_HW_TIMESTAMP_ALL_PEERS 0xffff
>>
>> /**
>> @@ -5625,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;
>> @@ -5775,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..dfb01d673094 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_BAND_MASK: Bitmask of bands supported by this
>> + * radio.
>> + *
>> + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges
>> + * supported by this radio.
>> + *
>> + * @__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_BAND_MASK,
>> + NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES,
>> +
>> + /* 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 3c0bca4238d3..c07abdf104ec 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -2392,6 +2392,78 @@ 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];
>
> Maybe allow radio[] array to be sparse (ie, idx 0 is there, idx 2 is there, idx 1 is NULL
> because user wants to remove that radio from MLO consideration for whatever reason?

Why should the user want to do this?

- Felix


2024-05-22 16:51:39

by Ben Greear

[permalink] [raw]
Subject: Re: [RFC 1/2] cfg80211: add support for advertising multiple radios belonging to a wiphy

On 5/22/24 07:46, Felix Fietkau wrote:
> On 22.05.24 16:23, Ben Greear wrote:
>> On 5/22/24 05:30, 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.
>>> With this change, the bands and supported frequencies 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 <[email protected]>
>>> ---
>>>   include/net/cfg80211.h       | 34 ++++++++++++++++
>>>   include/uapi/linux/nl80211.h | 48 +++++++++++++++++++++++
>>>   net/wireless/nl80211.c       | 75 ++++++++++++++++++++++++++++++++++++
>>>   3 files changed, 157 insertions(+)
>>>
>>> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>>> index f2ca495ac9f8..58d8375ffa11 100644
>>> --- a/include/net/cfg80211.h
>>> +++ b/include/net/cfg80211.h
>>> @@ -5407,6 +5407,34 @@ 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.
>>> + *
>>> + * @band_mask: Mask of enum nl80211_band describing the bands that the radio
>>> + *    can operate on.
>>> + * @num_freq_range: number of elements in @freq_range
>>> + * @freq_range: frequency range that the radio can operate on (optional)
>>> + */
>>> +struct wiphy_radio {
>>> +    u16 band_mask;
>>> +    u16 n_freq_range;
>>> +    const struct wiphy_radio_freq_range *freq_range;
>>> +};
>>
>> Do you think we might should add the radio_idx in here so that we don't
>> depend on position in the array in case we need to add/remove radios
>> for some reason?
>>
>> Or radio MAC addr or some other more persistent way to detect exactly
>> what this is in user-space?
>
> Why would radios be added/removed at run time?

I'm thinking I might want to have radio(s) work in legacy mode sometimes and then change
to combined mode.

Another possibility is unrecoverable firmware/radio crashes, where maybe only one of them
dies and so system could remove the wedged hardware from the radio list
if it cannot recover.

If putting radios in/out of combined mode is not feasible, then I will instead
want to be able to pin legacy STA/VAP/monitor ports to specific radios. It would
be nice if the radios could be reliably addressed from user-space without having
to query the frequency band and use that as unique identifier. Especially if someone
manages to make something like an mtk7996 that can do more than one radio per frequency
range.

And, is the hardware discrete enough that if you had two mtk7996 3-radio NICs,
could you actually do a 6-radio OFDMA configuration with a 'fake' virtual wiphy
device to hold the underlying wiphys/radios? If so, having better radio identifier
would be required at that point I think.

>
>>>   #define CFG80211_HW_TIMESTAMP_ALL_PEERS    0xffff
>>>   /**
>>> @@ -5625,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;
>>> @@ -5775,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..dfb01d673094 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_BAND_MASK: Bitmask of bands supported by this
>>> + *    radio.
>>> + *
>>> + * @NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES: Nested array of frequency ranges
>>> + *    supported by this radio.
>>> + *
>>> + * @__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_BAND_MASK,
>>> +    NL80211_WIPHY_RADIO_ATTR_FREQ_RANGES,
>>> +
>>> +    /* 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 3c0bca4238d3..c07abdf104ec 100644
>>> --- a/net/wireless/nl80211.c
>>> +++ b/net/wireless/nl80211.c
>>> @@ -2392,6 +2392,78 @@ 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];
>>
>> Maybe allow radio[] array to be sparse (ie, idx 0 is there, idx 2 is there, idx 1 is NULL
>> because user wants to remove that radio from MLO consideration for whatever reason?
>
> Why should the user want to do this?

unrecoverable radio failures and putting radio into legacy mode...

Thanks,
Ben

>
> - Felix
>


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com