diff mbox series

[11/13] wifi: mac80211: Add multi-hardware support in the iface comb helper

Message ID 20240328072916.1164195-12-quic_periyasa@quicinc.com (mailing list archive)
State RFC
Delegated to: Johannes Berg
Headers show
Series wifi: Add multi physical hardware iface combination support | expand

Commit Message

Karthikeyan Periyasamy March 28, 2024, 7:29 a.m. UTC 
From: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>

Currently, iface combination param supports single physical hardware.
To support multi physical hardware, add hardware specific checks on the
channel context creation and populate hardware specific parameters from
the channel definition for which the interface combination need to check.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00188-QCAHKSWPL_SILICONZ-1

Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
Co-developed-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
---
 net/mac80211/chan.c        |  33 +++++--
 net/mac80211/ieee80211_i.h |   3 +-
 net/mac80211/util.c        | 194 ++++++++++++++++++++++++++++++++++---
 3 files changed, 207 insertions(+), 23 deletions(-)

Comments

Johannes Berg March 28, 2024, 2:41 p.m. UTC | #1 
On Thu, 2024-03-28 at 12:59 +0530, Karthikeyan Periyasamy wrote:
> From: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
> 
> Currently, iface combination param supports single physical hardware.
> To support multi physical hardware, add hardware specific checks on the
> channel context creation and populate hardware specific parameters from
> the channel definition for which the interface combination need to check.

I haven't gone through this patch in detail, but again like on patch 9,
I'm not so sure about the implementation of the XOR behaviour of
checking global and per-HW limitations.

I probably wrote on the other patch that it seems it should check both,
but I realize now that doesn't make sense: After all, the top-level
combinations data must encode something that's supported _regardless_ of
channels, so likely only a subset of the combinations supported across
the different HW.

But that doesn't mean that I'm yet convinced that the design in patch 9
is actually good, with how it checks that depending on the 'chandef'
parameter. I'd like to explore other possibilities such as a different
function for that, for example. It could use the same underlying helpers
and mechanics, but it seems that might be better overall.

Or probably better yet: explore an approach where mac80211 doesn't even
have to _know_ about the cfg80211_get_hw_idx_by_chan() and all that, but
just prepares the information in a way that encodes enough data to
handle that, which really means going from

 int num_different_channels;
 int iftype_num[...];

to

 struct {
   enum nl80211_iftype iftype;
   u32 freq;
 } interfaces[];

or something like that.


I was almost going to write "links" instead of "interfaces" there, which
reminds me that none of this really even works well for MLO yet. Not
sure if that's better addressed as a separate first step?

johannes
Jeff Johnson March 28, 2024, 4:39 p.m. UTC | #2 
On 3/28/2024 7:41 AM, Johannes Berg wrote:
> I was almost going to write "links" instead of "interfaces" there, which
> reminds me that none of this really even works well for MLO yet. Not
> sure if that's better addressed as a separate first step?

we are following the sequence:
1) go from per-hw wiphy to wiphy that supports multi-hw (multiple patchsets
under review)
2) add hw-to-wiphy grouping infrastructure (hopefully coming soon)
3) add MLO on top of the grouping infrastructure

So we consider this to be part of the 1st phase
Karthikeyan Periyasamy April 23, 2024, 4:01 p.m. UTC | #3 
On 3/28/2024 8:11 PM, Johannes Berg wrote:
> On Thu, 2024-03-28 at 12:59 +0530, Karthikeyan Periyasamy wrote:
>> From: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
>>
>> Currently, iface combination param supports single physical hardware.
>> To support multi physical hardware, add hardware specific checks on the
>> channel context creation and populate hardware specific parameters from
>> the channel definition for which the interface combination need to check.
> 
> I haven't gone through this patch in detail, but again like on patch 9,
> I'm not so sure about the implementation of the XOR behaviour of
> checking global and per-HW limitations.
> 
> I probably wrote on the other patch that it seems it should check both,
> but I realize now that doesn't make sense: After all, the top-level
> combinations data must encode something that's supported _regardless_ of
> channels, so likely only a subset of the combinations supported across
> the different HW.
> 
> But that doesn't mean that I'm yet convinced that the design in patch 9
> is actually good, with how it checks that depending on the 'chandef'
> parameter. I'd like to explore other possibilities such as a different
> function for that, for example. It could use the same underlying helpers
> and mechanics, but it seems that might be better overall.
> 
> Or probably better yet: explore an approach where mac80211 doesn't even
> have to _know_ about the cfg80211_get_hw_idx_by_chan() and all that, but
> just prepares the information in a way that encodes enough data to
> handle that, which really means going from
> 
>   int num_different_channels;
>   int iftype_num[...];
> 
> to
> 
>   struct {
>     enum nl80211_iftype iftype;
>     u32 freq;
>   } interfaces[];
> 
> or something like that.
> 
> 
> I was almost going to write "links" instead of "interfaces" there, which
> reminds me that none of this really even works well for MLO yet. Not
> sure if that's better addressed as a separate first step?
> 

