RFC/PATCH: Allow wpa_supplicant to share scan results.

RFC/PATCH: Allow wpa_supplicant to share scan results.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 4506 bytes --]


The attached (and inline) patch allows wpa_supplicant to share scan results
between all VIFS on the same radio (phy).  This patch is a bit rough, but
it does appear to do what I was hoping for.

Please let me know if this has any chance and upstream inclusion.

I know I need to at least strip out some of the debug code, and make
this optional via the global config file, but I was hoping early feedback
might save more work later...

Thanks,
Ben


diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 39ac33b..6c2617a 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -44,6 +44,11 @@
  #include "mlme.h"
  #include "scan.h"

+#ifdef __linux__
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#endif

  static int wpa_supplicant_select_config(struct wpa_supplicant *wpa_s)
  {
@@ -810,8 +815,7 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
  	return 1;
  }

-
-static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+static void _wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
  					      union wpa_event_data *data)
  {
  	struct wpa_bss *selected;
@@ -851,7 +855,7 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
  		return;
  	}

-	wpa_printf(MSG_DEBUG, "New scan results available");
+	wpa_printf(MSG_DEBUG, "%s: New scan results available", wpa_s->ifname);
  	wpa_msg_ctrl(wpa_s, MSG_INFO, WPA_EVENT_SCAN_RESULTS);
  	wpas_notify_scan_results(wpa_s);

@@ -910,6 +914,85 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
  		}
  	}
  }
+
+static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+					      union wpa_event_data *data)
+{
+	_wpa_supplicant_event_scan_results(wpa_s, data);
+	
+#ifdef __linux__
+	{
+		char buf[90];
+		char phyname[30];
+		char phyname2[30];
+		int f;
+		int rv;
+		struct wpa_supplicant *ifs;
+
+		wpa_printf(MSG_DEBUG, "%s: Checking for peer VIFS in event_scan_results.\n",
+			   wpa_s->ifname);
+
+		snprintf(buf, sizeof(buf) - 1, "/sys/class/net/%s/phy80211/name", wpa_s->ifname);
+		f = open(buf, O_RDONLY);
+		if (f < 0) {
+			wpa_printf(MSG_DEBUG, "Could not open file: %s, error: %s",
+				   buf, strerror(errno));
+			return;
+		}
+
+		rv = read(f, phyname, sizeof(phyname) - 1);
+		close(f);
+		if (rv < 0) {
+			wpa_printf(MSG_DEBUG, "Could not read file: %s  error: %s",
+				   buf, strerror(errno));
+			return;
+		}
+		phyname[rv] = 0;
+		wpa_printf(MSG_DEBUG, "%s: Found phy name: %s", wpa_s->ifname, phyname);
+
+		/* Ok, we know our phy...now check other interfaces to see if they have the
+		 * same phy.  If so, they get updated with this same scan info.
+		 */
+		ifs = wpa_s->global->ifaces;
+		while (ifs) {
+			if (ifs == wpa_s)
+				ifs = ifs->next;
+			else {
+				snprintf(buf, sizeof(buf) - 1, "/sys/class/net/%s/phy80211/name",
+					 ifs->ifname);
+				wpa_printf(MSG_DEBUG, "Opening: %s", buf);
+				f = open(buf, O_RDONLY);
+				if (f < 0) {
+					wpa_printf(MSG_DEBUG, "Could not open file: %s, error: %s",
+						   buf, strerror(errno));
+					ifs = ifs->next;
+					continue;
+				}
+
+				rv = read(f, phyname2, sizeof(phyname2) - 1);
+				close(f);
+				if (rv < 0) {
+					wpa_printf(MSG_DEBUG, "Could not read file: %s  error: %s",
+						   buf, strerror(errno));
+					ifs = ifs->next;
+					continue;
+				}
+				phyname2[rv] = 0;
+				wpa_printf(MSG_DEBUG, "%s: Found phy name: %s",
+					   ifs->ifname, phyname2);
+				if (strcmp(phyname2, phyname) == 0) {
+					wpa_printf(MSG_DEBUG, "%s: Updating scan results from peer.",
+						   ifs->ifname);
+					/* Got a winner! */
+					_wpa_supplicant_event_scan_results(ifs, data);
+				}
+				ifs = ifs->next;
+			}
+		}
+	}
+#endif
+}
+
  #endif /* CONFIG_NO_SCAN_PROCESSING */


diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 0a603af..64f0ef5 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -530,8 +530,8 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
  {
  	enum wpa_states old_state = wpa_s->wpa_state;

-	wpa_printf(MSG_DEBUG, "State: %s -> %s",
-		   wpa_supplicant_state_txt(wpa_s->wpa_state),
+	wpa_printf(MSG_DEBUG, "%s: State: %s -> %s",
+		   wpa_s->ifname, wpa_supplicant_state_txt(wpa_s->wpa_state),
  		   wpa_supplicant_state_txt(state));

  	if (state != WPA_SCANNING)

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: supplicant_share_scan_results.patch --]
[-- Type: text/plain, Size: 3947 bytes --]

diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 39ac33b..6c2617a 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -44,6 +44,11 @@
 #include "mlme.h"
 #include "scan.h"
 
+#ifdef __linux__
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#endif
 
 static int wpa_supplicant_select_config(struct wpa_supplicant *wpa_s)
 {
@@ -810,8 +815,7 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
 	return 1;
 }
 
-
-static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+static void _wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
 					      union wpa_event_data *data)
 {
 	struct wpa_bss *selected;
@@ -851,7 +855,7 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
 		return;
 	}
 
