diff mbox series

[61/76] wifi: nl80211: add EML/MLD capabilities to per-iftype capabilities

Message ID 20220713114426.4dfc9ebd0461.Ice7b841051cfeb23da17bb2caa0e45191b34c4db@changeid
State New
Headers show
Series wifi: more MLO work | expand

Commit Message

Johannes Berg July 13, 2022, 9:44 a.m. UTC
From: Johannes Berg <johannes.berg@intel.com>

We have the per-interface type capabilities, currently for
extended capabilities, add the EML/MLD capabilities there
to have this advertised by the driver.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h       |  4 ++++
 include/uapi/linux/nl80211.h | 12 ++++++++++--
 net/wireless/nl80211.c       |  9 +++++++++
 3 files changed, 23 insertions(+), 2 deletions(-)

Comments

Wen Gong Aug. 11, 2023, 3:51 a.m. UTC | #1
On 7/13/2022 5:44 PM, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> We have the per-interface type capabilities, currently for
> extended capabilities, add the EML/MLD capabilities there
> to have this advertised by the driver.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>   include/net/cfg80211.h       |  4 ++++
>   include/uapi/linux/nl80211.h | 12 ++++++++++--
>   net/wireless/nl80211.c       |  9 +++++++++
>   3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index ca26e3b7341a..44abaa9d74ea 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -4993,12 +4993,16 @@ struct wiphy_vendor_command {
>    *	802.11-2012 8.4.2.29 for the defined fields.
>    * @extended_capabilities_mask: mask of the valid values
>    * @extended_capabilities_len: length of the extended capabilities
> + * @eml_capabilities: EML capabilities (for MLO)
> + * @mld_capa_and_ops: MLD capabilities and operations (for MLO)
>    */
>   struct wiphy_iftype_ext_capab {
>   	enum nl80211_iftype iftype;
>   	const u8 *extended_capabilities;
>   	const u8 *extended_capabilities_mask;
>   	u8 extended_capabilities_len;
> +	u16 eml_capabilities;
> +	u16 mld_capa_and_ops;
>   };
>   

Now there are many nl80211_band such as NL80211_BAND_2GHZ/
NL80211_BAND_5GHZ/NL80211_BAND_6GHZ... In the same interface, if some bands
support EML, and other bands not support EML, then how to handler this
case?

Could move the "u16 eml_capabilities" into struct 
ieee80211_sband_iftype_data
as well as "struct ieee80211_sta_eht_cap eht_cap"? Also same for "u16
mld_capa_and_ops".

[...]
Johannes Berg Aug. 11, 2023, 9:03 a.m. UTC | #2
On Fri, 2023-08-11 at 11:51 +0800, Wen Gong wrote:
> Now there are many nl80211_band such as NL80211_BAND_2GHZ/
> NL80211_BAND_5GHZ/NL80211_BAND_6GHZ... In the same interface, if some bands
> support EML, and other bands not support EML, then how to handler this
> case?

But ... these are MLD capabilities, not of the (associated) STA?

So not sure how that would make sense? What would you even _do_ with
that?

johannes
Wen Gong Aug. 11, 2023, 9:05 a.m. UTC | #3
On 8/11/2023 5:03 PM, Johannes Berg wrote:
> On Fri, 2023-08-11 at 11:51 +0800, Wen Gong wrote:
>> Now there are many nl80211_band such as NL80211_BAND_2GHZ/
>> NL80211_BAND_5GHZ/NL80211_BAND_6GHZ... In the same interface, if some bands
>> support EML, and other bands not support EML, then how to handler this
>> case?
> But ... these are MLD capabilities, not of the (associated) STA?
Yes, I know it.
>
> So not sure how that would make sense? What would you even _do_ with
> that?
I think another change is not move "u16 eml_capabilities", and only add 
a new
field "u16 eml_supp_bands" together with the "u16 eml_capabilities", the
eml_supp_bands is filled with the bit map of enum nl80211_band. Is that OK?
>
> johannes
Johannes Berg Aug. 11, 2023, 9:08 a.m. UTC | #4
On Fri, 2023-08-11 at 17:05 +0800, Wen Gong wrote:
> On 8/11/2023 5:03 PM, Johannes Berg wrote:
> > On Fri, 2023-08-11 at 11:51 +0800, Wen Gong wrote:
> > > Now there are many nl80211_band such as NL80211_BAND_2GHZ/
> > > NL80211_BAND_5GHZ/NL80211_BAND_6GHZ... In the same interface, if some bands
> > > support EML, and other bands not support EML, then how to handler this
> > > case?
> > But ... these are MLD capabilities, not of the (associated) STA?
> Yes, I know it.

Then you can't honestly be suggesting we move "MLD capa and ops" into
per-band, no?

> > So not sure how that would make sense? What would you even _do_ with
> > that?

> I think another change is not move "u16 eml_capabilities", and only add 
> a new
> field "u16 eml_supp_bands" together with the "u16 eml_capabilities", the
> eml_supp_bands is filled with the bit map of enum nl80211_band. Is that OK?
> 

And again, what would you do with it? Not advertise EML when you have
selected links that are not in the map? And if the links change
dynamically in the future? You'd probably need a bunch of validation for
that.

And usually you'd probably always connect to all the bands?

I really don't get it.

Btw this is all client - so your client implementation can just not send
the EML action frame (forgot the name right now) when the currently
active links are not compatible.

johannes
Wen Gong Aug. 11, 2023, 9:24 a.m. UTC | #5
On 8/11/2023 5:08 PM, Johannes Berg wrote:
> On Fri, 2023-08-11 at 17:05 +0800, Wen Gong wrote:
>> On 8/11/2023 5:03 PM, Johannes Berg wrote:
>>> On Fri, 2023-08-11 at 11:51 +0800, Wen Gong wrote:
>>>> Now there are many nl80211_band such as NL80211_BAND_2GHZ/
>>>> NL80211_BAND_5GHZ/NL80211_BAND_6GHZ... In the same interface, if some bands
>>>> support EML, and other bands not support EML, then how to handler this
>>>> case?
>>> But ... these are MLD capabilities, not of the (associated) STA?
>> Yes, I know it.
> Then you can't honestly be suggesting we move "MLD capa and ops" into
> per-band, no?
I know it just now after I send the suggestion😁
>>> So not sure how that would make sense? What would you even _do_ with
>>> that?
>> I think another change is not move "u16 eml_capabilities", and only add
>> a new
>> field "u16 eml_supp_bands" together with the "u16 eml_capabilities", the
>> eml_supp_bands is filled with the bit map of enum nl80211_band. Is that OK?
>>
> And again, what would you do with it? Not advertise EML when you have
> selected links that are not in the map? And if the links change
> dynamically in the future? You'd probably need a bunch of validation for
> that.
For example, for station, NOT support EML on 2 GHz, support EML on 5 
GHz/6 GHz.

When connect to 2/3 links AP, then EML should NOT add in assoc req while 
connect
to AP which is 2 GHz+5 GHz or 2 GHz+6 GHz or 2 GHz+5 GHz+6 GHz.

EML should add in assoc req while connect to AP which is 5 GHz+6 GHz.

For dynamic link change, it is another new topic, so I have not good advise
for that.😁
>
> And usually you'd probably always connect to all the bands?
>
> I really don't get it.

Sometimes AP is 3 links (2 GHz+5 GHz+6 GHz), but station only support 2 
links,

then station will not connect all bands, station need choose 2 bands to 
connect.

>
> Btw this is all client - so your client implementation can just not send
> the EML action frame (forgot the name right now) when the currently
> active links are not compatible.
I guess you are refer to "EML Operating Mode Notification frame".
> johannes
Johannes Berg Aug. 11, 2023, 9:27 a.m. UTC | #6
On Fri, 2023-08-11 at 17:24 +0800, Wen Gong wrote:
> > 
> > And again, what would you do with it? Not advertise EML when you have
> > selected links that are not in the map? And if the links change
> > dynamically in the future? You'd probably need a bunch of validation for
> > that.
> For example, for station, NOT support EML on 2 GHz, support EML on 5 
> GHz/6 GHz.
> 
> When connect to 2/3 links AP, then EML should NOT add in assoc req while 
> connect to AP which is 2 GHz+5 GHz or 2 GHz+6 GHz or 2 GHz+5 GHz+6 GHz.
> 
> EML should add in assoc req while connect to AP which is 5 GHz+6 GHz.

OK, at least that answers the question what you'd want to do with it :)

> For dynamic link change, it is another new topic, so I have not good advise
> for that.😁

But everyone's looking at that?

Honestly I'm not even sure - and you might know better - what the EML
capabilities matter for if you never enable EML.

So almost feels like if you never send the EML OMN frame (thanks for the
name!) the AP wouldn't really care, and that's something you could do
very easily without changing it, i.e. you pretend that your 2.4 GHz
actually has EML, you just never enable it in case 2.4 GHz is active?

johannes
Wen Gong Aug. 11, 2023, 9:54 a.m. UTC | #7
On 8/11/2023 5:27 PM, Johannes Berg wrote:
> On Fri, 2023-08-11 at 17:24 +0800, Wen Gong wrote:
>
[...]
> OK, at least that answers the question what you'd want to do with it :)
>
>> For dynamic link change, it is another new topic, so I have not good advise
>> for that.😁
> But everyone's looking at that?
I see there hare some patches in mac80211/cfg80211 for dynamic link 
removal such as
"wifi: mac80211: Support link removal using Reconfiguration ML element". 
For link
removal, it does not effect the EML issue I said. Only link add 
operation without (re)assoc
(35.3.6 ML reconfiguration of IEEE P802.11be™/D3.2 disallow (re)assoc 
for link add)
will impact the EML issue I said. But I did not see dynamic link add 
patch now. Will
you also do the patches for dynamic link add without (re)assoc?
>
> Honestly I'm not even sure - and you might know better - what the EML
> capabilities matter for if you never enable EML.
>
> So almost feels like if you never send the EML OMN frame (thanks for the
> name!) the AP wouldn't really care, and that's something you could do
> very easily without changing it, i.e. you pretend that your 2.4 GHz
> actually has EML, you just never enable it in case 2.4 GHz is active?
Your advise maybe feasible. Then no change is needed in nl80211 here for 
the EML.
> johannes
diff mbox series

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ca26e3b7341a..44abaa9d74ea 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -4993,12 +4993,16 @@  struct wiphy_vendor_command {
  *	802.11-2012 8.4.2.29 for the defined fields.
  * @extended_capabilities_mask: mask of the valid values
  * @extended_capabilities_len: length of the extended capabilities
+ * @eml_capabilities: EML capabilities (for MLO)
+ * @mld_capa_and_ops: MLD capabilities and operations (for MLO)
  */
 struct wiphy_iftype_ext_capab {
 	enum nl80211_iftype iftype;
 	const u8 *extended_capabilities;
 	const u8 *extended_capabilities_mask;
 	u8 extended_capabilities_len;
+	u16 eml_capabilities;
+	u16 mld_capa_and_ops;
 };
 
 /**
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 37bfc934325a..3fa586e38f88 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2368,8 +2368,10 @@  enum nl80211_commands {
  *
  * @NL80211_ATTR_IFTYPE_EXT_CAPA: Nested attribute of the following attributes:
  *	%NL80211_ATTR_IFTYPE, %NL80211_ATTR_EXT_CAPA,
- *	%NL80211_ATTR_EXT_CAPA_MASK, to specify the extended capabilities per
- *	interface type.
+ *	%NL80211_ATTR_EXT_CAPA_MASK, to specify the extended capabilities and
+ *	other interface-type specific capabilities per interface type. For MLO,
+ *	%NL80211_ATTR_EML_CAPABILITY and %NL80211_ATTR_MLD_CAPA_AND_OPS are
+ *	present.
  *
  * @NL80211_ATTR_MU_MIMO_GROUP_DATA: array of 24 bytes that defines a MU-MIMO
  *	groupID for monitor mode.
@@ -2709,6 +2711,9 @@  enum nl80211_commands {
  *	suites allowed as %NL80211_MAX_NR_AKM_SUITES which is the legacy maximum
  *	number prior to the introduction of this attribute.
  *
+ * @NL80211_ATTR_EML_CAPABILITY: EML Capability information (u16)
+ * @NL80211_ATTR_MLD_CAPA_AND_OPS: MLD Capabilities and Operations (u16)
+ *
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3231,6 +3236,9 @@  enum nl80211_attrs {
 
 	NL80211_ATTR_MAX_NUM_AKM_SUITES,
 
+	NL80211_ATTR_EML_CAPABILITY,
+	NL80211_ATTR_MLD_CAPA_AND_OPS,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 37ec8b3897b4..35fb2b0517d9 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2867,6 +2867,15 @@  static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev,
 					    capab->extended_capabilities_mask))
 					goto nla_put_failure;
 
+				if (rdev->wiphy.flags & WIPHY_FLAG_SUPPORTS_MLO &&
+				    (nla_put_u16(msg,
+						 NL80211_ATTR_EML_CAPABILITY,
+						 capab->eml_capabilities) ||
+				     nla_put_u16(msg,
+						 NL80211_ATTR_MLD_CAPA_AND_OPS,
+						 capab->mld_capa_and_ops)))
+					goto nla_put_failure;
+
 				nla_nest_end(msg, nested_ext_capab);
 				if (state->split)
 					break;