Message-ID: <20200115121041.10863-51-Jerome.Pouiller@silabs.com>
Date:   Wed, 15 Jan 2020 12:13:15 +0000
From:   Jérôme Pouiller <Jerome.Pouiller@...abs.com>
To:     "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        "David S . Miller" <davem@...emloft.net>,
        Jérôme Pouiller <Jerome.Pouiller@...abs.com>
Subject: [PATCH 50/65] staging: wfx: simplify the link-id allocation

From: Jérôme Pouiller <jerome.pouiller@...abs.com>

The "link-id" is a slot number provided to the chip. A link-id is
allocated to every station associated with the chip (mainly when the
chip is in AP mode). It is more or less the same thing than the
association ID, but it is limited to 14 values.

Firmware uses the link-id to track the power save status of the
stations.

The current code try to associate a link-id as soon as data are
exchanged with station. It is far easier to rely on sta_add() and
sta_remove().

Until now the value WFX_LINK_ID_NO_ASSOC, was only used when no more
link-id was available. Now, we also use this value for
not-yet-associated stations (that was its primary behavior).

Signed-off-by: Jérôme Pouiller <jerome.pouiller@...abs.com>
---
 drivers/staging/wfx/data_rx.c |   7 --
 drivers/staging/wfx/data_tx.c | 164 +---------------------------------
 drivers/staging/wfx/data_tx.h |  19 ----
 drivers/staging/wfx/queue.c   |   7 +-
 drivers/staging/wfx/queue.h   |   7 +-
 drivers/staging/wfx/sta.c     |  58 ++++--------
 drivers/staging/wfx/wfx.h     |   3 -
 7 files changed, 24 insertions(+), 241 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index e26bc665b2b3..699e2d60fa89 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -103,11 +103,9 @@ static int wfx_drop_encrypt_data(struct wfx_dev *wdev,
 void wfx_rx_cb(struct wfx_vif *wvif,
 	       const struct hif_ind_rx *arg, struct sk_buff *skb)
 {
-	int link_id = arg->rx_flags.peer_sta_id;
 	struct ieee80211_rx_status *hdr = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_hdr *frame = (struct ieee80211_hdr *)skb->data;
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
-	struct wfx_link_entry *entry = NULL;
 
 	memset(hdr, 0, sizeof(*hdr));
 
@@ -117,11 +115,6 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 	     ieee80211_is_beacon(frame->frame_control)))
 		goto drop;
 
-	if (link_id && link_id <= WFX_MAX_STA_IN_AP_MODE) {
-		entry = &wvif->link_id_db[link_id - 1];
-		entry->timestamp = jiffies;
-	}
-
 	if (arg->status == HIF_STATUS_MICFAILURE)
 		hdr->flag |= RX_FLAG_MMIC_ERROR;
 	else if (arg->status)
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 9313c8f5d4d8..8c9f986ec672 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -17,7 +17,6 @@
 #include "hif_tx_mib.h"
 
 #define WFX_INVALID_RATE_ID    15
-#define WFX_LINK_ID_NO_ASSOC   15
 #define WFX_LINK_ID_GC_TIMEOUT ((unsigned long)(10 * HZ))
 
 static int wfx_get_hw_rate(struct wfx_dev *wdev,
@@ -265,156 +264,6 @@ void wfx_tx_policy_init(struct wfx_vif *wvif)
 		list_add(&cache->cache[i].link, &cache->free);
 }
 