-	wpa_printf(MSG_DEBUG, "New scan results available");
+	wpa_printf(MSG_DEBUG, "%s: New scan results available", wpa_s->ifname);
 	wpa_msg_ctrl(wpa_s, MSG_INFO, WPA_EVENT_SCAN_RESULTS);
 	wpas_notify_scan_results(wpa_s);
 
@@ -910,6 +914,85 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
 		}
 	}
 }
+
+static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+					      union wpa_event_data *data)
+{
+	_wpa_supplicant_event_scan_results(wpa_s, data);
+	
+#ifdef __linux__
+	{
+		char buf[90];
+		char phyname[30];
+		char phyname2[30];
+		int f;
+		int rv;
+		struct wpa_supplicant *ifs;
+
+		wpa_printf(MSG_DEBUG, "%s: Checking for peer VIFS in event_scan_results.\n",
+			   wpa_s->ifname);
+
+		snprintf(buf, sizeof(buf) - 1, "/sys/class/net/%s/phy80211/name", wpa_s->ifname);
+		f = open(buf, O_RDONLY);
+		if (f < 0) {
+			wpa_printf(MSG_DEBUG, "Could not open file: %s, error: %s",
+				   buf, strerror(errno));
+			return;
+		}
+
+		rv = read(f, phyname, sizeof(phyname) - 1);
+		close(f);
+		if (rv < 0) {
+			wpa_printf(MSG_DEBUG, "Could not read file: %s  error: %s",
+				   buf, strerror(errno));
+			return;
+		}
+		phyname[rv] = 0;
+		wpa_printf(MSG_DEBUG, "%s: Found phy name: %s", wpa_s->ifname, phyname);
+
+		/* Ok, we know our phy...now check other interfaces to see if they have the
+		 * same phy.  If so, they get updated with this same scan info.
+		 */
+		ifs = wpa_s->global->ifaces;
+		while (ifs) {
+			if (ifs == wpa_s)
+				ifs = ifs->next;
+			else {
+				snprintf(buf, sizeof(buf) - 1, "/sys/class/net/%s/phy80211/name",
+					 ifs->ifname);
+				wpa_printf(MSG_DEBUG, "Opening: %s", buf);
+				f = open(buf, O_RDONLY);
+				if (f < 0) {
+					wpa_printf(MSG_DEBUG, "Could not open file: %s, error: %s",
+						   buf, strerror(errno));
+					ifs = ifs->next;
+					continue;
+				}
+
+				rv = read(f, phyname2, sizeof(phyname2) - 1);
+				close(f);
+				if (rv < 0) {
+					wpa_printf(MSG_DEBUG, "Could not read file: %s  error: %s",
+						   buf, strerror(errno));
+					ifs = ifs->next;
+					continue;
+				}
+				phyname2[rv] = 0;
+				wpa_printf(MSG_DEBUG, "%s: Found phy name: %s",
+					   ifs->ifname, phyname2);
+				if (strcmp(phyname2, phyname) == 0) {
+					wpa_printf(MSG_DEBUG, "%s: Updating scan results from peer.",
+						   ifs->ifname);
+					/* Got a winner! */
+					_wpa_supplicant_event_scan_results(ifs, data);
+				}
+				ifs = ifs->next;
+			}
+		}
+	}
+#endif
+}
+
 #endif /* CONFIG_NO_SCAN_PROCESSING */
 
 
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 0a603af..64f0ef5 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -530,8 +530,8 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
 {
 	enum wpa_states old_state = wpa_s->wpa_state;
 
-	wpa_printf(MSG_DEBUG, "State: %s -> %s",
-		   wpa_supplicant_state_txt(wpa_s->wpa_state),
+	wpa_printf(MSG_DEBUG, "%s: State: %s -> %s",
+		   wpa_s->ifname, wpa_supplicant_state_txt(wpa_s->wpa_state),
 		   wpa_supplicant_state_txt(state));
 
 	if (state != WPA_SCANNING)

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Jouni Malinen]

On Mon, Nov 15, 2010 at 04:22:30PM -0800, Ben Greear wrote:
> The attached (and inline) patch allows wpa_supplicant to share scan results
> between all VIFS on the same radio (phy).  This patch is a bit rough, but
> it does appear to do what I was hoping for.
> 
> Please let me know if this has any chance and upstream inclusion.

With the changes in wpa_supplicant/events.c, the likelihood of that
getting anywhere is about zero, but if the driver specific changes were
to be moved to src/drivers/driver_nl80211.c, this could be quite a
reasonable change to include in the upstream tree. Note the
global_init() and init2() struct wpa_driver_ops callbacks that should
make it easy to track the interfaces within driver_nl80211.c.

I would like to have more knowledge of virtual interfaces sharing the
same radio in the driver wrappers anyway, e.g., for shared_freq()
implementations. If this can be reliably detected from the driver (which
hopefully is the case with mac80211-based ones), it would be great to be
able to that without having to ask the user to configure anything.

> I know I need to at least strip out some of the debug code, and make
> this optional via the global config file, but I was hoping early feedback
> might save more work later...

What would be need for configuring this? In which case would it be
preferred to not process the scan results from other virtual interfaces
on the same radio? Wouldn't those scan events look exactly like the same
if some other application would have triggered them?


In addition, it might be interesting to considering sharing the BSS
table and scan result parsing in wpa_supplicant among the interface,
too, instead of just the scan completed events..

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Ben Greear]

