[wt PATCH 0/5] libertas: make association work again

[wt PATCH 0/5] libertas: make association work again

[Dan Williams]

This series makes libertas actually associate with APs after the
cfg80211 conversion.  There was a pretty big TODO where if cfg80211
didn't pass in the BSSID of the AP to associate with, then libertas
would fail because the firmware really, really, really wants a BSSID.
So lets do a scan to find that BSSID.  The other notable change is
a fix to make sure all supported rates are included in the firmware's
association request, lack of which caused some APs to deny the attempt.


Dan Williams (5):
  libertas: get the right # of scanned BSSes
  libertas: better scan response debugging
  libertas: better association request debugging
  libertas: fix association with some APs by using extended rates
  libertas: scan before assocation if no BSSID was given

 drivers/net/wireless/libertas/cfg.c  |  275 ++++++++++++++++++++++++++--------
 drivers/net/wireless/libertas/dev.h  |    5 +
 drivers/net/wireless/libertas/main.c |    1 +
 3 files changed, 219 insertions(+), 62 deletions(-)

[wt PATCH 1/5] libertas: get the right # of scanned BSSes

[Dan Williams]

Let's actually check the right field in the command response; and
if there aren't any reported BSSes, exit early with success.

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/cfg.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index 25f9027..b60f661 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -465,7 +465,15 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,
 	lbs_deb_enter(LBS_DEB_CFG80211);
 
 	bsssize = get_unaligned_le16(&scanresp->bssdescriptsize);
-	nr_sets = le16_to_cpu(resp->size);
+	nr_sets = le16_to_cpu(scanresp->nr_sets);
+
+	lbs_deb_scan("scan response: %d BSSs (%d bytes); resp size %d bytes\n",
+			nr_sets, bsssize, le16_to_cpu(resp->size));
+
+	if (nr_sets == 0) {
+		ret = 0;
+		goto done;
+	}
 
 	/*
 	 * The general layout of the scan response is described in chapter
-- 
1.7.2

[wt PATCH 2/5] libertas: better scan response debugging

[Dan Williams]

Make it a bit easier to debug scan results in the future.

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/cfg.c |   24 ++++++++++++++++++++----
 1 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index b60f661..c7da6c0 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -502,20 +502,31 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,
 
 	pos = scanresp->bssdesc_and_tlvbuffer;
 
+	lbs_deb_hex(LBS_DEB_SCAN, "SCAN_RSP", scanresp->bssdesc_and_tlvbuffer,
+			scanresp->bssdescriptsize);
+
 	tsfdesc = pos + bsssize;
 	tsfsize = 4 + 8 * scanresp->nr_sets;
+	lbs_deb_hex(LBS_DEB_SCAN, "SCAN_TSF", (u8 *) tsfdesc, tsfsize);
 
 	/* Validity check: we expect a Marvell-Local TLV */
 	i = get_unaligned_le16(tsfdesc);
 	tsfdesc += 2;
-	if (i != TLV_TYPE_TSFTIMESTAMP)
+	if (i != TLV_TYPE_TSFTIMESTAMP) {
+		lbs_deb_scan("scan response: invalid TSF Timestamp %d\n", i);
 		goto done;
+	}
+
 	/* Validity check: the TLV holds TSF values with 8 bytes each, so
 	 * the size in the TLV must match the nr_sets value */
 	i = get_unaligned_le16(tsfdesc);
 	tsfdesc += 2;
-	if (i / 8 != scanresp->nr_sets)
+	if (i / 8 != scanresp->nr_sets) {
+		lbs_deb_scan("scan response: invalid number of TSF timestamp "
+			     "sets (expected %d got %d)\n", scanresp->nr_sets,
+			     i / 8);
 		goto done;
+	}
 
 	for (i = 0; i < scanresp->nr_sets; i++) {
 		const u8 *bssid;
@@ -557,8 +568,11 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,
 			id = *pos++;
 			elen = *pos++;
 			left -= 2;
-			if (elen > left || elen == 0)
+			if (elen > left || elen == 0) {
+				lbs_deb_scan("scan response: invalid IE fmt\n");
 				goto done;
+			}
+
 			if (id == WLAN_EID_DS_PARAMS)
 				chan_no = *pos;
 			if (id == WLAN_EID_SSID) {
@@ -589,7 +603,9 @@ static int lbs_ret_scan(struct lbs_private *priv, unsigned long dummy,
 					capa, intvl, ie, ielen,
 					LBS_SCAN_RSSI_TO_MBM(rssi),
 					GFP_KERNEL);
-		}
+		} else
+			lbs_deb_scan("scan response: missing BSS channel IE\n");
+
 		tsfdesc += 8;
 	}
 	ret = 0;
-- 
1.7.2

[wt PATCH 3/5] libertas: better association request debugging

[Dan Williams]