Sure, will post this change as a separate RFC patch.
diff mbox series

Patch

diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 08a362892d9a..0558eafa45aa 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -47,24 +47,43 @@  int ieee80211_chanctx_refcount(struct ieee80211_local *local,
 	       ieee80211_chanctx_num_reserved(local, ctx);
 }
 
-static int ieee80211_num_chanctx(struct ieee80211_local *local)
+static int ieee80211_num_chanctx(struct ieee80211_local *local,
+				 const struct cfg80211_chan_def *chandef)
 {
 	struct ieee80211_chanctx *ctx;
-	int num = 0;
+	int num = 0, hw_idx = -1, ctx_idx;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	list_for_each_entry(ctx, &local->chanctx_list, list)
-		num++;
+	if (chandef && cfg80211_chandef_valid(chandef))
+		hw_idx = cfg80211_get_hw_idx_by_chan(local->hw.wiphy,
+						     chandef->chan);
+
+	if (hw_idx < 0) {
+		list_for_each_entry(ctx, &local->chanctx_list, list)
+			num++;
+	} else {
+		list_for_each_entry(ctx, &local->chanctx_list, list) {
+			ctx_idx = cfg80211_get_hw_idx_by_chan(local->hw.wiphy,
+							      ctx->conf.def.chan);
+			if (ctx_idx != hw_idx)
+				continue;
+
+			num++;
+		}
+	}
 
 	return num;
 }
 
-static bool ieee80211_can_create_new_chanctx(struct ieee80211_local *local)
+static
+bool ieee80211_can_create_new_chanctx(struct ieee80211_local *local,
+				      const struct cfg80211_chan_def *chandef)
 {
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	return ieee80211_num_chanctx(local) < ieee80211_max_num_channels(local);
+	return ieee80211_num_chanctx(local, chandef) <
+	       ieee80211_max_num_channels(local, chandef);
 }
 
 struct ieee80211_chanctx *
