* RFC/PATCH: Allow wpa_supplicant to share scan results. @ 2010-11-16 0:22 Ben Greear 2010-11-16 9:45 ` Jouni Malinen 0 siblings, 1 reply; 10+ messages in thread From: Ben Greear @ 2010-11-16 0:22 UTC (permalink / raw) To: hostap, linux-wireless@vger.kernel.org [-- 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) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 0:22 RFC/PATCH: Allow wpa_supplicant to share scan results Ben Greear @ 2010-11-16 9:45 ` Jouni Malinen 2010-11-16 17:03 ` Ben Greear 0 siblings, 1 reply; 10+ messages in thread From: Jouni Malinen @ 2010-11-16 9:45 UTC (permalink / raw) To: Ben Greear; +Cc: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 9:45 ` Jouni Malinen @ 2010-11-16 17:03 ` Ben Greear 2010-11-16 19:31 ` Jouni Malinen 0 siblings, 1 reply; 10+ messages in thread From: Ben Greear @ 2010-11-16 17:03 UTC (permalink / raw) To: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 17:03 ` Ben Greear @ 2010-11-16 19:31 ` Jouni Malinen 2010-11-16 19:57 ` Ben Greear 2010-11-16 20:39 ` Ben Greear 0 siblings, 2 replies; 10+ messages in thread From: Jouni Malinen @ 2010-11-16 19:31 UTC (permalink / raw) To: Ben Greear; +Cc: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 19:31 ` Jouni Malinen @ 2010-11-16 19:57 ` Ben Greear 2010-11-16 22:36 ` Jouni Malinen 2010-11-16 20:39 ` Ben Greear 1 sibling, 1 reply; 10+ messages in thread From: Ben Greear @ 2010-11-16 19:57 UTC (permalink / raw) To: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 19:57 ` Ben Greear @ 2010-11-16 22:36 ` Jouni Malinen 2010-11-16 22:52 ` Ben Greear 0 siblings, 1 reply; 10+ messages in thread From: Jouni Malinen @ 2010-11-16 22:36 UTC (permalink / raw) To: Ben Greear; +Cc: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 22:36 ` Jouni Malinen @ 2010-11-16 22:52 ` Ben Greear 2010-11-16 23:16 ` Johannes Berg 0 siblings, 1 reply; 10+ messages in thread From: Ben Greear @ 2010-11-16 22:52 UTC (permalink / raw) To: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 22:52 ` Ben Greear @ 2010-11-16 23:16 ` Johannes Berg 2010-11-16 23:40 ` Ben Greear 0 siblings, 1 reply; 10+ messages in thread From: Johannes Berg @ 2010-11-16 23:16 UTC (permalink / raw) To: Ben Greear; +Cc: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 23:16 ` Johannes Berg @ 2010-11-16 23:40 ` Ben Greear 0 siblings, 0 replies; 10+ messages in thread From: Ben Greear @ 2010-11-16 23:40 UTC (permalink / raw) To: Johannes Berg; +Cc: hostap, linux-wireless@vger.kernel.org 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: RFC/PATCH: Allow wpa_supplicant to share scan results. 2010-11-16 19:31 ` Jouni Malinen 2010-11-16 19:57 ` Ben Greear @ 2010-11-16 20:39 ` Ben Greear 1 sibling, 0 replies; 10+ messages in thread From: Ben Greear @ 2010-11-16 20:39 UTC (permalink / raw) To: hostap, linux-wireless@vger.kernel.org [-- 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) ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2010-11-16 23:40 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-16 0:22 RFC/PATCH: Allow wpa_supplicant to share scan results Ben Greear 2010-11-16 9:45 ` Jouni Malinen 2010-11-16 17:03 ` Ben Greear 2010-11-16 19:31 ` Jouni Malinen 2010-11-16 19:57 ` Ben Greear 2010-11-16 22:36 ` Jouni Malinen 2010-11-16 22:52 ` Ben Greear 2010-11-16 23:16 ` Johannes Berg 2010-11-16 23:40 ` Ben Greear 2010-11-16 20:39 ` Ben Greear
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).