| Message ID | 20120205110616.GA3985@w1.fi |
|---|---|
| State | Superseded |
| Headers | show |
Thanks Jouni for your email. Pls find my comments inline. > 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 was also thinking of something that can be set in wpa_global. I will update this part. > 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. This is mentioned in the case 3(a) below. If the interface is GO and channel switch is supported, we would move the GO (only a hook is provided at this point) and allow the assoc to proceed. > Have you looked at more details on how to implement the new driver > operation (concurrent-3.patch) with cfg80211/nl80211? I am yet to start on this part. > 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 am expecting the driver not to attempt a connection, if another interface is connected on a different channel (and FW roaming is supported). But again, this is debatable. Some drivers might by default give preference to STA operation. We might need to handle cases where supplicant gets the STA connected event while we already have a p2p connection on some other channel. Probably in this case, we can just do a p2p_group_remove with reason=freq_conflict. Two more changes from my side which are not in the current set of patches. a) I later felt that providing network_id would be better than providing ssid and bssid during WPA_EVENT_FREQ_CONFLICT. +▷⋅⋅▷⋅⋅⋅▷⋅⋅⋅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)); changed to +▷⋅⋅▷⋅⋅⋅▷⋅⋅⋅wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_FREQ_CONFLICT " id=%d", ssid->id);. b) I have added a new reason code to P2P_GROUP_REMOVE reason=FREQ_CONFLICT. This is useful when a p2p group gets removed when STA connection has more priority. Let me put the scenarios which we want to address and what have so far been handled by the current patches. Please add to this, if i missed any. 1. We got a STA connection already. Now a P2P Join is requested. Action: Check whether there is the GO to which Join is requested on a different channel. If channel is different, announce GROUP-FORMATION-FAILURE with reason as freq_conflict. Let the application decide further. This is handled by concurrent-2.patch 2. We got a STA connection already. Now a P2P GO Nego connection is requested. The supplicant automatically tries to start the new connection on the shared channel. There can be failures a) The go nego requesting device may not support the channel. b) The other device doesn't support channel (due to a shared interface on someother channel). If GO negotiation fails with status 7, it could be caused due to freq conflict. I think status code is good enough for application to know that it is a channel problem. 3. We got a P2P Connection present and now a STA association is attempted. This could include supplicant based roaming. It would be difficult to make a run-time based decision. So basically, the decision will be done based on the pre-set connection priority (whether STA or P2P connection has the priority). a) If the P2P interface is a GO, then try moving the GO to the STA channel (there by avoiding the conflict). b) If GO channel switch is not supported or if the interface is a P2P-client, then we need to terminate one of the connection. c) If STA has priority, the P2P connection is terminated indicating that it is caused due to frequency conflict. d) If P2P connection has priority, the STA ssid is disabled and upper framework is notified about that. Now the application/framwork can decide whether to terminate P2P in favor or STA or leave it like that. e) If the priority is not set, the default behavior is to notify the upper application or framework. The supplicant won't do any policing. Status: These 4a to 4e cases are hanlded by concurrent-4.patch. 4. supplicant gets the STA connected event while we already have a p2p connection on some other channel (roaming handled by driver/firmware). Status: Not yet handled. In short, the Action Items for me. 1. Move prioritize setting to wpa_global 2. Handle cases where we get STA CONNECTED event while we have a p2p_interface. I will get back to you again after making the above two changes. The rest probably we can take care as second step. 1) The concurrent-3.patch expansion [go_switch_channel]. - Jithu Jance On Sun, Feb 5, 2012 at 4:36 PM, Jouni Malinen <j@w1.fi> wrote: > 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=<iface> > 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. > > -- > Jouni Malinen PGP id EFC895FA > > _______________________________________________ > HostAP mailing list > HostAP@lists.shmoo.com > http://lists.shmoo.com/mailman/listinfo/hostap > >
On Tue, Feb 07, 2012 at 06:53:48AM +0530, Jithu Jance wrote: > I was also thinking of something that can be set in wpa_global. I will > update this part. OK, thanks. > > 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. > This is mentioned in the case 3(a) below. If the interface is GO and > channel switch is supported, > we would move the GO (only a hook is provided at this point) and allow the > assoc to proceed. No, this is different case. I was thinking of a case that does not have any concurrency at all, i.e., just a single P2P client interface that is associated with a GO that decides to change channels. Though, maybe I missed the part of shared_freq>0 not hitting in this particular sequence. > I am expecting the driver not to attempt a connection, if another interface > is connected on a different channel (and FW roaming is supported). But > again, this is debatable. Some drivers might > by default give preference to STA operation. We might need to handle cases > where supplicant gets the STA connected event while we already have a p2p > connection on some other channel. Probably in this case, we can just do > a p2p_group_remove with reason=freq_conflict. OK, that may be good enough assumption and resolution for the conflict. I guess some drivers could end up moving GO interface to a new channel automatically, so that may need some handling to address potential race with the events (GO channel frequency change and new association event at more or less the same time). > Two more changes from my side which are not in the current set of patches. > a) I later felt that providing network_id would be better than providing > ssid and bssid during WPA_EVENT_FREQ_CONFLICT. > +▷⋅⋅▷⋅⋅⋅▷⋅⋅⋅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)); > > changed to > > +▷⋅⋅▷⋅⋅⋅▷⋅⋅⋅wpa_msg(wpa_s, MSG_INFO, WPA_EVENT_FREQ_CONFLICT > " id=%d", ssid->id);. Agreed. > b) I have added a new reason code to P2P_GROUP_REMOVE > reason=FREQ_CONFLICT. This is useful when a p2p group gets removed when STA > connection has more priority. OK. > Let me put the scenarios which we want to address and what have so far been > handled by the current patches. Please add to this, if i missed any. > > 1. We got a STA connection already. Now a P2P Join is requested. > Action: Check whether there is the GO to which Join is requested on a > different channel. If > channel is different, announce GROUP-FORMATION-FAILURE with reason as > freq_conflict. Let > the application decide further. This is handled by concurrent-2.patch If the GO is already in the BSS table at the time the p2p_connect join is issued, that command may return failure immediately. For the case where a scan is started, this event message sounds fine. > 2. We got a STA connection already. Now a P2P GO Nego connection is > requested. The supplicant > automatically tries to start the new connection on the shared channel. > > There can be failures > a) The go nego requesting device may not support the channel. > b) The other device doesn't support channel (due to a shared interface > on someother channel). > > If GO negotiation fails with status 7, it could be caused due to freq > conflict. I think status code is good enough for application to know that > it is a channel problem. Yes, this case should already be handled. > 3. We got a P2P Connection present and now a STA association is attempted. > This could include > supplicant based roaming. It would be difficult to make a run-time based > decision. So basically, > the decision will be done based on the pre-set connection priority > (whether STA or P2P connection > has the priority). If the STA association request is triggered without a command from external entity, this sounds fine. If this is based on a new reassociation (etc.) command, there may be cases where the ctrl_iface command can be rejected immediately instead of having to go through this. Anyway, those can be handled as separate optimizations. > a) If the P2P interface is a GO, then try moving the GO to the STA > channel (there by avoiding the conflict). > > b) If GO channel switch is not supported or if the interface is a > P2P-client, then we need to terminate > one of the connection. > > c) If STA has priority, the P2P connection is terminated indicating that > it is caused due to frequency > conflict. OK. > d) If P2P connection has priority, the STA ssid is disabled and upper > framework is notified about that. > Now the application/framwork can decide whether to terminate P2P in > favor or STA or leave it like that. I'm not sure whether the STA network block needs to be disabled (I'm assuming you meant that with ssid getting disabled), but yes, the STA interface would not be allowed to associate on another channel. > e) If the priority is not set, the default behavior is to notify the > upper application or framework. The supplicant > won't do any policing. Hmm.. How is this different from (d) ? > 4. supplicant gets the STA connected event while we already have a p2p > connection on some other channel > (roaming handled by driver/firmware). > > Status: Not yet handled. This is related to the possibility of driver/firmware handling all channel changes internally. wpa_supplicant may need to be quite flexible in this area to allow various different architectures. > In short, the Action Items for me. > 1. Move prioritize setting to wpa_global > 2. Handle cases where we get STA CONNECTED event while we have a > p2p_interface. > > I will get back to you again after making the above two changes. OK, great. > The rest probably we can take care as second step. > 1) The concurrent-3.patch expansion [go_switch_channel]. Yeah, there should be no requirement to have all parts merged in at the same time.
P2P: Handle frequency conflicts on station mode association requests Signed-hostap: Jithu Jance <jithu@broadcom.com> 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 "