On 11/16/2010 01:45 AM, Jouni Malinen wrote:
> On Mon, Nov 15, 2010 at 04:22:30PM -0800, Ben Greear wrote:
>> The attached (and inline) patch allows wpa_supplicant to share scan results
>> between all VIFS on the same radio (phy).  This patch is a bit rough, but
>> it does appear to do what I was hoping for.
>>
>> Please let me know if this has any chance and upstream inclusion.
>
> With the changes in wpa_supplicant/events.c, the likelihood of that
> getting anywhere is about zero, but if the driver specific changes were
> to be moved to src/drivers/driver_nl80211.c, this could be quite a
> reasonable change to include in the upstream tree. Note the
> global_init() and init2() struct wpa_driver_ops callbacks that should
> make it easy to track the interfaces within driver_nl80211.c.

It would be easy enough to save the phyname in the wpa_s struct so it
doesn't need to be probed all the time.

As for the logic that actually propagates scan results to all of the
peer VIFS, do you mean you want that moved farther up into the driver
as well?

> I would like to have more knowledge of virtual interfaces sharing the
> same radio in the driver wrappers anyway, e.g., for shared_freq()
> implementations. If this can be reliably detected from the driver (which
> hopefully is the case with mac80211-based ones), it would be great to be
> able to that without having to ask the user to configure anything.

It simple to know the phy-name for a particular VIF (on the latest kernel),
so knowing parent-child and sibling relationships is also easy.  Older
kernels don't have the phy-name available in sysfs, but they also didn't
support multiple VIFS well anyway..so maybe just ignore them?

>
>> I know I need to at least strip out some of the debug code, and make
>> this optional via the global config file, but I was hoping early feedback
>> might save more work later...
>
> What would be need for configuring this? In which case would it be
> preferred to not process the scan results from other virtual interfaces
> on the same radio? Wouldn't those scan events look exactly like the same
> if some other application would have triggered them?

I was afraid that one VIF might scan with one set of scan-options and another
a different set.  However, I have no real understanding of how wpa_supplicant
works, so if you think it can be always enabled, then I'm fine with that.

Also, with it enabled, I can currently reliably crash ath9k.  This is a
kernel bug that hopefully can be fixed soon, but it would be one small reason to
keep the old behaviour available.

> In addition, it might be interesting to considering sharing the BSS
> table and scan result parsing in wpa_supplicant among the interface,
> too, instead of just the scan completed events..

I'm happy to test patches..but I probably don't have time to do more
than the bare minimum to get the shared-scan results functional at
this time.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Jouni Malinen]

On Tue, Nov 16, 2010 at 09:03:15AM -0800, Ben Greear wrote:
> It would be easy enough to save the phyname in the wpa_s struct so it
> doesn't need to be probed all the time.

phyname sounds like a Linux/mac80211/cfg80211 specific thing and
probably does not belong in wpa_s.. But yes, some kind of identifier for
interfaces sharing the same radio there or in the private driver wrapper
data would be fine (if it within driver_nl80211.c, phyname would be
perfectly valid thing to store at initialization).

> As for the logic that actually propagates scan results to all of the
> peer VIFS, do you mean you want that moved farther up into the driver
> as well?

I think I would be fine with either option as a starting point, i.e.,
driver_nl80211.c can maintain an internal list of interfaces and
propagate the scan complete event to each interface or there can be an
abstract radio identifier exposed by the driver wrapper to allow core
supplicant code to handle this. I'm assuming here that it is possible to
read the scan results from any vif in mac80211 regardless of which
generated the scan-completed event. Is that correct?

> It simple to know the phy-name for a particular VIF (on the latest kernel),
> so knowing parent-child and sibling relationships is also easy.  Older
> kernels don't have the phy-name available in sysfs, but they also didn't
> support multiple VIFS well anyway..so maybe just ignore them?

Yeah, I have no problems in ignoring old versions for this kind of new
functionality.

> I was afraid that one VIF might scan with one set of scan-options and another
> a different set.  However, I have no real understanding of how wpa_supplicant
> works, so if you think it can be always enabled, then I'm fine with that.

That can already happen even with a single vif when other applications
are requesting scans.. wpa_supplicant will have to handle it cleanly
anyway.

