diff mbox series

[1/3] wifi: cfg80211: Extend cfg80211_new_sta() for MLD AP

Message ID 20221206080226.1702646-2-quic_vjakkam@quicinc.com
State New
Headers show
Series [1/3] wifi: cfg80211: Extend cfg80211_new_sta() for MLD AP | expand

Commit Message

Veerendranath Jakkam Dec. 6, 2022, 8:02 a.m. UTC
Add support for drivers to indicate STA connection when user space SME
(e.g., hostapd) is not used for MLD AP. Add new parameters in struct
station_info to provide (re)association link ID, station's MLD address
and (Re)Association Response IEs in cfg80211_new_sta().

Signed-off-by: Veerendranath Jakkam <quic_vjakkam@quicinc.com>
---
 include/net/cfg80211.h | 22 ++++++++++++++++++++++
 net/wireless/nl80211.c | 16 ++++++++++++++++
 2 files changed, 38 insertions(+)

Comments

Johannes Berg Jan. 18, 2023, 3:41 p.m. UTC | #1
Hi,

> + * @mlo_params_valid: Indicates @assoc_link_id and @mld_addr fields are valid.
> + *	Drivers use this only in cfg80211_new_sta() calls when AP MLD's MLME/SME
> + *	is offload to driver. Drivers won't fill this information in
> + *	cfg80211_del_sta_sinfo(), get_station() and dump_station() callbacks.
> 
> + * @mld_addr: MLD address if the station is an MLD. Otherwise, set to all zeros.


> +	if (sinfo->mlo_params_valid) {
> +		if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
> +			       sinfo->assoc_link_id))
> +			goto nla_put_failure;
> +
> +		if (!is_zero_ether_addr(sinfo->mld_addr) &&
> +		    nla_put(msg, NL80211_ATTR_MLD_ADDR, ETH_ALEN,
> +			    sinfo->mld_addr))
> 


It'd be invalid, but now you could have mlo_params_valid == true &&
is_zero_ether_addr(sinfo->mld_addr), which would lead to a very strange
situation for userspace, it would see a link ID but no MLD address.

With the documented requirement that
	mlo_params_valid == (mld_addr is valid)

wouldn't it make sense to just remove mlo_params_valid, and lift the
is_zero_ether_addr() check to the outer condition?

johannes
Veerendranath Jakkam Jan. 21, 2023, 8:55 a.m. UTC | #2
Thanks Johannes for reviewing the patches.


On 1/18/2023 9:11 PM, Johannes Berg wrote:
> Hi,
>
>> + * @mlo_params_valid: Indicates @assoc_link_id and @mld_addr fields are valid.
>> + *	Drivers use this only in cfg80211_new_sta() calls when AP MLD's MLME/SME
>> + *	is offload to driver. Drivers won't fill this information in
>> + *	cfg80211_del_sta_sinfo(), get_station() and dump_station() callbacks.
>>
>> + * @mld_addr: MLD address if the station is an MLD. Otherwise, set to all zeros.
>
>> +	if (sinfo->mlo_params_valid) {
>> +		if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
>> +			       sinfo->assoc_link_id))
>> +			goto nla_put_failure;
>> +
>> +		if (!is_zero_ether_addr(sinfo->mld_addr) &&
>> +		    nla_put(msg, NL80211_ATTR_MLD_ADDR, ETH_ALEN,
>> +			    sinfo->mld_addr))
>>
>
> It'd be invalid, but now you could have mlo_params_valid == true &&
> is_zero_ether_addr(sinfo->mld_addr), which would lead to a very strange
> situation for userspace, it would see a link ID but no MLD address.


Only link ID and no MLD address also a valid combination. It indicates 
the connected STA is non-MLD.  User space needs the link ID information 
to determine "STA connected to which affiliated AP of the AP MLD".

>
> With the documented requirement that
> 	mlo_params_valid == (mld_addr is valid)


No, I think there is some misinterpretation of the documentation here. 
Let me submit new patch-set after adding more detailed text in the 
documentation.

Actually what I mean,  cfg80211 can consider parsing both the 
"assoc_link_id" and "mld_addr" fields when "mlo_params_valid" is true. 
"assoc_link_id" will be set to link ID of the affiliated AP on which STA 
(MLD/non-MLD) completed association. "mld_addr" will be set to MLD 
address of the STA if applicable (i.e., MLD STA) . Otherwise, will be 
set to zeros (i.e., non-MLD STA).

--

veeru
diff mbox series

Patch

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 03d4f4deadae..15619e0f5f0b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1882,6 +1882,22 @@  struct cfg80211_tid_stats {
  *	received packet with an FCS error matches the peer MAC address.
  * @airtime_link_metric: mesh airtime link metric.
  * @connected_to_as: true if mesh STA has a path to authentication server
+ * @mlo_params_valid: Indicates @assoc_link_id and @mld_addr fields are valid.
+ *	Drivers use this only in cfg80211_new_sta() calls when AP MLD's MLME/SME
+ *	is offload to driver. Drivers won't fill this information in
+ *	cfg80211_del_sta_sinfo(), get_station() and dump_station() callbacks.
+ * @assoc_link_id: Indicates link ID of the AP MLD link on which the station
+ *	association happened.
+ * @mld_addr: MLD address if the station is an MLD. Otherwise, set to all zeros.
+ * @assoc_resp_ies: IEs from (Re)Association Response.
+ *	This is used only when in AP mode with drivers that do not use user
+ *	space MLME/SME implementation. The information is provided only for the
+ *	cfg80211_new_sta() calls to notify user space of the IEs. Drivers won't
+ *	fill this information in cfg80211_del_sta_sinfo(), get_station() and
+ *	dump_station() callbacks. Userspace needs this information to determine
+ *	the other affiliated link stations information when MLD station is
+ *	connected.
+ * @assoc_resp_ies_len: Length of @assoc_resp_ies buffer in octets.
  */
 struct station_info {
 	u64 filled;
@@ -1941,6 +1957,12 @@  struct station_info {
 	u32 airtime_link_metric;
 
 	u8 connected_to_as;
+
+	bool mlo_params_valid;
+	u8 assoc_link_id;
+	u8 mld_addr[ETH_ALEN] __aligned(2);
+	const u8 *assoc_resp_ies;
+	size_t assoc_resp_ies_len;
 };
 
 /**
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 33a82ecab9d5..5a20bf8b85d6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -6527,6 +6527,22 @@  static int nl80211_send_station(struct sk_buff *msg, u32 cmd, u32 portid,
 		    sinfo->assoc_req_ies))
 		goto nla_put_failure;
 
+	if (sinfo->assoc_resp_ies_len &&
+	    nla_put(msg, NL80211_ATTR_RESP_IE, sinfo->assoc_resp_ies_len,
+		    sinfo->assoc_resp_ies))
+		goto nla_put_failure;
+
+	if (sinfo->mlo_params_valid) {
+		if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
+			       sinfo->assoc_link_id))
+			goto nla_put_failure;
+
+		if (!is_zero_ether_addr(sinfo->mld_addr) &&
+		    nla_put(msg, NL80211_ATTR_MLD_ADDR, ETH_ALEN,
+			    sinfo->mld_addr))
+			goto nla_put_failure;
+	}
+
 	cfg80211_sinfo_release_content(sinfo);
 	genlmsg_end(msg, hdr);
 	return 0;