Bring back the comment about FW v5 status codes from the pre-cfg80211
driver, and let through status codes that aren't remapped by the
firmware.

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/cfg.c |   37 +++++++++++++++++++++++++++-------
 1 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index c7da6c0..a9647e6 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -1071,7 +1071,7 @@ static int lbs_associate(struct lbs_private *priv,
 	pos += lbs_add_common_rates_tlv(pos, bss);
 
 	/* add auth type TLV */
-	if (priv->fwrelease >= 0x09000000)
+	if (MRVL_FW_MAJOR_REV(priv->fwrelease) >= 9)
 		pos += lbs_add_auth_type_tlv(pos, sme->auth_type);
 
 	/* add WPA/WPA2 TLV */
@@ -1082,6 +1082,9 @@ static int lbs_associate(struct lbs_private *priv,
 		(u16)(pos - (u8 *) &cmd->iebuf);
 	cmd->hdr.size = cpu_to_le16(len);
 
+	lbs_deb_hex(LBS_DEB_ASSOC, "ASSOC_CMD", (u8 *) cmd,
+			le16_to_cpu(cmd->hdr.size));
+
 	/* store for later use */
 	memcpy(priv->assoc_bss, bss->bssid, ETH_ALEN);
 
@@ -1089,14 +1092,28 @@ static int lbs_associate(struct lbs_private *priv,
 	if (ret)
 		goto done;
 
-
 	/* generate connect message to cfg80211 */
 
 	resp = (void *) cmd; /* recast for easier field access */
 	status = le16_to_cpu(resp->statuscode);
 
-	/* Convert statis code of old firmware */
-	if (priv->fwrelease < 0x09000000)
+	/* Older FW versions map the IEEE 802.11 Status Code in the association
+	 * response to the following values returned in resp->statuscode:
+	 *
+	 *    IEEE Status Code                Marvell Status Code
+	 *    0                       ->      0x0000 ASSOC_RESULT_SUCCESS
+	 *    13                      ->      0x0004 ASSOC_RESULT_AUTH_REFUSED
+	 *    14                      ->      0x0004 ASSOC_RESULT_AUTH_REFUSED
+	 *    15                      ->      0x0004 ASSOC_RESULT_AUTH_REFUSED
+	 *    16                      ->      0x0004 ASSOC_RESULT_AUTH_REFUSED
+	 *    others                  ->      0x0003 ASSOC_RESULT_REFUSED
+	 *
+	 * Other response codes:
+	 *    0x0001 -> ASSOC_RESULT_INVALID_PARAMETERS (unused)
+	 *    0x0002 -> ASSOC_RESULT_TIMEOUT (internal timer expired waiting for
+	 *                                    association response from the AP)
+	 */
+	if (MRVL_FW_MAJOR_REV(priv->fwrelease) <= 8) {
 		switch (status) {
 		case 0:
 			break;
@@ -1118,11 +1135,16 @@ static int lbs_associate(struct lbs_private *priv,
 			break;
 		default:
 			lbs_deb_assoc("association failure %d\n", status);
-			status = WLAN_STATUS_UNSPECIFIED_FAILURE;
+			/* v5 OLPC firmware does return the AP status code if
+			 * it's not one of the values above.  Let that through.
+			 */
+			break;
+		}
 	}
 
-	lbs_deb_assoc("status %d, capability 0x%04x\n", status,
-		      le16_to_cpu(resp->capability));
+	lbs_deb_assoc("status %d, statuscode 0x%04x, capability 0x%04x, "
+		      "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
+		      le16_to_cpu(resp->capability), le16_to_cpu(resp->aid));
 
 	resp_ie_len = le16_to_cpu(resp->hdr.size)
 		- sizeof(resp->hdr)
@@ -1142,7 +1164,6 @@ static int lbs_associate(struct lbs_private *priv,
 			netif_tx_wake_all_queues(priv->dev);
 	}
 
-
 done:
 	lbs_deb_leave_args(LBS_DEB_CFG80211, "ret %d", ret);
 	return ret;
-- 
1.7.2

[wt PATCH 4/5] libertas: fix association with some APs by using extended rates

[Dan Williams]

Some APs get pissy if you don't send the firmware the extended rates
in the association request's rates TLV.  Found this on a Linksys
WRT54G v2; it denies association with status code 18 unless you
add the extended rates too.  The old driver did this, but it got
lost in the cfg80211 conversion.

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/cfg.c |   56 +++++++++++++++++++++++++----------
 1 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index a9647e6..a8c0126 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -257,6 +257,29 @@ static int lbs_add_supported_rates_tlv(u8 *tlv)
 	return sizeof(rate_tlv->header) + i;
 }
 
+/* Add common rates from a TLV and return the new end of the TLV */
+static u8 *
+add_ie_rates(u8 *tlv, const u8 *ie, int *nrates)
+{
+	int hw, ap, ap_max = ie[1];
+	u8 hw_rate;
+
+	/* Advance past IE header */
+	ie += 2;
+
+	lbs_deb_hex(LBS_DEB_ASSOC, "AP IE Rates", (u8 *) ie, ap_max);
+
+	for (hw = 0; hw < ARRAY_SIZE(lbs_rates); hw++) {
+		hw_rate = lbs_rates[hw].bitrate / 5;
+		for (ap = 0; ap < ap_max; ap++) {
+			if (hw_rate == (ie[ap] & 0x7f)) {
+				*tlv++ = ie[ap];
+				*nrates = *nrates + 1;
+			}
+		}
+	}
+	return tlv;
+}
 
 /*
  * Adds a TLV with all rates the hardware *and* BSS supports.
@@ -264,8 +287,11 @@ static int lbs_add_supported_rates_tlv(u8 *tlv)
 static int lbs_add_common_rates_tlv(u8 *tlv, struct cfg80211_bss *bss)
 {
 	struct mrvl_ie_rates_param_set *rate_tlv = (void *)tlv;
-	const u8 *rates_eid = ieee80211_bss_get_ie(bss, WLAN_EID_SUPP_RATES);
-	int n;
+	const u8 *rates_eid, *ext_rates_eid;
+	int n = 0;
+
+	rates_eid = ieee80211_bss_get_ie(bss, WLAN_EID_SUPP_RATES);
+	ext_rates_eid = ieee80211_bss_get_ie(bss, WLAN_EID_EXT_SUPP_RATES);
 
 	/*
 	 * 01 00                   TLV_TYPE_RATES
@@ -275,26 +301,21 @@ static int lbs_add_common_rates_tlv(u8 *tlv, struct cfg80211_bss *bss)
 	rate_tlv->header.type = cpu_to_le16(TLV_TYPE_RATES);
 	tlv += sizeof(rate_tlv->header);
 
-	if (!rates_eid) {
+	/* Add basic rates */
+	if (rates_eid) {
+		tlv = add_ie_rates(tlv, rates_eid, &n);
+
+		/* Add extended rates, if any */
+		if (ext_rates_eid)
+			tlv = add_ie_rates(tlv, ext_rates_eid, &n);
+	} else {
+		lbs_deb_assoc("assoc: bss had no basic rate IE\n");
 		/* Fallback: add basic 802.11b rates */
 		*tlv++ = 0x82;
 		*tlv++ = 0x84;
 		*tlv++ = 0x8b;
 		*tlv++ = 0x96;
 		n = 4;
-	} else {
-		int hw, ap;
-		u8 ap_max = rates_eid[1];
-		n = 0;
-		for (hw = 0; hw < ARRAY_SIZE(lbs_rates); hw++) {
-			u8 hw_rate = lbs_rates[hw].bitrate / 5;
-			for (ap = 0; ap < ap_max; ap++) {
-				if (hw_rate == (rates_eid[ap+2] & 0x7f)) {
-					*tlv++ = rates_eid[ap+2];
-					n++;
-				}
-			}
-		}
 	}
 
 	rate_tlv->header.len = cpu_to_le16(n);
@@ -1024,6 +1045,7 @@ static int lbs_associate(struct lbs_private *priv,
 	int status;
 	int ret;
 	u8 *pos = &(cmd->iebuf[0]);
+	u8 *tmp;
 
 	lbs_deb_enter(LBS_DEB_CFG80211);
 
@@ -1068,7 +1090,9 @@ static int lbs_associate(struct lbs_private *priv,
 	pos += lbs_add_cf_param_tlv(pos);
 
 	/* add rates TLV */
+	tmp = pos + 4; /* skip Marvell IE header */
 	pos += lbs_add_common_rates_tlv(pos, bss);
+	lbs_deb_hex(LBS_DEB_ASSOC, "Common Rates", tmp, pos - tmp);
 
 	/* add auth type TLV */
 	if (MRVL_FW_MAJOR_REV(priv->fwrelease) <= 9)
-- 
1.7.2

[wt PATCH 5/5] libertas: scan before assocation if no BSSID was given

[Dan Williams]

Fix this leftover TODO from the cfg80211 conversion by doing a scan
if cfg80211 didn't pass in the BSSID for us.  Since the scan code
uses so much of the cfg80211_scan_request structure to build up the
firmware command, we just fake one when the scan request is triggered
internally.  But we need to make sure that internal 'fake' cfg82011
scan request does get back to cfg82011 via cfg80211_scan_done().

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/cfg.c  |  148 ++++++++++++++++++++++++++--------
 drivers/net/wireless/libertas/dev.h  |    5 +
 drivers/net/wireless/libertas/main.c |    1 +
 3 files changed, 121 insertions(+), 33 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index a8c0126..623b81e 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -715,8 +715,13 @@ static void lbs_scan_worker(struct work_struct *work)
 
 	if (priv->scan_channel >= priv->scan_req->n_channels) {
 		/* Mark scan done */
-		cfg80211_scan_done(priv->scan_req, false);
+		if (priv->internal_scan)
+			kfree(priv->scan_req);
+		else
+			cfg80211_scan_done(priv->scan_req, false);
+
 		priv->scan_req = NULL;
+		priv->last_scan = jiffies;
 	}
 
 	/* Restart network */
@@ -727,10 +732,33 @@ static void lbs_scan_worker(struct work_struct *work)
 
 	kfree(scan_cmd);
 
+	/* Wake up anything waiting on scan completion */
+	if (priv->scan_req == NULL) {
+		lbs_deb_scan("scan: waking up waiters\n");
+		wake_up_all(&priv->scan_q);
+	}
+
  out_no_scan_cmd:
 	lbs_deb_leave(LBS_DEB_SCAN);
 }
 
+static void _internal_start_scan(struct lbs_private *priv, bool internal,
+	struct cfg80211_scan_request *request)
+{
+	lbs_deb_enter(LBS_DEB_CFG80211);
+
+	lbs_deb_scan("scan: ssids %d, channels %d, ie_len %zd\n",
+		request->n_ssids, request->n_channels, request->ie_len);
+
+	priv->scan_channel = 0;
+	queue_delayed_work(priv->work_thread, &priv->scan_work,
+		msecs_to_jiffies(50));
+
+	priv->scan_req = request;
+	priv->internal_scan = internal;
+
+	lbs_deb_leave(LBS_DEB_CFG80211);
+}
 
 static int lbs_cfg_scan(struct wiphy *wiphy,
 	struct net_device *dev,
@@ -747,18 +775,11 @@ static int lbs_cfg_scan(struct wiphy *wiphy,
 		goto out;
 	}
 
-	lbs_deb_scan("scan: ssids %d, channels %d, ie_len %zd\n",
-		request->n_ssids, request->n_channels, request->ie_len);
-
-	priv->scan_channel = 0;
-	queue_delayed_work(priv->work_thread, &priv->scan_work,
-		msecs_to_jiffies(50));
+	_internal_start_scan(priv, false, request);
 
 	if (priv->surpriseremoved)
 		ret = -EIO;
 
-	priv->scan_req = request;
-
  out:
 	lbs_deb_leave_args(LBS_DEB_CFG80211, "ret %d", ret);
 	return ret;
@@ -1193,7 +1214,62 @@ done:
 	return ret;
 }
 
+static struct cfg80211_scan_request *
+_new_connect_scan_req(struct wiphy *wiphy, struct cfg80211_connect_params *sme)
+{
+	struct cfg80211_scan_request *creq = NULL;
+	int i, n_channels = 0;
+	enum ieee80211_band band;
+
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+		if (wiphy->bands[band])
+			n_channels += wiphy->bands[band]->n_channels;
+	}
+
+	creq = kzalloc(sizeof(*creq) + sizeof(struct cfg80211_ssid) +
+		       n_channels * sizeof(void *),
+		       GFP_ATOMIC);
+	if (!creq)
+		return NULL;
+
+	/* SSIDs come after channels */
+	creq->ssids = (void *)&creq->channels[n_channels];
+	creq->n_channels = n_channels;
+	creq->n_ssids = 1;
+
+	/* Scan all available channels */
+	i = 0;
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+		int j;
+
+		if (!wiphy->bands[band])
+			continue;
+
+		for (j = 0; j < wiphy->bands[band]->n_channels; j++) {
+			/* ignore disabled channels */
+			if (wiphy->bands[band]->channels[j].flags &
+						IEEE80211_CHAN_DISABLED)
+				continue;
+
+			creq->channels[i] = &wiphy->bands[band]->channels[j];
+			i++;
+		}
+	}
+	if (i) {
+		/* Set real number of channels specified in creq->channels[] */
+		creq->n_channels = i;
+
+		/* Scan for the SSID we're going to connect to */
+		memcpy(creq->ssids[0].ssid, sme->ssid, sme->ssid_len);
+		creq->ssids[0].ssid_len = sme->ssid_len;
+	} else {
+		/* No channels found... */
+		kfree(creq);
+		creq = NULL;
+	}
 
+	return creq;
+}
 
 static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
 			   struct cfg80211_connect_params *sme)
@@ -1205,37 +1281,43 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
 
 	lbs_deb_enter(LBS_DEB_CFG80211);
 
-	if (sme->bssid) {
-		bss = cfg80211_get_bss(wiphy, sme->channel, sme->bssid,
-			sme->ssid, sme->ssid_len,
-			WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
-	} else {
-		/*
-		 * Here we have an impedance mismatch. The firmware command
-		 * CMD_802_11_ASSOCIATE always needs a BSSID, it cannot
-		 * connect otherwise. However, for the connect-API of
-		 * cfg80211 the bssid is purely optional. We don't get one,
-		 * except the user specifies one on the "iw" command line.
-		 *
-		 * If we don't got one, we could initiate a scan and look
-		 * for the best matching cfg80211_bss entry.
-		 *
-		 * Or, better yet, net/wireless/sme.c get's rewritten into
-		 * something more generally useful.
+	if (!sme->bssid) {
+		/* Run a scan if one isn't in-progress already and if the last
+		 * scan was done more than 2 seconds ago.
 		 */
-		lbs_pr_err("TODO: no BSS specified\n");
-		ret = -ENOTSUPP;
-		goto done;
-	}
+		if (priv->scan_req == NULL &&
+		    time_after(jiffies, priv->last_scan + (2 * HZ))) {
+			struct cfg80211_scan_request *creq;
 
+			creq = _new_connect_scan_req(wiphy, sme);
+			if (!creq) {
+				ret = -EINVAL;
+				goto done;
+			}
+
+			lbs_deb_assoc("assoc: scanning for compatible AP\n");
+			_internal_start_scan(priv, true, creq);
+		}
+
+		/* Wait for any in-progress scan to complete */
+		lbs_deb_assoc("assoc: waiting for scan to complete\n");
+		wait_event_interruptible_timeout(priv->scan_q,
+						 (priv->scan_req == NULL),
+						 (15 * HZ));
+		lbs_deb_assoc("assoc: scanning competed\n");
+	}
 
+	/* Find the BSS we want using available scan results */
+	bss = cfg80211_get_bss(wiphy, sme->channel, sme->bssid,
+		sme->ssid, sme->ssid_len,
+		WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
 	if (!bss) {
-		lbs_pr_err("assicate: bss %pM not in scan results\n",
+		lbs_pr_err("assoc: bss %pM not in scan results\n",
 			   sme->bssid);
 		ret = -ENOENT;
 		goto done;
 	}
-	lbs_deb_assoc("trying %pM", sme->bssid);
+	lbs_deb_assoc("trying %pM\n", bss->bssid);
 	lbs_deb_assoc("cipher 0x%x, key index %d, key len %d\n",
 		      sme->crypto.cipher_group,
 		      sme->key_idx, sme->key_len);
@@ -1298,7 +1380,7 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
 	lbs_set_radio(priv, preamble, 1);
 
 	/* Do the actual association */
-	lbs_associate(priv, bss, sme);
+	ret = lbs_associate(priv, bss, sme);
 
  done:
 	if (bss)
diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
index 3c7e255..f062ed5 100644
--- a/drivers/net/wireless/libertas/dev.h
+++ b/drivers/net/wireless/libertas/dev.h
@@ -161,6 +161,11 @@ struct lbs_private {
 	/** Scanning */
 	struct delayed_work scan_work;
 	int scan_channel;
+	/* Queue of things waiting for scan completion */
+	wait_queue_head_t scan_q;
+	/* Whether the scan was initiated internally and not by cfg80211 */
+	bool internal_scan;
+	unsigned long last_scan;
 };
 
 extern struct cmd_confirm_sleep confirm_sleep;
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 2589671..24958a8 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -719,6 +719,7 @@ static int lbs_init_adapter(struct lbs_private *priv)
 	priv->deep_sleep_required = 0;
 	priv->wakeup_dev_required = 0;
 	init_waitqueue_head(&priv->ds_awake_q);
+	init_waitqueue_head(&priv->scan_q);
 	priv->authtype_auto = 1;
 	priv->is_host_sleep_configured = 0;
 	priv->is_host_sleep_activated = 0;
-- 
1.7.2

Re: [wt PATCH 0/5] libertas: make association work again

[Johannes Berg]

On Thu, 2010-07-29 at 23:07 -0700, Dan Williams wrote:
> This series makes libertas actually associate with APs after the
> cfg80211 conversion.  There was a pretty big TODO where if cfg80211
> didn't pass in the BSSID of the AP to associate with, then libertas
> would fail because the firmware really, really, really wants a BSSID.
> So lets do a scan to find that BSSID.  The other notable change is
> a fix to make sure all supported rates are included in the firmware's
> association request, lack of which caused some APs to deny the attempt.

I'm a bit tempted to say we could use the connect logic in cfg80211 to
provide this? cfg80211 _already_ provides this if the hardware wants
auth()/assoc() rather than connect() calls, so it seems fairly simple to
also (optionally) do it here?

johannes

Re: [wt PATCH 5/5] libertas: scan before assocation if no BSSID was given

[Johannes Berg]

On Thu, 2010-07-29 at 23:18 -0700, Dan Williams wrote:
> Fix this leftover TODO from the cfg80211 conversion by doing a scan
> if cfg80211 didn't pass in the BSSID for us.  Since the scan code
> uses so much of the cfg80211_scan_request structure to build up the
> firmware command, we just fake one when the scan request is triggered
> internally.  But we need to make sure that internal 'fake' cfg82011
> scan request does get back to cfg82011 via cfg80211_scan_done().

I think you mean "does _not_ get back".

johannes

Re: [wt PATCH 0/5] libertas: make association work again

[Holger Schurig]

> I'm a bit tempted to say we could use the connect logic in cfg80211 to
> provide this? cfg80211 _already_ provides this if the hardware wants
> auth()/assoc() rather than connect() calls, so it seems fairly simple to
> also (optionally) do it here?

Some months ago I was thinking the same. I thought that the auth/assoc and 
connect driver APIs where quite unorthogonal. However, I did not understand 
all of cfg80211's state machinery to make the connect branch of the API also 
support a BSSID.

If that could be done this way, I'd prefer it over a libertas-internal hack.

Basically we'd need this to be controller from the driver:

do scan before connect
use connect() or auth()/assoc() to connect

That way ...

... some driver can say "no scan, use connect" (orinoco, e.g.)
... or "scan, use connect" (libertas)
... or "scan, use auth/assoc" (mac80211)

-- 
http://www.holgerschurig.de

Re: [wt PATCH 0/5] libertas: make association work again

[Dan Williams]

On Fri, 2010-07-30 at 08:35 +0200, Johannes Berg wrote:
> On Thu, 2010-07-29 at 23:07 -0700, Dan Williams wrote:
> > This series makes libertas actually associate with APs after the
> > cfg80211 conversion.  There was a pretty big TODO where if cfg80211
> > didn't pass in the BSSID of the AP to associate with, then libertas
> > would fail because the firmware really, really, really wants a BSSID.
> > So lets do a scan to find that BSSID.  The other notable change is
> > a fix to make sure all supported rates are included in the firmware's
> > association request, lack of which caused some APs to deny the attempt.
> 
> I'm a bit tempted to say we could use the connect logic in cfg80211 to
> provide this? cfg80211 _already_ provides this if the hardware wants
> auth()/assoc() rather than connect() calls, so it seems fairly simple to
> also (optionally) do it here?

Yeah, I thought about that too.  Do you want me to work on that now
instead of letting this patch through?  I can revert the relevant bits
of this later if that's acceptable too.

Dan

[wt PATCH 5/5 v2] libertas: scan before assocation if no BSSID was given

[Dan Williams]

Fix this leftover TODO from the cfg80211 conversion by doing a scan
if cfg80211 didn't pass in the BSSID for us.  Since the scan code
uses so much of the cfg80211_scan_request structure to build up the
firmware command, we just fake one when the scan request is triggered
internally.  But we need to make sure that internal 'fake' cfg82011
scan request does not get back to cfg82011 via cfg80211_scan_done().

Signed-off-by: Dan Williams <dcbw@redhat.com>
---
 drivers/net/wireless/libertas/cfg.c  |  148 ++++++++++++++++++++++++++--------
 drivers/net/wireless/libertas/dev.h  |    5 +
 drivers/net/wireless/libertas/main.c |    1 +
 3 files changed, 121 insertions(+), 33 deletions(-)

v2: fixed commit message

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index a8c0126..623b81e 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -715,8 +715,13 @@ static void lbs_scan_worker(struct work_struct *work)
 
 	if (priv->scan_channel >= priv->scan_req->n_channels) {
 		/* Mark scan done */
-		cfg80211_scan_done(priv->scan_req, false);
+		if (priv->internal_scan)
+			kfree(priv->scan_req);
+		else
+			cfg80211_scan_done(priv->scan_req, false);
+
 		priv->scan_req = NULL;
+		priv->last_scan = jiffies;
 	}
 
 	/* Restart network */
@@ -727,10 +732,33 @@ static void lbs_scan_worker(struct work_struct *work)
 
 	kfree(scan_cmd);
 
+	/* Wake up anything waiting on scan completion */
+	if (priv->scan_req == NULL) {
+		lbs_deb_scan("scan: waking up waiters\n");
+		wake_up_all(&priv->scan_q);
+	}
+
  out_no_scan_cmd:
 	lbs_deb_leave(LBS_DEB_SCAN);
 }
 
+static void _internal_start_scan(struct lbs_private *priv, bool internal,
+	struct cfg80211_scan_request *request)
+{
+	lbs_deb_enter(LBS_DEB_CFG80211);
+
+	lbs_deb_scan("scan: ssids %d, channels %d, ie_len %zd\n",
+		request->n_ssids, request->n_channels, request->ie_len);
+
+	priv->scan_channel = 0;
+	queue_delayed_work(priv->work_thread, &priv->scan_work,
+		msecs_to_jiffies(50));
+
+	priv->scan_req = request;
+	priv->internal_scan = internal;
+
+	lbs_deb_leave(LBS_DEB_CFG80211);
+}
 
 static int lbs_cfg_scan(struct wiphy *wiphy,
 	struct net_device *dev,
@@ -747,18 +775,11 @@ static int lbs_cfg_scan(struct wiphy *wiphy,
 		goto out;
 	}
 
-	lbs_deb_scan("scan: ssids %d, channels %d, ie_len %zd\n",
-		request->n_ssids, request->n_channels, request->ie_len);
-
-	priv->scan_channel = 0;
-	queue_delayed_work(priv->work_thread, &priv->scan_work,
-		msecs_to_jiffies(50));
+	_internal_start_scan(priv, false, request);
 
 	if (priv->surpriseremoved)
 		ret = -EIO;
 
-	priv->scan_req = request;
-
  out:
 	lbs_deb_leave_args(LBS_DEB_CFG80211, "ret %d", ret);
 	return ret;
@@ -1193,7 +1214,62 @@ done:
 	return ret;
 }
 
+static struct cfg80211_scan_request *
+_new_connect_scan_req(struct wiphy *wiphy, struct cfg80211_connect_params *sme)
+{
+	struct cfg80211_scan_request *creq = NULL;
+	int i, n_channels = 0;
+	enum ieee80211_band band;
+
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+		if (wiphy->bands[band])
+			n_channels += wiphy->bands[band]->n_channels;
+	}
+
+	creq = kzalloc(sizeof(*creq) + sizeof(struct cfg80211_ssid) +
+		       n_channels * sizeof(void *),
+		       GFP_ATOMIC);
+	if (!creq)
+		return NULL;
+
+	/* SSIDs come after channels */
+	creq->ssids = (void *)&creq->channels[n_channels];
+	creq->n_channels = n_channels;
+	creq->n_ssids = 1;
+
+	/* Scan all available channels */
+	i = 0;
+	for (band = 0; band < IEEE80211_NUM_BANDS; band++) {
+		int j;
+
+		if (!wiphy->bands[band])
+			continue;
+
+		for (j = 0; j < wiphy->bands[band]->n_channels; j++) {
+			/* ignore disabled channels */
+			if (wiphy->bands[band]->channels[j].flags &
+						IEEE80211_CHAN_DISABLED)
+				continue;
+
+			creq->channels[i] = &wiphy->bands[band]->channels[j];
+			i++;
+		}
+	}
+	if (i) {
+		/* Set real number of channels specified in creq->channels[] */
+		creq->n_channels = i;
+
+		/* Scan for the SSID we're going to connect to */
+		memcpy(creq->ssids[0].ssid, sme->ssid, sme->ssid_len);
+		creq->ssids[0].ssid_len = sme->ssid_len;
+	} else {
+		/* No channels found... */
+		kfree(creq);
+		creq = NULL;
+	}
 
+	return creq;
+}
 
 static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
 			   struct cfg80211_connect_params *sme)
