Linux-Wireless Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] wifi: mac80211: element parsing cleanups
@ 2024-02-28  8:48 Johannes Berg
  2024-02-28  8:48 ` [PATCH 1/8] wifi: mac80211: update scratch_pos after defrag Johannes Berg
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless

There are some small issues in how element parsing works, and
I'm going to reuse it a bit differently for CSA. But before,
clean it up a bit, and in particular hide the scratch buffer,
which was used oddly in a few places.

johannes


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/8] wifi: mac80211: update scratch_pos after defrag
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 2/8] wifi: mac80211: remove unnecessary ML element type check Johannes Berg
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless
  Cc: Johannes Berg, Benjamin Berg, Ilan Peer, Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

The scratch_pos update here was lost after defrag, so any
other uses of the scratch buffer might overwrite it.

Fixes: a286de1aa38f ("wifi: mac80211: Rename multi_link")
Reviewed-by: Benjamin Berg <benjamin.berg@intel.com>
Reviewed-by: Ilan Peer <ilan.peer@intel.com>
Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/parse.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 196a882e4c19..233c761823d3 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -800,6 +800,7 @@ static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
 
 	elems->ml_basic = (const void *)elems->scratch_pos;
 	elems->ml_basic_len = ml_len;
+	elems->scratch_pos += ml_len;
 
 	ieee80211_mle_get_sta_prof(elems, params->link_id);
 	prof = elems->prof;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/8] wifi: mac80211: remove unnecessary ML element type check
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
  2024-02-28  8:48 ` [PATCH 1/8] wifi: mac80211: update scratch_pos after defrag Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 3/8] wifi: mac80211: add ieee80211_vif_link_active() helper Johannes Berg
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless; +Cc: Johannes Berg, Ilan Peer, Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

At this point, since it's taken from elems->ml_basic which
is stored only if it's of type basic, we don't really need
to check again if it's basic.

Reviewed-by: Ilan Peer <ilan.peer@intel.com>
Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/parse.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 233c761823d3..ae0f14bd952a 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -723,10 +723,6 @@ static void ieee80211_mle_get_sta_prof(struct ieee802_11_elems *elems,
 	if (!ml || !ml_len)
 		return;
 
-	if (le16_get_bits(ml->control, IEEE80211_ML_CONTROL_TYPE) !=
-	    IEEE80211_ML_CONTROL_TYPE_BASIC)
-		return;
-
 	for_each_mle_subelement(sub, (u8 *)ml, ml_len) {
 		struct ieee80211_mle_per_sta_profile *prof = (void *)sub->data;
 		ssize_t sta_prof_len;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/8] wifi: mac80211: add ieee80211_vif_link_active() helper
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
  2024-02-28  8:48 ` [PATCH 1/8] wifi: mac80211: update scratch_pos after defrag Johannes Berg
  2024-02-28  8:48 ` [PATCH 2/8] wifi: mac80211: remove unnecessary ML element type check Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 4/8] wifi: mac80211: remove unnecessary ML element checks Johannes Berg
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless
  Cc: Johannes Berg, Ilan Peer, Emmanuel Grumbach,
	Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

We sometimes need to check if a link is active, and this
is complicated by the fact that active_links has no bits
set when the vif isn't (acting as) an MLD. Add a small
new helper ieee80211_vif_link_active() to make that a bit
easier, and use it in a few places.

Reviewed-by: Ilan Peer <ilan.peer@intel.com>
Reviewed-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/mac80211.h    | 15 +++++++++++++++
 net/mac80211/cfg.c        |  3 +--
 net/mac80211/chan.c       |  3 +--
 net/mac80211/driver-ops.c | 14 +++++---------
 net/mac80211/util.c       |  3 +--
 5 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 34d66d0a24b1..6c6d8210d637 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2011,6 +2011,21 @@ static inline bool ieee80211_vif_is_mld(const struct ieee80211_vif *vif)
 	return vif->valid_links != 0;
 }
 
