linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/13] staging: wfx: multiple fixes
@ 2020-07-01 15:06 Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 01/13] staging: wfx: associate tx_queues to vifs Jerome Pouiller
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:06 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

Hello,

This series fixes a few issues/improvements discovered during the last months.

Jérôme Pouiller (13):
  staging: wfx: associate tx_queues to vifs
  staging: wfx: check the vif ID of the Tx confirmations
  staging: wfx: correctly retrieve vif ID from Tx confirmation
  staging: wfx: add tracepoint "queues_stats"
  staging: wfx: load the firmware faster
  staging: wfx: improve protection against malformed HIF messages
  staging: wfx: fix unexpected calls to ieee80211_sta_set_buffered()
  staging: wfx: drop counter of buffered frames
  staging: wfx: fix handling of frames without RSSI data
  staging: wfx: simplify handling of encrypted frames
  staging: wfx: fix CCMP/TKIP replay protection
  staging: wfx: add a debugfs entry to force ps_timeout
  staging: wfx: always enable FastPs in combo with new firmwares

 drivers/staging/wfx/bh.c      |  36 ++++----
 drivers/staging/wfx/data_rx.c |  85 ++++++-------------
 drivers/staging/wfx/data_tx.c | 127 +++++++++++-----------------
 drivers/staging/wfx/data_tx.h |   3 +-
 drivers/staging/wfx/debug.c   |  23 +++++
 drivers/staging/wfx/fwio.c    |   9 +-
 drivers/staging/wfx/hif_rx.c  |  22 +----
 drivers/staging/wfx/main.c    |   4 +-
 drivers/staging/wfx/queue.c   | 152 ++++++++++++++++------------------
 drivers/staging/wfx/queue.h   |  13 ++-
 drivers/staging/wfx/sta.c     |  36 +++-----
 drivers/staging/wfx/sta.h     |   4 +-
 drivers/staging/wfx/traces.h  |  51 ++++++++++++
 drivers/staging/wfx/wfx.h     |   5 +-
 14 files changed, 270 insertions(+), 300 deletions(-)

-- 
2.27.0


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