@@ -1029,7 +1048,7 @@  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)) {
+		if (ieee80211_can_create_new_chanctx(local, &chanreq->oper)) {
 			new_ctx = ieee80211_new_chanctx(local, chanreq, mode);
 			if (IS_ERR(new_ctx))
 				return PTR_ERR(new_ctx);
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 202bbffec746..42f4211c622b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -2600,7 +2600,8 @@  int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 				 const struct cfg80211_chan_def *chandef,
 				 enum ieee80211_chanctx_mode chanmode,
 				 u8 radar_detect);
-int ieee80211_max_num_channels(struct ieee80211_local *local);
+int ieee80211_max_num_channels(struct ieee80211_local *local,
+			       const struct cfg80211_chan_def *chandef);
 void ieee80211_recalc_chanctx_chantype(struct ieee80211_local *local,
 				       struct ieee80211_chanctx *ctx);
 
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index b1f3b1eb053d..f8ffc8ad0cd8 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -3926,6 +3926,44 @@  static u8 ieee80211_chanctx_radar_detect(struct ieee80211_local *local,
 	return radar_detect;
 }
 
+static void
+ieee80211_get_per_hw_sdata_active_iface(struct ieee80211_sub_if_data *sdata,
+					struct iface_comb_per_hw_params *per_hw,
+					int iftype_num[NUM_NL80211_IFTYPES],
+					int *total)
+{
+	struct ieee80211_local *local = sdata->local;
+	unsigned int link_id;
+	int hw_idx;
+
+	for (link_id = 0; link_id < ARRAY_SIZE(sdata->link); link_id++) {
+		struct ieee80211_link_data *link;
+		struct ieee80211_chanctx *ctx;
+
+		link = sdata_dereference(sdata->link[link_id], sdata);
+		if (!link)
+			continue;
+
+		ctx = ieee80211_link_get_chanctx(link);
+		if (ctx &&
+		    ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
+			ctx = ctx->replace_ctx;
+
+		hw_idx = -1;
+		if (ctx && cfg80211_chandef_valid(&ctx->conf.def))
+			hw_idx = cfg80211_get_hw_idx_by_chan(local->hw.wiphy,
+							     ctx->conf.def.chan);
+
+		if (hw_idx >= 0)
+			per_hw[hw_idx].iftype_num[sdata->wdev.iftype]++;
+		else
+			iftype_num[sdata->wdev.iftype]++;
+
+		if (total)
+			(*total)++;
+	}
+}
+
 int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 				 const struct cfg80211_chan_def *chandef,
 				 enum ieee80211_chanctx_mode chanmode,
@@ -3936,9 +3974,12 @@  int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 	enum nl80211_iftype iftype = sdata->wdev.iftype;
 	struct ieee80211_chanctx *ctx;
 	int total = 1;
+	struct iface_comb_per_hw_params *per_hw __free(kfree) = NULL;
 	struct iface_combination_params params = {
 		.radar_detect = radar_detect,
 	};
+	bool is_per_hw;
+	int hchan_idx = -1;
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
@@ -3969,26 +4010,68 @@  int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 		return 0;
 	}
 
-	if (chandef)
-		params.num_different_channels = 1;
+	is_per_hw = cfg80211_per_hw_iface_comb_advertised(local->hw.wiphy);
+	if (is_per_hw) {
+		per_hw = kcalloc(local->hw.wiphy->num_hw, sizeof(*per_hw),
+				 GFP_KERNEL);
+		if (!per_hw)
+			return -ENOMEM;
+
+		if (chandef && cfg80211_chandef_valid(chandef)) {
+			hchan_idx = cfg80211_get_hw_idx_by_chan(local->hw.wiphy,
+								chandef->chan);
+			if (hchan_idx < 0)
+				goto skip;
 
-	if (iftype != NL80211_IFTYPE_UNSPECIFIED)
-		params.iftype_num[iftype] = 1;
+			per_hw[hchan_idx].num_different_channels = 1;
 
+			if (iftype != NL80211_IFTYPE_UNSPECIFIED)
+				per_hw[hchan_idx].iftype_num[iftype] = 1;
+		}
+	} else {
+		if (chandef)
+			params.num_different_channels = 1;
+
+		if (iftype != NL80211_IFTYPE_UNSPECIFIED)
+			params.iftype_num[iftype] = 1;
+	}
+
+skip:
 	list_for_each_entry(ctx, &local->chanctx_list, list) {
 		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
 			continue;
+
+		if (is_per_hw) {
+			if (WARN_ON(!cfg80211_chandef_valid(&ctx->conf.def)))
+				continue;
+
+			hchan_idx = cfg80211_get_hw_idx_by_chan(local->hw.wiphy,
+								ctx->conf.def.chan);
+			if (WARN_ON(hchan_idx < 0))
+				continue;
+		}
+
 		params.radar_detect |=
 			ieee80211_chanctx_radar_detect(local, ctx);
+
 		if (ctx->mode == IEEE80211_CHANCTX_EXCLUSIVE) {
-			params.num_different_channels++;
+			if (is_per_hw)
+				per_hw[hchan_idx].num_different_channels++;
+			else
+				params.num_different_channels++;
+
 			continue;
 		}
+
 		if (chandef && chanmode == IEEE80211_CHANCTX_SHARED &&
 		    cfg80211_chandef_compatible(chandef,
 						&ctx->conf.def))
 			continue;
-		params.num_different_channels++;
+
+		if (is_per_hw)
+			per_hw[hchan_idx].num_different_channels++;
+		else
+			params.num_different_channels++;
 	}
 
 	list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
@@ -4002,13 +4085,25 @@  int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
 					    wdev_iter->iftype, 0, 1))
 			continue;
 
-		params.iftype_num[wdev_iter->iftype]++;
-		total++;
+		if (is_per_hw) {
+			ieee80211_get_per_hw_sdata_active_iface(sdata_iter,
+								per_hw,
+								params.iftype_num,
+								&total);
+		} else {
+			params.iftype_num[wdev_iter->iftype]++;
+			total++;
+		}
 	}
 
 	if (total == 1 && !params.radar_detect)
 		return 0;
 
