diff mbox series

[v2,07/20] nl80211: Use valid_links bitmask for bss->links array

Message ID 20240220131827.17766-8-benjamin@sipsolutions.net
State Accepted
Headers show
Series Various mostly WNM related fixes and cleanups | expand

Commit Message

Benjamin Berg Feb. 20, 2024, 1:18 p.m. UTC
From: Benjamin Berg <benjamin.berg@intel.com>

Most places in the codebase use a valid_links bitmask with an array.
Switch the bss->links array instead of having a link_id inside.

Signed-off-by: Benjamin Berg <benjamin.berg@intel.com>
---
 src/drivers/driver_nl80211.c       | 179 ++++++++++-------------------
 src/drivers/driver_nl80211.h       |  29 ++++-
 src/drivers/driver_nl80211_event.c |  33 +++---
 src/utils/common.h                 |   8 ++
 4 files changed, 111 insertions(+), 138 deletions(-)
diff mbox series

Patch

diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index bc01c4426..639767f21 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -2289,7 +2289,6 @@  static void * wpa_driver_nl80211_drv_init(void *ctx, const char *ifname,
 {
 	struct wpa_driver_nl80211_data *drv;
 	struct i802_bss *bss;
-	unsigned int i;
 	char path[128], buf[200], *pos;
 	ssize_t len;
 	int ret;
@@ -2389,16 +2388,12 @@  skip_wifi_status:
 	}
 
 	/*
-	 * Set the default link to be the first one, and set its address to that
-	 * of the interface.
+	 * Use link ID 0 for the single "link" of a non-MLD.
 	 */
+	bss->valid_links = 0;
 	bss->flink = &bss->links[0];
-	bss->n_links = 1;
 	os_memcpy(bss->flink->addr, bss->addr, ETH_ALEN);
 
-	for (i = 0; i < MAX_NUM_MLD_LINKS; i++)
-		bss->links[i].link_id = NL80211_DRV_LINK_ID_NA;
-
 	return bss;
 
 failed:
@@ -3073,10 +3068,11 @@  wpa_driver_nl80211_finish_drv_init(struct wpa_driver_nl80211_data *drv,
 
 
 static int wpa_driver_nl80211_del_beacon(struct i802_bss *bss,
-					 struct i802_link *link)
+					 int link_id)
 {
 	struct nl_msg *msg;
 	struct wpa_driver_nl80211_data *drv = bss->drv;
+	struct i802_link *link = nl80211_get_link(bss, link_id);
 
 	if (!link->beacon_set)
 		return 0;
@@ -3091,13 +3087,12 @@  static int wpa_driver_nl80211_del_beacon(struct i802_bss *bss,
 	if (!msg)
 		return -ENOBUFS;
 
-	if (link->link_id != NL80211_DRV_LINK_ID_NA) {
+	if (link_id != NL80211_DRV_LINK_ID_NA) {
 		wpa_printf(MSG_DEBUG,
 			   "nl80211: MLD: stop beaconing on link=%u",
-			   link->link_id);
+			   link_id);
 
-		if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID,
-			       link->link_id)) {
+		if (nla_put_u8(msg, NL80211_ATTR_MLO_LINK_ID, link_id)) {
 			nlmsg_free(msg);
 			return -ENOBUFS;
 		}
@@ -3109,10 +3104,10 @@  static int wpa_driver_nl80211_del_beacon(struct i802_bss *bss,
 
 static void wpa_driver_nl80211_del_beacon_all(struct i802_bss *bss)
 {
-	unsigned int i;
+	unsigned int link_id;
 
-	for (i = 0; i < bss->n_links; i++)
-		wpa_driver_nl80211_del_beacon(bss, &bss->links[i]);
+	for_each_link_default(bss->valid_links, link_id, NL80211_DRV_LINK_ID_NA)
+		wpa_driver_nl80211_del_beacon(bss, link_id);
 }
 
 
@@ -4191,21 +4186,6 @@  int wpa_driver_nl80211_authenticate_retry(struct wpa_driver_nl80211_data *drv)
 }
 
 
-struct i802_link * nl80211_get_link(struct i802_bss *bss, s8 link_id)
-{
-	unsigned int i;
-
-	for (i = 0; i < bss->n_links; i++) {
-		if (bss->links[i].link_id != link_id)
-			continue;
-
-		return &bss->links[i];
-	}
-
-	return bss->flink;
-}
-
-
 static void nl80211_link_set_freq(struct i802_bss *bss, s8 link_id, int freq)
 {
 	struct i802_link *link = nl80211_get_link(bss, link_id);
@@ -4217,9 +4197,9 @@  static void nl80211_link_set_freq(struct i802_bss *bss, s8 link_id, int freq)
 static int nl80211_get_link_freq(struct i802_bss *bss, const u8 *addr,
 				 bool bss_freq_debug)
 {
-	size_t i;
+	u8 i;
 
-	for (i = 0; i < bss->n_links; i++) {
+	for_each_link(bss->valid_links, i) {
 		if (ether_addr_equal(bss->links[i].addr, addr)) {
 			wpa_printf(MSG_DEBUG,
 				   "nl80211: Use link freq=%d for address "
@@ -5060,20 +5040,17 @@  static int wpa_driver_nl80211_set_ap(void *priv,
 #endif /* CONFIG_MESH */
 
 	if (params->mld_ap) {
-		size_t i;
-
-		for (i = 0; i < bss->n_links; i++) {
-			if (bss->links[i].link_id == params->mld_link_id) {
-				link = &bss->links[i];
-				break;
-			}
-		}
-
-		if (i == bss->n_links) {
-			wpa_printf(MSG_DEBUG, "nl80211: Link ID=%u not found",
-				   params->mld_link_id);
+		if (!nl80211_link_valid(bss->valid_links,
+					params->mld_link_id)) {
+			wpa_printf(MSG_DEBUG, "nl80211: Link ID=%u invalid (valid: 0x%04x)",
+				   params->mld_link_id, bss->valid_links);
 			return -EINVAL;
 		}
+
+		link = nl80211_get_link(bss, params->mld_link_id);
+	} else if (bss->valid_links) {
+		wpa_printf(MSG_DEBUG, "nl80211: MLD configuration expected");
+		return -EINVAL;
 	}
 
 	beacon_set = params->reenable ? 0 : link->beacon_set;
@@ -5483,27 +5460,6 @@  fail:
 }
 
 
-static bool nl80211_link_valid(struct i802_bss *bss, s8 link_id)
-{
-	unsigned int i;
-
-	if (link_id < 0)
-		return false;
-
-	for (i = 0; i < bss->n_links; i++) {
-		wpa_printf(MSG_DEBUG, "nl80211: %s - i=%u, link_id=%u",
-			   __func__, i, bss->links[i].link_id);
-		if (bss->links[i].link_id == NL80211_DRV_LINK_ID_NA)
-			continue;
-
-		if (bss->links[i].link_id == link_id)
-			return true;
-	}
-
-	return false;
-}
-
-
 static int nl80211_set_channel(struct i802_bss *bss,
 			       struct hostapd_freq_params *freq, int set_chan)
 {
@@ -5524,7 +5480,7 @@  static int nl80211_set_channel(struct i802_bss *bss,
 		return -1;
 	}
 
-	if (nl80211_link_valid(bss, freq->link_id)) {
+	if (nl80211_link_valid(bss->valid_links, freq->link_id)) {
 		wpa_printf(MSG_DEBUG, "nl80211: Set link_id=%u for freq",
 			   freq->link_id);
 
@@ -8886,7 +8842,6 @@  static int wpa_driver_nl80211_if_add(void *priv, enum wpa_driver_if_type type,
 
 	if (type == WPA_IF_AP_BSS && setup_ap) {
 		struct i802_bss *new_bss = os_zalloc(sizeof(*new_bss));
-		unsigned int i;
 
 		if (new_bss == NULL) {
 			if (added)
@@ -8894,10 +8849,6 @@  static int wpa_driver_nl80211_if_add(void *priv, enum wpa_driver_if_type type,
 			return -1;
 		}
 
-		/* Initialize here before any failure path */
-		for (i = 0; i < MAX_NUM_MLD_LINKS; i++)
-			new_bss->links[i].link_id = NL80211_DRV_LINK_ID_NA;
-
 		if (bridge &&
 		    i802_check_bridge(drv, new_bss, bridge, ifname) < 0) {
 			wpa_printf(MSG_ERROR, "nl80211: Failed to add the new "
@@ -8922,7 +8873,7 @@  static int wpa_driver_nl80211_if_add(void *priv, enum wpa_driver_if_type type,
 		new_bss->drv = drv;
 		new_bss->next = drv->first_bss->next;
 		new_bss->flink = &new_bss->links[0];
-		new_bss->n_links = 1;
+		new_bss->valid_links = 0;
 		os_memcpy(new_bss->flink->addr, new_bss->addr, ETH_ALEN);
 
 		new_bss->flink->freq = drv->first_bss->flink->freq;
@@ -9464,33 +9415,35 @@  static void nl80211_remove_links(struct i802_bss *bss)
 	struct wpa_driver_nl80211_data *drv = bss->drv;
 	struct nl_msg *msg;
 	int ret;
-	u8 link_id;
-
-	if (bss->n_links == 0)
-		return;
+	u8 link_id, i;
 
-	while (bss->links[0].link_id != NL80211_DRV_LINK_ID_NA) {
-		struct i802_link *link = &bss->links[0];
+	for_each_link(bss->valid_links, link_id) {
+		struct i802_link *link = &bss->links[link_id];
 
 		wpa_printf(MSG_DEBUG, "nl80211: MLD: remove link_id=%u",
-			   link->link_id);
-
-		wpa_driver_nl80211_del_beacon(bss, link);
+			   link_id);
 
-		link_id = link->link_id;
+		wpa_driver_nl80211_del_beacon(bss, link_id);
 
 		/* First remove the link locally */
-		if (bss->n_links == 1) {
-			bss->flink->link_id = NL80211_DRV_LINK_ID_NA;
-			os_memcpy(bss->flink->addr, bss->addr, ETH_ALEN);
-		} else {
-			struct i802_link *other = &bss->links[bss->n_links - 1];
+		bss->valid_links &= ~BIT(link_id);
+		os_memset(link->addr, 0, ETH_ALEN);
 
-			os_memcpy(link, other, sizeof(*link));
-			other->link_id = NL80211_DRV_LINK_ID_NA;
-			os_memset(other->addr, 0, ETH_ALEN);
+		/* Choose new deflink if we are removing that link */
+		if (bss->flink == link) {
+			for_each_link_default(bss->valid_links, i, 0) {
+				bss->flink = &bss->links[i];
+				break;
+			}
+		}
 
-			bss->n_links--;
+		/* If this was the last link, then reset default link */
+		if (!bss->valid_links) {
+			/* Does keeping freq/bandwidth make sense? */
+			if (bss->flink != link)
+				os_memcpy(bss->flink, link, sizeof(*link));
+
+			os_memcpy(bss->flink->addr, bss->addr, ETH_ALEN);
 		}
 
 		/* Remove the link from the kernel */
@@ -9501,7 +9454,7 @@  static void nl80211_remove_links(struct i802_bss *bss)
 			wpa_printf(MSG_ERROR,
 				   "nl80211: remove link (%d) failed",
 				   link_id);
-			return;
+			continue;
 		}
 
 		ret = send_and_recv_cmd(drv, msg);
@@ -9509,7 +9462,7 @@  static void nl80211_remove_links(struct i802_bss *bss)
 			wpa_printf(MSG_ERROR,
 				   "nl80211: remove link (%d) failed. ret=%d (%s)",
 				   link_id, ret, strerror(-ret));
-			return;
+			continue;
 		}
 	}
 }
@@ -9524,7 +9477,7 @@  static int wpa_driver_nl80211_deinit_ap(void *priv)
 		return -1;
 
 	/* Stop beaconing */
-	wpa_driver_nl80211_del_beacon(bss, bss->flink);
+	wpa_driver_nl80211_del_beacon(bss, NL80211_DRV_LINK_ID_NA);
 
 	nl80211_remove_links(bss);
 
@@ -13791,7 +13744,6 @@  static int nl80211_link_add(void *priv, u8 link_id, const u8 *addr)
 	struct i802_bss *bss = priv;
 	struct wpa_driver_nl80211_data *drv = bss->drv;
 	struct nl_msg *msg;
-	unsigned int idx, i;
 	int ret;
 
 	wpa_printf(MSG_DEBUG, "nl80211: MLD: add link_id=%u, addr=" MACSTR,
@@ -13804,32 +13756,24 @@  static int nl80211_link_add(void *priv, u8 link_id, const u8 *addr)
 		return -EINVAL;
 	}
 
-	if (bss->n_links >= MAX_NUM_MLD_LINKS) {
-		wpa_printf(MSG_DEBUG, "nl80211: MLD: already have n_links=%zu",
-			   bss->n_links);
+	if (link_id >= MAX_NUM_MLD_LINKS) {
+		wpa_printf(MSG_DEBUG,
+			   "nl80211: invalid link_id=%u", link_id);
 		return -EINVAL;
 	}
 
-	for (i = 0; i < bss->n_links; i++) {
-		if (bss->links[i].link_id == link_id &&
-		    bss->links[i].beacon_set) {
-			wpa_printf(MSG_DEBUG,
-				   "nl80211: MLD: link already set");
-			return -EINVAL;
-		}
+	if (bss->valid_links & BIT(link_id)) {
+		wpa_printf(MSG_DEBUG,
+			   "nl80211: MLD: link already set");
+		return -EINVAL;
 	}
 
-	/* try using the first link entry, assuming it is not beaconing yet */
-	if (bss->n_links == 1 &&
-	    bss->flink->link_id == NL80211_DRV_LINK_ID_NA) {
+	if (!bss->valid_links) {
+		/* Becoming MLD, verify we were not beaconing */
 		if (bss->flink->beacon_set) {
 			wpa_printf(MSG_DEBUG, "nl80211: BSS already beaconing");
 			return -EINVAL;
 		}
-
-		idx = 0;
-	} else {
-		idx = bss->n_links;
 	}
 
 	msg = nl80211_drv_msg(drv, 0, NL80211_CMD_ADD_LINK);
@@ -13847,12 +13791,15 @@  static int nl80211_link_add(void *priv, u8 link_id, const u8 *addr)
 		return ret;
 	}
 
-	bss->links[idx].link_id = link_id;
-	os_memcpy(bss->links[idx].addr, addr, ETH_ALEN);
+	os_memcpy(bss->links[link_id].addr, addr, ETH_ALEN);
+
+	/* The new link is the first one, make it the default */
+	if (!bss->valid_links)
+		bss->flink = &bss->links[link_id];
 
-	bss->n_links = idx + 1;
+	bss->valid_links |= BIT(link_id);
 
-	wpa_printf(MSG_DEBUG, "nl80211: MLD: n_links=%zu", bss->n_links);
+	wpa_printf(MSG_DEBUG, "nl80211: MLD: valid_links=0x%04x", bss->valid_links);
 	return 0;
 }
 
diff --git a/src/drivers/driver_nl80211.h b/src/drivers/driver_nl80211.h
index 8bb70ecfc..f1e20045b 100644
--- a/src/drivers/driver_nl80211.h
+++ b/src/drivers/driver_nl80211.h
@@ -55,18 +55,16 @@  struct nl80211_wiphy_data {
 struct i802_link {
 	unsigned int beacon_set:1;
 
-	s8 link_id;
 	int freq;
 	int bandwidth;
 	u8 addr[ETH_ALEN];
-	void *ctx;
 };
 
 struct i802_bss {
 	struct wpa_driver_nl80211_data *drv;
 	struct i802_bss *next;
 
-	size_t n_links;
+	u16 valid_links;
 	struct i802_link links[MAX_NUM_MLD_LINKS];
 	struct i802_link *flink;
 
@@ -352,7 +350,30 @@  int process_bss_event(struct nl_msg *msg, void *arg);
 const char * nl80211_iftype_str(enum nl80211_iftype mode);
 
 void nl80211_restore_ap_mode(struct i802_bss *bss);
-struct i802_link * nl80211_get_link(struct i802_bss *bss, s8 link_id);
+
+static inline struct i802_link *
+nl80211_get_link(struct i802_bss *bss, s8 link_id)
+{
+	if (link_id < 0 || link_id >= MAX_NUM_MLD_LINKS)
+		return bss->flink;
+
+	if (BIT(link_id) & bss->valid_links)
+		return &bss->links[link_id];
+
+	return bss->flink;
+}
+
+static inline bool nl80211_link_valid(u16 links, s8 link_id)
+{
+	if (link_id < 0 || link_id >= MAX_NUM_MLD_LINKS)
+		return false;
+
+	if (links & BIT(link_id))
+		return true;
+
+	return false;
+}
+
 
 static inline bool
 nl80211_attr_supported(struct wpa_driver_nl80211_data *drv, unsigned int attr)
diff --git a/src/drivers/driver_nl80211_event.c b/src/drivers/driver_nl80211_event.c
index d6c628cef..be0e369a2 100644
--- a/src/drivers/driver_nl80211_event.c
+++ b/src/drivers/driver_nl80211_event.c
@@ -1610,18 +1610,17 @@  static void mlme_event_unprot_beacon(struct wpa_driver_nl80211_data *drv,
 }
 
 
-static struct i802_link *
-nl80211_get_mld_link_by_freq(struct i802_bss *bss, unsigned int freq)
+static s8
+nl80211_get_link_id_by_freq(struct i802_bss *bss, unsigned int freq)
 {
 	unsigned int i;
 
-	for (i = 0; i < bss->n_links; i++) {
-		if ((unsigned int) bss->links[i].freq == freq &&
-		    bss->links[i].link_id != -1)
-			return &bss->links[i];
+	for_each_link(bss->valid_links, i) {
+		if ((unsigned int) bss->links[i].freq == freq)
+			return i;
 	}
 
-	return NULL;
+	return NL80211_DRV_LINK_ID_NA;
 }
 
 
@@ -1655,12 +1654,12 @@  static void mlme_event(struct i802_bss *bss,
 	/* Determine the MLD link either by an explicitly provided link id or
 	 * finding a match based on the frequency. */
 	if (link)
-		mld_link = nl80211_get_link(bss, nla_get_u8(link));
+		link_id = nla_get_u8(link);
 	else if (freq)
-		mld_link = nl80211_get_mld_link_by_freq(bss, nla_get_u32(freq));
+		link_id = nl80211_get_link_id_by_freq(bss, nla_get_u32(freq));
 
-	if (mld_link)
-		link_id = mld_link->link_id;
+	if (nl80211_link_valid(bss->valid_links, link_id))
+		mld_link = nl80211_get_link(bss, link_id);
 
 	data = nla_data(frame);
 	len = nla_len(frame);
@@ -1912,7 +1911,7 @@  static void mlme_event_dh_event(struct wpa_driver_nl80211_data *drv,
 	os_memset(&data, 0, sizeof(data));
 	addr = nla_data(tb[NL80211_ATTR_MAC]);
 
-	if (bss->links[0].link_id == NL80211_DRV_LINK_ID_NA &&
+	if (!bss->valid_links &&
 	    (tb[NL80211_ATTR_MLO_LINK_ID] ||
 	     tb[NL80211_ATTR_MLD_ADDR])) {
 		wpa_printf(MSG_ERROR,
@@ -2176,7 +2175,7 @@  static void nl80211_new_station_event(struct wpa_driver_nl80211_data *drv,
 		u8 *req_ies = NULL, *resp_ies = NULL;
 		size_t req_ies_len = 0, resp_ies_len = 0;
 
-		if (bss->links[0].link_id == NL80211_DRV_LINK_ID_NA &&
+		if (!bss->valid_links &&
 		    (tb[NL80211_ATTR_MLO_LINK_ID] ||
 		     tb[NL80211_ATTR_MLD_ADDR])) {
 			wpa_printf(MSG_ERROR,
@@ -2444,7 +2443,6 @@  static void nl80211_radar_event(struct wpa_driver_nl80211_data *drv,
 {
 	union wpa_event_data data;
 	enum nl80211_radar_event event_type;
-	struct i802_link *mld_link = NULL;
 
 	if (!tb[NL80211_ATTR_WIPHY_FREQ] || !tb[NL80211_ATTR_RADAR_EVENT])
 		return;
@@ -2455,10 +2453,9 @@  static void nl80211_radar_event(struct wpa_driver_nl80211_data *drv,
 	event_type = nla_get_u32(tb[NL80211_ATTR_RADAR_EVENT]);
 
 	if (data.dfs_event.freq) {
-		mld_link = nl80211_get_mld_link_by_freq(drv->first_bss,
-							data.dfs_event.freq);
-		if (mld_link)
-			data.dfs_event.link_id = mld_link->link_id;
+		data.dfs_event.link_id =
+			nl80211_get_link_id_by_freq(drv->first_bss,
+						       data.dfs_event.freq);
 	}
 
 	/* Check HT params */
diff --git a/src/utils/common.h b/src/utils/common.h
index 3a8d9e022..2b2611d02 100644
--- a/src/utils/common.h
+++ b/src/utils/common.h
@@ -603,6 +603,14 @@  char * get_param(const char *cmd, const char *param);
        for ((__i) = 0; (__i) < MAX_NUM_MLD_LINKS; (__i)++)     \
                if ((__links) & BIT(__i))
 
+/* Iterate all links, or, if no link is defined, iterate given index */
+#define for_each_link_default(_links, _i, _def_idx)	\
+	for ((_i) = (_links) ? 0 : (_def_idx);		\
+	     (_i) < MAX_NUM_MLD_LINKS ||		\
+		(!(_links) && (_i) == (_def_idx));	\
+	     (_i)++)					\
+		if (!(_links) || (_links) & BIT(_i))
+
 void forced_memzero(void *ptr, size_t len);
 
 /*