* [PATCH 01/13] staging: wfx: associate tx_queues to vifs
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
@ 2020-07-01 15:06 ` Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 02/13] staging: wfx: check the vif ID of the Tx confirmations Jerome Pouiller
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:06 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

The device handles 4 queues (one per AC) for each virtual interface (and
maximum 4 virtual interfaces). Until now the driver unified the queue of
all interfaces and handled only 4 queues for whole device.

This architecture did not allow to balance the traffic between the vif. So,
this patch relocate the queues into the vif and change the API accordingly.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c |  57 +++++++++-----
 drivers/staging/wfx/main.c    |   3 +-
 drivers/staging/wfx/queue.c   | 141 +++++++++++++++-------------------
 drivers/staging/wfx/queue.h   |  15 ++--
 drivers/staging/wfx/sta.c     |   4 +-
 drivers/staging/wfx/wfx.h     |   2 +-
 6 files changed, 111 insertions(+), 111 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index f042ef36b408e..ce3048c94961c 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -408,7 +408,7 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 
 	// Auxiliary operations
 	wfx_tx_manage_pm(wvif, hdr, tx_priv, sta);
-	wfx_tx_queues_put(wvif->wdev, skb);
+	wfx_tx_queues_put(wvif, skb);
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
 		schedule_work(&wvif->update_tim_work);
 	wfx_bh_request_tx(wvif->wdev);
@@ -539,7 +539,7 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	const struct wfx_tx_priv *tx_priv;
 	struct sk_buff *skb;
 
-	skb = wfx_pending_get(wvif->wdev, arg->packet_id);
+	skb = wfx_pending_get(wvif, arg->packet_id);
 	if (!skb) {
 		dev_warn(wvif->wdev->dev, "received unknown packet_id (%#.8x) from chip\n",
 			 arg->packet_id);
@@ -582,34 +582,50 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	wfx_skb_dtor(wvif, skb);
 }
 
+static void wfx_flush_vif(struct wfx_vif *wvif, u32 queues,
+			  struct sk_buff_head *dropped)
+{
+	struct wfx_queue *queue;
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		if (!(BIT(i) & queues))
+			continue;
+		queue = &wvif->tx_queue[i];
+		if (dropped)
+			wfx_tx_queue_drop(wvif, queue, dropped);
+	}
+	if (wvif->wdev->chip_frozen)
+		return;
+	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+		if (!(BIT(i) & queues))
+			continue;
+		queue = &wvif->tx_queue[i];
+		if (wait_event_timeout(wvif->wdev->tx_dequeue,
+				       wfx_tx_queue_empty(wvif, queue),
+				       msecs_to_jiffies(1000)) <= 0)
+			dev_warn(wvif->wdev->dev,
+				 "frames queued while flushing tx queues?");
+	}
+}
+
 void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	       u32 queues, bool drop)
 {
 	struct wfx_dev *wdev = hw->priv;
 	struct sk_buff_head dropped;
-	struct wfx_queue *queue;
 	struct wfx_vif *wvif;
 	struct hif_msg *hif;
 	struct sk_buff *skb;
-	int vif_id = -1;
-	int i;
 
-	if (vif)
-		vif_id = ((struct wfx_vif *)vif->drv_priv)->id;
 	skb_queue_head_init(&dropped);
-	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
-		if (!(BIT(i) & queues))
-			continue;
-		queue = &wdev->tx_queue[i];
-		if (drop)
-			wfx_tx_queue_drop(wdev, queue, vif_id, &dropped);
-		if (wdev->chip_frozen)
-			continue;
-		if (wait_event_timeout(wdev->tx_dequeue,
-				       wfx_tx_queue_empty(wdev, queue, vif_id),
-				       msecs_to_jiffies(1000)) <= 0)
-			dev_warn(wdev->dev,
-				 "frames queued while flushing tx queues?");
+	if (vif) {
+		wvif = (struct wfx_vif *)vif->drv_priv;
+		wfx_flush_vif(wvif, queues, drop ? &dropped : NULL);
+	} else {
+		wvif = NULL;
+		while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
+			wfx_flush_vif(wvif, queues, drop ? &dropped : NULL);
 	}
 	wfx_tx_flush(wdev);
 	if (wdev->chip_frozen)
@@ -623,4 +639,3 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		wfx_skb_dtor(wvif, skb);
 	}
 }
-
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 6bd96f4763884..80e4474cc3314 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -349,8 +349,9 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 	init_completion(&wdev->firmware_ready);
 	INIT_DELAYED_WORK(&wdev->cooling_timeout_work,
 			  wfx_cooling_timeout_work);
+	skb_queue_head_init(&wdev->tx_pending);
+	init_waitqueue_head(&wdev->tx_dequeue);
 	wfx_init_hif_cmd(&wdev->hif_cmd);
-	wfx_tx_queues_init(wdev);
 
 	if (devm_add_action_or_reset(dev, wfx_free_common, wdev))
 		return NULL;
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 93ea2b72febd0..7ec36598d9a83 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -57,84 +57,57 @@ void wfx_tx_lock_flush(struct wfx_dev *wdev)
 	wfx_tx_flush(wdev);
 }
 
-void wfx_tx_queues_init(struct wfx_dev *wdev)
+void wfx_tx_queues_init(struct wfx_vif *wvif)
 {
 	int i;
 
-	skb_queue_head_init(&wdev->tx_pending);
-	init_waitqueue_head(&wdev->tx_dequeue);
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
-		skb_queue_head_init(&wdev->tx_queue[i].normal);
-		skb_queue_head_init(&wdev->tx_queue[i].cab);
+		skb_queue_head_init(&wvif->tx_queue[i].normal);
+		skb_queue_head_init(&wvif->tx_queue[i].cab);
 	}
 }
 
-void wfx_tx_queues_check_empty(struct wfx_dev *wdev)
+void wfx_tx_queues_check_empty(struct wfx_vif *wvif)
 {
 	int i;
 
-	WARN_ON(!skb_queue_empty_lockless(&wdev->tx_pending));
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
-		WARN_ON(atomic_read(&wdev->tx_queue[i].pending_frames));
-		WARN_ON(!skb_queue_empty_lockless(&wdev->tx_queue[i].normal));
-		WARN_ON(!skb_queue_empty_lockless(&wdev->tx_queue[i].cab));
+		WARN_ON(atomic_read(&wvif->tx_queue[i].pending_frames));
+		WARN_ON(!skb_queue_empty_lockless(&wvif->tx_queue[i].normal));
+		WARN_ON(!skb_queue_empty_lockless(&wvif->tx_queue[i].cab));
 	}
 }
 
-static bool __wfx_tx_queue_empty(struct wfx_dev *wdev,
-				 struct sk_buff_head *skb_queue, int vif_id)
+bool wfx_tx_queue_empty(struct wfx_vif *wvif, struct wfx_queue *queue)
 {
-	struct hif_msg *hif_msg;
-	struct sk_buff *skb;
-
-	spin_lock_bh(&skb_queue->lock);
-	skb_queue_walk(skb_queue, skb) {
-		hif_msg = (struct hif_msg *)skb->data;
-		if (vif_id < 0 || hif_msg->interface == vif_id) {
-			spin_unlock_bh(&skb_queue->lock);
-			return false;
-		}
-	}
-	spin_unlock_bh(&skb_queue->lock);
-	return true;
-}
-
-bool wfx_tx_queue_empty(struct wfx_dev *wdev,
-			struct wfx_queue *queue, int vif_id)
-{
-	return __wfx_tx_queue_empty(wdev, &queue->normal, vif_id) &&
-	       __wfx_tx_queue_empty(wdev, &queue->cab, vif_id);
+	return skb_queue_empty(&queue->normal) && skb_queue_empty(&queue->cab);
 }
 
-static void __wfx_tx_queue_drop(struct wfx_dev *wdev,
-				struct sk_buff_head *skb_queue, int vif_id,
+static void __wfx_tx_queue_drop(struct wfx_vif *wvif,
+				struct sk_buff_head *skb_queue,
 				struct sk_buff_head *dropped)
 {
 	struct sk_buff *skb, *tmp;
-	struct hif_msg *hif_msg;
 
 	spin_lock_bh(&skb_queue->lock);
 	skb_queue_walk_safe(skb_queue, skb, tmp) {
-		hif_msg = (struct hif_msg *)skb->data;
-		if (vif_id < 0 || hif_msg->interface == vif_id) {
-			__skb_unlink(skb, skb_queue);
-			skb_queue_head(dropped, skb);
-		}
+		__skb_unlink(skb, skb_queue);
+		skb_queue_head(dropped, skb);
 	}
 	spin_unlock_bh(&skb_queue->lock);
 }
 
-void wfx_tx_queue_drop(struct wfx_dev *wdev, struct wfx_queue *queue,
-		       int vif_id, struct sk_buff_head *dropped)
+void wfx_tx_queue_drop(struct wfx_vif *wvif, struct wfx_queue *queue,
+		       struct sk_buff_head *dropped)
 {
-	__wfx_tx_queue_drop(wdev, &queue->cab, vif_id, dropped);
-	__wfx_tx_queue_drop(wdev, &queue->normal, vif_id, dropped);
-	wake_up(&wdev->tx_dequeue);
+	__wfx_tx_queue_drop(wvif, &queue->cab, dropped);
+	__wfx_tx_queue_drop(wvif, &queue->normal, dropped);
+	wake_up(&wvif->wdev->tx_dequeue);
 }
 
-void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb)
+void wfx_tx_queues_put(struct wfx_vif *wvif, struct sk_buff *skb)
 {
-	struct wfx_queue *queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
+	struct wfx_queue *queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
 	struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(skb);
 
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
@@ -146,39 +119,45 @@ void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb)
 void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped)
 {
 	struct wfx_queue *queue;
+	struct wfx_vif *wvif;
+	struct hif_msg *hif;
 	struct sk_buff *skb;
 
 	WARN(!wdev->chip_frozen, "%s should only be used to recover a frozen device",
 	     __func__);
 	while ((skb = skb_dequeue(&wdev->tx_pending)) != NULL) {
-		queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
-		WARN_ON(skb_get_queue_mapping(skb) > 3);
-		WARN_ON(!atomic_read(&queue->pending_frames));
-		atomic_dec(&queue->pending_frames);
+		hif = (struct hif_msg *)skb->data;
+		wvif = wdev_to_wvif(wdev, hif->interface);
+		if (wvif) {
+			queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
+			WARN_ON(skb_get_queue_mapping(skb) > 3);
+			WARN_ON(!atomic_read(&queue->pending_frames));
+			atomic_dec(&queue->pending_frames);
+		}
 		skb_queue_head(dropped, skb);
 	}
 }
 
-struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
+struct sk_buff *wfx_pending_get(struct wfx_vif *wvif, u32 packet_id)
 {
 	struct wfx_queue *queue;
 	struct hif_req_tx *req;
 	struct sk_buff *skb;
 
-	spin_lock_bh(&wdev->tx_pending.lock);
-	skb_queue_walk(&wdev->tx_pending, skb) {
+	spin_lock_bh(&wvif->wdev->tx_pending.lock);
+	skb_queue_walk(&wvif->wdev->tx_pending, skb) {
 		req = wfx_skb_txreq(skb);
 		if (req->packet_id == packet_id) {
-			spin_unlock_bh(&wdev->tx_pending.lock);
-			queue = &wdev->tx_queue[skb_get_queue_mapping(skb)];
+			spin_unlock_bh(&wvif->wdev->tx_pending.lock);
+			queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
 			WARN_ON(skb_get_queue_mapping(skb) > 3);
 			WARN_ON(!atomic_read(&queue->pending_frames));
 			atomic_dec(&queue->pending_frames);
-			skb_unlink(skb, &wdev->tx_pending);
+			skb_unlink(skb, &wvif->wdev->tx_pending);
 			return skb;
 		}
 	}
-	spin_unlock_bh(&wdev->tx_pending.lock);
+	spin_unlock_bh(&wvif->wdev->tx_pending.lock);
 	WARN(1, "cannot find packet in pending queue");
 	return NULL;
 }
@@ -221,7 +200,6 @@ unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 
 bool wfx_tx_queues_has_cab(struct wfx_vif *wvif)
 {
-	struct wfx_dev *wdev = wvif->wdev;
 	int i;
 
 	if (wvif->vif->type != NL80211_IFTYPE_AP)
@@ -229,33 +207,39 @@ bool wfx_tx_queues_has_cab(struct wfx_vif *wvif)
 	for (i = 0; i < IEEE80211_NUM_ACS; ++i)
 		// Note: since only AP can have mcast frames in queue and only
 		// one vif can be AP, all queued frames has same interface id
-		if (!skb_queue_empty_lockless(&wdev->tx_queue[i].cab))
+		if (!skb_queue_empty_lockless(&wvif->tx_queue[i].cab))
 			return true;
 	return false;
 }
 
 static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 {
-	struct wfx_queue *sorted_queues[IEEE80211_NUM_ACS];
+	struct wfx_queue *queues[IEEE80211_NUM_ACS * ARRAY_SIZE(wdev->vif)];
+	int i, j, num_queues = 0;
 	struct wfx_vif *wvif;
 	struct hif_msg *hif;
 	struct sk_buff *skb;
-	int i, j;
 
-	// bubble sort
-	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
-		sorted_queues[i] = &wdev->tx_queue[i];
-		for (j = i; j > 0; j--)
-			if (atomic_read(&sorted_queues[j]->pending_frames) <
-			    atomic_read(&sorted_queues[j - 1]->pending_frames))
-				swap(sorted_queues[j - 1], sorted_queues[j]);
+	// sort the queues
+	wvif = NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
+		for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+			WARN_ON(num_queues >= ARRAY_SIZE(queues));
+			queues[num_queues] = &wvif->tx_queue[i];
+			for (j = num_queues; j > 0; j--)
+				if (atomic_read(&queues[j]->pending_frames) <
+				    atomic_read(&queues[j - 1]->pending_frames))
+					swap(queues[j - 1], queues[j]);
+			num_queues++;
+		}
 	}
+
 	wvif = NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
 		if (!wvif->after_dtim_tx_allowed)
 			continue;
-		for (i = 0; i < IEEE80211_NUM_ACS; i++) {
-			skb = skb_dequeue(&sorted_queues[i]->cab);
+		for (i = 0; i < num_queues; i++) {
+			skb = skb_dequeue(&queues[i]->cab);
 			if (!skb)
 				continue;
 			// Note: since only AP can have mcast frames in queue
@@ -263,21 +247,20 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 			// same interface id
 			hif = (struct hif_msg *)skb->data;
 			WARN_ON(hif->interface != wvif->id);
-			WARN_ON(sorted_queues[i] !=
-				&wdev->tx_queue[skb_get_queue_mapping(skb)]);
-			atomic_inc(&sorted_queues[i]->pending_frames);
+			WARN_ON(queues[i] !=
+				&wvif->tx_queue[skb_get_queue_mapping(skb)]);
+			atomic_inc(&queues[i]->pending_frames);
 			return skb;
 		}
 		// No more multicast to sent
 		wvif->after_dtim_tx_allowed = false;
 		schedule_work(&wvif->update_tim_work);
 	}
-	for (i = 0; i < IEEE80211_NUM_ACS; i++) {
-		skb = skb_dequeue(&sorted_queues[i]->normal);
+
+	for (i = 0; i < num_queues; i++) {
+		skb = skb_dequeue(&queues[i]->normal);
 		if (skb) {
-			WARN_ON(sorted_queues[i] !=
-				&wdev->tx_queue[skb_get_queue_mapping(skb)]);
-			atomic_inc(&sorted_queues[i]->pending_frames);
+			atomic_inc(&queues[i]->pending_frames);
 			return skb;
 		}
 	}
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index 0c3b7244498e3..dfbbe4b111113 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -25,18 +25,17 @@ void wfx_tx_unlock(struct wfx_dev *wdev);
 void wfx_tx_flush(struct wfx_dev *wdev);
 void wfx_tx_lock_flush(struct wfx_dev *wdev);
 
-void wfx_tx_queues_init(struct wfx_dev *wdev);
-void wfx_tx_queues_check_empty(struct wfx_dev *wdev);
+void wfx_tx_queues_init(struct wfx_vif *wvif);
+void wfx_tx_queues_check_empty(struct wfx_vif *wvif);
 bool wfx_tx_queues_has_cab(struct wfx_vif *wvif);
-void wfx_tx_queues_put(struct wfx_dev *wdev, struct sk_buff *skb);
+void wfx_tx_queues_put(struct wfx_vif *wvif, struct sk_buff *skb);
 struct hif_msg *wfx_tx_queues_get(struct wfx_dev *wdev);
 
-bool wfx_tx_queue_empty(struct wfx_dev *wdev, struct wfx_queue *queue,
-			int vif_id);
-void wfx_tx_queue_drop(struct wfx_dev *wdev, struct wfx_queue *queue,
-		       int vif_id, struct sk_buff_head *dropped);
+bool wfx_tx_queue_empty(struct wfx_vif *wvif, struct wfx_queue *queue);
+void wfx_tx_queue_drop(struct wfx_vif *wvif, struct wfx_queue *queue,
+		       struct sk_buff_head *dropped);
 
-struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
+struct sk_buff *wfx_pending_get(struct wfx_vif *wvif, u32 packet_id);
 void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped);
 unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 					  struct sk_buff *skb);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index d855d87c21028..85d4bc2949882 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -805,6 +805,7 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 
 	hif_set_macaddr(wvif, vif->addr);
 
+	wfx_tx_queues_init(wvif);
 	wfx_tx_policy_init(wvif);
 	wvif = NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
@@ -823,6 +824,7 @@ void wfx_remove_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 
 	wait_for_completion_timeout(&wvif->set_pm_mode_complete, msecs_to_jiffies(300));
+	wfx_tx_queues_check_empty(wvif);
 
 	mutex_lock(&wdev->conf_mutex);
 	WARN(wvif->link_id_map != 1, "corrupted state");
@@ -855,5 +857,5 @@ void wfx_stop(struct ieee80211_hw *hw)
 {
 	struct wfx_dev *wdev = hw->priv;
 
-	wfx_tx_queues_check_empty(wdev);
+	WARN_ON(!skb_queue_empty_lockless(&wdev->tx_pending));
 }
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 73e216733ce4f..0c44b733ef6fe 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -48,7 +48,6 @@ struct wfx_dev {
 	struct mutex		conf_mutex;
 
 	struct wfx_hif_cmd	hif_cmd;
-	struct wfx_queue	tx_queue[4];
 	struct sk_buff_head	tx_pending;
 	wait_queue_head_t	tx_dequeue;
 	atomic_t		tx_lock;
@@ -75,6 +74,7 @@ struct wfx_vif {
 
 	struct delayed_work	beacon_loss_work;
 
+	struct wfx_queue	tx_queue[4];
 	struct tx_policy_cache	tx_policy_cache;
 	struct work_struct	tx_policy_upload_work;
 
-- 
2.27.0


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

* [PATCH 02/13] staging: wfx: check the vif ID of the Tx confirmations
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 01/13] staging: wfx: associate tx_queues to vifs Jerome Pouiller
@ 2020-07-01 15:06 ` Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 03/13] staging: wfx: correctly retrieve vif ID from Tx confirmation Jerome Pouiller
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:06 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