> Also, with it enabled, I can currently reliably crash ath9k.  This is a
> kernel bug that hopefully can be fixed soon, but it would be one small reason to
> keep the old behaviour available.

What exactly is the trigger for the crash? Multiple vifs? Or this type
of optimization for re-using scan results? Anyway, I don't think we
should care about that in the context of designing what should be added
into wpa_supplicant. If a kernel driver is buggy, it will be fixed, or
you better not try to use this feature ;-).

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Ben Greear]

On 11/16/2010 11:31 AM, Jouni Malinen wrote:
> On Tue, Nov 16, 2010 at 09:03:15AM -0800, Ben Greear wrote:
>> It would be easy enough to save the phyname in the wpa_s struct so it
>> doesn't need to be probed all the time.
>
> phyname sounds like a Linux/mac80211/cfg80211 specific thing and
> probably does not belong in wpa_s.. But yes, some kind of identifier for
> interfaces sharing the same radio there or in the private driver wrapper
> data would be fine (if it within driver_nl80211.c, phyname would be
> perfectly valid thing to store at initialization).
>
>> As for the logic that actually propagates scan results to all of the
>> peer VIFS, do you mean you want that moved farther up into the driver
>> as well?
>
> I think I would be fine with either option as a starting point, i.e.,
> driver_nl80211.c can maintain an internal list of interfaces and
> propagate the scan complete event to each interface or there can be an
> abstract radio identifier exposed by the driver wrapper to allow core
> supplicant code to handle this. I'm assuming here that it is possible to
> read the scan results from any vif in mac80211 regardless of which
> generated the scan-completed event. Is that correct?

I earlier attempted to put this sharing/propagation directly into
the kernel (instead of -EBUSY, the kernel would just keep a list of
interested interfaces and then send results to all interested parties).

However, Johannes didn't like this approach, so I'm trying to do a
similar thing in user-space.  My understanding is that you cannot
read scan results from the kernel for one VIF if they were initiated on another
VIF.  I might be wrong about that, however.  At any rate, you certainly can't
have two VIFS on the same phy scanning at the same time, as you get -EBUSY instead.
That makes wpa_supplicant take a long time to scan & associate lots
of VIFS, and speeding that up is my primary goal at this point.

I think I see where to propagate the scan results in the hostap NL driver, except
that I cannot find a global list of driver-data structs.  I could
do some casting of drv->ctx->global, but perhaps we are not supposed
to do that sort of thing?

If I put the phyname (or radio_name, or whatever) in wpa_s, I can propagate
in the events.c code, but I'm not too sure how to set that value from within
the NL driver since we are given the wpa_s as a void* (which I assume means
we should not be assuming what struct it really is).

>> It simple to know the phy-name for a particular VIF (on the latest kernel),
>> so knowing parent-child and sibling relationships is also easy.  Older
>> kernels don't have the phy-name available in sysfs, but they also didn't
>> support multiple VIFS well anyway..so maybe just ignore them?
>
> Yeah, I have no problems in ignoring old versions for this kind of new
> functionality.
>
>> I was afraid that one VIF might scan with one set of scan-options and another
>> a different set.  However, I have no real understanding of how wpa_supplicant
>> works, so if you think it can be always enabled, then I'm fine with that.
>
> That can already happen even with a single vif when other applications
> are requesting scans.. wpa_supplicant will have to handle it cleanly
> anyway.

One other thing I was worried about:  My patch is going to send scan results
to interfaces that have not successfully requested them (they may have not
requested at all, or they may have tried and received EBUSY).  Will that be
an issue?

>> Also, with it enabled, I can currently reliably crash ath9k.  This is a
>> kernel bug that hopefully can be fixed soon, but it would be one small reason to
>> keep the old behaviour available.
>
> What exactly is the trigger for the crash? Multiple vifs? Or this type
> of optimization for re-using scan results? Anyway, I don't think we
> should care about that in the context of designing what should be added
> into wpa_supplicant. If a kernel driver is buggy, it will be fixed, or
> you better not try to use this feature ;-).

It only crashes with the wpa_supplicant patch I posted (and the kernel
patch as well, it turns out).  It's almost certainly due to faster
association attempts, but I haven't looked any farther.  I plan to
test this on ath5k as well..that might shed some light.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 5971 bytes --]

Please let me know if this is moving in the right direction.  The
ath9k crash actually killed my file-system, so still re-installing
the OS on my test box...  This code is un-tested, but it does compile.

diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index aef8554..0f81b41 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -718,6 +718,11 @@ struct wpa_driver_ops {
  	 */
  	int (*get_ssid)(void *priv, u8 *ssid);

+	/** Return the physical radio name for this device, if known.
+	 * The returned data must not be modified by the caller.
+	 */
+	const char* (*get_radio_name)(void *priv);
+
  	/**
  	 * set_key - Configure encryption key
  	 * @ifname: Interface name (for multi-SSID/VLAN support)
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 51bf446..ec9b0be 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -18,6 +18,9 @@

  #include "includes.h"
  #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
  #include <net/if.h>
  #include <netlink/genl/genl.h>
  #include <netlink/genl/family.h>
@@ -27,6 +30,7 @@
  #include <linux/filter.h>
  #include "nl80211_copy.h"

+
  #include "common.h"
  #include "eloop.h"
  #include "common/ieee802_11_defs.h"
@@ -104,6 +108,7 @@ struct i802_bss {
  	struct i802_bss *next;
  	int ifindex;
  	char ifname[IFNAMSIZ + 1];
+	char phyname[32];
  	unsigned int beacon_set:1;
  };

@@ -364,6 +369,12 @@ static int wpa_driver_nl80211_get_ssid(void *priv, u8 *ssid)
  	return drv->ssid_len;
  }

+static const char *wpa_driver_nl80211_get_radio_name(void *priv)
+{
+	struct i802_bss *bss = priv;
+	return bss->phyname;
+}
+

  static void wpa_driver_nl80211_event_link(struct wpa_driver_nl80211_data *drv,
  					  char *buf, size_t len, int del)
@@ -1673,6 +1684,29 @@ static void * wpa_driver_nl80211_init(void *ctx, const char *ifname)
  		return NULL;
  	}

+	/* Find phy this interface belongs to. */
+	{
+		char buf[90];
+		int f;
+		int rv;
+
+		bss->phyname[0] = 0;
+		snprintf(buf, sizeof(buf) - 1, "/sys/class/net/%s/phy80211/name", ifname);
+		f = open(buf, O_RDONLY);
+		if (f < 0)
+			wpa_printf(MSG_DEBUG, "Could not open file: %s, error: %s",
+				   buf, strerror(errno));
+		else {
+			rv = read(f, bss->phyname, sizeof(bss->phyname) - 1);
+			close(f);
+			if (rv < 0)
+				wpa_printf(MSG_DEBUG, "Could not read file: %s  error: %s",
+					   buf, strerror(errno));
+			else
+				bss->phyname[rv] = 0;
+		}
+	}
+
  	drv->ioctl_sock = socket(PF_INET, SOCK_DGRAM, 0);
  	if (drv->ioctl_sock < 0) {
  		perror("socket(PF_INET,SOCK_DGRAM)");
@@ -5952,6 +5986,7 @@ const struct wpa_driver_ops wpa_driver_nl80211_ops = {
  	.desc = "Linux nl80211/cfg80211",
  	.get_bssid = wpa_driver_nl80211_get_bssid,
  	.get_ssid = wpa_driver_nl80211_get_ssid,
+	.get_radio_name = wpa_driver_nl80211_get_radio_name,
  	.set_key = wpa_driver_nl80211_set_key,
  	.scan2 = wpa_driver_nl80211_scan,
  	.get_scan_results2 = wpa_driver_nl80211_get_scan_results,
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 39ac33b..d7d2227 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -44,7 +44,6 @@
  #include "mlme.h"
  #include "scan.h"

-
  static int wpa_supplicant_select_config(struct wpa_supplicant *wpa_s)
  {
  	struct wpa_ssid *ssid, *old_ssid;
@@ -810,8 +809,7 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
  	return 1;
  }

-
-static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+static void _wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
  					      union wpa_event_data *data)
  {
  	struct wpa_bss *selected;
@@ -851,7 +849,7 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
  		return;
  	}

-	wpa_printf(MSG_DEBUG, "New scan results available");
+	wpa_printf(MSG_DEBUG, "%s: New scan results available", wpa_s->ifname);
  	wpa_msg_ctrl(wpa_s, MSG_INFO, WPA_EVENT_SCAN_RESULTS);
  	wpas_notify_scan_results(wpa_s);

@@ -910,6 +908,46 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
  		}
  	}
  }
+
+static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+					      union wpa_event_data *data)
+{
+	const char* rn = NULL;
+	_wpa_supplicant_event_scan_results(wpa_s, data);
+	
+	if (wpa_s->driver->get_radio_name) {
+		rn = wpa_s->driver->get_radio_name(wpa_s->drv_priv);
+	}
+
+	if (rn && rn[0]) {
+		struct wpa_supplicant *ifs;
+		wpa_printf(MSG_DEBUG, "%s: Checking for peer VIFS in event_scan_results.\n",
+			   wpa_s->ifname);
+
+		/* Ok, we know our phy...now check other interfaces
+		 * to see if they have the same phy.  If so, they get
+		 * updated with this same scan info.
+		 */
+		ifs = wpa_s->global->ifaces;
+		while (ifs) {
+			if ((ifs == wpa_s) ||
+			    !ifs->driver->get_radio_name)
+				ifs = ifs->next;
+			else {
+				const char* rn2;
+				rn2 = ifs->driver->get_radio_name(ifs->drv_priv);
+				if (rn2 && (strcmp(rn, rn2) == 0)) {
+					wpa_printf(MSG_DEBUG, "%s: Updating scan results from peer.",
+						   ifs->ifname);
+					/* Got a winner! */
+					_wpa_supplicant_event_scan_results(ifs, data);
+				}
+				ifs = ifs->next;
+			}
+		}
+	}
+}
+
  #endif /* CONFIG_NO_SCAN_PROCESSING */


diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 0a603af..64f0ef5 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -530,8 +530,8 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
  {
  	enum wpa_states old_state = wpa_s->wpa_state;

-	wpa_printf(MSG_DEBUG, "State: %s -> %s",
-		   wpa_supplicant_state_txt(wpa_s->wpa_state),
+	wpa_printf(MSG_DEBUG, "%s: State: %s -> %s",
+		   wpa_s->ifname, wpa_supplicant_state_txt(wpa_s->wpa_state),
  		   wpa_supplicant_state_txt(state));

  	if (state != WPA_SCANNING)

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: supplicant_share_scan_results2.patch --]
[-- Type: text/plain, Size: 5628 bytes --]

diff --git a/src/drivers/driver.h b/src/drivers/driver.h
index aef8554..0f81b41 100644
--- a/src/drivers/driver.h
+++ b/src/drivers/driver.h
@@ -718,6 +718,11 @@ struct wpa_driver_ops {
 	 */
 	int (*get_ssid)(void *priv, u8 *ssid);
 
+	/** Return the physical radio name for this device, if known.
+	 * The returned data must not be modified by the caller.
+	 */
+	const char* (*get_radio_name)(void *priv);
+
 	/**
 	 * set_key - Configure encryption key
 	 * @ifname: Interface name (for multi-SSID/VLAN support)
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c
index 51bf446..ec9b0be 100644
--- a/src/drivers/driver_nl80211.c
+++ b/src/drivers/driver_nl80211.c
@@ -18,6 +18,9 @@
 
 #include "includes.h"
 #include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
 #include <net/if.h>
 #include <netlink/genl/genl.h>
 #include <netlink/genl/family.h>
@@ -27,6 +30,7 @@
 #include <linux/filter.h>
 #include "nl80211_copy.h"
 
+
 #include "common.h"
 #include "eloop.h"
 #include "common/ieee802_11_defs.h"
@@ -104,6 +108,7 @@ struct i802_bss {
 	struct i802_bss *next;
 	int ifindex;
 	char ifname[IFNAMSIZ + 1];
+	char phyname[32];
 	unsigned int beacon_set:1;
 };
 
@@ -364,6 +369,12 @@ static int wpa_driver_nl80211_get_ssid(void *priv, u8 *ssid)
 	return drv->ssid_len;
 }
 
+static const char *wpa_driver_nl80211_get_radio_name(void *priv)
+{
+	struct i802_bss *bss = priv;
+	return bss->phyname;
+}
+
 
 static void wpa_driver_nl80211_event_link(struct wpa_driver_nl80211_data *drv,
 					  char *buf, size_t len, int del)
@@ -1673,6 +1684,29 @@ static void * wpa_driver_nl80211_init(void *ctx, const char *ifname)
 		return NULL;
 	}
 
+	/* Find phy this interface belongs to. */
+	{
+		char buf[90];
+		int f;
+		int rv;
+
+		bss->phyname[0] = 0;
+		snprintf(buf, sizeof(buf) - 1, "/sys/class/net/%s/phy80211/name", ifname);
+		f = open(buf, O_RDONLY);
+		if (f < 0)
+			wpa_printf(MSG_DEBUG, "Could not open file: %s, error: %s",
+				   buf, strerror(errno));
+		else {
+			rv = read(f, bss->phyname, sizeof(bss->phyname) - 1);
+			close(f);
+			if (rv < 0)
+				wpa_printf(MSG_DEBUG, "Could not read file: %s  error: %s",
+					   buf, strerror(errno));
+			else
+				bss->phyname[rv] = 0;
+		}
+	}
+
 	drv->ioctl_sock = socket(PF_INET, SOCK_DGRAM, 0);
 	if (drv->ioctl_sock < 0) {
 		perror("socket(PF_INET,SOCK_DGRAM)");
@@ -5952,6 +5986,7 @@ const struct wpa_driver_ops wpa_driver_nl80211_ops = {
 	.desc = "Linux nl80211/cfg80211",
 	.get_bssid = wpa_driver_nl80211_get_bssid,
 	.get_ssid = wpa_driver_nl80211_get_ssid,
+	.get_radio_name = wpa_driver_nl80211_get_radio_name,
 	.set_key = wpa_driver_nl80211_set_key,
 	.scan2 = wpa_driver_nl80211_scan,
 	.get_scan_results2 = wpa_driver_nl80211_get_scan_results,
diff --git a/wpa_supplicant/events.c b/wpa_supplicant/events.c
index 39ac33b..d7d2227 100644
--- a/wpa_supplicant/events.c
+++ b/wpa_supplicant/events.c
@@ -44,7 +44,6 @@
 #include "mlme.h"
 #include "scan.h"
 
-
 static int wpa_supplicant_select_config(struct wpa_supplicant *wpa_s)
 {
 	struct wpa_ssid *ssid, *old_ssid;
@@ -810,8 +809,7 @@ static int wpa_supplicant_need_to_roam(struct wpa_supplicant *wpa_s,
 	return 1;
 }
 
-
-static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+static void _wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
 					      union wpa_event_data *data)
 {
 	struct wpa_bss *selected;
@@ -851,7 +849,7 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
 		return;
 	}
 
-	wpa_printf(MSG_DEBUG, "New scan results available");
+	wpa_printf(MSG_DEBUG, "%s: New scan results available", wpa_s->ifname);
 	wpa_msg_ctrl(wpa_s, MSG_INFO, WPA_EVENT_SCAN_RESULTS);
 	wpas_notify_scan_results(wpa_s);
 
@@ -910,6 +908,46 @@ static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
 		}
 	}
 }
+
+static void wpa_supplicant_event_scan_results(struct wpa_supplicant *wpa_s,
+					      union wpa_event_data *data)
+{
+	const char* rn = NULL;
+	_wpa_supplicant_event_scan_results(wpa_s, data);
+	
+	if (wpa_s->driver->get_radio_name) {
+		rn = wpa_s->driver->get_radio_name(wpa_s->drv_priv);
+	}
+
+	if (rn && rn[0]) {
+		struct wpa_supplicant *ifs;
+		wpa_printf(MSG_DEBUG, "%s: Checking for peer VIFS in event_scan_results.\n",
+			   wpa_s->ifname);
+
+		/* Ok, we know our phy...now check other interfaces
+		 * to see if they have the same phy.  If so, they get
+		 * updated with this same scan info.
+		 */
+		ifs = wpa_s->global->ifaces;
+		while (ifs) {
+			if ((ifs == wpa_s) ||
+			    !ifs->driver->get_radio_name)
+				ifs = ifs->next;
+			else {
+				const char* rn2;
+				rn2 = ifs->driver->get_radio_name(ifs->drv_priv);
+				if (rn2 && (strcmp(rn, rn2) == 0)) {
+					wpa_printf(MSG_DEBUG, "%s: Updating scan results from peer.",
+						   ifs->ifname);
+					/* Got a winner! */
+					_wpa_supplicant_event_scan_results(ifs, data);
+				}
+				ifs = ifs->next;
+			}
+		}
+	}
+}
+
 #endif /* CONFIG_NO_SCAN_PROCESSING */
 
 
diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c
index 0a603af..64f0ef5 100644
--- a/wpa_supplicant/wpa_supplicant.c
+++ b/wpa_supplicant/wpa_supplicant.c
@@ -530,8 +530,8 @@ void wpa_supplicant_set_state(struct wpa_supplicant *wpa_s,
 {
 	enum wpa_states old_state = wpa_s->wpa_state;
 
-	wpa_printf(MSG_DEBUG, "State: %s -> %s",
-		   wpa_supplicant_state_txt(wpa_s->wpa_state),
+	wpa_printf(MSG_DEBUG, "%s: State: %s -> %s",
+		   wpa_s->ifname, wpa_supplicant_state_txt(wpa_s->wpa_state),
 		   wpa_supplicant_state_txt(state));
 
 	if (state != WPA_SCANNING)

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Jouni Malinen]

On Tue, Nov 16, 2010 at 11:57:09AM -0800, Ben Greear wrote:
> I earlier attempted to put this sharing/propagation directly into
> the kernel (instead of -EBUSY, the kernel would just keep a list of
> interested interfaces and then send results to all interested parties).
> 
> However, Johannes didn't like this approach, so I'm trying to do a
> similar thing in user-space.

Johannes is not the only one having something against the kernel
approach.. ;-)

>  My understanding is that you cannot
> read scan results from the kernel for one VIF if they were initiated on another
> VIF.  I might be wrong about that, however.  At any rate, you certainly can't
> have two VIFS on the same phy scanning at the same time, as you get -EBUSY instead.
> That makes wpa_supplicant take a long time to scan & associate lots
> of VIFS, and speeding that up is my primary goal at this point.

Hmm.. Maybe I'm missing something in your patch, but it seems to be
doing exactly what you are describing it should not be doing.. It seems
to share the scan-done event with all interfaces that are from the same
radio and then each interface would try to read the scan result from
their own VIF which would be different from the one that actually
initiated the scan.. Similarly, I did not notice any changes that would
actually restrict requesting new scans when there is a known scan
request pending on another interface that shares the same radio.

Are these still waiting to be implemented or did I miss something in
your patch? Anyway, they approach in the newer version looked reasonable
to me based on what I believe you are now trying to do.

> I think I see where to propagate the scan results in the hostap NL driver, except
> that I cannot find a global list of driver-data structs.  I could
> do some casting of drv->ctx->global, but perhaps we are not supposed
> to do that sort of thing?

You would need to maintain a local list in driver_nl80211.c after
converting it to use global_init/init2 callbacks.. Anyway, it sounds
like this would not be the approach to make this work cleanly with
wpa_supplicant becoming more aware of when to request new scans and with
the assumed limitation on which vif the scan results can be read from.

> If I put the phyname (or radio_name, or whatever) in wpa_s, I can propagate
> in the events.c code, but I'm not too sure how to set that value from within
> the NL driver since we are given the wpa_s as a void* (which I assume means
> we should not be assuming what struct it really is).

It looks like you found the way I was hoping for with the new
get_radio_name() driver op.

> One other thing I was worried about:  My patch is going to send scan results
> to interfaces that have not successfully requested them (they may have not
> requested at all, or they may have tried and received EBUSY).  Will that be
> an issue?

If your assumption above is correct, yes. Instead of sharing the
scan-done event, this change should like share the scan_res from
wpa_supplicant_get_scan_results() for all interfaces sharing the radio.
It could also be nice to add a for-each-vif-in-my-radio wrapper to take
care of the calls since I would expect that to be needed for other
areas, too.. Like for restricting new scan requests if there is a
pending scan on another interface that happens to share the radio.
("Restricting" here could mean delaying or dropping the new request
completely.. For more optimized behavior, this could eventually end up
being able to merge multiple requests into one, too..)

> It only crashes with the wpa_supplicant patch I posted (and the kernel
> patch as well, it turns out).  It's almost certainly due to faster
> association attempts, but I haven't looked any farther.  I plan to
> test this on ath5k as well..that might shed some light.

Huh.. Well, anyway, as I said, that should not matter in the context of
what we want to do with wpa_supplicant.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Ben Greear]

On 11/16/2010 02:36 PM, Jouni Malinen wrote:
> On Tue, Nov 16, 2010 at 11:57:09AM -0800, Ben Greear wrote:
>> I earlier attempted to put this sharing/propagation directly into
>> the kernel (instead of -EBUSY, the kernel would just keep a list of
>> interested interfaces and then send results to all interested parties).
>>
>> However, Johannes didn't like this approach, so I'm trying to do a
>> similar thing in user-space.
>
> Johannes is not the only one having something against the kernel
> approach.. ;-)