@@ -1205,37 +1281,43 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
 
 	lbs_deb_enter(LBS_DEB_CFG80211);
 
-	if (sme->bssid) {
-		bss = cfg80211_get_bss(wiphy, sme->channel, sme->bssid,
-			sme->ssid, sme->ssid_len,
-			WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
-	} else {
-		/*
-		 * Here we have an impedance mismatch. The firmware command
-		 * CMD_802_11_ASSOCIATE always needs a BSSID, it cannot
-		 * connect otherwise. However, for the connect-API of
-		 * cfg80211 the bssid is purely optional. We don't get one,
-		 * except the user specifies one on the "iw" command line.
-		 *
-		 * If we don't got one, we could initiate a scan and look
-		 * for the best matching cfg80211_bss entry.
-		 *
-		 * Or, better yet, net/wireless/sme.c get's rewritten into
-		 * something more generally useful.
+	if (!sme->bssid) {
+		/* Run a scan if one isn't in-progress already and if the last
+		 * scan was done more than 2 seconds ago.
 		 */
-		lbs_pr_err("TODO: no BSS specified\n");
-		ret = -ENOTSUPP;
-		goto done;
-	}
+		if (priv->scan_req == NULL &&
+		    time_after(jiffies, priv->last_scan + (2 * HZ))) {
+			struct cfg80211_scan_request *creq;
 
+			creq = _new_connect_scan_req(wiphy, sme);
+			if (!creq) {
+				ret = -EINVAL;
+				goto done;
+			}
+
+			lbs_deb_assoc("assoc: scanning for compatible AP\n");
+			_internal_start_scan(priv, true, creq);
+		}
+
+		/* Wait for any in-progress scan to complete */
+		lbs_deb_assoc("assoc: waiting for scan to complete\n");
+		wait_event_interruptible_timeout(priv->scan_q,
+						 (priv->scan_req == NULL),
+						 (15 * HZ));
+		lbs_deb_assoc("assoc: scanning competed\n");
+	}
 