When the driver has sent a frame on a virtual interface (vif), it
expects to receive the confirmation on the same vif.

This patch add a check for that.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 7ec36598d9a83..6069143369f30 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -142,14 +142,18 @@ struct sk_buff *wfx_pending_get(struct wfx_vif *wvif, u32 packet_id)
 {
 	struct wfx_queue *queue;
 	struct hif_req_tx *req;
+	struct hif_msg *hif;
 	struct sk_buff *skb;
 
 	spin_lock_bh(&wvif->wdev->tx_pending.lock);
 	skb_queue_walk(&wvif->wdev->tx_pending, skb) {
-		req = wfx_skb_txreq(skb);
+		hif = (struct hif_msg *)skb->data;
+		req = (struct hif_req_tx *)hif->body;
 		if (req->packet_id == packet_id) {
 			spin_unlock_bh(&wvif->wdev->tx_pending.lock);
 			queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
+			WARN(hif->interface != wvif->id, "sent frame %08x on vif %d, but get reply on vif %d",
+			     req->packet_id, hif->interface, wvif->id);
 			WARN_ON(skb_get_queue_mapping(skb) > 3);
 			WARN_ON(!atomic_read(&queue->pending_frames));
 			atomic_dec(&queue->pending_frames);
-- 
2.27.0


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

* [PATCH 03/13] staging: wfx: correctly retrieve vif ID from Tx confirmation
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 01/13] staging: wfx: associate tx_queues to vifs Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 02/13] staging: wfx: check the vif ID of the Tx confirmations Jerome Pouiller
@ 2020-07-01 15:06 ` Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 04/13] staging: wfx: add tracepoint "queues_stats" Jerome Pouiller
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:06 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

The device is able to send multiple Tx confirmations in the one reply.
In this case, there is only one vif identifier for all the
confirmations.

Unfortunately, to generate this kind of messages the device squashes all
the confirmations whatever their vif ID and use the vif ID of the first
confirmation. So, the driver cannot rely on the vif ID mentioned in the
header. Fortunately, using the packet_id, the driver can retrieve the Tx
request and the associated vif.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 16 ++++++++++------
 drivers/staging/wfx/data_tx.h |  2 +-
 drivers/staging/wfx/hif_rx.c  | 14 ++------------
 drivers/staging/wfx/queue.c   | 22 ++++++++++++----------
 drivers/staging/wfx/queue.h   |  2 +-
 5 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index ce3048c94961c..dcec722afb174 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -533,25 +533,29 @@ static void wfx_tx_fill_rates(struct wfx_dev *wdev,
 		dev_dbg(wdev->dev, "%d more retries than expected\n", tx_count);
 }
 
-void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
+void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)
 {
 	struct ieee80211_tx_info *tx_info;
 	const struct wfx_tx_priv *tx_priv;
+	struct wfx_vif *wvif;
 	struct sk_buff *skb;
 
-	skb = wfx_pending_get(wvif, arg->packet_id);
+	skb = wfx_pending_get(wdev, arg->packet_id);
 	if (!skb) {
-		dev_warn(wvif->wdev->dev, "received unknown packet_id (%#.8x) from chip\n",
+		dev_warn(wdev->dev, "received unknown packet_id (%#.8x) from chip\n",
 			 arg->packet_id);
 		return;
 	}
+	wvif = wdev_to_wvif(wdev, ((struct hif_msg *)skb->data)->interface);
+	WARN_ON(!wvif);
+	if (!wvif)
+		return;
 	tx_info = IEEE80211_SKB_CB(skb);
 	tx_priv = wfx_skb_tx_priv(skb);
-	_trace_tx_stats(arg, skb,
-			wfx_pending_get_pkt_us_delay(wvif->wdev, skb));
+	_trace_tx_stats(arg, skb, wfx_pending_get_pkt_us_delay(wdev, skb));
 
 	// You can touch to tx_priv, but don't touch to tx_info->status.
-	wfx_tx_fill_rates(wvif->wdev, tx_info, arg);
+	wfx_tx_fill_rates(wdev, tx_info, arg);
 	if (tx_priv->has_sta)
 		wfx_tx_update_sta(wvif, wfx_skb_hdr80211(skb));
 	skb_trim(skb, skb->len - wfx_tx_get_icv_len(tx_priv->hw_key));
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index 54fff24508fb9..b1727ddecd5e2 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -44,7 +44,7 @@ void wfx_tx_policy_upload_work(struct work_struct *work);
 
 void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	    struct sk_buff *skb);
-void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg);
+void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg);
 void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	       u32 queues, bool drop);
 
diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index bb156033d1e16..e3ebd910fabfd 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -63,13 +63,8 @@ static int hif_tx_confirm(struct wfx_dev *wdev,
 			  const struct hif_msg *hif, const void *buf)
 {
 	const struct hif_cnf_tx *body = buf;
-	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
 
-	WARN_ON(!wvif);
-	if (!wvif)
-		return -EFAULT;
-
-	wfx_tx_confirm_cb(wvif, body);
+	wfx_tx_confirm_cb(wdev, body);
 	return 0;
 }
 
@@ -77,16 +72,11 @@ static int hif_multi_tx_confirm(struct wfx_dev *wdev,
 				const struct hif_msg *hif, const void *buf)
 {
 	const struct hif_cnf_multi_transmit *body = buf;
-	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
 	int i;
 
 	WARN(body->num_tx_confs <= 0, "corrupted message");
-	WARN_ON(!wvif);
-	if (!wvif)
-		return -EFAULT;
-
 	for (i = 0; i < body->num_tx_confs; i++)
-		wfx_tx_confirm_cb(wvif, &body->tx_conf_payload[i]);
+		wfx_tx_confirm_cb(wdev, &body->tx_conf_payload[i]);
 	return 0;
 }
 
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 6069143369f30..678f622639093 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -138,30 +138,32 @@ void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped)
 	}
 }
 
-struct sk_buff *wfx_pending_get(struct wfx_vif *wvif, u32 packet_id)
+struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id)
 {
 	struct wfx_queue *queue;
 	struct hif_req_tx *req;
+	struct wfx_vif *wvif;
 	struct hif_msg *hif;
 	struct sk_buff *skb;
 
-	spin_lock_bh(&wvif->wdev->tx_pending.lock);
-	skb_queue_walk(&wvif->wdev->tx_pending, skb) {
+	spin_lock_bh(&wdev->tx_pending.lock);
+	skb_queue_walk(&wdev->tx_pending, skb) {
 		hif = (struct hif_msg *)skb->data;
 		req = (struct hif_req_tx *)hif->body;
-		if (req->packet_id == packet_id) {
-			spin_unlock_bh(&wvif->wdev->tx_pending.lock);
+		if (req->packet_id != packet_id)
+			continue;
+		spin_unlock_bh(&wdev->tx_pending.lock);
+		wvif = wdev_to_wvif(wdev, hif->interface);
+		if (wvif) {
 			queue = &wvif->tx_queue[skb_get_queue_mapping(skb)];
-			WARN(hif->interface != wvif->id, "sent frame %08x on vif %d, but get reply on vif %d",
-			     req->packet_id, hif->interface, wvif->id);
 			WARN_ON(skb_get_queue_mapping(skb) > 3);
 			WARN_ON(!atomic_read(&queue->pending_frames));
 			atomic_dec(&queue->pending_frames);
-			skb_unlink(skb, &wvif->wdev->tx_pending);
-			return skb;
 		}
+		skb_unlink(skb, &wdev->tx_pending);
+		return skb;
 	}
-	spin_unlock_bh(&wvif->wdev->tx_pending.lock);
+	spin_unlock_bh(&wdev->tx_pending.lock);
 	WARN(1, "cannot find packet in pending queue");
 	return NULL;
 }
diff --git a/drivers/staging/wfx/queue.h b/drivers/staging/wfx/queue.h
index dfbbe4b111113..22d7c936907f4 100644
--- a/drivers/staging/wfx/queue.h
+++ b/drivers/staging/wfx/queue.h
@@ -35,7 +35,7 @@ bool wfx_tx_queue_empty(struct wfx_vif *wvif, struct wfx_queue *queue);
 void wfx_tx_queue_drop(struct wfx_vif *wvif, struct wfx_queue *queue,
 		       struct sk_buff_head *dropped);
 
-struct sk_buff *wfx_pending_get(struct wfx_vif *wvif, u32 packet_id);
+struct sk_buff *wfx_pending_get(struct wfx_dev *wdev, u32 packet_id);
 void wfx_pending_drop(struct wfx_dev *wdev, struct sk_buff_head *dropped);
 unsigned int wfx_pending_get_pkt_us_delay(struct wfx_dev *wdev,
 					  struct sk_buff *skb);
-- 
2.27.0


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

* [PATCH 04/13] staging: wfx: add tracepoint "queues_stats"
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (2 preceding siblings ...)
  2020-07-01 15:06 ` [PATCH 03/13] staging: wfx: correctly retrieve vif ID from Tx confirmation Jerome Pouiller
@ 2020-07-01 15:06 ` Jerome Pouiller
  2020-07-01 15:06 ` [PATCH 05/13] staging: wfx: load the firmware faster Jerome Pouiller
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:06 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

It is useful to check which queue the driver choose to send to the
hardware.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/queue.c  |  3 +++
 drivers/staging/wfx/traces.h | 51 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index 678f622639093..6e31591651432 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -12,6 +12,7 @@
 #include "wfx.h"
 #include "sta.h"
 #include "data_tx.h"
+#include "traces.h"
 
 void wfx_tx_lock(struct wfx_dev *wdev)
 {
@@ -256,6 +257,7 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 			WARN_ON(queues[i] !=
 				&wvif->tx_queue[skb_get_queue_mapping(skb)]);
 			atomic_inc(&queues[i]->pending_frames);
+			trace_queues_stats(wdev, queues[i]);
 			return skb;
 		}
 		// No more multicast to sent
@@ -267,6 +269,7 @@ static struct sk_buff *wfx_tx_queues_get_skb(struct wfx_dev *wdev)
 		skb = skb_dequeue(&queues[i]->normal);
 		if (skb) {
 			atomic_inc(&queues[i]->pending_frames);
+			trace_queues_stats(wdev, queues[i]);
 			return skb;
 		}
 	}
