diff mbox series

[RFC,v3,8/8] wifi: mac80211: add wiphy radio assignment and validation

Message ID 9b331a61b8d53284b044bc594cf9952c60164e5f.1717696995.git-series.nbd@nbd.name
State Superseded
Headers show
Series cfg80211/mac80211: support defining multiple radios per wiphy | expand

Commit Message

Felix Fietkau June 6, 2024, 6:07 p.m. UTC
Validate number of channels and interface combinations per radio.
Assign each channel context to a radio.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
 net/mac80211/chan.c | 70 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 65 insertions(+), 5 deletions(-)

Comments

Johannes Berg June 7, 2024, 9:44 a.m. UTC | #1
On Thu, 2024-06-06 at 20:07 +0200, Felix Fietkau wrote:
> 
> +static bool
> +ieee80211_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;
> +
> +	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;

IMHO should be inclusive only on one side of the range. Can always make
it work since channels are at least a few MHz apart, but the purist in
me says it's easier to grok if the end is not inclusive :)

Maybe this should be a cfg80211 helper like

struct wiphy_radio *wiphy_get_radio(struct wiphy *wiphy, ... *chandef);

which could also check that the _whole_ chandef fits (though that should
probably also be handled elsewhere, like chandef_valid), and you can use
it to get the radio pointer and then check for == below.

The point would be to have a single place with this kind of range logic.
This is only going to get done by mac80211 now, but it'd really be
awkward if some other driver had some other logic later.

>  	if (!curr_ctx || (curr_ctx->replace_state ==
>  			  IEEE80211_CHANCTX_WILL_BE_REPLACED) ||
> @@ -1096,6 +1117,12 @@ ieee80211_replace_chanctx(struct ieee80211_local *local,
>  			if (!list_empty(&ctx->reserved_links))
>  				continue;
>  
> +			if (ctx->conf.radio_idx >= 0) {
> +					radio = &wiphy->radio[ctx->conf.radio_idx];
> +					if (!ieee80211_radio_freq_match(radio, chanreq))
> +							continue;
> +			}

something happened to indentation here :)

> +static bool
> +ieee80211_find_available_radio(struct ieee80211_local *local,
> +			       const struct ieee80211_chan_req *chanreq,
> +			       int *radio_idx)
> +{
> +	struct wiphy *wiphy = local->hw.wiphy;
> +	const struct wiphy_radio *radio;
> +	int i;
> +
> +	*radio_idx = -1;
> +	if (!wiphy->n_radio)
> +		return true;
> +
> +	for (i = 0; i < wiphy->n_radio; i++) {
> +		radio = &wiphy->radio[i];
> +		if (!ieee80211_radio_freq_match(radio, chanreq))
> +			continue;
> +
> +		if (!ieee80211_can_create_new_chanctx(local, i))
> +			continue;
> +
> +		*radio_idx = i;
> +		return true;
> +	}
> +
> +	return false;
> +}

which would also get used here to find the radio first, though would
have to differentiate n_radio still, of course.

johannes
Felix Fietkau June 7, 2024, 9:53 a.m. UTC | #2
On 07.06.24 11:44, Johannes Berg wrote:
> On Thu, 2024-06-06 at 20:07 +0200, Felix Fietkau wrote:
>> 
>> +static bool
>> +ieee80211_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;
>> +
>> +	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;
> 
> IMHO should be inclusive only on one side of the range. Can always make
> it work since channels are at least a few MHz apart, but the purist in
> me says it's easier to grok if the end is not inclusive :)

Sure, no problem.

> Maybe this should be a cfg80211 helper like
> 
> struct wiphy_radio *wiphy_get_radio(struct wiphy *wiphy, ... *chandef);

I didn't add such a helper, in case we get hardware where multiple 
radios support the same band. That's why ieee80211_find_available_radio 
loops over all radios until it finds one that matches both the freq 
range and the ifcomb constraints.

> which could also check that the _whole_ chandef fits (though that should
> probably also be handled elsewhere, like chandef_valid), and you can use
> it to get the radio pointer and then check for == below.
> 
> The point would be to have a single place with this kind of range logic.
> This is only going to get done by mac80211 now, but it'd really be
> awkward if some other driver had some other logic later.

I will move a variation of the freq range match helper to cfg80211.

>>  	if (!curr_ctx || (curr_ctx->replace_state ==
>>  			  IEEE80211_CHANCTX_WILL_BE_REPLACED) ||
>> @@ -1096,6 +1117,12 @@ ieee80211_replace_chanctx(struct ieee80211_local *local,
>>  			if (!list_empty(&ctx->reserved_links))
>>  				continue;
>>  
>> +			if (ctx->conf.radio_idx >= 0) {
>> +					radio = &wiphy->radio[ctx->conf.radio_idx];
>> +					if (!ieee80211_radio_freq_match(radio, chanreq))
>> +							continue;
>> +			}
> 
> something happened to indentation here :)

Will fix :)

>> +static bool
>> +ieee80211_find_available_radio(struct ieee80211_local *local,
>> +			       const struct ieee80211_chan_req *chanreq,
>> +			       int *radio_idx)
>> +{
>> +	struct wiphy *wiphy = local->hw.wiphy;
>> +	const struct wiphy_radio *radio;
>> +	int i;
>> +
>> +	*radio_idx = -1;
>> +	if (!wiphy->n_radio)
>> +		return true;
>> +
>> +	for (i = 0; i < wiphy->n_radio; i++) {
>> +		radio = &wiphy->radio[i];
>> +		if (!ieee80211_radio_freq_match(radio, chanreq))
>> +			continue;
>> +
>> +		if (!ieee80211_can_create_new_chanctx(local, i))
>> +			continue;
>> +
>> +		*radio_idx = i;
>> +		return true;
>> +	}
>> +
>> +	return false;
>> +}
> 
> which would also get used here to find the radio first, though would
> have to differentiate n_radio still, of course.

See above

- Felix
diff mbox series

Patch

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index ac49c2c71d2b..257ee3b1100b 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -696,14 +696,15 @@  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, -1);
+	ctx = ieee80211_alloc_chanctx(local, chanreq, mode, radio_idx);
 	if (!ctx)
 		return ERR_PTR(-ENOMEM);
 