+	/* Find the BSS we want using available scan results */
+	bss = cfg80211_get_bss(wiphy, sme->channel, sme->bssid,
+		sme->ssid, sme->ssid_len,
+		WLAN_CAPABILITY_ESS, WLAN_CAPABILITY_ESS);
 	if (!bss) {
-		lbs_pr_err("assicate: bss %pM not in scan results\n",
+		lbs_pr_err("assoc: bss %pM not in scan results\n",
 			   sme->bssid);
 		ret = -ENOENT;
 		goto done;
 	}
-	lbs_deb_assoc("trying %pM", sme->bssid);
+	lbs_deb_assoc("trying %pM\n", bss->bssid);
 	lbs_deb_assoc("cipher 0x%x, key index %d, key len %d\n",
 		      sme->crypto.cipher_group,
 		      sme->key_idx, sme->key_len);
@@ -1298,7 +1380,7 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
 	lbs_set_radio(priv, preamble, 1);
 
 	/* Do the actual association */
-	lbs_associate(priv, bss, sme);
+	ret = lbs_associate(priv, bss, sme);
 
  done:
 	if (bss)
diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
index 3c7e255..f062ed5 100644
--- a/drivers/net/wireless/libertas/dev.h
+++ b/drivers/net/wireless/libertas/dev.h
@@ -161,6 +161,11 @@ struct lbs_private {
 	/** Scanning */
 	struct delayed_work scan_work;
 	int scan_channel;
+	/* Queue of things waiting for scan completion */
+	wait_queue_head_t scan_q;
+	/* Whether the scan was initiated internally and not by cfg80211 */
+	bool internal_scan;
+	unsigned long last_scan;
 };
 
 extern struct cmd_confirm_sleep confirm_sleep;
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index 2589671..24958a8 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -719,6 +719,7 @@ static int lbs_init_adapter(struct lbs_private *priv)
 	priv->deep_sleep_required = 0;
 	priv->wakeup_dev_required = 0;
 	init_waitqueue_head(&priv->ds_awake_q);
+	init_waitqueue_head(&priv->scan_q);
 	priv->authtype_auto = 1;
 	priv->is_host_sleep_configured = 0;
 	priv->is_host_sleep_activated = 0;
-- 
1.7.2

Re: [wt PATCH 0/5] libertas: make association work again

[Johannes Berg]

On Wed, 2010-08-04 at 00:41 -0500, Dan Williams wrote:
> On Fri, 2010-07-30 at 08:35 +0200, Johannes Berg wrote:
> > On Thu, 2010-07-29 at 23:07 -0700, Dan Williams wrote:
> > > This series makes libertas actually associate with APs after the
> > > cfg80211 conversion.  There was a pretty big TODO where if cfg80211
> > > didn't pass in the BSSID of the AP to associate with, then libertas
> > > would fail because the firmware really, really, really wants a BSSID.
> > > So lets do a scan to find that BSSID.  The other notable change is
> > > a fix to make sure all supported rates are included in the firmware's
> > > association request, lack of which caused some APs to deny the attempt.
> > 
> > I'm a bit tempted to say we could use the connect logic in cfg80211 to
> > provide this? cfg80211 _already_ provides this if the hardware wants
> > auth()/assoc() rather than connect() calls, so it seems fairly simple to
> > also (optionally) do it here?
> 
> Yeah, I thought about that too.  Do you want me to work on that now
> instead of letting this patch through?  I can revert the relevant bits
> of this later if that's acceptable too.

Either way works for me since I don't care about any cruft in
libertas ;)