diff --git a/drivers/staging/wfx/traces.h b/drivers/staging/wfx/traces.h
index 0b6fbd5186381..d376db2f1891b 100644
--- a/drivers/staging/wfx/traces.h
+++ b/drivers/staging/wfx/traces.h
@@ -439,6 +439,57 @@ TRACE_EVENT(tx_stats,
 );
 #define _trace_tx_stats(tx_cnf, skb, delay) trace_tx_stats(tx_cnf, skb, delay)
 
+TRACE_EVENT(queues_stats,
+	TP_PROTO(struct wfx_dev *wdev, const struct wfx_queue *elected_queue),
+	TP_ARGS(wdev, elected_queue),
+	TP_STRUCT__entry(
+		__field(int, vif_id)
+		__field(int, queue_id)
+		__array(int, hw, IEEE80211_NUM_ACS * 2)
+		__array(int, drv, IEEE80211_NUM_ACS * 2)
+		__array(int, cab, IEEE80211_NUM_ACS * 2)
+	),
+	TP_fast_assign(
+		const struct wfx_queue *queue;
+		struct wfx_vif *wvif;
+		int i, j;
+
+		for (j = 0; j < IEEE80211_NUM_ACS * 2; j++) {
+			__entry->hw[j] = -1;
+			__entry->drv[j] = -1;
+			__entry->cab[j] = -1;
+		}
+		__entry->vif_id = -1;
+		__entry->queue_id = -1;
+		wvif = NULL;
+		while ((wvif = wvif_iterate(wdev, wvif)) != NULL) {
+			for (i = 0; i < IEEE80211_NUM_ACS; i++) {
+				j = wvif->id * IEEE80211_NUM_ACS + i;
+				WARN_ON(j >= IEEE80211_NUM_ACS * 2);
+				queue = &wvif->tx_queue[i];
+				__entry->hw[j] = atomic_read(&queue->pending_frames);
+				__entry->drv[j] = skb_queue_len(&queue->normal);
+				__entry->cab[j] = skb_queue_len(&queue->cab);
+				if (queue == elected_queue) {
+					__entry->vif_id = wvif->id;
+					__entry->queue_id = i;
+				}
+			}
+		}
+	),
+	TP_printk("got skb from %d/%d, pend. hw/norm/cab: [ %d/%d/%d %d/%d/%d %d/%d/%d %d/%d/%d ] [ %d/%d/%d %d/%d/%d %d/%d/%d %d/%d/%d ]",
+		__entry->vif_id, __entry->queue_id,
+		__entry->hw[0], __entry->drv[0], __entry->cab[0],
+		__entry->hw[1], __entry->drv[1], __entry->cab[1],
+		__entry->hw[2], __entry->drv[2], __entry->cab[2],
+		__entry->hw[3], __entry->drv[3], __entry->cab[3],
+		__entry->hw[4], __entry->drv[4], __entry->cab[4],
+		__entry->hw[5], __entry->drv[5], __entry->cab[5],
+		__entry->hw[6], __entry->drv[6], __entry->cab[6],
+		__entry->hw[7], __entry->drv[7], __entry->cab[7]
+	)
+);
+
 #endif
 
 /* This part must be outside protection */
-- 
2.27.0


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

