diff mbox series

[RFC,4/4] wifi: nl80211: send iface combination to user space in multi-hardware wiphy

Message ID 20220920100518.19705-5-quic_vthiagar@quicinc.com (mailing list archive)
State RFC
Delegated to: Johannes Berg
Headers show
Series wifi: cfg80211/mac80211: capability advertisement infra for multi-hw abstraction under one wiphy | expand

Commit Message

Vasanthakumar Thiagarajan Sept. 20, 2022, 10:05 a.m. UTC
A new nested nl attribute is added to the same existing NL command to
advertise the iface combination capability for each underlying hardware
when driver groups more than one physical hardware under one wiphy to
enable MLO among them.

Signed-off-by: Vasanthakumar Thiagarajan <quic_vthiagar@quicinc.com>
---
 include/uapi/linux/nl80211.h | 50 ++++++++++++++++++++++++-
 net/wireless/nl80211.c       | 71 ++++++++++++++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 1 deletion(-)

Comments

Johannes Berg Oct. 21, 2022, 12:25 p.m. UTC | #1
On Tue, 2022-09-20 at 15:35 +0530, Vasanthakumar Thiagarajan wrote:
> A new nested nl attribute is added to the same existing NL command to
> advertise the iface combination capability for each underlying hardware
> when driver groups more than one physical hardware under one wiphy to
> enable MLO among them.

That's a good point - are we assuming this implies you can do MLO across
the groups? Maybe somebody would want to use this advertisement without
allowing MLO? (Not sure that's useful vs. multiple wiphys though, but
maybe to simplify driver if there are some devices w/o MLO?)

> +		for (l = 0; l < c->iface_hw_list->n_limits; l++) {
> +			struct nlattr *limit;
> +
> +			limit = nla_nest_start(msg, l + 1);
> +			if (!limit)
> +				return -ENOBUFS;
> +
> +			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_MAX,
> +					c->iface_hw_list[i].limits[l].max))
> +				return -ENOBUFS;
> +
> +			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_TYPES,
> +					c->iface_hw_list[i].limits[l].types))
> +				return -ENOBUFS;
> +
> +			nla_nest_end(msg, limit);
> +		}
> +		nla_nest_end(msg, limits);


Feels like this part is kind of pre-existing code, or should be, could
it be refactored?

> +		if (nla_put_u32(msg,
> +				NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS,
> +				c->iface_hw_list[i].num_different_channels))
> +			return -ENOBUFS;
> +
> +		if (nla_put_u16(msg,
> +				NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM,
> +				c->iface_hw_list[i].max_interfaces))
> +			return -ENOBUFS;
> +
> +		nla_nest_end(msg, hw_combi);

And even this feels like it must already exist in some way? Wouldn't it
even be easier to parse for userspace if it wasn't a separate set of
attributes?

johannes
Vasanthakumar Thiagarajan Oct. 21, 2022, 1:31 p.m. UTC | #2
On 10/21/2022 5:55 PM, Johannes Berg wrote:
> On Tue, 2022-09-20 at 15:35 +0530, Vasanthakumar Thiagarajan wrote:
>> A new nested nl attribute is added to the same existing NL command to
>> advertise the iface combination capability for each underlying hardware
>> when driver groups more than one physical hardware under one wiphy to
>> enable MLO among them.
> 
> That's a good point - are we assuming this implies you can do MLO across
> the groups? Maybe somebody would want to use this advertisement without
> allowing MLO? (Not sure that's useful vs. multiple wiphys though, but
> maybe to simplify driver if there are some devices w/o MLO?)

Good point. Yes, this can be used even without MLO (i.e. not quite tied 
to MLO feature). Ill update the doc accordingly. Thanks.

> 
>> +		for (l = 0; l < c->iface_hw_list->n_limits; l++) {
>> +			struct nlattr *limit;
>> +
>> +			limit = nla_nest_start(msg, l + 1);
>> +			if (!limit)
>> +				return -ENOBUFS;
>> +
>> +			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_MAX,
>> +					c->iface_hw_list[i].limits[l].max))
>> +				return -ENOBUFS;
>> +
>> +			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_TYPES,
>> +					c->iface_hw_list[i].limits[l].types))
>> +				return -ENOBUFS;
>> +
>> +			nla_nest_end(msg, limit);
>> +		}
>> +		nla_nest_end(msg, limits);
> 
> 
> Feels like this part is kind of pre-existing code, or should be, could
> it be refactored?
> 

