Scanning improvements for multiple VIFS.

DISCUSSION Ben Greear 2 participants 5 messages Nov 12, 2010 – Nov 15, 2010 802.11 (including cfg80211/NL80211)

Share & tools

Ben Greear

author

2010-11-12 23:23 UTC #0 lore raw

When I am using lots of virtual stations, each of them will attempt to scan when started. Most of them get EBUSY
until finally everyone is associated. This can take a long time.

I was considering trying something a bit different.

In nl80211_trigger_scan, if rdev->scan_req is != NULL, instead of returning EBUSY, what if we set a flag in the VIF that said "I want scan results too."

Then, when whatever is scanning is finished, it would send scan results to all interested vifs.

Does that sound like something that could work?

Thanks,
Ben

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Ben Greear

author

2010-11-13 01:00 UTC #1 lore raw

Quoted 4 lines · Ben Greear

When I am using lots of virtual stations, each of them will attempt to scan when started. Most of them get EBUSY
until finally everyone is associated. This can take a long time.

Quoted 8 lines

I was considering trying something a bit different.
In nl80211_trigger_scan, if rdev->scan_req is != NULL, instead of returning EBUSY, what if we set a flag in the VIF that said "I want scan results too."
Then, when whatever is scanning is finished, it would send scan results to all interested vifs.

Maybe something like the attached? I crashed my ath9k box with DMA errors and such with this patch, but not sure if it is the root of the problem or not...

I'll test some more later.

Take it easy,
Ben

[ white space damaged, I'm sure ]

include/net/cfg80211.h +12 -1

index e0950c8..dd8d830 100644
@@ -694,6 +694,17 @@ struct cfg80211_ssid {
 	u8 ssid_len;
 };
 
+
+/**
+ * struct cfg80211_srd -- Scan-Req device container.
+ * @next:  Next container in list.
+ * @dev:  network device.
+ */
+struct cfg80211_srd {
+	struct cfg80211_srd *next;
+	struct net_device *dev;
+};
+
 /**
  * struct cfg80211_scan_request - scan request description
  *

@@ -718,7 +729,7 @@ struct cfg80211_scan_request {
 
 	/* internal */
 	struct wiphy *wiphy;
-	struct net_device *dev;
+	struct cfg80211_srd* devs;
 	bool aborted;
 	bool can_scan_one;

net/wireless/core.c +13 -3

index 9c21ebf..20ae948 100644
@@ -640,9 +640,19 @@ static void wdev_cleanup_work(struct work_struct *work)
 
 	cfg80211_lock_rdev(rdev);
 
-	if (WARN_ON(rdev->scan_req && rdev->scan_req->dev == wdev->netdev)) {
-		rdev->scan_req->aborted = true;
-		___cfg80211_scan_done(rdev, true);
+	if (rdev->scan_req) {
+		struct cfg80211_srd *ptr = rdev->scan_req->devs;
+		while (ptr) {
+			/* Maybe just remove from linked-list if not
+			 * the primary?
+			 */
+			if (WARN_ON(ptr->dev == wdev->netdev)) {
+				rdev->scan_req->aborted = true;
+				___cfg80211_scan_done(rdev, true);
+				break;
+			}
+			ptr = ptr->next;
+		}
 	}
 
 	cfg80211_unlock_rdev(rdev);

net/wireless/nl80211.c +17 -3

index 72d0cc8..343271f 100644
@@ -2857,6 +2857,7 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
 	enum ieee80211_band band;
 	size_t ie_len;
 	bool do_all_chan = true;
+	struct cfg80211_srd *srd;
 
 	if (!is_valid_ie_attr(info->attrs[NL80211_ATTR_IE]))
 		return -EINVAL;

@@ -2866,8 +2867,18 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
 	if (!rdev->ops->scan)
 		return -EOPNOTSUPP;
 
-	if (rdev->scan_req)
-		return -EBUSY;
+	srd = kmalloc(sizeof(*srd), GFP_KERNEL);
+	if (!srd)
+		return -ENOMEM;
+	
+	if (rdev->scan_req) {
+		dev_hold(dev);
+		srd->dev = dev;
+		/* Initial requestor remains at the front of the list */
+		srd->next = rdev->scan_req->devs->next;
+		rdev->scan_req->devs->next = srd;
+		return 0;
+	}
 
 	if (info->attrs[NL80211_ATTR_SCAN_FREQUENCIES]) {
 		n_channels = validate_scan_freqs(

@@ -2993,7 +3004,9 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
 		       request->ie_len);
 	}
 
-	request->dev = dev;
+	srd->dev = dev;
+	srd->next = NULL;
+	request->devs = srd;
 	request->wiphy = &rdev->wiphy;
 
 	rdev->scan_req = request;

@@ -3006,6 +3019,7 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
  out_free:
 		rdev->scan_req = NULL;
 		kfree(request);
+		kfree(srd);
 	}
 
 	return err;

net/wireless/scan.c +43 ~~-19~~

111 lines not on this page · raw message

net/wireless/sme.c +17 -3

index 3e0e638..aae4617 100644
@@ -79,13 +79,24 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
 	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
 	struct cfg80211_scan_request *request;
 	int n_channels, err;
+	struct cfg80211_srd *srd;
 
 	ASSERT_RTNL();
 	ASSERT_RDEV_LOCK(rdev);
 	ASSERT_WDEV_LOCK(wdev);
 
-	if (rdev->scan_req)
-		return -EBUSY;
+	srd = kmalloc(sizeof(*srd), GFP_KERNEL);
+	if (!srd)
+		return -ENOMEM;
+	
+	if (rdev->scan_req) {
+		dev_hold(wdev->netdev);
+		srd->dev = wdev->netdev;
+		/* Initial requestor remains at the front of the list */
+		srd->next = rdev->scan_req->devs->next;
+		rdev->scan_req->devs->next = srd;
+		return 0;
+	}
 
 	if (wdev->conn->params.channel) {
 		n_channels = 1;

@@ -133,7 +144,9 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
 		wdev->conn->params.ssid_len);
 	request->ssids[0].ssid_len = wdev->conn->params.ssid_len;
 
-	request->dev = wdev->netdev;
+	srd->dev = wdev->netdev;
+	srd->next = NULL;
+	request->devs = srd;
 	request->wiphy = &rdev->wiphy;
 
 	rdev->scan_req = request;

@@ -146,6 +159,7 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
 	} else {
 		rdev->scan_req = NULL;
 		kfree(request);
+		kfree(srd);
 	}
 	return err;
 }

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Johannes Berg