+/**
+ * ieee80211_vif_link_active - check if a given link is active
+ * @vif: the vif
+ * @link_id: the link ID to check
+ * Return: %true if the vif is an MLD and the link is active, or if
+ *	the vif is not an MLD and the link ID is 0; %false otherwise.
+ */
+static inline bool ieee80211_vif_link_active(const struct ieee80211_vif *vif,
+					     unsigned int link_id)
+{
+	if (!ieee80211_vif_is_mld(vif))
+		return link_id == 0;
+	return vif->active_links & BIT(link_id);
+}
+
 #define for_each_vif_active_link(vif, link, link_id)				\
 	for (link_id = 0; link_id < ARRAY_SIZE((vif)->link_conf); link_id++)	\
 		if ((!(vif)->active_links ||					\
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e57ba4f7a589..821a83e487df 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3155,8 +3155,7 @@ int __ieee80211_request_smps_mgd(struct ieee80211_sub_if_data *sdata,
 	if (WARN_ON_ONCE(sdata->vif.type != NL80211_IFTYPE_STATION))
 		return -EINVAL;
 
-	if (ieee80211_vif_is_mld(&sdata->vif) &&
-	    !(sdata->vif.active_links & BIT(link->link_id)))
+	if (!ieee80211_vif_link_active(&sdata->vif, link->link_id))
 		return 0;
 
 	old_req = link->u.mgd.req_smps;
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 38acdc458c7c..80e4b9784131 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -1701,8 +1701,7 @@ int ieee80211_link_use_channel(struct ieee80211_link_data *link,
 
 	lockdep_assert_wiphy(local->hw.wiphy);
 
-	if (sdata->vif.active_links &&
-	    !(sdata->vif.active_links & BIT(link->link_id))) {
+	if (!ieee80211_vif_link_active(&sdata->vif, link->link_id)) {
 		ieee80211_link_update_chanreq(link, chanreq);
 		return 0;
 	}
diff --git a/net/mac80211/driver-ops.c b/net/mac80211/driver-ops.c
index 3b7f70073fc3..dce37ba8ebe3 100644
--- a/net/mac80211/driver-ops.c
+++ b/net/mac80211/driver-ops.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright 2015 Intel Deutschland GmbH
- * Copyright (C) 2022-2023 Intel Corporation
+ * Copyright (C) 2022-2024 Intel Corporation
  */
 #include <net/mac80211.h>
 #include "ieee80211_i.h"
@@ -214,8 +214,7 @@ int drv_conf_tx(struct ieee80211_local *local,
 	if (!check_sdata_in_driver(sdata))
 		return -EIO;
 
-	if (sdata->vif.active_links &&
-	    !(sdata->vif.active_links & BIT(link->link_id)))
+	if (!ieee80211_vif_link_active(&sdata->vif, link->link_id))
 		return 0;
 
 	if (params->cw_min == 0 || params->cw_min > params->cw_max) {
@@ -315,8 +314,7 @@ int drv_assign_vif_chanctx(struct ieee80211_local *local,
 	if (!check_sdata_in_driver(sdata))
 		return -EIO;
 
-	if (sdata->vif.active_links &&
-	    !(sdata->vif.active_links & BIT(link_conf->link_id)))
+	if (!ieee80211_vif_link_active(&sdata->vif, link_conf->link_id))
 		return 0;
 
 	trace_drv_assign_vif_chanctx(local, sdata, link_conf, ctx);
@@ -343,8 +341,7 @@ void drv_unassign_vif_chanctx(struct ieee80211_local *local,
 	if (!check_sdata_in_driver(sdata))
 		return;
 
-	if (sdata->vif.active_links &&
-	    !(sdata->vif.active_links & BIT(link_conf->link_id)))
+	if (!ieee80211_vif_link_active(&sdata->vif, link_conf->link_id))
 		return;
 
 	trace_drv_unassign_vif_chanctx(local, sdata, link_conf, ctx);
@@ -461,8 +458,7 @@ void drv_link_info_changed(struct ieee80211_local *local,
 	if (!check_sdata_in_driver(sdata))
 		return;
 
-	if (sdata->vif.active_links &&
-	    !(sdata->vif.active_links & BIT(link_id)))
+	if (!ieee80211_vif_link_active(&sdata->vif, link_id))
 		return;
 
 	trace_drv_link_info_changed(local, sdata, info, changed);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 627bd5a8bda5..d7c4d162321f 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1935,8 +1935,7 @@ int ieee80211_reconfig(struct ieee80211_local *local)
 		for (link_id = 0;
 		     link_id < ARRAY_SIZE(sdata->vif.link_conf);
 		     link_id++) {
-			if (ieee80211_vif_is_mld(&sdata->vif) &&
-			    !(sdata->vif.active_links & BIT(link_id)))
+			if (!ieee80211_vif_link_active(&sdata->vif, link_id))
 				continue;
 
 			link = sdata_dereference(sdata->link[link_id], sdata);
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/8] wifi: mac80211: remove unnecessary ML element checks
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
                   ` (2 preceding siblings ...)
  2024-02-28  8:48 ` [PATCH 3/8] wifi: mac80211: add ieee80211_vif_link_active() helper Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 5/8] wifi: mac80211: simplify multi-link element parsing Johannes Berg
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless; +Cc: Johannes Berg, Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

Given the prior changes to ieee80211_mle_size_ok(), we
can now pass NULL to for_each_mle_subelement(), so no
longer need to check for that here explicitly.

Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/parse.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index ae0f14bd952a..d231aaecc219 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -720,9 +720,6 @@ static void ieee80211_mle_get_sta_prof(struct ieee802_11_elems *elems,
 	ssize_t ml_len = elems->ml_basic_len;
 	const struct element *sub;
 
-	if (!ml || !ml_len)
-		return;
-
 	for_each_mle_subelement(sub, (u8 *)ml, ml_len) {
 		struct ieee80211_mle_per_sta_profile *prof = (void *)sub->data;
 		ssize_t sta_prof_len;
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/8] wifi: mac80211: simplify multi-link element parsing
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
                   ` (3 preceding siblings ...)
  2024-02-28  8:48 ` [PATCH 4/8] wifi: mac80211: remove unnecessary ML element checks Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 6/8] wifi: mac80211: defragment reconfiguration MLE when parsing Johannes Berg
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless; +Cc: Johannes Berg, Ilan Peer, Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

We shouldn't assign elems->ml_basic{,len} before defragmentation,
and we don't need elems->ml_reconf{,len} at all since we don't do
defragmentation. Clean that up a bit. This does require always
defragmention even when it may not be needed, but that's easier
to reason about.

Reviewed-by: Ilan Peer <ilan.peer@intel.com>
Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ieee80211_i.h |  6 ++----
 net/mac80211/mlme.c        |  6 ++----
 net/mac80211/parse.c       | 12 ++++--------
 3 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4bec625a84d1..e8ca9ad12e62 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1736,7 +1736,6 @@ struct ieee802_11_elems {
 	const struct ieee80211_eht_cap_elem *eht_cap;
 	const struct ieee80211_eht_operation *eht_operation;
 	const struct ieee80211_multi_link_elem *ml_basic;
-	const struct ieee80211_multi_link_elem *ml_reconf;
 	const struct ieee80211_bandwidth_indication *bandwidth_indication;
 	const struct ieee80211_ttlm_elem *ttlm[IEEE80211_TTLM_MAX_CNT];
 
@@ -1764,12 +1763,11 @@ struct ieee802_11_elems {
 
 	/* mult-link element can be de-fragmented and thus u8 is not sufficient */
 	size_t ml_basic_len;
-	size_t ml_reconf_len;
 
-	/* The basic Multi-Link element in the original IEs */
+	/* The basic Multi-Link element in the original elements */
 	const struct element *ml_basic_elem;
 
-	/* The reconfiguration Multi-Link element in the original IEs */
+	/* The reconfiguration Multi-Link element in the original elements */
 	const struct element *ml_reconf_elem;
 
 	u8 ttlm_num;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index d5678a817771..f4544f167005 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5760,7 +5760,7 @@ static void ieee80211_ml_reconfiguration(struct ieee80211_sub_if_data *sdata,
 	u8 link_id;
 	u32 delay;
 
-	if (!ieee80211_vif_is_mld(&sdata->vif) || !elems->ml_reconf)
+	if (!ieee80211_vif_is_mld(&sdata->vif) || !elems->ml_reconf_elem)
 		return;
 
 	ml_len = cfg80211_defragment_element(elems->ml_reconf_elem,
@@ -5773,9 +5773,7 @@ static void ieee80211_ml_reconfiguration(struct ieee80211_sub_if_data *sdata,
 	if (ml_len < 0)
 		return;
 
-	elems->ml_reconf = (const void *)elems->scratch_pos;
-	elems->ml_reconf_len = ml_len;
-	ml = elems->ml_reconf;
+	ml = (const void *)elems->scratch_pos;
 
 	/* Directly parse the sub elements as the common information doesn't
 	 * hold any useful information.
diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index d231aaecc219..8bdf6e7efa58 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -129,19 +129,15 @@ ieee80211_parse_extension_element(u32 *crc,
 			switch (le16_get_bits(mle->control,
 					      IEEE80211_ML_CONTROL_TYPE)) {
 			case IEEE80211_ML_CONTROL_TYPE_BASIC:
-				if (elems->ml_basic) {
+				if (elems->ml_basic_elem) {
 					elems->parse_error |=
 						IEEE80211_PARSE_ERR_DUP_NEST_ML_BASIC;
 					break;
 				}
 				elems->ml_basic_elem = (void *)elem;
-				elems->ml_basic = data;
-				elems->ml_basic_len = len;
 				break;
 			case IEEE80211_ML_CONTROL_TYPE_RECONF:
 				elems->ml_reconf_elem = (void *)elem;
-				elems->ml_reconf = data;
-				elems->ml_reconf_len = len;
 				break;
 			default:
 				break;
@@ -776,9 +772,6 @@ static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
 	const struct element *non_inherit = NULL;
 	const u8 *end;
 
-	if (params->link_id == -1)
-		return;
-
 	ml_len = cfg80211_defragment_element(elems->ml_basic_elem,
 					     elems->ie_start,
 					     elems->total_len,
@@ -795,6 +788,9 @@ static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
 	elems->ml_basic_len = ml_len;
 	elems->scratch_pos += ml_len;
 
+	if (params->link_id == -1)
+		return;
+
 	ieee80211_mle_get_sta_prof(elems, params->link_id);
 	prof = elems->prof;
 
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/8] wifi: mac80211: defragment reconfiguration MLE when parsing
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
                   ` (4 preceding siblings ...)
  2024-02-28  8:48 ` [PATCH 5/8] wifi: mac80211: simplify multi-link element parsing Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 7/8] wifi: mac80211: remove unneeded scratch_len subtraction Johannes Berg
  2024-02-28  8:48 ` [PATCH 8/8] wifi: mac80211: hide element parsing internals Johannes Berg
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless; +Cc: Johannes Berg, Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

Using the scratch buffer (without advancing it) here in the
mlme.c code seems somewhat wrong, defragment the reconfig
multi-link element already when parsing. This might be a bit
more work in certain cases, but makes the whole thing more
regular.

Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ieee80211_i.h |  2 ++
 net/mac80211/mlme.c        | 19 +++----------------
 net/mac80211/parse.c       | 22 ++++++++++++++++++++++
 3 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e8ca9ad12e62..768f614731a7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1736,6 +1736,7 @@ struct ieee802_11_elems {
 	const struct ieee80211_eht_cap_elem *eht_cap;
 	const struct ieee80211_eht_operation *eht_operation;
 	const struct ieee80211_multi_link_elem *ml_basic;
+	const struct ieee80211_multi_link_elem *ml_reconf;
 	const struct ieee80211_bandwidth_indication *bandwidth_indication;
 	const struct ieee80211_ttlm_elem *ttlm[IEEE80211_TTLM_MAX_CNT];
 
@@ -1763,6 +1764,7 @@ struct ieee802_11_elems {
 
 	/* mult-link element can be de-fragmented and thus u8 is not sufficient */
 	size_t ml_basic_len;
+	size_t ml_reconf_len;
 
 	/* The basic Multi-Link element in the original elements */
 	const struct element *ml_basic_elem;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index f4544f167005..e6a82f28a7c6 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -5752,33 +5752,20 @@ static void ieee80211_ml_reconf_work(struct wiphy *wiphy,
 static void ieee80211_ml_reconfiguration(struct ieee80211_sub_if_data *sdata,
 					 struct ieee802_11_elems *elems)
 {
-	const struct ieee80211_multi_link_elem *ml;
 	const struct element *sub;
-	ssize_t ml_len;
 	unsigned long removed_links = 0;
 	u16 link_removal_timeout[IEEE80211_MLD_MAX_NUM_LINKS] = {};
 	u8 link_id;
 	u32 delay;
 
-	if (!ieee80211_vif_is_mld(&sdata->vif) || !elems->ml_reconf_elem)
+	if (!ieee80211_vif_is_mld(&sdata->vif) || !elems->ml_reconf)
 		return;
 
-	ml_len = cfg80211_defragment_element(elems->ml_reconf_elem,
-					     elems->ie_start,
-					     elems->total_len,
-					     elems->scratch_pos,
-					     elems->scratch + elems->scratch_len -
-					     elems->scratch_pos,
-					     WLAN_EID_FRAGMENT);
-	if (ml_len < 0)
-		return;
-
-	ml = (const void *)elems->scratch_pos;
-
 	/* Directly parse the sub elements as the common information doesn't
 	 * hold any useful information.
 	 */
-	for_each_mle_subelement(sub, (u8 *)ml, ml_len) {
+	for_each_mle_subelement(sub, (const u8 *)elems->ml_reconf,
+				elems->ml_reconf_len) {
 		struct ieee80211_mle_per_sta_profile *prof = (void *)sub->data;
 		u8 *pos = prof->variable;
 		u16 control;
diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 8bdf6e7efa58..804323858977 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -819,6 +819,26 @@ static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
 	_ieee802_11_parse_elems_full(&sub, elems, non_inherit);
 }
 
+static void
+ieee80211_mle_defrag_reconf(struct ieee802_11_elems *elems)
+{
+	ssize_t ml_len;
+
+	ml_len = cfg80211_defragment_element(elems->ml_reconf_elem,
+					     elems->ie_start,
+					     elems->total_len,
+					     elems->scratch_pos,
+					     elems->scratch +
+					     elems->scratch_len -
+					     elems->scratch_pos,
+					     WLAN_EID_FRAGMENT);
+	if (ml_len < 0)
+		return;
+	elems->ml_reconf = (void *)elems->scratch_pos;
+	elems->ml_reconf_len = ml_len;
+	elems->scratch_pos += ml_len;
+}
+
 struct ieee802_11_elems *
 ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params)
 {
@@ -864,6 +884,8 @@ ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params)
 
 	ieee80211_mle_parse_link(elems, params);
 
+	ieee80211_mle_defrag_reconf(elems);
+
 	if (elems->tim && !elems->parse_error) {
 		const struct ieee80211_tim_ie *tim_ie = elems->tim;
 
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 7/8] wifi: mac80211: remove unneeded scratch_len subtraction
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
                   ` (5 preceding siblings ...)
  2024-02-28  8:48 ` [PATCH 6/8] wifi: mac80211: defragment reconfiguration MLE when parsing Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  2024-02-28  8:48 ` [PATCH 8/8] wifi: mac80211: hide element parsing internals Johannes Berg
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless; +Cc: Johannes Berg, Miriam Rachel Korenblit, Ilan Peer

From: Johannes Berg <johannes.berg@intel.com>

We're always using "scratch + len - pos", so we don't need
to subtract here to calculate the remaining length. Remove
the unnecessary subtraction.

Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Reviewed-by: Ilan Peer <ilan.peer@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/parse.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 804323858977..73e52504ed97 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -862,7 +862,6 @@ ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params)
 					      elems, params->bss,
 					      nontransmitted_profile);
 	elems->scratch_pos += nontransmitted_profile_len;
-	elems->scratch_len -= nontransmitted_profile_len;
 	non_inherit = cfg80211_find_ext_elem(WLAN_EID_EXT_NON_INHERITANCE,
 					     nontransmitted_profile,
 					     nontransmitted_profile_len);
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 8/8] wifi: mac80211: hide element parsing internals
  2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
                   ` (6 preceding siblings ...)
  2024-02-28  8:48 ` [PATCH 7/8] wifi: mac80211: remove unneeded scratch_len subtraction Johannes Berg
@ 2024-02-28  8:48 ` Johannes Berg
  7 siblings, 0 replies; 9+ messages in thread
From: Johannes Berg @ 2024-02-28  8:48 UTC (permalink / raw
  To: linux-wireless; +Cc: Johannes Berg, Miriam Rachel Korenblit

From: Johannes Berg <johannes.berg@intel.com>

Rework the data structures to hide element parsing internals
from the users.

Reviewed-by: Miriam Rachel Korenblit <miriam.rachel.korenblit@intel.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/ieee80211_i.h |  14 -----
 net/mac80211/parse.c       | 118 ++++++++++++++++++++++++-------------
 net/mac80211/tests/elems.c |   4 +-
 3 files changed, 78 insertions(+), 58 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 768f614731a7..a8ac238bd197 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1766,12 +1766,6 @@ struct ieee802_11_elems {
 	size_t ml_basic_len;
 	size_t ml_reconf_len;
 
-	/* The basic Multi-Link element in the original elements */
-	const struct element *ml_basic_elem;
-
-	/* The reconfiguration Multi-Link element in the original elements */
-	const struct element *ml_reconf_elem;
-
 	u8 ttlm_num;
 
 	/*
@@ -1784,14 +1778,6 @@ struct ieee802_11_elems {
 
 	/* whether/which parse error occurred while retrieving these elements */
 	u8 parse_error;
-
-	/*
-	 * scratch buffer that can be used for various element parsing related
-	 * tasks, e.g., element de-fragmentation etc.
-	 */
-	size_t scratch_len;
-	u8 *scratch_pos;
-	u8 scratch[] __counted_by(scratch_len);
 };
 
 static inline struct ieee80211_local *hw_to_local(
diff --git a/net/mac80211/parse.c b/net/mac80211/parse.c
index 73e52504ed97..55e5497f8978 100644
--- a/net/mac80211/parse.c
+++ b/net/mac80211/parse.c
@@ -34,12 +34,32 @@
 #include "led.h"
 #include "wep.h"
 
+struct ieee80211_elems_parse {
+	/* must be first for kfree to work */
+	struct ieee802_11_elems elems;
+
+	/* The basic Multi-Link element in the original elements */
+	const struct element *ml_basic_elem;
+
+	/* The reconfiguration Multi-Link element in the original elements */
+	const struct element *ml_reconf_elem;
+
+	/*
+	 * scratch buffer that can be used for various element parsing related
+	 * tasks, e.g., element de-fragmentation etc.
+	 */
+	size_t scratch_len;
+	u8 *scratch_pos;
+	u8 scratch[] __counted_by(scratch_len);
+};
+
 static void
 ieee80211_parse_extension_element(u32 *crc,
 				  const struct element *elem,
-				  struct ieee802_11_elems *elems,
+				  struct ieee80211_elems_parse *elems_parse,
 				  struct ieee80211_elems_parse_params *params)
 {
+	struct ieee802_11_elems *elems = &elems_parse->elems;
 	const void *data = elem->data + 1;
 	bool calc_crc = false;
 	u8 len;
@@ -129,15 +149,15 @@ ieee80211_parse_extension_element(u32 *crc,
 			switch (le16_get_bits(mle->control,
 					      IEEE80211_ML_CONTROL_TYPE)) {
 			case IEEE80211_ML_CONTROL_TYPE_BASIC:
-				if (elems->ml_basic_elem) {
+				if (elems_parse->ml_basic_elem) {
 					elems->parse_error |=
 						IEEE80211_PARSE_ERR_DUP_NEST_ML_BASIC;
 					break;
 				}
-				elems->ml_basic_elem = (void *)elem;
+				elems_parse->ml_basic_elem = elem;
 				break;
 			case IEEE80211_ML_CONTROL_TYPE_RECONF:
-				elems->ml_reconf_elem = (void *)elem;
+				elems_parse->ml_reconf_elem = elem;
 				break;
 			default:
 				break;
@@ -169,9 +189,10 @@ ieee80211_parse_extension_element(u32 *crc,
 
 static u32
 _ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params,
-			     struct ieee802_11_elems *elems,
+			     struct ieee80211_elems_parse *elems_parse,
 			     const struct element *check_inherit)
 {
+	struct ieee802_11_elems *elems = &elems_parse->elems;
 	const struct element *elem;
 	bool calc_crc = params->filter != 0;
 	DECLARE_BITMAP(seen_elems, 256);
@@ -586,7 +607,8 @@ _ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params,
 		case WLAN_EID_EXTENSION:
 			ieee80211_parse_extension_element(calc_crc ?
 								&crc : NULL,
-							  elem, elems, params);
+							  elem, elems_parse,
+							  params);
 			break;
 		case WLAN_EID_S1G_CAPABILITIES:
 			if (params->mode != IEEE80211_CONN_MODE_S1G)
@@ -709,9 +731,11 @@ static size_t ieee802_11_find_bssid_profile(const u8 *start, size_t len,
 	return found ? profile_len : 0;
 }
 
-static void ieee80211_mle_get_sta_prof(struct ieee802_11_elems *elems,
-				       u8 link_id)
+static void
+ieee80211_mle_get_sta_prof(struct ieee80211_elems_parse *elems_parse,
+			   u8 link_id)
 {
+	struct ieee802_11_elems *elems = &elems_parse->elems;
 	const struct ieee80211_multi_link_elem *ml = elems->ml_basic;
 	ssize_t ml_len = elems->ml_basic_len;
 	const struct element *sub;
@@ -741,26 +765,27 @@ static void ieee80211_mle_get_sta_prof(struct ieee802_11_elems *elems,
 		sta_prof_len =
 			cfg80211_defragment_element(sub,
 						    (u8 *)ml, ml_len,
-						    elems->scratch_pos,
-						    elems->scratch +
-							elems->scratch_len -
-							elems->scratch_pos,
+						    elems_parse->scratch_pos,
+						    elems_parse->scratch +
+							elems_parse->scratch_len -
+							elems_parse->scratch_pos,
 						    IEEE80211_MLE_SUBELEM_FRAGMENT);
 
 		if (sta_prof_len < 0)
 			return;
 
-		elems->prof = (void *)elems->scratch_pos;
+		elems->prof = (void *)elems_parse->scratch_pos;
 		elems->sta_prof_len = sta_prof_len;
-		elems->scratch_pos += sta_prof_len;
+		elems_parse->scratch_pos += sta_prof_len;
 
 		return;
 	}
 }
 
-static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
+static void ieee80211_mle_parse_link(struct ieee80211_elems_parse *elems_parse,
 				     struct ieee80211_elems_parse_params *params)
 {
+	struct ieee802_11_elems *elems = &elems_parse->elems;
 	struct ieee80211_mle_per_sta_profile *prof;
 	struct ieee80211_elems_parse_params sub = {
 		.mode = params->mode,
@@ -772,26 +797,26 @@ static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
 	const struct element *non_inherit = NULL;
 	const u8 *end;
 
-	ml_len = cfg80211_defragment_element(elems->ml_basic_elem,
+	ml_len = cfg80211_defragment_element(elems_parse->ml_basic_elem,
 					     elems->ie_start,
 					     elems->total_len,
-					     elems->scratch_pos,
-					     elems->scratch +
-						elems->scratch_len -
-						elems->scratch_pos,
+					     elems_parse->scratch_pos,
+					     elems_parse->scratch +
+						elems_parse->scratch_len -
+						elems_parse->scratch_pos,
 					     WLAN_EID_FRAGMENT);
 
 	if (ml_len < 0)
 		return;
 
-	elems->ml_basic = (const void *)elems->scratch_pos;
+	elems->ml_basic = (const void *)elems_parse->scratch_pos;
 	elems->ml_basic_len = ml_len;
-	elems->scratch_pos += ml_len;
+	elems_parse->scratch_pos += ml_len;
 
 	if (params->link_id == -1)
 		return;
 
-	ieee80211_mle_get_sta_prof(elems, params->link_id);
+	ieee80211_mle_get_sta_prof(elems_parse, params->link_id);
 	prof = elems->prof;
 
 	if (!prof)
@@ -816,57 +841,66 @@ static void ieee80211_mle_parse_link(struct ieee802_11_elems *elems,
 
 	non_inherit = cfg80211_find_ext_elem(WLAN_EID_EXT_NON_INHERITANCE,
 					     sub.start, sub.len);
-	_ieee802_11_parse_elems_full(&sub, elems, non_inherit);
+	_ieee802_11_parse_elems_full(&sub, elems_parse, non_inherit);
 }
 
 static void
-ieee80211_mle_defrag_reconf(struct ieee802_11_elems *elems)
+ieee80211_mle_defrag_reconf(struct ieee80211_elems_parse *elems_parse)
 {
+	struct ieee802_11_elems *elems = &elems_parse->elems;
 	ssize_t ml_len;
 
-	ml_len = cfg80211_defragment_element(elems->ml_reconf_elem,
+	ml_len = cfg80211_defragment_element(elems_parse->ml_reconf_elem,
 					     elems->ie_start,
 					     elems->total_len,
-					     elems->scratch_pos,
-					     elems->scratch +
-					     elems->scratch_len -
-					     elems->scratch_pos,
+					     elems_parse->scratch_pos,
+					     elems_parse->scratch +
+						elems_parse->scratch_len -
+						elems_parse->scratch_pos,
 					     WLAN_EID_FRAGMENT);
 	if (ml_len < 0)
 		return;
-	elems->ml_reconf = (void *)elems->scratch_pos;
+	elems->ml_reconf = (void *)elems_parse->scratch_pos;
 	elems->ml_reconf_len = ml_len;
-	elems->scratch_pos += ml_len;
+	elems_parse->scratch_pos += ml_len;
 }
 
 struct ieee802_11_elems *
 ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params)
 {
+	struct ieee80211_elems_parse *elems_parse;
 	struct ieee802_11_elems *elems;
 	const struct element *non_inherit = NULL;
 	u8 *nontransmitted_profile;
 	int nontransmitted_profile_len = 0;
 	size_t scratch_len = 3 * params->len;
 
-	elems = kzalloc(struct_size(elems, scratch, scratch_len), GFP_ATOMIC);
-	if (!elems)
+	BUILD_BUG_ON(offsetof(typeof(*elems_parse), elems) != 0);
+
+	elems_parse = kzalloc(struct_size(elems_parse, scratch, scratch_len),
+			      GFP_ATOMIC);
+	if (!elems_parse)
 		return NULL;
+
+	elems_parse->scratch_len = scratch_len;
+	elems_parse->scratch_pos = elems_parse->scratch;
+
+	elems = &elems_parse->elems;
 	elems->ie_start = params->start;
 	elems->total_len = params->len;
-	elems->scratch_len = scratch_len;
-	elems->scratch_pos = elems->scratch;
 
-	nontransmitted_profile = elems->scratch_pos;
+	nontransmitted_profile = elems_parse->scratch_pos;
 	nontransmitted_profile_len =
 		ieee802_11_find_bssid_profile(params->start, params->len,
 					      elems, params->bss,
 					      nontransmitted_profile);
-	elems->scratch_pos += nontransmitted_profile_len;
+	elems_parse->scratch_pos += nontransmitted_profile_len;
 	non_inherit = cfg80211_find_ext_elem(WLAN_EID_EXT_NON_INHERITANCE,
 					     nontransmitted_profile,
 					     nontransmitted_profile_len);
 
-	elems->crc = _ieee802_11_parse_elems_full(params, elems, non_inherit);
+	elems->crc = _ieee802_11_parse_elems_full(params, elems_parse,
+						  non_inherit);
 
 	/* Override with nontransmitted profile, if found */
 	if (nontransmitted_profile_len) {
@@ -878,12 +912,12 @@ ieee802_11_parse_elems_full(struct ieee80211_elems_parse_params *params)
 			.link_id = params->link_id,
 		};
 
-		_ieee802_11_parse_elems_full(&sub, elems, NULL);
+		_ieee802_11_parse_elems_full(&sub, elems_parse, NULL);
 	}
 
-	ieee80211_mle_parse_link(elems, params);
+	ieee80211_mle_parse_link(elems_parse, params);
 
-	ieee80211_mle_defrag_reconf(elems);
+	ieee80211_mle_defrag_reconf(elems_parse);
 
 	if (elems->tim && !elems->parse_error) {
 		const struct ieee80211_tim_ie *tim_ie = elems->tim;
diff --git a/net/mac80211/tests/elems.c b/net/mac80211/tests/elems.c
index 30fc0acb7ac2..a413ba29f759 100644
--- a/net/mac80211/tests/elems.c
+++ b/net/mac80211/tests/elems.c
@@ -2,7 +2,7 @@
 /*
  * KUnit tests for element parsing
  *
- * Copyright (C) 2023 Intel Corporation
+ * Copyright (C) 2023-2024 Intel Corporation
  */
 #include <kunit/test.h>
 #include "../ieee80211_i.h"
@@ -69,7 +69,7 @@ static void mle_defrag(struct kunit *test)
 	if (IS_ERR_OR_NULL(parsed))
 		goto free_skb;
 
-	KUNIT_EXPECT_NOT_NULL(test, parsed->ml_basic_elem);
+	KUNIT_EXPECT_NOT_NULL(test, parsed->ml_basic);
 	KUNIT_EXPECT_EQ(test,
 			parsed->ml_basic_len,
 			2 /* control */ +
-- 
2.43.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-02-28  8:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-28  8:48 [PATCH 0/8] wifi: mac80211: element parsing cleanups Johannes Berg
2024-02-28  8:48 ` [PATCH 1/8] wifi: mac80211: update scratch_pos after defrag Johannes Berg
2024-02-28  8:48 ` [PATCH 2/8] wifi: mac80211: remove unnecessary ML element type check Johannes Berg
2024-02-28  8:48 ` [PATCH 3/8] wifi: mac80211: add ieee80211_vif_link_active() helper Johannes Berg
2024-02-28  8:48 ` [PATCH 4/8] wifi: mac80211: remove unnecessary ML element checks Johannes Berg
2024-02-28  8:48 ` [PATCH 5/8] wifi: mac80211: simplify multi-link element parsing Johannes Berg
2024-02-28  8:48 ` [PATCH 6/8] wifi: mac80211: defragment reconfiguration MLE when parsing Johannes Berg
2024-02-28  8:48 ` [PATCH 7/8] wifi: mac80211: remove unneeded scratch_len subtraction Johannes Berg
2024-02-28  8:48 ` [PATCH 8/8] wifi: mac80211: hide element parsing internals Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).