| Message ID | 1421904138-6385-2-git-send-email-masashi.honma@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Jan 22, 2015 at 02:22:17PM +0900, Masashi Honma wrote: > Our patch for mac80211 has already merged. > http://git.kernel.org/cgit/linux/kernel/git/linville/wireless-testing.git/commit/?id=2ae70efcea7a695a62bb47170d3fb16674b8dbea > > The patch fires new peer notification frequently. The notification restarts SAE > authentication when it will be received while SAE authentication is in progress. > > So this patch suppresses such undesired new peer notifications. This makes sense, authsae already does something similar. > diff --git a/wpa_supplicant/mesh_mpm.c b/wpa_supplicant/mesh_mpm.c > index 51f82f5..cddeb01 100644 > --- a/wpa_supplicant/mesh_mpm.c > +++ b/wpa_supplicant/mesh_mpm.c > @@ -537,11 +537,12 @@ void wpa_mesh_new_mesh_peer(struct wpa_supplicant *wpa_s, const u8 *addr, > int ret = 0; > > sta = ap_get_sta(data, addr); > - if (!sta) { > - sta = ap_sta_add(data, addr); > - if (!sta) > - return; > - } > + if (sta) > + goto end; > + > + sta = ap_sta_add(data, addr); > + if (!sta) > + return; ... but I think it's a little odd to use goto here when it's not really an error path. What about just suppressing them with something like: /* ignore new peer events while SAE auth in progress */ if (sta && sta->sae && sta->sae->state != SAE_ACCEPTED) return; Would that work?
On Thu, 2015-01-22 at 14:22 +0900, Masashi Honma wrote: > Our patch for mac80211 has already merged. > http://git.kernel.org/cgit/linux/kernel/git/linville/wireless-testing.git/commit/?id=2ae70efcea7a695a62bb47170d3fb16674b8dbea > > The patch fires new peer notification frequently. The notification restarts SAE > authentication when it will be received while SAE authentication is in progress. Isn't that more of a kernel bug then? johannes
On Thu, Jan 22, 2015 at 09:39:53PM +0100, Johannes Berg wrote: > On Thu, 2015-01-22 at 14:22 +0900, Masashi Honma wrote: > > Our patch for mac80211 has already merged. > > http://git.kernel.org/cgit/linux/kernel/git/linville/wireless-testing.git/commit/?id=2ae70efcea7a695a62bb47170d3fb16674b8dbea > > > > The patch fires new peer notification frequently. The notification restarts SAE > > authentication when it will be received while SAE authentication is in progress. > > Isn't that more of a kernel bug then? Perhaps it's not too late to revert that patch for 3.19, rather than deal with all these workarounds forever.
On Thu, Jan 22, 2015 at 09:39:53PM +0100, Johannes Berg wrote: > On Thu, 2015-01-22 at 14:22 +0900, Masashi Honma wrote: > > Our patch for mac80211 has already merged. > > http://git.kernel.org/cgit/linux/kernel/git/linville/wireless-testing.git/commit/?id=2ae70efcea7a695a62bb47170d3fb16674b8dbea > > > > The patch fires new peer notification frequently. The notification restarts SAE > > authentication when it will be received while SAE authentication is in progress. > > Isn't that more of a kernel bug then? I went ahead and sent out a revert for this -- besides breaking existing wpa_s builds, I think we can take a different approach that won't involve getting lots of events just to wind up ignoring them.
2015-01-23 6:53 GMT+09:00 Bob Copeland <me@bobcopeland.com>: > I went ahead and sent out a revert for this -- besides breaking existing > wpa_s builds, I think we can take a different approach that won't involve > getting lots of events just to wind up ignoring them. Thank you for your review. I don't think the kernel patch is fully wrong. The patch is useful to trigger next peering process after peering fail. But the patch has a side effect. This wpa_supplicant patch suppress only the side effect.
On Fri, 2015-01-23 at 10:31 +0900, Masashi Honma wrote: > 2015-01-23 6:53 GMT+09:00 Bob Copeland <me@bobcopeland.com>: > > I went ahead and sent out a revert for this -- besides breaking existing > > wpa_s builds, I think we can take a different approach that won't involve > > getting lots of events just to wind up ignoring them. > > Thank you for your review. > I don't think the kernel patch is fully wrong. The patch is useful to trigger > next peering process after peering fail. But the patch has a side effect. This > wpa_supplicant patch suppress only the side effect. It's still introducing a regression using old supplicant, so I guess you have to find a better solution - perhaps wpa_s requesting the new behaviour in some way. johannes
2015-01-23 18:36 GMT+09:00 Johannes Berg <johannes@sipsolutions.net>: > It's still introducing a regression using old supplicant, so I guess you > have to find a better solution - perhaps wpa_s requesting the new > behaviour in some way. I will drop this patch because Bob's new patch solves this problem. http://lists.shmoo.com/pipermail/hostap/2015-January/031932.html
diff --git a/wpa_supplicant/mesh_mpm.c b/wpa_supplicant/mesh_mpm.c index 51f82f5..cddeb01 100644 --- a/wpa_supplicant/mesh_mpm.c +++ b/wpa_supplicant/mesh_mpm.c @@ -537,11 +537,12 @@ void wpa_mesh_new_mesh_peer(struct wpa_supplicant *wpa_s, const u8 *addr, int ret = 0; sta = ap_get_sta(data, addr); - if (!sta) { - sta = ap_sta_add(data, addr); - if (!sta) - return; - } + if (sta) + goto end; + + sta = ap_sta_add(data, addr); + if (!sta) + return; /* initialize sta */ if (copy_supp_rates(wpa_s, sta, elems)) @@ -611,10 +612,15 @@ void wpa_mesh_new_mesh_peer(struct wpa_supplicant *wpa_s, const u8 *addr, return; } - if (conf->security == MESH_CONF_SEC_NONE) - mesh_mpm_plink_open(wpa_s, sta, PLINK_OPEN_SENT); - else - mesh_rsn_auth_sae_sta(wpa_s, sta); + end: + if (sta->plink_state == PLINK_LISTEN) { + if (!sta->my_lid) + mesh_mpm_init_link(wpa_s, sta); + if (conf->security == MESH_CONF_SEC_NONE) + mesh_mpm_plink_open(wpa_s, sta, PLINK_OPEN_SENT); + else if (sta->sae == NULL || sta->sae->state == SAE_NOTHING) + mesh_rsn_auth_sae_sta(wpa_s, sta); + } }