johannes

Re: [wt PATCH 0/5] libertas: make association work again

[Dan Williams]

On Wed, 2010-08-04 at 09:46 +0200, Johannes Berg wrote:
> On Wed, 2010-08-04 at 00:41 -0500, Dan Williams wrote:
> > On Fri, 2010-07-30 at 08:35 +0200, Johannes Berg wrote:
> > > On Thu, 2010-07-29 at 23:07 -0700, Dan Williams wrote:
> > > > This series makes libertas actually associate with APs after the
> > > > cfg80211 conversion.  There was a pretty big TODO where if cfg80211
> > > > didn't pass in the BSSID of the AP to associate with, then libertas
> > > > would fail because the firmware really, really, really wants a BSSID.
> > > > So lets do a scan to find that BSSID.  The other notable change is
> > > > a fix to make sure all supported rates are included in the firmware's
> > > > association request, lack of which caused some APs to deny the attempt.
> > > 
> > > I'm a bit tempted to say we could use the connect logic in cfg80211 to
> > > provide this? cfg80211 _already_ provides this if the hardware wants
> > > auth()/assoc() rather than connect() calls, so it seems fairly simple to
> > > also (optionally) do it here?
> > 
> > Yeah, I thought about that too.  Do you want me to work on that now
> > instead of letting this patch through?  I can revert the relevant bits
> > of this later if that's acceptable too.
> 
> Either way works for me since I don't care about any cruft in
> libertas ;)

