linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jerome Pouiller <Jerome.Pouiller@silabs.com>
To: devel@driverdev.osuosl.org, linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Kalle Valo" <kvalo@codeaurora.org>,
	"David S . Miller" <davem@davemloft.net>,
	"Jérôme Pouiller" <jerome.pouiller@silabs.com>
Subject: [PATCH 02/16] staging: wfx: use ieee80211_beacon_loss() provided by mac80211
Date: Mon, 20 Apr 2020 18:02:57 +0200	[thread overview]
Message-ID: <20200420160311.57323-3-Jerome.Pouiller@silabs.com> (raw)
In-Reply-To: <20200420160311.57323-1-Jerome.Pouiller@silabs.com>

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

The firmware is able to filter beacons and send a notification if one or
multiple beacons are not received. Note that it send this notification
only once. Only if it receive beacons gain, it send a new notification.

Currently, the driver handle the connection loss itself (see
wfx_cqm_bssloss_sm()). It send null frames and watch the answers.

This patch fixes all this mess:
  - settle firmware to send a notification on the first beacon loss
  - call ieee80211_beacon_loss() and let mac80211 handle all the process
  - since we do have notification for each beacon loss, add a period
    task that call ieee80211_beacon_loss() until we receive "REGAIN"
    notification.

Thus, we can drop the ugly wfx_cqm_bssloss_sm() and
wfx_bss_params_work().

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 drivers/staging/wfx/data_tx.c |   7 ---
 drivers/staging/wfx/queue.c   |  11 ----
 drivers/staging/wfx/sta.c     | 106 ++++++----------------------------
 drivers/staging/wfx/sta.h     |   1 -
 drivers/staging/wfx/wfx.h     |   7 +--
 5 files changed, 20 insertions(+), 112 deletions(-)

diff --git a/drivers/staging/wfx/data_tx.c b/drivers/staging/wfx/data_tx.c
index c30e4f5b6e2d..9c1a91207dd8 100644
--- a/drivers/staging/wfx/data_tx.c
+++ b/drivers/staging/wfx/data_tx.c
@@ -547,9 +547,6 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 	memset(tx_info->pad, 0, sizeof(tx_info->pad));
 
 	if (!arg->status) {
-		if (wvif->bss_loss_state &&
-		    arg->packet_id == wvif->bss_loss_confirm_id)
-			wfx_cqm_bssloss_sm(wvif, 0, 1, 0);
 		tx_info->status.tx_time =
 		arg->media_delay - arg->tx_queue_delay;
 		if (tx_info->flags & IEEE80211_TX_CTL_NO_ACK)
@@ -563,10 +560,6 @@ void wfx_tx_confirm_cb(struct wfx_vif *wvif, const struct hif_cnf_tx *arg)
 			schedule_work(&wvif->update_tim_work);
 		}
 		tx_info->flags |= IEEE80211_TX_STAT_TX_FILTERED;
-	} else {
-		if (wvif->bss_loss_state &&
-		    arg->packet_id == wvif->bss_loss_confirm_id)
-			wfx_cqm_bssloss_sm(wvif, 0, 0, 1);
 	}
 	wfx_skb_dtor(wvif->wdev, skb);
 }
diff --git a/drivers/staging/wfx/queue.c b/drivers/staging/wfx/queue.c
index e6d7d0e45156..e9573e9d009f 100644
--- a/drivers/staging/wfx/queue.c
+++ b/drivers/staging/wfx/queue.c
@@ -266,17 +266,6 @@ static bool wfx_handle_tx_data(struct wfx_dev *wdev, struct sk_buff *skb)
 	if (!wvif)
 		return false;
 
-	// FIXME: mac80211 is smart enough to handle BSS loss. Driver should not
-	// try to do anything about that.
-	if (ieee80211_is_nullfunc(frame->frame_control)) {
-		mutex_lock(&wvif->bss_loss_lock);
-		if (wvif->bss_loss_state) {
-			wvif->bss_loss_confirm_id = req->packet_id;
-			req->queue_id.queue_id = HIF_QUEUE_ID_VOICE;
-		}
-		mutex_unlock(&wvif->bss_loss_lock);
-	}
-
 	// FIXME: identify the exact scenario matched by this condition. Does it
 	// happen yet?
 	if (ieee80211_has_protected(frame->frame_control) &&
diff --git a/drivers/staging/wfx/sta.c b/drivers/staging/wfx/sta.c
index c0c3eb945967..ca84724e531c 100644
--- a/drivers/staging/wfx/sta.c
+++ b/drivers/staging/wfx/sta.c
@@ -59,60 +59,6 @@ static void wfx_free_event_queue(struct wfx_vif *wvif)
 	__wfx_free_event_queue(&list);
 }
 