+	if (is_per_hw) {
+		params.n_per_hw = local->hw.wiphy->num_hw;
+		params.per_hw = per_hw;
+	}
+
 	return cfg80211_check_combinations(local->hw.wiphy, &params);
 }
 
@@ -4022,31 +4117,100 @@  ieee80211_iter_max_chans(const struct ieee80211_iface_combination *c,
 					  c->num_different_channels);
 }
 
-int ieee80211_max_num_channels(struct ieee80211_local *local)
+static void
+ieee80211_iter_per_hw_max_chans(const struct ieee80211_iface_combination *c,
+				int hw_chan_idx, void *data)
+{
+	u32 *max_num_diff_chans = data;
+	u32 max_supp_diff_chans = 0;
+	int i;
+
+	for (i = 0; i < c->n_hw_list; i++) {
+		const struct ieee80211_iface_per_hw *h;
+
+		h = &c->iface_hw_list[i];
+		if (hw_chan_idx != -1) {
+			if (h->hw_chans_idx == hw_chan_idx) {
+				max_supp_diff_chans = h->num_different_channels;
+				break;
+			}
+			continue;
+		}
+		max_supp_diff_chans += h->num_different_channels;
+	}
+
+	*max_num_diff_chans = max(*max_num_diff_chans, max_supp_diff_chans);
+}
+
+int ieee80211_max_num_channels(struct ieee80211_local *local,
+			       const struct cfg80211_chan_def *chandef)
 {
 	struct ieee80211_sub_if_data *sdata;
 	struct ieee80211_chanctx *ctx;
+	struct iface_comb_per_hw_params *per_hw __free(kfree) = NULL;
+	struct iface_combination_params params = {0};
+	void (*iter)(const struct ieee80211_iface_combination *c,
+		     int hw_chan_idx, void *data) = ieee80211_iter_max_chans;
 	u32 max_num_different_channels = 1;
+	bool is_per_hw;
 	int err;
-	struct iface_combination_params params = {0};
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
+	is_per_hw = cfg80211_per_hw_iface_comb_advertised(local->hw.wiphy);
+	if (is_per_hw) {
+		per_hw = kcalloc(local->hw.wiphy->num_hw,
+				 sizeof(*params.per_hw),
+				 GFP_KERNEL);
+		if (!per_hw)
+			return -ENOMEM;
+
+		params.chandef = chandef;
+		iter = ieee80211_iter_per_hw_max_chans;
+	}
+
 	list_for_each_entry(ctx, &local->chanctx_list, list) {
-		if (ctx->replace_state == IEEE80211_CHANCTX_WILL_BE_REPLACED)
+		if (ctx->replace_state ==
+		    IEEE80211_CHANCTX_WILL_BE_REPLACED)
 			continue;
 
-		params.num_different_channels++;
+		if (is_per_hw) {
+			int hw_idx;
+
+			if (WARN_ON(!cfg80211_chandef_valid(&ctx->conf.def)))
+				continue;
+
+			hw_idx = cfg80211_get_hw_idx_by_chan(local->hw.wiphy,
+							     ctx->conf.def.chan);
+			if (WARN_ON(hw_idx < 0))
+				continue;
+
+			per_hw[hw_idx].num_different_channels++;
+		} else {
+			params.num_different_channels++;
+		}
 
 		params.radar_detect |=
 			ieee80211_chanctx_radar_detect(local, ctx);
 	}
 
-	list_for_each_entry_rcu(sdata, &local->interfaces, list)
-		params.iftype_num[sdata->wdev.iftype]++;
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (is_per_hw && ieee80211_sdata_running(sdata))
+			ieee80211_get_per_hw_sdata_active_iface(sdata,
+								per_hw,
+								params.iftype_num,
+								NULL);
+		else
+			params.iftype_num[sdata->wdev.iftype]++;
+	}
+
+	if (is_per_hw) {
+		params.n_per_hw = local->hw.wiphy->num_hw;
+		params.per_hw = per_hw;
+	}
 
 	err = cfg80211_iter_combinations(local->hw.wiphy, &params,
-					 ieee80211_iter_max_chans,
+					 iter,
 					 &max_num_different_channels);
 	if (err < 0)
 		return err;