John, if you don't mind applying this patch I'll work on generic
scan-before-connect support in cfg80211 in a further patch series.

Dan

Re: [wt PATCH 0/5] libertas: make association work again

[John W. Linville]

On Wed, Aug 04, 2010 at 02:15:46PM -0500, Dan Williams wrote:
> On Wed, 2010-08-04 at 09:46 +0200, Johannes Berg wrote:
> > On Wed, 2010-08-04 at 00:41 -0500, Dan Williams wrote:
> > > On Fri, 2010-07-30 at 08:35 +0200, Johannes Berg wrote:
> > > > On Thu, 2010-07-29 at 23:07 -0700, Dan Williams wrote:
> > > > > This series makes libertas actually associate with APs after the
> > > > > cfg80211 conversion.  There was a pretty big TODO where if cfg80211
> > > > > didn't pass in the BSSID of the AP to associate with, then libertas
> > > > > would fail because the firmware really, really, really wants a BSSID.
> > > > > So lets do a scan to find that BSSID.  The other notable change is
> > > > > a fix to make sure all supported rates are included in the firmware's
> > > > > association request, lack of which caused some APs to deny the attempt.
> > > > 
> > > > I'm a bit tempted to say we could use the connect logic in cfg80211 to
> > > > provide this? cfg80211 _already_ provides this if the hardware wants
> > > > auth()/assoc() rather than connect() calls, so it seems fairly simple to
> > > > also (optionally) do it here?
> > > 
> > > Yeah, I thought about that too.  Do you want me to work on that now
> > > instead of letting this patch through?  I can revert the relevant bits
> > > of this later if that's acceptable too.
> > 
> > Either way works for me since I don't care about any cruft in
> > libertas ;)
> 
> John, if you don't mind applying this patch I'll work on generic
> scan-before-connect support in cfg80211 in a further patch series.