-void wfx_cqm_bssloss_sm(struct wfx_vif *wvif, int init, int good, int bad)
-{
-	int tx = 0;
-
-	mutex_lock(&wvif->bss_loss_lock);
-	cancel_work_sync(&wvif->bss_params_work);
-
-	if (init) {
-		schedule_delayed_work(&wvif->bss_loss_work, HZ);
-		wvif->bss_loss_state = 0;
-
-		if (!atomic_read(&wvif->wdev->tx_lock))
-			tx = 1;
-	} else if (good) {
-		cancel_delayed_work_sync(&wvif->bss_loss_work);
-		wvif->bss_loss_state = 0;
-		schedule_work(&wvif->bss_params_work);
-	} else if (bad) {
-		/* FIXME Should we just keep going until we time out? */
-		if (wvif->bss_loss_state < 3)
-			tx = 1;
-	} else {
-		cancel_delayed_work_sync(&wvif->bss_loss_work);
-		wvif->bss_loss_state = 0;
-	}
-
-	/* Spit out a NULL packet to our AP if necessary */
-	// FIXME: call ieee80211_beacon_loss/ieee80211_connection_loss instead
-	if (tx) {
-		struct sk_buff *skb;
-		struct ieee80211_hdr *hdr;
-		struct ieee80211_tx_control control = { };
-
-		wvif->bss_loss_state++;
-
-		skb = ieee80211_nullfunc_get(wvif->wdev->hw, wvif->vif, false);
-		if (!skb)
-			goto end;
-		hdr = (struct ieee80211_hdr *)skb->data;
-		memset(IEEE80211_SKB_CB(skb), 0,
-		       sizeof(*IEEE80211_SKB_CB(skb)));
-		IEEE80211_SKB_CB(skb)->control.vif = wvif->vif;
-		IEEE80211_SKB_CB(skb)->driver_rates[0].idx = 0;
-		IEEE80211_SKB_CB(skb)->driver_rates[0].count = 1;
-		IEEE80211_SKB_CB(skb)->driver_rates[1].idx = -1;
-		rcu_read_lock(); // protect control.sta
-		control.sta = ieee80211_find_sta(wvif->vif, hdr->addr1);
-		wfx_tx(wvif->wdev->hw, &control, skb);
-		rcu_read_unlock();
-	}
-end:
-	mutex_unlock(&wvif->bss_loss_lock);
-}
-
 static void wfx_filter_beacon(struct wfx_vif *wvif, bool filter_beacon)
 {
 	const struct hif_ie_table_entry filter_ies[] = {
@@ -339,6 +285,17 @@ static void wfx_event_report_rssi(struct wfx_vif *wvif, u8 raw_rcpi_rssi)
 	ieee80211_cqm_rssi_notify(wvif->vif, cqm_evt, rcpi_rssi, GFP_KERNEL);
 }
 
+static void wfx_beacon_loss_work(struct work_struct *work)
+{
+	struct wfx_vif *wvif = container_of(to_delayed_work(work),
+					    struct wfx_vif, beacon_loss_work);
+	struct ieee80211_bss_conf *bss_conf = &wvif->vif->bss_conf;
+
+	ieee80211_beacon_loss(wvif->vif);
+	schedule_delayed_work(to_delayed_work(work),
+			      msecs_to_jiffies(bss_conf->beacon_int));
+}
+
 static void wfx_event_handler_work(struct work_struct *work)
 {
 	struct wfx_vif *wvif =
@@ -354,12 +311,10 @@ static void wfx_event_handler_work(struct work_struct *work)
 	list_for_each_entry(event, &list, link) {
 		switch (event->evt.event_id) {
 		case HIF_EVENT_IND_BSSLOST:
-			mutex_lock(&wvif->scan_lock);
-			wfx_cqm_bssloss_sm(wvif, 1, 0, 0);
-			mutex_unlock(&wvif->scan_lock);
+			schedule_delayed_work(&wvif->beacon_loss_work, 0);
 			break;
 		case HIF_EVENT_IND_BSSREGAINED:
-			wfx_cqm_bssloss_sm(wvif, 0, 0, 0);
+			cancel_delayed_work(&wvif->beacon_loss_work);
 			break;
 		case HIF_EVENT_IND_RCPI_RSSI:
 			wfx_event_report_rssi(wvif,
@@ -379,26 +334,6 @@ static void wfx_event_handler_work(struct work_struct *work)
 	__wfx_free_event_queue(&list);
 }
 
-static void wfx_bss_loss_work(struct work_struct *work)
-{
-	struct wfx_vif *wvif = container_of(work, struct wfx_vif,
-					    bss_loss_work.work);
-
-	ieee80211_connection_loss(wvif->vif);
-}
-
-static void wfx_bss_params_work(struct work_struct *work)
-{
-	struct wfx_vif *wvif = container_of(work, struct wfx_vif,
-					    bss_params_work);
-
-	mutex_lock(&wvif->wdev->conf_mutex);
-	wvif->bss_params.bss_flags.lost_count_only = 1;
-	hif_set_bss_params(wvif, &wvif->bss_params);
-	wvif->bss_params.bss_flags.lost_count_only = 0;
-	mutex_unlock(&wvif->wdev->conf_mutex);
-}
-
 // Call it with wdev->conf_mutex locked
 static void wfx_do_unjoin(struct wfx_vif *wvif)
 {
@@ -418,10 +353,10 @@ static void wfx_do_unjoin(struct wfx_vif *wvif)
 		hif_set_block_ack_policy(wvif, 0xFF, 0xFF);
 	wfx_free_event_queue(wvif);
 	cancel_work_sync(&wvif->event_handler_work);
-	wfx_cqm_bssloss_sm(wvif, 0, 0, 0);
 
 	memset(&wvif->bss_params, 0, sizeof(wvif->bss_params));
 	wfx_tx_unlock(wvif->wdev);
+	cancel_delayed_work_sync(&wvif->beacon_loss_work);
 }
 
 static void wfx_set_mfp(struct wfx_vif *wvif,
@@ -615,9 +550,9 @@ static void wfx_join_finalize(struct wfx_vif *wvif,
 	else
 		hif_dual_cts_protection(wvif, false);
 
-	wfx_cqm_bssloss_sm(wvif, 0, 0, 0);
-
-	wvif->bss_params.beacon_lost_count = 20;
+	// beacon_loss_count is defined to 7 in net/mac80211/mlme.c. Let's use
+	// the same value.
+	wvif->bss_params.beacon_lost_count = 7;
 	wvif->bss_params.aid = info->aid;
 
 	hif_set_association_mode(wvif, info);
@@ -904,12 +839,10 @@ int wfx_add_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif)
 
 	wvif->link_id_map = 1; // link-id 0 is reserved for multicast
 	INIT_WORK(&wvif->update_tim_work, wfx_update_tim_work);
+	INIT_DELAYED_WORK(&wvif->beacon_loss_work, wfx_beacon_loss_work);
 
 	memset(&wvif->bss_params, 0, sizeof(wvif->bss_params));
 
-	mutex_init(&wvif->bss_loss_lock);
-	INIT_DELAYED_WORK(&wvif->bss_loss_work, wfx_bss_loss_work);
-
 	wvif->wep_default_key_id = -1;
 	INIT_WORK(&wvif->wep_key_work, wfx_wep_key_work);
 
@@ -919,7 +852,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->bss_params_work, wfx_bss_params_work);
 	INIT_WORK(&wvif->tx_policy_upload_work, wfx_tx_policy_upload_work);
 
 	mutex_init(&wvif->scan_lock);
@@ -974,8 +906,8 @@ void wfx_remove_interface(struct ieee80211_hw *hw,
 	/* FIXME: In add to reset MAC address, try to reset interface */
 	hif_set_macaddr(wvif, NULL);
 
-	wfx_cqm_bssloss_sm(wvif, 0, 0, 0);
 	wfx_free_event_queue(wvif);
+	cancel_delayed_work_sync(&wvif->beacon_loss_work);
 
 	wdev->vif[wvif->id] = NULL;
 	wvif->vif = NULL;
diff --git a/drivers/staging/wfx/sta.h b/drivers/staging/wfx/sta.h
index 31097057563a..6d7734c65902 100644
--- a/drivers/staging/wfx/sta.h
+++ b/drivers/staging/wfx/sta.h
@@ -81,7 +81,6 @@ void wfx_unassign_vif_chanctx(struct ieee80211_hw *hw,
 void wfx_suspend_resume_mc(struct wfx_vif *wvif, enum sta_notify_cmd cmd);
 
 // Other Helpers
-void wfx_cqm_bssloss_sm(struct wfx_vif *wvif, int init, int good, int bad);
 u32 wfx_rate_mask_to_hw(struct wfx_dev *wdev, u32 rates);
 
 #endif /* WFX_STA_H */
diff --git a/drivers/staging/wfx/wfx.h b/drivers/staging/wfx/wfx.h
index b5d2d0f07740..2747c7cdf4d1 100644
--- a/drivers/staging/wfx/wfx.h
+++ b/drivers/staging/wfx/wfx.h
@@ -69,14 +69,10 @@ struct wfx_vif {
 	int			id;
 	enum wfx_state		state;
 
-	int			bss_loss_state;
-	u32			bss_loss_confirm_id;
-	struct mutex		bss_loss_lock;
-	struct delayed_work	bss_loss_work;
-
 	u32			link_id_map;
 
 	bool			after_dtim_tx_allowed;
+	struct delayed_work	beacon_loss_work;
 
 	s8			wep_default_key_id;
 	struct sk_buff		*wep_pending_skb;
@@ -92,7 +88,6 @@ struct wfx_vif {
 
 	unsigned long		uapsd_mask;
 	struct hif_req_set_bss_params bss_params;
-	struct work_struct	bss_params_work;
 
 	int			join_complete_status;
 
-- 
2.26.1


  parent reply	other threads:[~2020-04-20 16:05 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-20 16:02 [PATCH 00/16] staging: wfx: rework the handling of the connection loss Jerome Pouiller
2020-04-20 16:02 ` [PATCH 01/16] staging: wfx: simplify the check if the the device is associated Jerome Pouiller
2020-04-20 16:02 ` Jerome Pouiller [this message]
2020-04-20 16:02 ` [PATCH 03/16] staging: wfx: drop useless attribute 'bss_params' Jerome Pouiller
2020-04-20 16:02 ` [PATCH 04/16] staging: wfx: handle firmware events synchronously Jerome Pouiller
2020-04-20 16:03 ` [PATCH 05/16] staging: wfx: also fix network parameters for IBSS networks Jerome Pouiller
2020-04-20 16:03 ` [PATCH 06/16] staging: wfx: dual CTS is never necessary Jerome Pouiller
2020-04-20 16:03 ` [PATCH 07/16] staging: wfx: field operational_rate_set is ignored by firmware Jerome Pouiller
2020-04-20 16:03 ` [PATCH 08/16] staging: wfx: simplify hif_set_bss_params() Jerome Pouiller
2020-04-20 16:03 ` [PATCH 09/16] staging: wfx: drop useless update of field basic_rate_set Jerome Pouiller
2020-04-20 16:03 ` [PATCH 10/16] staging: wfx: introduce wfx_set_default_unicast_key() Jerome Pouiller
2020-04-20 16:03 ` [PATCH 11/16] staging: wfx: keys are kept during whole firmware life Jerome Pouiller
2020-04-20 16:03 ` [PATCH 12/16] staging: wfx: drop protection for asynchronous join during scan Jerome Pouiller
2020-04-20 16:03 ` [PATCH 13/16] staging: wfx: drop useless checks in wfx_do_unjoin() Jerome Pouiller
2020-04-20 16:03 ` [PATCH 14/16] staging: wfx: simplify wfx_remove_interface() Jerome Pouiller
2020-04-20 16:03 ` [PATCH 15/16] staging: wfx: drop unused enum wfx_state Jerome Pouiller
2020-04-20 16:03 ` [PATCH 16/16] staging: wfx: drop unused attribute 'join_complete_status' Jerome Pouiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200420160311.57323-3-Jerome.Pouiller@silabs.com \
    --to=jerome.pouiller@silabs.com \
    --cc=davem@davemloft.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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).