Well, this is all good for supplicant, but same problem exists for
un-encrypted interfaces that use the built-in kernel scanning.  However,
I can carry my own patch if needed..it's still useful to get this
working with supplicant.

>>   My understanding is that you cannot
>> read scan results from the kernel for one VIF if they were initiated on another
>> VIF.  I might be wrong about that, however.  At any rate, you certainly can't
>> have two VIFS on the same phy scanning at the same time, as you get -EBUSY instead.
>> That makes wpa_supplicant take a long time to scan&  associate lots
>> of VIFS, and speeding that up is my primary goal at this point.
>
> Hmm.. Maybe I'm missing something in your patch, but it seems to be
> doing exactly what you are describing it should not be doing.. It seems

Ok, I think I mis-understood your original question.  You can read shared results
from wpa_supplicant data structures..but not directly from the kernel.

> to share the scan-done event with all interfaces that are from the same
> radio and then each interface would try to read the scan result from
> their own VIF which would be different from the one that actually
> initiated the scan.. Similarly, I did not notice any changes that would
> actually restrict requesting new scans when there is a known scan
> request pending on another interface that shares the same radio.
>
> Are these still waiting to be implemented or did I miss something in
> your patch? Anyway, they approach in the newer version looked reasonable
> to me based on what I believe you are now trying to do.