OK...which of these need to be in 2.6.36?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [wt PATCH 0/5] libertas: make association work again

[Dan Williams]

On Wed, 2010-08-04 at 15:34 -0400, John W. Linville wrote:
> On Wed, Aug 04, 2010 at 02:15:46PM -0500, Dan Williams wrote:
> > On Wed, 2010-08-04 at 09:46 +0200, Johannes Berg wrote:
> > > On Wed, 2010-08-04 at 00:41 -0500, Dan Williams wrote:
> > > > On Fri, 2010-07-30 at 08:35 +0200, Johannes Berg wrote:
> > > > > On Thu, 2010-07-29 at 23:07 -0700, Dan Williams wrote:
> > > > > > This series makes libertas actually associate with APs after the
> > > > > > cfg80211 conversion.  There was a pretty big TODO where if cfg80211
> > > > > > didn't pass in the BSSID of the AP to associate with, then libertas
> > > > > > would fail because the firmware really, really, really wants a BSSID.
> > > > > > So lets do a scan to find that BSSID.  The other notable change is
> > > > > > a fix to make sure all supported rates are included in the firmware's
> > > > > > association request, lack of which caused some APs to deny the attempt.
> > > > > 
> > > > > I'm a bit tempted to say we could use the connect logic in cfg80211 to
> > > > > provide this? cfg80211 _already_ provides this if the hardware wants
> > > > > auth()/assoc() rather than connect() calls, so it seems fairly simple to
> > > > > also (optionally) do it here?
> > > > 
> > > > Yeah, I thought about that too.  Do you want me to work on that now
> > > > instead of letting this patch through?  I can revert the relevant bits
> > > > of this later if that's acceptable too.
> > > 
> > > Either way works for me since I don't care about any cruft in
> > > libertas ;)
> > 
> > John, if you don't mind applying this patch I'll work on generic
> > scan-before-connect support in cfg80211 in a further patch series.
> 
> OK...which of these need to be in 2.6.36?

They should stick with the libertas cfg80211 bits, wherever they are.
If you've pushed whatever was in wireless-testing into 2.6.36, then this
latest series of 5 should go there too.

Dan