From patchwork Sun Feb 5 11:06:16 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: [RFC] P2P: Handling single channel concurrency From: Jouni Malinen X-Patchwork-Id: 139624 Message-Id: <20120205110616.GA3985@w1.fi> To: Jithu Jance Cc: hostap@lists.shmoo.com Date: Sun, 5 Feb 2012 13:06:16 +0200 On Mon, Jan 16, 2012 at 11:11:39AM -0800, Jithu Jance wrote: > I made some changes to the previously sent patch and is attached below. Kindly review the same. Thanks! There was some whitespace corruption in the patch, but I think I managed to get the changes correctly. I split the patch into four pieces for easier review and did some cleanup. Please check the attached files. > A new config param "prioritize" has been added to provide the default user preference setting. If it is not used, then event is > indicated to application/above framework to take appropriate action. Adding a mechanism to figure out which interface is preferred is fine, but I'm not sure this configuration parameter is the best way of doing this. I added the other wpa_s pointer as an argument to the wpas_is_interface_prioritized() function to allow somewhat more complex mechanism to be implemented if desired. In addition, I changed wpa_s->conf->prioritize to wpa_s->parent->conf->prioritize so that this parameter is used consistently from a single configuration file in P2P use cases. Though, this may not be correct for cases where multiple interfaces are started from command line. See concurrent-1.patch for these. Is fixed ifname configuration the best way of configuring the preference? I was thinking more of a new ctrl_iface command that could be used on any interface (say, "SET_PREFERRED"). I guess the same could be handled with the prioritize field ("SET prioritize wlan3" on the parent interface), but this information should really be in struct wpa_global and not in struct wpa_config (and in there, it could be a pointer to the preferred wpa_s rather than having to store an interface name and that SET_PREFERRED would be used to set/change this). > I also added a new event to indicate that a STATION > connection is in conflict with a P2P connection. If prioritize= is set for STA interface, then it will disconnect P2P and then > proceed with STA connection. If it is not set, then it will indicate user application of the frequency conflict. OK. This is in concurrent-4.patch. I moved the no-priority-set part into wpas_is_interface_prioritized() to provide a cleaner separation between the P2P code and interface preferences. It looks like this patch may have somewhat strange behavior for the case where a P2P client interface is using wpa_supplicant-controlled "roaming", i.e., changing frequencies to follow the GO that is doing a channel switch. In that case, we should allow the association request even if the interface is not marked prioritized since that association operation itself is going to resolve the frequency conflict. > In scenarios where a STA connection already exists and a P2P join is attempted on a different channel, I am failing the P2P connection when frequency > conflict is detected. I have appended a reason code to GROUP-FORMATION-FAILURE to indicate that this is a case of frequency conflict. I know that > changing an external event is dangerous. But was doubtful on whether to add another specific event to handle this case. so i just appended a reason code to > the existing event. Please advise whether it is better to add another event or handle it in some other way. This sounds fine (concurrent-2.patch). Have you looked at more details on how to implement the new driver operation (concurrent-3.patch) with cfg80211/nl80211? How would station mode roaming cases handled, e.g., what if the driver were to indicate that it reassociated a legacy STA interface on another channel? Do we need code for that in wpa_supplicant or is the expectation here that the driver would stop any conflicting interface in such a case (e.g., indicate a GO interface cannot operate anymore)? I'm not sure how exactly to coordinate a operating channel move in this type of case when legacy STA interface would like to move to another frequency and a GO interface is running. P2P: Handle frequency conflicts on station mode association requests Signed-hostap: Jithu Jance diff --git a/src/common/wpa_ctrl.h b/src/common/wpa_ctrl.h index d13ba02..0813a79 100644 --- a/src/common/wpa_ctrl.h +++ b/src/common/wpa_ctrl.h @@ -62,6 +62,8 @@ extern "C" { #define WPA_EVENT_BSS_ADDED "CTRL-EVENT-BSS-ADDED " /** A BSS entry was removed (followed by BSS entry id and BSSID) */ #define WPA_EVENT_BSS_REMOVED "CTRL-EVENT-BSS-REMOVED " +/** Notification of frequency conflict */ +#define WPA_EVENT_FREQ_CONFLICT "CTRL-EVENT-FREQ-CONFLICT " /** WPS overlap detected in PBC mode */ #define WPS_EVENT_OVERLAP "WPS-OVERLAP-DETECTED " diff --git a/wpa_supplicant/p2p_supplicant.c b/wpa_supplicant/p2p_supplicant.c index a6298a7..5ed7295 100644 --- a/wpa_supplicant/p2p_supplicant.c +++ b/wpa_supplicant/p2p_supplicant.c @@ -68,6 +68,55 @@ static void wpas_p2p_group_idle_timeout(void *eloop_ctx, void *timeout_ctx); static void wpas_p2p_set_group_idle_timeout(struct wpa_supplicant *wpa_s); +int wpas_p2p_handle_frequency_conflicts(struct wpa_supplicant *wpa_s, int freq) +{ + struct wpa_supplicant *iface; + int res; + + for (iface = wpa_s->global->ifaces; iface; iface = iface->next) { + if (iface->p2p_group_interface == NOT_P2P_GROUP_INTERFACE || + iface->current_ssid == NULL || + iface->current_ssid->frequency == freq) + continue; + + if (iface->p2p_group_interface == P2P_GROUP_INTERFACE_GO) { + /* + * Try to see whether we can move the GO. If it is not + * possible, remove the GO interface. + */ + if (wpa_drv_switch_channel(iface, freq) == 0) { + wpa_dbg(iface, MSG_DEBUG, "P2P: GO moved to " + "freq %d", freq); + iface->current_ssid->frequency = freq; + continue; + } + } + + /* + * If GO cannot be moved or if the conflicting interface is a + * P2P Client, remove the interface depending up on the + * connection priority. + */ + res = wpas_is_interface_prioritized(wpa_s, iface); + if (res > 0) { + /* + * Newly requested connection has priority over + * existing P2P connection. So remove the interface. + */ + wpas_p2p_disconnect(iface); + } else if (res < 0) { + /* + * No priority set. Notify the application to take + * action. + */ + return -1; + } + } + + return 0; +} + + static void wpas_p2p_scan_res_handler(struct wpa_supplicant *wpa_s, struct wpa_scan_results *scan_res) { diff --git a/wpa_supplicant/p2p_supplicant.h b/wpa_supplicant/p2p_supplicant.h index 605741d..7194681 100644 --- a/wpa_supplicant/p2p_supplicant.h +++ b/wpa_supplicant/p2p_supplicant.h @@ -31,6 +31,8 @@ void wpas_p2p_remain_on_channel_cb(struct wpa_supplicant *wpa_s, unsigned int freq, unsigned int duration); void wpas_p2p_cancel_remain_on_channel_cb(struct wpa_supplicant *wpa_s, unsigned int freq); +int wpas_p2p_handle_frequency_conflicts(struct wpa_supplicant *wpa_s, + int freq); int wpas_p2p_group_remove(struct wpa_supplicant *wpa_s, const char *ifname); int wpas_p2p_group_add(struct wpa_supplicant *wpa_s, int persistent_group, int freq); diff --git a/wpa_supplicant/wpa_supplicant.c b/wpa_supplicant/wpa_supplicant.c index 4a7a5be..9d2fe95 100644 --- a/wpa_supplicant/wpa_supplicant.c +++ b/wpa_supplicant/wpa_supplicant.c @@ -1110,6 +1110,7 @@ void wpa_supplicant_associate(struct wpa_supplicant *wpa_s, struct wpa_driver_capa capa; int assoc_failed = 0; struct wpa_ssid *old_ssid; + int freq = 0; #ifdef CONFIG_HT_OVERRIDES struct ieee80211_ht_capabilities htcaps; struct ieee80211_ht_capabilities htcaps_mask; @@ -1423,6 +1424,35 @@ void wpa_supplicant_associate(struct wpa_supplicant *wpa_s, wpa_supplicant_apply_ht_overrides(wpa_s, ssid, ¶ms); #endif /* CONFIG_HT_OVERRIDES */ + /* + * If multichannel concurrency is not supported, check for any + * frequency conflict and take appropriate action. + */ + if (!(wpa_s->drv_flags & WPA_DRIVER_FLAGS_MULTI_CHANNEL_CONCURRENT) && + (freq = wpa_drv_shared_freq(wpa_s)) > 0 && freq != params.freq) { + wpa_msg(wpa_s, MSG_INFO, "Shared interface with conflicting " + "frequency found (%d != %d)", freq, params.freq); +#ifdef CONFIG_P2P + if (wpas_p2p_handle_frequency_conflicts(wpa_s, params.freq) < + 0) { + /* + * Handling conflicts failed. Disable the current + * connection request and notify the upper layers to + * take appropriate action. + */ + wpa_printf(MSG_DEBUG, "Prioritize param not set. " + "Notifying upper layers to handle the " + "case"); + wpa_supplicant_disable_network(wpa_s, ssid); + wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_FREQ_CONFLICT + " ssid=%s bssid=" MACSTR, + wpa_ssid_txt(ssid->ssid, ssid->ssid_len), + MAC2STR(wpa_s->pending_bssid)); + os_memset(wpa_s->pending_bssid, 0, ETH_ALEN); + } +#endif /* CONFIG_P2P */ + } + ret = wpa_drv_associate(wpa_s, ¶ms); if (ret < 0) { wpa_msg(wpa_s, MSG_INFO, "Association request to the driver "