* [PATCH 05/13] staging: wfx: load the firmware faster
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (3 preceding siblings ...)
  2020-07-01 15:06 ` [PATCH 04/13] staging: wfx: add tracepoint "queues_stats" Jerome Pouiller
@ 2020-07-01 15:06 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 06/13] staging: wfx: improve protection against malformed HIF messages Jerome Pouiller
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:06 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

During the loading of the firmware, the WFX_DCA_GET register provide the
number available bytes in the receiving buffer. It is not necessary to
access to the WFX_DCA_GET after sent of each firmware fragment.

This patch allows to send the firmware:
  - in 64ms instead of 130ms using SDIO bus
  - in 78ms instead of 115ms using SPI bus

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/fwio.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wfx/fwio.c b/drivers/staging/wfx/fwio.c
index 72bb3d2a96138..d9a886f3e64be 100644
--- a/drivers/staging/wfx/fwio.c
+++ b/drivers/staging/wfx/fwio.c
@@ -188,15 +188,14 @@ static int upload_firmware(struct wfx_dev *wdev, const u8 *data, size_t len)
 	while (offs < len) {
 		start = ktime_get();
 		for (;;) {
-			ret = sram_reg_read(wdev, WFX_DCA_GET, &bytes_done);
-			if (ret < 0)
-				return ret;
 			now = ktime_get();
-			if (offs +
-			    DNLD_BLOCK_SIZE - bytes_done < DNLD_FIFO_SIZE)
+			if (offs + DNLD_BLOCK_SIZE - bytes_done < DNLD_FIFO_SIZE)
 				break;
 			if (ktime_after(now, ktime_add_ms(start, DCA_TIMEOUT)))
 				return -ETIMEDOUT;
+			ret = sram_reg_read(wdev, WFX_DCA_GET, &bytes_done);
+			if (ret < 0)
+				return ret;
 		}
 		if (ktime_compare(now, start))
 			dev_dbg(wdev->dev, "answer after %lldus\n",
-- 
2.27.0


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

* [PATCH 06/13] staging: wfx: improve protection against malformed HIF messages
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (4 preceding siblings ...)
  2020-07-01 15:06 ` [PATCH 05/13] staging: wfx: load the firmware faster Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 07/13] staging: wfx: fix unexpected calls to ieee80211_sta_set_buffered() Jerome Pouiller
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

As discussed here[1], if a message was smaller than the size of the
message header, it could be incorrectly processed.

[1] https://lore.kernel.org/driverdev-devel/2302785.6C7ODC2LYm@pc-42/

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/bh.c | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/wfx/bh.c b/drivers/staging/wfx/bh.c
index 1cbaf8bb4fa38..53ae0b5abcdd8 100644
--- a/drivers/staging/wfx/bh.c
+++ b/drivers/staging/wfx/bh.c
@@ -57,7 +57,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	int release_count;
 	int piggyback = 0;
 
-	WARN(read_len < 4, "corrupted read");
 	WARN(read_len > round_down(0xFFF, 2) * sizeof(u16),
 	     "%s: request exceed WFx capability", __func__);
 
@@ -76,7 +75,27 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 	hif = (struct hif_msg *)skb->data;
 	WARN(hif->encrypted & 0x1, "unsupported encryption type");
 	if (hif->encrypted == 0x2) {
-		if (wfx_sl_decode(wdev, (void *)hif)) {
+		if (WARN(read_len < sizeof(struct hif_sl_msg), "corrupted read"))
+			goto err;
+		computed_len = le16_to_cpu(((struct hif_sl_msg *)hif)->len);
+		computed_len = round_up(computed_len - sizeof(u16), 16);
+		computed_len += sizeof(struct hif_sl_msg);
+		computed_len += sizeof(struct hif_sl_tag);
+	} else {
+		if (WARN(read_len < sizeof(struct hif_msg), "corrupted read"))
+			goto err;
+		computed_len = le16_to_cpu(hif->len);
+		computed_len = round_up(computed_len, 2);
+	}
+	if (computed_len != read_len) {
+		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
+			computed_len, read_len);
+		print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET, 16, 1,
+			       hif, read_len, true);
+		goto err;
+	}
+	if (hif->encrypted == 0x2) {
+		if (wfx_sl_decode(wdev, (struct hif_sl_msg *)hif)) {
 			dev_kfree_skb(skb);
 			// If frame was a confirmation, expect trouble in next
 			// exchange. However, it is harmless to fail to decode
@@ -84,19 +103,6 @@ static int rx_helper(struct wfx_dev *wdev, size_t read_len, int *is_cnf)
 			// piggyback is probably correct.
 			return piggyback;
 		}
-		computed_len =
-			round_up(le16_to_cpu(hif->len) - sizeof(hif->len), 16) +
-			sizeof(struct hif_sl_msg) +
-			sizeof(struct hif_sl_tag);
-	} else {
-		computed_len = round_up(le16_to_cpu(hif->len), 2);
-	}
-	if (computed_len != read_len) {
-		dev_err(wdev->dev, "inconsistent message length: %zu != %zu\n",
-			computed_len, read_len);
-		print_hex_dump(KERN_INFO, "hif: ", DUMP_PREFIX_OFFSET, 16, 1,
-			       hif, read_len, true);
-		goto err;
 	}
 
 	if (!(hif->id & HIF_ID_IS_INDICATION)) {
-- 
2.27.0


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

* [PATCH 07/13] staging: wfx: fix unexpected calls to ieee80211_sta_set_buffered()
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (5 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 06/13] staging: wfx: improve protection against malformed HIF messages Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 08/13] staging: wfx: drop counter of buffered frames Jerome Pouiller
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

When a station go to sleep, the driver receive the status REQUEUE and
forward this answer to mac80211. So, neither the driver, neither the
hardware buffer the frames. So the call to ieee80211_sta_set_buffered is
useless.

In add, it seems that mac80211 does not expect to receive
ieee80211_sta_set_buffered(false) after the station is asleep(). When
the device send data to a station, the following sequence can be
observed:

   - Mac80211 call wfx_sta_notify(awake).
   - The driver calls ieee80211_sta_set_buffered(true). Since the
     station is awake, its TIM is not set.
   - Mac80211 receive a power save notification from the station, so it
     calls wfx_sta_notify(asleep).
   - Then, since the driver has declared it has buffered some frames,
     the TIM of the station should be set. This action is delayed by
     mac80211.
   - The device also notice the station go to sleep. It replies the
     REQUEUE status for the buffered frames. The driver forward this
     status to mac80211.
   - There is no more frames in queues, so the driver call
     ieee80211_sta_set_buffered(false).
   - Mac80211 updates the TIM but since there is no frames buffered by
     the driver, it set the TIM for the station to 0.

Anyway, correctly use the ieee80211_sta_set_buffered() API solves the
problem.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index dcec722afb174..3244a768345c5 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -225,7 +225,6 @@ static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
 		sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
 		spin_lock_bh(&sta_priv->lock);
 		sta_priv->buffered[tid]++;
-		ieee80211_sta_set_buffered(sta, tid, true);
 		spin_unlock_bh(&sta_priv->lock);
 	}
 }
@@ -471,8 +470,6 @@ static void wfx_tx_update_sta(struct wfx_vif *wvif, struct ieee80211_hdr *hdr)
 		spin_lock_bh(&sta_priv->lock);
 		WARN(!sta_priv->buffered[tid], "inconsistent notification");
 		sta_priv->buffered[tid]--;
-		if (!sta_priv->buffered[tid])
-			ieee80211_sta_set_buffered(sta, tid, false);
 		spin_unlock_bh(&sta_priv->lock);
 	} else {
 		dev_dbg(wvif->wdev->dev, "%s: sta does not exist anymore\n",
-- 
2.27.0


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

* [PATCH 08/13] staging: wfx: drop counter of buffered frames
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (6 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 07/13] staging: wfx: fix unexpected calls to ieee80211_sta_set_buffered() Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 09/13] staging: wfx: fix handling of frames without RSSI data Jerome Pouiller
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

Since the driver does not call ieee80211_sta_set_buffered() anymore, it
is no more necessary to maintain a counter of buffered frames for each
stations.

This change allows to simplify the processing in multiple places in the
driver.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c | 50 -----------------------------------
 drivers/staging/wfx/data_tx.h |  1 -
 drivers/staging/wfx/sta.c     |  8 ------
 drivers/staging/wfx/sta.h     |  3 ---
 4 files changed, 62 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 3244a768345c5..5c744d9c8c114 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -213,22 +213,6 @@ static bool ieee80211_is_action_back(struct ieee80211_hdr *hdr)
 	return true;
 }
 
-static void wfx_tx_manage_pm(struct wfx_vif *wvif, struct ieee80211_hdr *hdr,
-			     struct wfx_tx_priv *tx_priv,
-			     struct ieee80211_sta *sta)
-{
-	struct wfx_sta_priv *sta_priv;
-	int tid = ieee80211_get_tid(hdr);
-
-	if (sta) {
-		tx_priv->has_sta = true;
-		sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
-		spin_lock_bh(&sta_priv->lock);
-		sta_priv->buffered[tid]++;
-		spin_unlock_bh(&sta_priv->lock);
-	}
-}
-
 static u8 wfx_tx_get_link_id(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 			     struct ieee80211_hdr *hdr)
 {
@@ -406,7 +390,6 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta,
 	req->tx_flags.retry_policy_index = wfx_tx_get_rate_id(wvif, tx_info);
 
 	// Auxiliary operations
-	wfx_tx_manage_pm(wvif, hdr, tx_priv, sta);
 	wfx_tx_queues_put(wvif, skb);
 	if (tx_info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM)
 		schedule_work(&wvif->update_tim_work);
@@ -449,35 +432,6 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 	ieee80211_tx_status_irqsafe(wdev->hw, skb);
 }
 