maintainer

2010-11-13 05:19 UTC #2 lore raw

Quoted 6 lines

Quoted 6 lines
In nl80211_trigger_scan, if rdev->scan_req is != NULL, instead of returning EBUSY, what if we set a flag in the VIF that said "I want scan results too."
Then, when whatever is scanning is finished, it would send scan results to all interested vifs.

But the scans results could technically be different if they've been done on a different interface. Besides, you can handle it in userspace. You've been trying for weeks to avoid changing your userspace by changing the kernel, which is still the wrong approach.

johannes

Ben Greear

author

2010-11-13 18:19 UTC #3 lore raw

Quoted 9 lines · Johannes Berg

Quoted 6 lines
Quoted 6 lines
In nl80211_trigger_scan, if rdev->scan_req is != NULL, instead of returning EBUSY, what if we set a flag in the VIF that said "I want scan results too."
Then, when whatever is scanning is finished, it would send scan results to all interested vifs.
But the scans results could technically be different if they've been done on a different interface. Besides,

I was wondering about that...I was hoping that some results were better than none, and that supplicant or whatever would re-scan if it didn't find what it was looking for.

I'd also be happy to add a flag that would allow user-space to request this feature enabled or not per-scan attempt, and disable it by default so that no one is surprised by new behaviour.

Quoted 3 lines

you can handle it in userspace. You've been trying for weeks to avoid changing your userspace by changing the kernel, which is still the wrong approach.

From what I can tell, you are right about wpa_supplicant controlled interfaces (perhaps requires some supplicant hacking, but logically it can be done), but what about non-encrypted interfaces that do not even need supplicant? It seems user-space has very little to do with how these scan and associated aside from setting the initial values.

Thanks,
Ben

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

Ben Greear

author

2010-11-15 19:10 UTC #4 lore raw

In reply to Johannes Berg

Quoted 4 lines · Johannes Berg

Quoted 4 lines
Quoted 4 lines
In nl80211_trigger_scan, if rdev->scan_req is != NULL, instead of returning EBUSY, what if we set a flag in the VIF that said "I want scan results too."

Quoted 8 lines

Quoted 2 lines
Quoted 2 lines
Then, when whatever is scanning is finished, it would send scan results to all interested vifs.
But the scans results could technically be different if they've been done on a different interface. Besides, you can handle it in userspace. You've been trying for weeks to avoid changing your userspace by changing the kernel, which is still the wrong approach.

I just attempted to start wpa_supplicant to control 30 STAs. It still seems to scan sequentially.

I guess I need to hack wpa_supplicant to cache scan results?

Thanks,
Ben

Quoted 2 lines

johannes

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com