-/* Link ID related functions */
-
-static int wfx_alloc_link_id(struct wfx_vif *wvif, const u8 *mac)
-{
-	int i, ret = 0;
-	unsigned long oldest;
-
-	spin_lock_bh(&wvif->ps_state_lock);
-	for (i = 0; i < WFX_MAX_STA_IN_AP_MODE; ++i) {
-		if (!wvif->link_id_db[i].status) {
-			ret = i + 1;
-			break;
-		} else if (wvif->link_id_db[i].status != WFX_LINK_HARD &&
-			   !wvif->wdev->tx_queue_stats.link_map_cache[i + 1]) {
-			if (!ret || time_after(oldest, wvif->link_id_db[i].timestamp)) {
-				oldest = wvif->link_id_db[i].timestamp;
-				ret = i + 1;
-			}
-		}
-	}
-
-	if (ret) {
-		struct wfx_link_entry *entry = &wvif->link_id_db[ret - 1];
-
-		entry->status = WFX_LINK_RESERVE;
-		ether_addr_copy(entry->mac, mac);
-		wfx_tx_lock(wvif->wdev);
-
-		if (!schedule_work(&wvif->link_id_work))
-			wfx_tx_unlock(wvif->wdev);
-	} else {
-		dev_info(wvif->wdev->dev, "no more link-id available\n");
-	}
-	spin_unlock_bh(&wvif->ps_state_lock);
-	return ret;
-}
-
-int wfx_find_link_id(struct wfx_vif *wvif, const u8 *mac)
-{
-	int i, ret = 0;
-
-	spin_lock_bh(&wvif->ps_state_lock);
-	for (i = 0; i < WFX_MAX_STA_IN_AP_MODE; ++i) {
-		if (ether_addr_equal(mac, wvif->link_id_db[i].mac) &&
-		    wvif->link_id_db[i].status) {
-			wvif->link_id_db[i].timestamp = jiffies;
-			ret = i + 1;
-			break;
-		}
-	}
-	spin_unlock_bh(&wvif->ps_state_lock);
-	return ret;
-}
-
-static int wfx_map_link(struct wfx_vif *wvif,
-			struct wfx_link_entry *link_entry, int sta_id)
-{
-	int ret;
-
-	ret = hif_map_link(wvif, link_entry->mac, 0, sta_id);
-
-	if (ret == 0)
-		/* Save the MAC address currently associated with the peer
-		 * for future unmap request
-		 */
-		ether_addr_copy(link_entry->old_mac, link_entry->mac);
-
-	return ret;
-}
-
-int wfx_unmap_link(struct wfx_vif *wvif, int sta_id)
-{
-	u8 *mac_addr = NULL;
-
-	if (sta_id)
-		mac_addr = wvif->link_id_db[sta_id - 1].old_mac;
-
-	return hif_map_link(wvif, mac_addr, 1, sta_id);
-}
-
-void wfx_link_id_gc_work(struct work_struct *work)
-{
-	struct wfx_vif *wvif =
-		container_of(work, struct wfx_vif, link_id_gc_work.work);
-	unsigned long now = jiffies;
-	unsigned long next_gc = -1;
-	long ttl;
-	u32 mask;
-	int i;
-
-	if (wvif->state != WFX_STATE_AP)
-		return;
-
-	wfx_tx_lock_flush(wvif->wdev);
-	spin_lock_bh(&wvif->ps_state_lock);
-	for (i = 0; i < WFX_MAX_STA_IN_AP_MODE; ++i) {
-		bool need_reset = false;
-
-		mask = BIT(i + 1);
-		if (wvif->link_id_db[i].status == WFX_LINK_RESERVE ||
-		    (wvif->link_id_db[i].status == WFX_LINK_HARD &&
-		     !(wvif->link_id_map & mask))) {
-			if (wvif->link_id_map & mask) {
-				wvif->sta_asleep_mask &= ~mask;
-				wvif->pspoll_mask &= ~mask;
-				need_reset = true;
-			}
-			wvif->link_id_map |= mask;
-			if (wvif->link_id_db[i].status != WFX_LINK_HARD)
-				wvif->link_id_db[i].status = WFX_LINK_SOFT;
-
-			spin_unlock_bh(&wvif->ps_state_lock);
-			if (need_reset)
-				wfx_unmap_link(wvif, i + 1);
-			wfx_map_link(wvif, &wvif->link_id_db[i], i + 1);
-			next_gc = min(next_gc, WFX_LINK_ID_GC_TIMEOUT);
-			spin_lock_bh(&wvif->ps_state_lock);
-		} else if (wvif->link_id_db[i].status == WFX_LINK_SOFT) {
-			ttl = wvif->link_id_db[i].timestamp - now +
-					WFX_LINK_ID_GC_TIMEOUT;
-			if (ttl <= 0) {
-				need_reset = true;
-				wvif->link_id_db[i].status = WFX_LINK_OFF;
-				wvif->link_id_map &= ~mask;
-				wvif->sta_asleep_mask &= ~mask;
-				wvif->pspoll_mask &= ~mask;
-				spin_unlock_bh(&wvif->ps_state_lock);
-				wfx_unmap_link(wvif, i + 1);
-				spin_lock_bh(&wvif->ps_state_lock);
-			} else {
-				next_gc = min_t(unsigned long, next_gc, ttl);
-			}
-		}
-	}
-	spin_unlock_bh(&wvif->ps_state_lock);
-	if (next_gc != -1)
-		schedule_delayed_work(&wvif->link_id_gc_work, next_gc);
-	wfx_tx_unlock(wvif->wdev);
-}
-
-void wfx_link_id_work(struct work_struct *work)
-{
-	struct wfx_vif *wvif =
-		container_of(work, struct wfx_vif, link_id_work);
-
-	wfx_tx_flush(wvif->wdev);
-	wfx_link_id_gc_work(&wvif->link_id_gc_work.work);
-	wfx_tx_unlock(wvif->wdev);
-}
-
 /* Tx implementation */
 
 static bool ieee80211_is_action_back(struct ieee80211_hdr *hdr)