I wasn't planning to add further restrictions.  Currently, other vifs
making requests would get EBUSY, and that seems to be handled fine.
It's *possible* that someday multiple VIFs can scan simultaneously
in the kernel, so I don't think it's worth adding extra checks in supplicant.

>> One other thing I was worried about:  My patch is going to send scan results
>> to interfaces that have not successfully requested them (they may have not
>> requested at all, or they may have tried and received EBUSY).  Will that be
>> an issue?
>
> If your assumption above is correct, yes. Instead of sharing the
> scan-done event, this change should like share the scan_res from
> wpa_supplicant_get_scan_results() for all interfaces sharing the radio.

Something is needed to kick the other VIFs and tell them there are
scan results available.  That is why I put the logic in events.c
as I did.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Johannes Berg]

On Tue, 2010-11-16 at 14:52 -0800, Ben Greear wrote:

> Well, this is all good for supplicant, but same problem exists for
> un-encrypted interfaces that use the built-in kernel scanning.  However,
> I can carry my own patch if needed..it's still useful to get this
> working with supplicant.

You do know, of course, that you should be running the supplicant anyway
even then, for various "simple" reasons like roaming etc.

johannes

Re: RFC/PATCH: Allow wpa_supplicant to share scan results.

[Ben Greear]

On 11/16/2010 03:16 PM, Johannes Berg wrote:
> On Tue, 2010-11-16 at 14:52 -0800, Ben Greear wrote:
>
>> Well, this is all good for supplicant, but same problem exists for
>> un-encrypted interfaces that use the built-in kernel scanning.  However,
>> I can carry my own patch if needed..it's still useful to get this
>> working with supplicant.
>
> You do know, of course, that you should be running the supplicant anyway
> even then, for various "simple" reasons like roaming etc.

You'd be amazed at the things I don't know :)

I'll give that a try.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com