-static struct ieee80211_hdr *wfx_skb_hdr80211(struct sk_buff *skb)
-{
-	struct hif_msg *hif = (struct hif_msg *)skb->data;
-	struct hif_req_tx *req = (struct hif_req_tx *)hif->body;
-
-	return (struct ieee80211_hdr *)(req->frame + req->data_flags.fc_offset);
-}
-
-static void wfx_tx_update_sta(struct wfx_vif *wvif, struct ieee80211_hdr *hdr)
-{
-	int tid = ieee80211_get_tid(hdr);
-	struct wfx_sta_priv *sta_priv;
-	struct ieee80211_sta *sta;
-
-	rcu_read_lock(); // protect sta
-	sta = ieee80211_find_sta(wvif->vif, hdr->addr1);
-	if (sta) {
-		sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
-		spin_lock_bh(&sta_priv->lock);
-		WARN(!sta_priv->buffered[tid], "inconsistent notification");
-		sta_priv->buffered[tid]--;
-		spin_unlock_bh(&sta_priv->lock);
-	} else {
-		dev_dbg(wvif->wdev->dev, "%s: sta does not exist anymore\n",
-			__func__);
-	}
-	rcu_read_unlock();
-}
-
 static void wfx_skb_dtor(struct wfx_vif *wvif, struct sk_buff *skb)
 {
 	struct hif_msg *hif = (struct hif_msg *)skb->data;
@@ -553,8 +507,6 @@ void wfx_tx_confirm_cb(struct wfx_dev *wdev, const struct hif_cnf_tx *arg)
 
 	// You can touch to tx_priv, but don't touch to tx_info->status.
 	wfx_tx_fill_rates(wdev, tx_info, arg);
-	if (tx_priv->has_sta)
-		wfx_tx_update_sta(wvif, wfx_skb_hdr80211(skb));
 	skb_trim(skb, skb->len - wfx_tx_get_icv_len(tx_priv->hw_key));
 
 	// From now, you can touch to tx_info->status, but do not touch to
@@ -634,8 +586,6 @@ void wfx_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	while ((skb = skb_dequeue(&dropped)) != NULL) {
 		hif = (struct hif_msg *)skb->data;
 		wvif = wdev_to_wvif(wdev, hif->interface);
-		if (wfx_skb_tx_priv(skb)->has_sta)
-			wfx_tx_update_sta(wvif, wfx_skb_hdr80211(skb));
 		ieee80211_tx_info_clear_status(IEEE80211_SKB_CB(skb));
 		wfx_skb_dtor(wvif, skb);
 	}
diff --git a/drivers/staging/wfx/data_tx.h b/drivers/staging/wfx/data_tx.h
index b1727ddecd5e2..cff7b9ff99a99 100644
--- a/drivers/staging/wfx/data_tx.h
+++ b/drivers/staging/wfx/data_tx.h
@@ -36,7 +36,6 @@ struct tx_policy_cache {
 struct wfx_tx_priv {
 	ktime_t xmit_timestamp;
 	struct ieee80211_key_conf *hw_key;
-	bool has_sta;
 } __packed;
 
 void wfx_tx_policy_init(struct wfx_vif *wvif);
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 85d4bc2949882..2c0ab51fc92da 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -430,7 +430,6 @@ int wfx_sta_add(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 	struct wfx_sta_priv *sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
 
-	spin_lock_init(&sta_priv->lock);
 	sta_priv->vif_id = wvif->id;
 
 	// In station mode, the firmware interprets new link-id as a TDLS peer.
@@ -450,14 +449,7 @@ int wfx_sta_remove(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 {
 	struct wfx_vif *wvif = (struct wfx_vif *)vif->drv_priv;
 	struct wfx_sta_priv *sta_priv = (struct wfx_sta_priv *)&sta->drv_priv;
-	int i;
 
-	for (i = 0; i < ARRAY_SIZE(sta_priv->buffered); i++)
-		if (sta_priv->buffered[i])
-			// Not an error if paired with trace in
-			// wfx_tx_update_sta()
-			dev_dbg(wvif->wdev->dev, "release station while %d pending frame on queue %d",
-				sta_priv->buffered[i], i);
 	// See note in wfx_sta_add()
 	if (!sta_priv->link_id)
 		return 0;
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index 8a20ad9ae017e..43808cef4785c 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -16,9 +16,6 @@ struct wfx_vif;
 struct wfx_sta_priv {
 	int link_id;
 	int vif_id;
-	int buffered[IEEE80211_NUM_TIDS];
-	// Ensure atomicity of "buffered" and calls to ieee80211_sta_set_buffered()
-	spinlock_t lock;
 };
 
 // mac80211 interface
-- 
2.27.0


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

* [PATCH 09/13] staging: wfx: fix handling of frames without RSSI data
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (7 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 08/13] staging: wfx: drop counter of buffered frames Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 10/13] staging: wfx: simplify handling of encrypted frames Jerome Pouiller
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

It seems that in the old days, the RSSI information could be missing. In
this case, in order to not pollute the RSSI stats, the frame was
dropped (!).

It is far better to mark the frame with the flag RX_FLAG_NO_SIGNAL_VAL.

In add, the problem seems now fixed in the firmware (at least, it has
not been encountered with recent firmwares).

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_rx.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 0e959ebc38b56..316c2f1537fe5 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -73,12 +73,6 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 
 	memset(hdr, 0, sizeof(*hdr));
 
-	// FIXME: Why do we drop these frames?
-	if (!arg->rcpi_rssi &&
-	    (ieee80211_is_probe_resp(frame->frame_control) ||
-	     ieee80211_is_beacon(frame->frame_control)))
-		goto drop;
-
 	if (arg->status == HIF_STATUS_RX_FAIL_MIC)
 		hdr->flag |= RX_FLAG_MMIC_ERROR;
 	else if (arg->status)
@@ -102,6 +96,10 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 		hdr->rate_idx = arg->rxed_rate;
 	}
 
+	if (!arg->rcpi_rssi) {
+		hdr->flag |= RX_FLAG_NO_SIGNAL_VAL;
+		dev_info(wvif->wdev->dev, "received frame without RSSI data\n");
+	}
 	hdr->signal = arg->rcpi_rssi / 2 - 110;
 	hdr->antenna = 0;
 
-- 
2.27.0


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

* [PATCH 10/13] staging: wfx: simplify handling of encrypted frames
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (8 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 09/13] staging: wfx: fix handling of frames without RSSI data Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 11/13] staging: wfx: fix CCMP/TKIP replay protection Jerome Pouiller
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

We don't want mac80211 try to check MMIC and other security mechanisms.
So, the driver remove all the data related to the encryption (IV, ICV,
MMIC).

However, enabling RX_FLAG_PN_VALIDATED is sufficient for that.

So, drop the useless function wfx_drop_encrypt_data() and enable
RX_FLAG_PN_VALIDATED.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_rx.c | 60 ++---------------------------------
 1 file changed, 2 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 316c2f1537fe5..60e2e5cb4656a 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -13,57 +13,6 @@
 #include "bh.h"
 #include "sta.h"
 
-static int wfx_drop_encrypt_data(struct wfx_dev *wdev,
-				 const struct hif_ind_rx *arg,
-				 struct sk_buff *skb)
-{
-	struct ieee80211_hdr *frame = (struct ieee80211_hdr *)skb->data;
-	size_t hdrlen = ieee80211_hdrlen(frame->frame_control);
-	size_t iv_len, icv_len;
-
-	/* Oops... There is no fast way to ask mac80211 about
-	 * IV/ICV lengths. Even defineas are not exposed.
-	 */
-	switch (arg->rx_flags.encryp) {
-	case HIF_RI_FLAGS_WEP_ENCRYPTED:
-		iv_len = 4 /* WEP_IV_LEN */;
-		icv_len = 4 /* WEP_ICV_LEN */;
-		break;
-	case HIF_RI_FLAGS_TKIP_ENCRYPTED:
-		iv_len = 8 /* TKIP_IV_LEN */;
-		icv_len = 4 /* TKIP_ICV_LEN */
-			+ 8 /*MICHAEL_MIC_LEN*/;
-		break;
-	case HIF_RI_FLAGS_AES_ENCRYPTED:
-		iv_len = 8 /* CCMP_HDR_LEN */;
-		icv_len = 8 /* CCMP_MIC_LEN */;
-		break;
-	case HIF_RI_FLAGS_WAPI_ENCRYPTED:
-		iv_len = 18 /* WAPI_HDR_LEN */;
-		icv_len = 16 /* WAPI_MIC_LEN */;
-		break;
-	default:
-		dev_err(wdev->dev, "unknown encryption type %d\n",
-			arg->rx_flags.encryp);
-		return -EIO;
-	}
-
-	/* Firmware strips ICV in case of MIC failure. */
-	if (arg->status == HIF_STATUS_RX_FAIL_MIC)
-		icv_len = 0;
-
-	if (skb->len < hdrlen + iv_len + icv_len) {
-		dev_warn(wdev->dev, "malformed SDU received\n");
-		return -EIO;
-	}
-
-	/* Remove IV, ICV and MIC */
-	skb_trim(skb, skb->len - icv_len);
-	memmove(skb->data + iv_len, skb->data, hdrlen);
-	skb_pull(skb, iv_len);
-	return 0;
-}
-
 void wfx_rx_cb(struct wfx_vif *wvif,
 	       const struct hif_ind_rx *arg, struct sk_buff *skb)
 {
@@ -103,13 +52,8 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 	hdr->signal = arg->rcpi_rssi / 2 - 110;
 	hdr->antenna = 0;
 
-	if (arg->rx_flags.encryp) {
-		if (wfx_drop_encrypt_data(wvif->wdev, arg, skb))
-			goto drop;
-		hdr->flag |= RX_FLAG_DECRYPTED | RX_FLAG_IV_STRIPPED;
-		if (arg->rx_flags.encryp == HIF_RI_FLAGS_TKIP_ENCRYPTED)
-			hdr->flag |= RX_FLAG_MMIC_STRIPPED;
-	}
+	if (arg->rx_flags.encryp)
+		hdr->flag |= RX_FLAG_DECRYPTED | RX_FLAG_PN_VALIDATED;
 
 	/* Filter block ACK negotiation: fully controlled by firmware */
 	if (ieee80211_is_action(frame->frame_control) &&
-- 
2.27.0


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

* [PATCH 11/13] staging: wfx: fix CCMP/TKIP replay protection
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (9 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 10/13] staging: wfx: simplify handling of encrypted frames Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 12/13] staging: wfx: add a debugfs entry to force ps_timeout Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 13/13] staging: wfx: always enable FastPs in combo with new firmwares Jerome Pouiller
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

To enable the TKIP/CCMP replay protection, the frames has to be
processed in the right order. However, the device is not able to
re-order the frames during BlockAck sessions.

Mac80211 is able to reorder the frames, but it need the information
about the BlockAck sessions start and stop. Unfortunately, since the
BlockAck is fully handled by the hardware, these frames were not
forwarded to the host. So, if the driver ask to mac80211 to apply the
replay protection, it drop all misordered frames.

So, until now, the driver explicitly asked to mac80211 to not apply
the CCMP/TKIP replay protection.

The situation has changed with the API 3.4 of the device firmware. The
firmware forward the BlockAck information. Mac80211 is now able to
correctly reorder the frames. There is no more reasons to drop
cryptographic data.

This patch also impact the older firmwares. There will be a performance
impact on these firmware (since the misordered frames will dropped).
However, we can't keep the replay protection disabled.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_rx.c | 31 ++++++++++++++++++++++++++-----
 drivers/staging/wfx/data_tx.c |  3 ++-
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wfx/data_rx.c b/drivers/staging/wfx/data_rx.c
index 60e2e5cb4656a..6fb0788807426 100644
--- a/drivers/staging/wfx/data_rx.c
+++ b/drivers/staging/wfx/data_rx.c
@@ -13,6 +13,24 @@
 #include "bh.h"
 #include "sta.h"
 
+static void wfx_rx_handle_ba(struct wfx_vif *wvif, struct ieee80211_mgmt *mgmt)
+{
+	int params, tid;
+
+	switch (mgmt->u.action.u.addba_req.action_code) {
+	case WLAN_ACTION_ADDBA_REQ:
+		params = le16_to_cpu(mgmt->u.action.u.addba_req.capab);
+		tid = (params & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
+		ieee80211_start_rx_ba_session_offl(wvif->vif, mgmt->sa, tid);
+		break;
+	case WLAN_ACTION_DELBA:
+		params = le16_to_cpu(mgmt->u.action.u.delba.params);
+		tid = (params &  IEEE80211_DELBA_PARAM_TID_MASK) >> 12;
+		ieee80211_stop_rx_ba_session_offl(wvif->vif, mgmt->sa, tid);
+		break;
+	}
+}
+
 void wfx_rx_cb(struct wfx_vif *wvif,
 	       const struct hif_ind_rx *arg, struct sk_buff *skb)
 {
@@ -53,15 +71,18 @@ void wfx_rx_cb(struct wfx_vif *wvif,
 	hdr->antenna = 0;
 
 	if (arg->rx_flags.encryp)
-		hdr->flag |= RX_FLAG_DECRYPTED | RX_FLAG_PN_VALIDATED;
+		hdr->flag |= RX_FLAG_DECRYPTED;
 
-	/* Filter block ACK negotiation: fully controlled by firmware */
+	// Block ack negociation is offloaded by the firmware. However,
+	// re-ordering must be done by the mac80211.
 	if (ieee80211_is_action(frame->frame_control) &&
-	    arg->rx_flags.match_uc_addr &&
-	    mgmt->u.action.category == WLAN_CATEGORY_BACK)
+	    mgmt->u.action.category == WLAN_CATEGORY_BACK &&
+	    skb->len > IEEE80211_MIN_ACTION_SIZE) {
+		wfx_rx_handle_ba(wvif, mgmt);
 		goto drop;
+	}
+
 	ieee80211_rx_irqsafe(wvif->wdev->hw, skb);
-
 	return;
 
 drop:
diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index 5c744d9c8c114..3acf4eb0214dc 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -418,7 +418,8 @@ void wfx_tx(struct ieee80211_hw *hw, struct ieee80211_tx_control *control,
 		wvif = wvif_iterate(wdev, NULL);
 	if (WARN_ON(!wvif))
 		goto drop;
-	// FIXME: why?
+	// Because of TX_AMPDU_SETUP_IN_HW, mac80211 does not try to send any
+	// BlockAck session management frame. The check below exist just in case.
 	if (ieee80211_is_action_back(hdr)) {
 		dev_info(wdev->dev, "drop BA action\n");
 		goto drop;
-- 
2.27.0


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

* [PATCH 12/13] staging: wfx: add a debugfs entry to force ps_timeout
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (10 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 11/13] staging: wfx: fix CCMP/TKIP replay protection Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  2020-07-01 15:07 ` [PATCH 13/13] staging: wfx: always enable FastPs in combo with new firmwares Jerome Pouiller
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

In some advanced usage or debug scenarios, it could interesting to
change the value of ps_timeout or eventually to force use of PS-Poll
frames.

The wext API (used by iwconfig) provide a way to change ps_timeout.
However, this API is obsolete and it seems a little weird to use (it
seems it does apply the change, so the user have to disable then
re-enable de power save)

On side of nl80211, there is no way to change the ps_timeout.

This patch provides a file in debugfs to change the value of ps_timeout.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/debug.c | 23 +++++++++++++++++++++++
 drivers/staging/wfx/main.c  |  1 +
 drivers/staging/wfx/sta.c   | 10 +++++++---
 drivers/staging/wfx/sta.h   |  1 +
 drivers/staging/wfx/wfx.h   |  1 +
 5 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/wfx/debug.c b/drivers/staging/wfx/debug.c
index 10d649985696a..3f1712b7c919d 100644
--- a/drivers/staging/wfx/debug.c
+++ b/drivers/staging/wfx/debug.c
@@ -334,6 +334,28 @@ static const struct file_operations wfx_send_hif_msg_fops = {
 	.read = wfx_send_hif_msg_read,
 };
 
+static int wfx_ps_timeout_set(void *data, u64 val)
+{
+	struct wfx_dev *wdev = (struct wfx_dev *)data;
+	struct wfx_vif *wvif;
+
+	wdev->force_ps_timeout = val;
+	wvif = NULL;
+	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
+		wfx_update_pm(wvif);
+	return 0;
+}
+
+static int wfx_ps_timeout_get(void *data, u64 *val)
+{
+	struct wfx_dev *wdev = (struct wfx_dev *)data;
+
+	*val = wdev->force_ps_timeout;
+	return 0;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(wfx_ps_timeout_fops, wfx_ps_timeout_get, wfx_ps_timeout_set, "%lld\n");
+
 int wfx_debug_init(struct wfx_dev *wdev)
 {
 	struct dentry *d;
@@ -348,6 +370,7 @@ int wfx_debug_init(struct wfx_dev *wdev)
 			    &wfx_burn_slk_key_fops);
 	debugfs_create_file("send_hif_msg", 0600, d, wdev,
 			    &wfx_send_hif_msg_fops);
+	debugfs_create_file("ps_timeout", 0600, d, wdev, &wfx_ps_timeout_fops);
 
 	return 0;
 }
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 80e4474cc3314..62e3634556ec0 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -352,6 +352,7 @@ struct wfx_dev *wfx_init_common(struct device *dev,
 	skb_queue_head_init(&wdev->tx_pending);
 	init_waitqueue_head(&wdev->tx_dequeue);
 	wfx_init_hif_cmd(&wdev->hif_cmd);
+	wdev->force_ps_timeout = -1;
 
 	if (devm_add_action_or_reset(dev, wfx_free_common, wdev))
 		return NULL;
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index 2c0ab51fc92da..fdf4f48ddc2ce 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -217,20 +217,24 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
 		// are differents.
 		if (enable_ps)
 			*enable_ps = true;
-		if (wvif->bss_not_support_ps_poll)
+		if (wvif->wdev->force_ps_timeout > -1)
+			return wvif->wdev->force_ps_timeout;
+		else if (wvif->bss_not_support_ps_poll)
 			return 30;
 		else
 			return 0;
 	}
 	if (enable_ps)
 		*enable_ps = wvif->vif->bss_conf.ps;
-	if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
+	if (wvif->wdev->force_ps_timeout > -1)
+		return wvif->wdev->force_ps_timeout;
+	else if (wvif->vif->bss_conf.assoc && wvif->vif->bss_conf.ps)
 		return conf->dynamic_ps_timeout;
 	else
 		return -1;
 }
 
-static int wfx_update_pm(struct wfx_vif *wvif)
+int wfx_update_pm(struct wfx_vif *wvif)
 {
 	int ps_timeout;
 	bool ps;
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index 43808cef4785c..6b15a64ac9e28 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -66,6 +66,7 @@ void wfx_cooling_timeout_work(struct work_struct *work);
 void wfx_suspend_hot_dev(struct wfx_dev *wdev, enum sta_notify_cmd cmd);
 void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd cmd);
 void wfx_event_report_rssi(struct wfx_vif *wvif, u8 raw_rcpi_rssi);
+int wfx_update_pm(struct wfx_vif *wvif);
 
 // Other Helpers
 void wfx_reset(struct wfx_vif *wvif);
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 0c44b733ef6fe..477c08fc713fa 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -59,6 +59,7 @@ struct wfx_dev {
 	struct mutex		rx_stats_lock;
 	struct hif_tx_power_loop_info tx_power_loop_info;
 	struct mutex		tx_power_loop_info_lock;
+	int			force_ps_timeout;
 };
 
 struct wfx_vif {
-- 
2.27.0


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

* [PATCH 13/13] staging: wfx: always enable FastPs in combo with new firmwares
  2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
                   ` (11 preceding siblings ...)
  2020-07-01 15:07 ` [PATCH 12/13] staging: wfx: add a debugfs entry to force ps_timeout Jerome Pouiller
@ 2020-07-01 15:07 ` Jerome Pouiller
  12 siblings, 0 replies; 14+ messages in thread
From: Jerome Pouiller @ 2020-07-01 15:07 UTC (permalink / raw)
  To: devel, linux-wireless
  Cc: netdev, linux-kernel, Greg Kroah-Hartman, Kalle Valo,
	David S . Miller, Jérôme Pouiller

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

When multiple interface on different channels are in use. It is
necessary to advertise the AP that the device is no more awake before to
switch to the other channel.

Until now, PS-Poll was the preferred mechanism for that. However.  The
new firmwares (>= 3.7) now nicely support FastPS.

FastPS improves bandwidth and compatibility with AP.

This patch drop the complex and rarely used mechanism introduced in the
commit dd5eba1bb5b4f ("staging: wfx: fix support for AP that do not
support PS-Poll") and use FastPS as soon as it is possible.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/hif_rx.c |  8 +-------
 drivers/staging/wfx/sta.c    | 16 +++-------------
 drivers/staging/wfx/wfx.h    |  2 --
 3 files changed, 4 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/wfx/hif_rx.c b/drivers/staging/wfx/hif_rx.c
index e3ebd910fabfd..cc7c0cf226ba1 100644
--- a/drivers/staging/wfx/hif_rx.c
+++ b/drivers/staging/wfx/hif_rx.c
@@ -149,7 +149,6 @@ static int hif_event_indication(struct wfx_dev *wdev,
 	struct wfx_vif *wvif = wdev_to_wvif(wdev, hif->interface);
 	const struct hif_ind_event *body = buf;
 	int type = le32_to_cpu(body->event_id);
-	int cause;
 
 	if (!wvif) {
 		dev_warn(wdev->dev, "received event for non-existent vif\n");
@@ -168,13 +167,8 @@ static int hif_event_indication(struct wfx_dev *wdev,
 		dev_dbg(wdev->dev, "ignore BSSREGAINED indication\n");
 		break;
 	case HIF_EVENT_IND_PS_MODE_ERROR:
-		cause = le32_to_cpu(body->event_data.ps_mode_error);
 		dev_warn(wdev->dev, "error while processing power save request: %d\n",
-			 cause);
-		if (cause == HIF_PS_ERROR_AP_NOT_RESP_TO_POLL) {
-			wvif->bss_not_support_ps_poll = true;
-			schedule_work(&wvif->update_pm_work);
-		}
+			 le32_to_cpu(body->event_data.ps_mode_error));
 		break;
 	default:
 		dev_warn(wdev->dev, "unhandled event indication: %.2x\n",
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index fdf4f48ddc2ce..4e30ab17a93d4 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -219,10 +219,10 @@ static int wfx_get_ps_timeout(struct wfx_vif *wvif, bool *enable_ps)
 			*enable_ps = true;
 		if (wvif->wdev->force_ps_timeout > -1)
 			return wvif->wdev->force_ps_timeout;
-		else if (wvif->bss_not_support_ps_poll)
-			return 30;
-		else
+		else if (wfx_api_older_than(wvif->wdev, 3, 2))
 			return 0;
+		else
+			return 30;
 	}
 	if (enable_ps)
 		*enable_ps = wvif->vif->bss_conf.ps;
@@ -255,14 +255,6 @@ int wfx_update_pm(struct wfx_vif *wvif)
 	return hif_set_pm(wvif, ps, ps_timeout);
 }
 
-static void wfx_update_pm_work(struct work_struct *work)
-{
-	struct wfx_vif *wvif = container_of(work, struct wfx_vif,
-					    update_pm_work);
-
-	wfx_update_pm(wvif);
-}
-
 int wfx_conf_tx(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 		   u16 queue, const struct ieee80211_tx_queue_params *params)
 {
@@ -372,7 +364,6 @@ void wfx_reset(struct wfx_vif *wvif)
 		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
 	wfx_tx_unlock(wdev);
 	wvif->join_in_progress = false;
-	wvif->bss_not_support_ps_poll = false;
 	cancel_delayed_work_sync(&wvif->beacon_loss_work);
 	wvif =  NULL;
 	while ((wvif = wvif_iterate(wdev, wvif)) != NULL)
@@ -790,7 +781,6 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 
 	init_completion(&wvif->set_pm_mode_complete);
 	complete(&wvif->set_pm_mode_complete);
-	INIT_WORK(&wvif->update_pm_work, wfx_update_pm_work);
 	INIT_WORK(&wvif->tx_policy_upload_work, wfx_tx_policy_upload_work);
 
 	mutex_init(&wvif->scan_lock);
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index 477c08fc713fa..38e24d7f72f24 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -93,8 +93,6 @@ struct wfx_vif {
 	bool			scan_abort;
 	struct ieee80211_scan_request *scan_req;
 
-	bool			bss_not_support_ps_poll;
-	struct work_struct	update_pm_work;
 	struct completion	set_pm_mode_complete;
 };
 
-- 
2.27.0


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

end of thread, other threads:[~2020-07-01 15:09 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-01 15:06 [PATCH 00/13] staging: wfx: multiple fixes Jerome Pouiller
2020-07-01 15:06 ` [PATCH 01/13] staging: wfx: associate tx_queues to vifs Jerome Pouiller
2020-07-01 15:06 ` [PATCH 02/13] staging: wfx: check the vif ID of the Tx confirmations Jerome Pouiller
2020-07-01 15:06 ` [PATCH 03/13] staging: wfx: correctly retrieve vif ID from Tx confirmation Jerome Pouiller
2020-07-01 15:06 ` [PATCH 04/13] staging: wfx: add tracepoint "queues_stats" Jerome Pouiller
2020-07-01 15:06 ` [PATCH 05/13] staging: wfx: load the firmware faster Jerome Pouiller
2020-07-01 15:07 ` [PATCH 06/13] staging: wfx: improve protection against malformed HIF messages Jerome Pouiller
2020-07-01 15:07 ` [PATCH 07/13] staging: wfx: fix unexpected calls to ieee80211_sta_set_buffered() Jerome Pouiller
2020-07-01 15:07 ` [PATCH 08/13] staging: wfx: drop counter of buffered frames Jerome Pouiller
2020-07-01 15:07 ` [PATCH 09/13] staging: wfx: fix handling of frames without RSSI data Jerome Pouiller
2020-07-01 15:07 ` [PATCH 10/13] staging: wfx: simplify handling of encrypted frames Jerome Pouiller
2020-07-01 15:07 ` [PATCH 11/13] staging: wfx: fix CCMP/TKIP replay protection Jerome Pouiller
2020-07-01 15:07 ` [PATCH 12/13] staging: wfx: add a debugfs entry to force ps_timeout Jerome Pouiller
2020-07-01 15:07 ` [PATCH 13/13] staging: wfx: always enable FastPs in combo with new firmwares Jerome Pouiller

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 NNTP newsgroup(s).