let me check, thanks.

>> +		if (nla_put_u32(msg,
>> +				NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS,
>> +				c->iface_hw_list[i].num_different_channels))
>> +			return -ENOBUFS;
>> +
>> +		if (nla_put_u16(msg,
>> +				NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM,
>> +				c->iface_hw_list[i].max_interfaces))
>> +			return -ENOBUFS;
>> +
>> +		nla_nest_end(msg, hw_combi);
> 
> And even this feels like it must already exist in some way? Wouldn't it
> even be easier to parse for userspace if it wasn't a separate set of
> attributes?
>

Yes, reusing existing attribute will be simpler. let me check that.

Thanks for the review!


Vasanth
diff mbox series

Patch

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 070b31277402..678da076b122 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -5779,6 +5779,10 @@  enum nl80211_iface_limit_attrs {
  * @NL80211_IFACE_COMB_BI_MIN_GCD: u32 attribute specifying the minimum GCD of
  *	different beacon intervals supported by all the interface combinations
  *	in this group (if not present, all beacon intervals be identical).
+ * @NL80211_IFACE_COMB_PER_HW_COMB: nested attribute specifying the interface
+ *	combination for each underlying hardware when multiple hardware are
+ *	registered under one wiphy,
+ *	see &enum nl80211_if_comb_per_hw_comb_attrs.
  * @NUM_NL80211_IFACE_COMB: number of attributes
  * @MAX_NL80211_IFACE_COMB: highest attribute number
  *
@@ -5795,7 +5799,19 @@  enum nl80211_iface_limit_attrs {
  *	numbers = [ #{STA} <= 1, #{P2P-client,P2P-GO} <= 3 ], max = 4
  *	=> allows a STA plus three P2P interfaces
  *
- * The list of these four possibilities could completely be contained
+ *	When describing per-hardware combinations in multi-hardware in
+ *	one wiphy model, the first possibility can further include the finer
+ *	capabilities like below
+ *	hw_chan_idx = 0, numbers = [ #{STA} <= 1, #{AP} <= 1 ],
+ *	channels = 1, max = 2
+ *	=> allows a STA plus an AP interface on the underlying hardware mac
+ *	   advertised at index 0 in wiphy @hw_chans array.
+ *	hw_chan_idx = 1, numbers = [ #{STA} <= 1, #{AP} <= 2 ],
+ *	channels = 1, max = 3
+ *	=> allows a STA plus two AP interfaces on the underlying hardware mac
+ *	   advertised at index 1 in wiphy @hw_chans array.
+ *
+ * The list of these five possibilities could completely be contained
  * within the %NL80211_ATTR_INTERFACE_COMBINATIONS attribute to indicate
  * that any of these groups must match.
  *
@@ -5814,12 +5830,44 @@  enum nl80211_if_combination_attrs {
 	NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS,
 	NL80211_IFACE_COMB_RADAR_DETECT_REGIONS,
 	NL80211_IFACE_COMB_BI_MIN_GCD,
+	NL80211_IFACE_COMB_PER_HW_COMB,
 
 	/* keep last */
 	NUM_NL80211_IFACE_COMB,
 	MAX_NL80211_IFACE_COMB = NUM_NL80211_IFACE_COMB - 1
 };
 
+/**
+ * enum nl80211_if_comb_per_hw_comb_attrs - per-hardware iface combination
+ * attributes with multi-hw radios in one wiphy model
+ *
+ * @NL80211_IFACE_COMB_PER_HW_COMB_UNSPEC: (reserved)
+ * @NL80211_IFACE_COMB_PER_HW_COMB_HW_IDX: u8 attribute specifying the index
+ *	to the wiphy @hw_chans list for which the iface combination is being
+ *	described.
+ * @NL80211_IFACE_COMB_PER_HW_COMB_LIMITS: nested attribute containing the
+ *	limits for the given interface types, see
+ *	&enum nl80211_iface_limit_attrs.
+ * @NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM: u32 attribute giving the maximum
+ *	number of interfaces that can be created in this group. This number
+ *	does not apply to the interfaces purely managed in software.
+ * @NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS: u32 attribute specifying the
+ *	number of different channels that can be used in this group.
+ * @NUM_NL80211_IFACE_COMB_PER_HW_COMB: number of attributes
+ * @MAX_NL80211_IFACE_COMB_PER_HW_COMB: highest attribute number
+ */
+enum nl80211_if_comb_per_hw_comb_attrs {
+	NL80211_IFACE_COMB_PER_HW_COMB_UNSPEC,
+	NL80211_IFACE_COMB_PER_HW_COMB_HW_IDX,
+	NL80211_IFACE_COMB_PER_HW_COMB_LIMITS,
+	NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM,
+	NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS,
+
+	/* keep last */
+	NUM_NL80211_IFACE_COMB_PER_HW_COMB,
+	MAX_NL80211_IFACE_COMB_PER_HW_COMB =
+			NUM_NL80211_IFACE_COMB_PER_HW_COMB - 1
+};
 
 /**
  * enum nl80211_plink_state - state of a mesh peer link finite state machine
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b7d466010e81..1f3b79e10697 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1595,6 +1595,74 @@  static int nl80211_put_iftypes(struct sk_buff *msg, u32 attr, u16 ifmodes)
 	return -ENOBUFS;
 }
 
+static int
+nl80211_put_per_hw_iface_combinations(struct wiphy *wiphy, struct sk_buff *msg,
+				      const struct ieee80211_iface_combination *c)
+{
+	struct nlattr *hw_combis;
+	int i;
+
+	if (!wiphy->num_hw)
+		return 0;
+
+	hw_combis = nla_nest_start(msg, NL80211_IFACE_COMB_PER_HW_COMB);
+	if (!hw_combis)
+		return -ENOBUFS;
+
+	for (i = 0; i < c->n_hw_list; i++) {
+		struct nlattr *hw_combi, *limits;
+		int l;
+
+		hw_combi = nla_nest_start(msg, i + 1);
+		if (!hw_combi)
+			return -ENOBUFS;
+
+		if (nla_put_u8(msg, NL80211_IFACE_COMB_PER_HW_COMB_HW_IDX,
+			       c->iface_hw_list[i].hw_chans_idx))
+			return -ENOBUFS;
+
+		limits = nla_nest_start(msg,
+					NL80211_IFACE_COMB_PER_HW_COMB_LIMITS);
+		if (!limits)
+			return -ENOBUFS;
+
+		for (l = 0; l < c->iface_hw_list->n_limits; l++) {
+			struct nlattr *limit;
+
+			limit = nla_nest_start(msg, l + 1);
+			if (!limit)
+				return -ENOBUFS;
+
+			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_MAX,
+					c->iface_hw_list[i].limits[l].max))
+				return -ENOBUFS;
+
+			if (nla_put_u16(msg, NL80211_IFACE_LIMIT_TYPES,
+					c->iface_hw_list[i].limits[l].types))
+				return -ENOBUFS;
+
+			nla_nest_end(msg, limit);
+		}
+		nla_nest_end(msg, limits);
+
+		if (nla_put_u32(msg,
+				NL80211_IFACE_COMB_PER_HW_COMB_NUM_CHANNELS,
+				c->iface_hw_list[i].num_different_channels))
+			return -ENOBUFS;
+
+		if (nla_put_u16(msg,
+				NL80211_IFACE_COMB_PER_HW_COMB_MAXIMUM,
+				c->iface_hw_list[i].max_interfaces))
+			return -ENOBUFS;
+
+		nla_nest_end(msg, hw_combi);
+	}
+
+	nla_nest_end(msg, hw_combis);
+
+	return 0;
+}
+
 static int nl80211_put_iface_combinations(struct wiphy *wiphy,
 					  struct sk_buff *msg,
 					  bool large)
@@ -1658,6 +1726,9 @@  static int nl80211_put_iface_combinations(struct wiphy *wiphy,
 				c->beacon_int_min_gcd))
 			goto nla_put_failure;
 
+		if (large && nl80211_put_per_hw_iface_combinations(wiphy, msg, c))
+			goto nla_put_failure;
+
 		nla_nest_end(msg, nl_combi);
 	}