diff mbox

[11/12] mesh: Send peering open frame again if beacon from listen state peer is received

Message ID CAFk-A4=-M5kftCoupfDSdY==XXPrTnRz8adxVBhHRHTZMxHT1A@mail.gmail.com
State Superseded
Headers show

Commit Message

Masashi Honma Nov. 6, 2014, 8:35 a.m. UTC 
I need extra sta_remove for open connection.
Anyway I will re-send all patches as version 2.


2014-11-06 11:48 GMT+09:00 Masashi Honma <masashi.honma@gmail.com>:
> 2014-11-05 22:09 GMT+09:00 Bob Copeland <me@bobcopeland.com>:
>> I fixed a similar problem with authsae here:
>> https://github.com/cozybit/authsae/commit/295164a83717ce59ca280468fc2f7edcea6b3cbf
>
> Great!
>
> I could fix this problem by adding one line based on your idea.
> Thank you !
>
> diff --git a/wpa_supplicant/mesh_rsn.c b/wpa_supplicant/mesh_rsn.c
> index 94dbfd1..06e3e07 100644
> --- a/wpa_supplicant/mesh_rsn.c
> +++ b/wpa_supplicant/mesh_rsn.c
> @@ -37,6 +37,9 @@ void mesh_auth_timer(void *eloop_ctx, void *user_data)
>                 " (attempt %d) ",
>                 MAC2STR(sta->addr), sta->sae_auth_retry);
>          if (sta->sae_auth_retry < MESH_AUTH_RETRY) {
> +            if (wpa_drv_sta_remove(wpa_s, sta->addr) < 0)
> +                wpa_printf(MSG_ERROR, "AUTH: Failed to remove "
> +                       "STA " MACSTR, MAC2STR(sta->addr));
>              mesh_rsn_auth_sae_sta(wpa_s, sta);
>          } else {
>              /* block the STA if exceeded the number of attempts */

Comments

Masashi Honma Nov. 7, 2014, 8:11 a.m. UTC | #1 
I think I need to consider more about this.

So I will drop this patch from the patch set for now.

As thomas said, to use scan result is reasonable way (thanks thomas).
Because the sequence scan -> find peer -> connect -> timeout -> scan -> ... is
similar to infrastructure BSS.
If NEW_PEER_CANDIDATE based connection trigger can not avoid this issue,
I will consider scan based.


2014-11-06 19:29 GMT+09:00 Nishikawa, Kenzoh <Kenzoh.Nishikawa@jp.sony.com>:
>> 2014-11-05 22:09 GMT+09:00 Bob Copeland <me at bobcopeland.com>:
>>> I fixed a similar problem with authsae here:
>>> https://github.com/cozybit/authsae/commit/295164a83717ce59ca280468fc2f7edcea6b3cbf
>>
>> Great!
>>
>> I could fix this problem by adding one line based on your idea.
>> Thank you !
>
> I found the problem for this solution.
>
> NodeA registers NodeB by receiving the beacon and sends open frames.
> But NodeB ignore the open frames because NodeB has not registered NodeA.
> So NodeA goes to Holding state and forget NodeB by the solution.
> Then NodeB starts sending open frames after receiving the NodeA beacon
> and fails since NodeA doesn't remember NodeB anymore.
> And repeats this forever.
>
> Nearly same problem happens when a peer is deleted since mesh_mpm_fsm_restart()
> is called with the reason code "WLAN_REASON_MESH_CLOSE_RCVD".
>
Bob Copeland Nov. 7, 2014, 12:21 p.m. UTC | #2 
On Thu, Nov 06, 2014 at 10:29:51AM +0000, Nishikawa, Kenzoh wrote:
> > 2014-11-05 22:09 GMT+09:00 Bob Copeland <me at bobcopeland.com>:
> >> I fixed a similar problem with authsae here:
> >> https://github.com/cozybit/authsae/commit/295164a83717ce59ca280468fc2f7edcea6b3cbf
> >
> > Great!
> >
> > I could fix this problem by adding one line based on your idea.
> > Thank you !
> 
> I found the problem for this solution.
> 
> NodeA registers NodeB by receiving the beacon and sends open frames.
> But NodeB ignore the open frames because NodeB has not registered NodeA.
> So NodeA goes to Holding state and forget NodeB by the solution.
> Then NodeB starts sending open frames after receiving the NodeA beacon
> and fails since NodeA doesn't remember NodeB anymore.
> And repeats this forever.
> 
> Nearly same problem happens when a peer is deleted since mesh_mpm_fsm_restart()
> is called with the reason code "WLAN_REASON_MESH_CLOSE_RCVD".

Ah, too bad.  I guess authsae must suffer from this same problem then
if SAE succeeds but plink establishment fails, as it doesn't delete
peers inside the ampe.c state machine.  Hmm.
Nishikawa, Kenzoh Nov. 9, 2014, 6:36 a.m. UTC | #3 
> Ah, too bad.  I guess authsae must suffer from this same problem then if SAE succeeds but plink establishment > fails, as it doesn't delete peers inside the ampe.c state machine.  Hmm.

Using the existence of a sta_info as a flag of peering success is the root cause of this problem.

Peering should be managed by the state machine independently.
A sta_info should be related to the existence of the node, the beacon of the node independently.
So the existence of a sta_info should not be related to the peering success.

I think the original patch and the patch to be committed to the open80211s can solve the problem.
Bob Copeland Nov. 11, 2014, 1:26 p.m. UTC | #4 
On Sun, Nov 09, 2014 at 06:36:12AM +0000, Nishikawa, Kenzoh wrote:
> Using the existence of a sta_info as a flag of peering success is the
> root cause of this problem.
> 
> Peering should be managed by the state machine independently.
> A sta_info should be related to the existence of the node, the beacon
> of the node independently.
> So the existence of a sta_info should not be related to the peering success.
>
> I think the original patch and the patch to be committed to the
> open80211s can solve the problem.

It does feel kind of kludgy to have to delete the station when we
know it's there just to make the MPM happy, so I suppose I'm not
opposed to the proposed kernel patch, especially as it does at least
filter on user_mpm and plink state, giving userspace the option to
not care.  We'll need to check how authsae handles new peer candidate
events if it already knows about the station or if auth is in progress.

In any case, I guess the kernel patch needs to be submitted upstream
for discussion.

The submitted mesh patch needs to be cleaned up a bit to remove the
unrelated changes, though.
diff mbox

Patch

diff --git a/wpa_supplicant/mesh_mpm.c b/wpa_supplicant/mesh_mpm.c
index 4f86578..c58eaf7 100644
--- a/wpa_supplicant/mesh_mpm.c
+++ b/wpa_supplicant/mesh_mpm.c
@@ -420,6 +420,9 @@  static void plink_timer(void *eloop_ctx, void *user_data)
        case PLINK_HOLDING:
                /* holding timer */
                mesh_mpm_fsm_restart(wpa_s, sta);
+               if (wpa_drv_sta_remove(wpa_s, sta->addr) < 0)
+                       wpa_printf(MSG_ERROR, "Failed to remove STA " MACSTR,
+                                  MAC2STR(sta->addr));
                break;
        default:
                break;
diff --git a/wpa_supplicant/mesh_rsn.c b/wpa_supplicant/mesh_rsn.c
index 94dbfd1..06e3e07 100644
--- a/wpa_supplicant/mesh_rsn.c
+++ b/wpa_supplicant/mesh_rsn.c
@@ -37,6 +37,9 @@  void mesh_auth_timer(void *eloop_ctx, void *user_data)
                           " (attempt %d) ",
                           MAC2STR(sta->addr), sta->sae_auth_retry);
                if (sta->sae_auth_retry < MESH_AUTH_RETRY) {
+                       if (wpa_drv_sta_remove(wpa_s, sta->addr) < 0)
+                               wpa_printf(MSG_ERROR, "AUTH: Failed to remove "
+                                          "STA " MACSTR, MAC2STR(sta->addr));
                        mesh_rsn_auth_sae_sta(wpa_s, sta);
                } else {
                        /* block the STA if exceeded the number of attempts */