@@ -447,9 +296,6 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 		if (wvif->sta_asleep_mask)
 			schedule_work(&wvif->mcast_start_work);
 	}
-
-	if (tx_priv->raw_link_id)
-		wvif->link_id_db[tx_priv->raw_link_id - 1].timestamp = jiffies;
 	spin_unlock_bh(&wvif->ps_state_lock);
 
 	if (sta && tx_priv->tid < WFX_MAX_TID) {
@@ -468,7 +314,6 @@ static u8 wfx_tx_get_raw_link_id(struct wfx_vif *wvif,
 	struct wfx_sta_priv *sta_priv =
 		sta ? (struct wfx_sta_priv *) &sta->drv_priv : NULL;
 	const u8 *da = ieee80211_get_DA(hdr);
-	int ret;
 
 	if (sta_priv && sta_priv->link_id)
 		return sta_priv->link_id;
@@ -476,14 +321,7 @@ static u8 wfx_tx_get_raw_link_id(struct wfx_vif *wvif,
 		return 0;
 	if (is_multicast_ether_addr(da))
 		return 0;
-	ret = wfx_find_link_id(wvif, da);
-	if (!ret)
-		ret = wfx_alloc_link_id(wvif, da);
-	if (!ret) {
-		dev_err(wvif->wdev->dev, "no more link-id available\n");
-		return WFX_LINK_ID_NO_ASSOC;
-	}
-	return ret;
+	return WFX_LINK_ID_NO_ASSOC;
 }
 
 static void wfx_tx_fixup_rates(struct ieee80211_tx_rate *rates)
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index d02a7b325b27..83720b343484 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -18,20 +18,6 @@ struct wfx_tx_priv;
 struct wfx_dev;
 struct wfx_vif;
 
-enum wfx_link_status {
-	WFX_LINK_OFF,
-	WFX_LINK_RESERVE,
-	WFX_LINK_SOFT,
-	WFX_LINK_HARD,
-};
-
-struct wfx_link_entry {
-	unsigned long		timestamp;
-	enum wfx_link_status	status;
-	u8			mac[ETH_ALEN];
-	u8			old_mac[ETH_ALEN];
-};
-
 struct tx_policy {
 	struct list_head link;
 	int usage_count;
@@ -63,11 +49,6 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg);
 void wfx_skb_dtor(struct wfx_dev *wdev, struct sk_buff *skb);
 
-int wfx_unmap_link(struct wfx_vif *wvif, int link_id);
-void wfx_link_id_work(struct work_struct *work);
-void wfx_link_id_gc_work(struct work_struct *work);
-int wfx_find_link_id(struct wfx_vif *wvif, const u8 *mac);
-
 static inline struct wfx_tx_priv *wfx_skb_tx_priv(struct sk_buff *skb)
 {
 	struct ieee80211_tx_info *tx_info;
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 92bb9a794f30..3d40388739e3 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -378,13 +378,8 @@ static bool hif_handle_tx_data(struct wfx_vif *wvif, struct sk_buff *skb,
 			action = do_drop;
 		break;
 	case NL80211_IFTYPE_AP:
-		if (!wvif->state) {
+		if (!wvif->state)
 			action = do_drop;
-		} else if (!(BIT(tx_priv->raw_link_id) &
-			     (BIT(0) | wvif->link_id_map))) {
-			dev_warn(wvif->wdev->dev, "a frame with expired link-id is dropped\n");
-			action = do_drop;
-		}
 		break;
 	case NL80211_IFTYPE_ADHOC:
 		if (wvif->state != WFX_STATE_IBSS)
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 21566e48b2c2..813c2d09e034 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -13,9 +13,10 @@
 #include "hif_api_cmd.h"
 
 #define WFX_MAX_STA_IN_AP_MODE    14
-#define WFX_LINK_ID_AFTER_DTIM    (WFX_MAX_STA_IN_AP_MODE + 1)
-#define WFX_LINK_ID_UAPSD         (WFX_MAX_STA_IN_AP_MODE + 2)
-#define WFX_LINK_ID_MAX           (WFX_MAX_STA_IN_AP_MODE + 3)
+#define WFX_LINK_ID_NO_ASSOC      15
+#define WFX_LINK_ID_AFTER_DTIM    (WFX_LINK_ID_NO_ASSOC + 1)
+#define WFX_LINK_ID_UAPSD         (WFX_LINK_ID_NO_ASSOC + 2)
+#define WFX_LINK_ID_MAX           (WFX_LINK_ID_NO_ASSOC + 3)
 
 struct wfx_dev;
 struct wfx_vif;
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 47271b7eeaab..84853aa90f4b 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -573,28 +573,26 @@ static void wfx_unjoin_work(struct work_struct *work)
 int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		struct ieee80211_sta *sta)
 {
-	struct wfx_dev *wdev = hw->priv;
 	struct wfx_vif *wvif = (struct wfx_vif *) vif->drv_priv;
 	struct wfx_sta_priv *sta_priv = (struct wfx_sta_priv *) &sta->drv_priv;
-	struct wfx_link_entry *entry;
 
 	spin_lock_init(&sta_priv->lock);
-	if (wvif->vif->type != NL80211_IFTYPE_AP)
-		return 0;
-
 	sta_priv->vif_id = wvif->id;
-	sta_priv->link_id = wfx_find_link_id(wvif, sta->addr);
-	if (!sta_priv->link_id) {
-		dev_warn(wdev->dev, "mo more link-id available\n");
-		return -ENOENT;
-	}
 
-	entry = &wvif->link_id_db[sta_priv->link_id - 1];
+	// FIXME: in station mode, the current API interprets new link-id as a
+	// tdls peer.
+	if (vif->type == NL80211_IFTYPE_STATION)
+		return 0;
+	sta_priv->link_id = ffz(wvif->link_id_map);
+	wvif->link_id_map |= BIT(sta_priv->link_id);
+	WARN_ON(!sta_priv->link_id);
+	WARN_ON(sta_priv->link_id >= WFX_MAX_STA_IN_AP_MODE);
+	hif_map_link(wvif, sta->addr, 0, sta_priv->link_id);
+
 	spin_lock_bh(&wvif->ps_state_lock);
 	if ((sta->uapsd_queues & IEEE80211_WMM_IE_STA_QOSINFO_AC_MASK) ==
 					IEEE80211_WMM_IE_STA_QOSINFO_AC_MASK)
 		wvif->sta_asleep_mask |= BIT(sta_priv->link_id);
-	entry->status = WFX_LINK_HARD;
 	spin_unlock_bh(&wvif->ps_state_lock);
 	return 0;
 }
@@ -602,23 +600,15 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 int wfx_sta_remove(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		   struct ieee80211_sta *sta)
 {
-	struct wfx_dev *wdev = hw->priv;
 	struct wfx_vif *wvif = (struct wfx_vif *) vif->drv_priv;
 	struct wfx_sta_priv *sta_priv = (struct wfx_sta_priv *) &sta->drv_priv;
-	struct wfx_link_entry *entry;
 
-	if (wvif->vif->type != NL80211_IFTYPE_AP || !sta_priv->link_id)
+	// FIXME: see note in wfx_sta_add()
+	if (vif->type == NL80211_IFTYPE_STATION)
 		return 0;
-
-	entry = &wvif->link_id_db[sta_priv->link_id - 1];
-	spin_lock_bh(&wvif->ps_state_lock);
-	entry->status = WFX_LINK_RESERVE;
-	entry->timestamp = jiffies;
-	wfx_tx_lock(wdev);
-	if (!schedule_work(&wvif->link_id_work))
-		wfx_tx_unlock(wdev);
-	spin_unlock_bh(&wvif->ps_state_lock);
-	flush_work(&wvif->link_id_work);
+	// FIXME add a mutex?
+	hif_map_link(wvif, sta->addr, 1, sta_priv->link_id);
+	wvif->link_id_map &= ~BIT(sta_priv->link_id);
 	return 0;
 }
 
@@ -627,8 +617,6 @@ static int wfx_start_ap(struct wfx_vif *wvif)
 	int ret;
 
 	wvif->beacon_int = wvif->vif->bss_conf.beacon_int;
-	memset(&wvif->link_id_db, 0, sizeof(wvif->link_id_db));
-
 	wvif->wdev->tx_burst_idx = -1;
 	ret = hif_start(wvif, &wvif->vif->bss_conf, wvif->channel);
 	if (ret)
@@ -861,7 +849,7 @@ static void wfx_ps_notify(struct wfx_vif *wvif, enum sta_notify_cmd notify_cmd,
 		dev_warn(wvif->wdev->dev, "unsupported notify command\n");
 		bit = 0;
 	} else {
-		bit = wvif->link_id_map;
+		bit = wvif->link_id_map & ~1;
 	}
 	prev = wvif->sta_asleep_mask & bit;
 
@@ -1114,9 +1102,7 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	wvif->vif = vif;
 	wvif->wdev = wdev;
 
-	INIT_WORK(&wvif->link_id_work, wfx_link_id_work);
-	INIT_DELAYED_WORK(&wvif->link_id_gc_work, wfx_link_id_gc_work);
-
+	wvif->link_id_map = 1; // link-id 0 is reserved for multicast
 	spin_lock_init(&wvif->ps_state_lock);
 	INIT_WORK(&wvif->update_tim_work, wfx_update_tim_work);
 
@@ -1171,11 +1157,11 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 {
 	struct wfx_dev *wdev = hw->priv;
 	struct wfx_vif *wvif = (struct wfx_vif *) vif->drv_priv;
-	int i;
 
 	wait_for_completion_timeout(&wvif->set_pm_mode_complete, msecs_to_jiffies(300));
 
 	mutex_lock(&wdev->conf_mutex);
+	WARN(wvif->link_id_map != 1, "corrupted state");
 	switch (wvif->state) {
 	case WFX_STATE_PRE_STA:
 	case WFX_STATE_STA:
@@ -1185,13 +1171,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 			wfx_tx_unlock(wdev);
 		break;
 	case WFX_STATE_AP:
-		for (i = 0; wvif->link_id_map; ++i) {
-			if (wvif->link_id_map & BIT(i)) {
-				wfx_unmap_link(wvif, i);
-				wvif->link_id_map &= ~BIT(i);
-			}
-		}
-		memset(wvif->link_id_db, 0, sizeof(wvif->link_id_db));
 		wvif->sta_asleep_mask = 0;
 		wvif->mcast_tx = false;
 		wvif->aid0_bit_set = false;
@@ -1213,7 +1192,6 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 
 	wfx_cqm_bssloss_sm(wvif, 0, 0, 0);
 	cancel_work_sync(&wvif->unjoin_work);
-	cancel_delayed_work_sync(&wvif->link_id_gc_work);
 	del_timer_sync(&wvif->mcast_timeout);
 	wfx_free_event_queue(wvif);
 
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index cb359150e2ad..365aacc073fb 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -74,9 +74,6 @@ struct wfx_vif {
 	struct delayed_work	bss_loss_work;
 
 	u32			link_id_map;
-	struct wfx_link_entry	link_id_db[WFX_MAX_STA_IN_AP_MODE];
-	struct delayed_work	link_id_gc_work;
-	struct work_struct	link_id_work;
 
 	bool			aid0_bit_set;
 	bool			mcast_tx;
-- 
2.25.0

Powered by blists - more mailing lists