@@ -1060,6 +1061,24 @@  int ieee80211_link_unreserve_chanctx(struct ieee80211_link_data *link)
 	return 0;
 }
 
+static bool
+ieee80211_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;
+
+	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 struct ieee80211_chanctx *
 ieee80211_replace_chanctx(struct ieee80211_local *local,
 			  const struct ieee80211_chan_req *chanreq,
@@ -1067,6 +1086,8 @@  ieee80211_replace_chanctx(struct ieee80211_local *local,
 			  struct ieee80211_chanctx *curr_ctx)
 {
 	struct ieee80211_chanctx *new_ctx, *ctx;
+	struct wiphy *wiphy = local->hw.wiphy;
+	const struct wiphy_radio *radio;
 
 	if (!curr_ctx || (curr_ctx->replace_state ==
 			  IEEE80211_CHANCTX_WILL_BE_REPLACED) ||
@@ -1096,6 +1117,12 @@  ieee80211_replace_chanctx(struct ieee80211_local *local,
 			if (!list_empty(&ctx->reserved_links))
 				continue;
 
+			if (ctx->conf.radio_idx >= 0) {
+					radio = &wiphy->radio[ctx->conf.radio_idx];
+					if (!ieee80211_radio_freq_match(radio, chanreq))
+							continue;
+			}
+
 			curr_ctx = ctx;
 			break;
 		}
@@ -1125,6 +1152,34 @@  ieee80211_replace_chanctx(struct ieee80211_local *local,
 	return new_ctx;
 }
 
+static bool
+ieee80211_find_available_radio(struct ieee80211_local *local,
+			       const struct ieee80211_chan_req *chanreq,
+			       int *radio_idx)
+{
+	struct wiphy *wiphy = local->hw.wiphy;
+	const struct wiphy_radio *radio;
+	int i;
+
+	*radio_idx = -1;
+	if (!wiphy->n_radio)
+		return true;
+
+	for (i = 0; i < wiphy->n_radio; i++) {
+		radio = &wiphy->radio[i];
+		if (!ieee80211_radio_freq_match(radio, chanreq))
+			continue;
+
+		if (!ieee80211_can_create_new_chanctx(local, i))
+			continue;
+
+		*radio_idx = i;
+		return true;
+	}
+
+	return false;
+}
+
 int ieee80211_link_reserve_chanctx(struct ieee80211_link_data *link,
 				   const struct ieee80211_chan_req *chanreq,
 				   enum ieee80211_chanctx_mode mode,
@@ -1133,6 +1188,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;
+	int radio_idx;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
@@ -1142,9 +1198,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, -1))
+		if (ieee80211_can_create_new_chanctx(local, -1) &&
+		    ieee80211_find_available_radio(local, chanreq, &radio_idx))
 			new_ctx = ieee80211_new_chanctx(local, chanreq, mode,
-							false);
+							false, radio_idx);
 		else
 			new_ctx = ieee80211_replace_chanctx(local, chanreq,
 							    mode, curr_ctx);
@@ -1755,6 +1812,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);
@@ -1785,9 +1843,11 @@  int _ieee80211_link_use_channel(struct ieee80211_link_data *link,
 	/* Note: context is now reserved */
 	if (ctx)
 		reserved = true;
+	else if (!ieee80211_find_available_radio(local, chanreq, &radio_idx))
+		ctx = ERR_PTR(-EBUSY);
 	else
 		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;