2014-06-28 20:54:17

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: mesh_plink: handle confirm frames with new plid

[oops, +linux-wireless]
On Sat, Jun 28, 2014 at 04:35:25PM -0400, Bob Copeland wrote:
> The 802.11 standard says when processing a plink confirm
> frame:
>
> "If the peerLinkID in the mesh peering instance has not been
> set, the Local Link ID field of the Mesh Peering Confirm
> request shall be copied into the peerLinkID in the mesh
> peering instance."
>
> We were only doing this when receiving an open peering frame,
> but it could happen that the open frame gets lost and so we
> should handle this case rather than rejecting the confirm and
> failing the whole peering process.
>
> Reported-by: Yu Niiro <[email protected]>
> Signed-off-by: Bob Copeland <[email protected]>
> ---
> net/mac80211/mesh_plink.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
> index 63b8741..c47194d 100644
> --- a/net/mac80211/mesh_plink.c
> +++ b/net/mac80211/mesh_plink.c
> @@ -959,7 +959,8 @@ mesh_plink_get_event(struct ieee80211_sub_if_data *sdata,
> if (!matches_local)
> event = CNF_RJCT;
> if (!mesh_plink_free_count(sdata) ||
> - (sta->llid != llid || sta->plid != plid))
> + sta->llid != llid ||
> + (sta->plid && sta->plid != plid))
> event = CNF_IGNR;
> else
> event = CNF_ACPT;
> @@ -1080,6 +1081,10 @@ mesh_process_plink_frame(struct ieee80211_sub_if_data *sdata,
> goto unlock_rcu;
> }
>
> + /* 802.11-2012 13.3.7.2 - update plid on CNF if not set */
> + if (!sta->plid && event == CNF_ACPT)
> + sta->plid = plid;
> +
> changed |= mesh_plink_fsm(sdata, sta, event);
>
> unlock_rcu:
> --
> 2.0.0.rc2
>

--
Bob Copeland %% http://www.bobcopeland.com


2014-07-03 15:10:38

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: mesh_plink: handle confirm frames with new plid

On Thu, Jul 03, 2014 at 10:28:37AM -0400, Bob Copeland wrote:
> > What is the consequence if we don't handle this case? Is the peer
> > going to do the re-auth again?
> >
> > Regards,
> > Chun-Yeow
>
> It shouldn't be a big problem -- looking at 802.11-2012 figure 13-2:

Actually, thinking about it more, a worse case is if B reaches
ESTAB and then A reboots. Now, A sends an Open, gets back
a Confirm that A will ignore (due to no plid). A will still
timeout and send a Close frame to restart everything, but we
would have to wait quite a while to re-establish the peering.

--
Bob Copeland %% http://www.bobcopeland.com

2014-07-03 12:36:29

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: mesh_plink: handle confirm frames with new plid

Hi, Bob

What is the consequence if we don't handle this case? Is the peer
going to do the re-auth again?

Regards,
Chun-Yeow

On Sun, Jun 29, 2014 at 4:54 AM, Bob Copeland <[email protected]> wrote:
> [oops, +linux-wireless]
> On Sat, Jun 28, 2014 at 04:35:25PM -0400, Bob Copeland wrote:
>> The 802.11 standard says when processing a plink confirm
>> frame:
>>
>> "If the peerLinkID in the mesh peering instance has not been
>> set, the Local Link ID field of the Mesh Peering Confirm
>> request shall be copied into the peerLinkID in the mesh
>> peering instance."
>>
>> We were only doing this when receiving an open peering frame,
>> but it could happen that the open frame gets lost and so we
>> should handle this case rather than rejecting the confirm and
>> failing the whole peering process.
>>
>> Reported-by: Yu Niiro <[email protected]>
>> Signed-off-by: Bob Copeland <[email protected]>
>> ---
>> net/mac80211/mesh_plink.c | 7 ++++++-
>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
>> index 63b8741..c47194d 100644
>> --- a/net/mac80211/mesh_plink.c
>> +++ b/net/mac80211/mesh_plink.c
>> @@ -959,7 +959,8 @@ mesh_plink_get_event(struct ieee80211_sub_if_data *sdata,
>> if (!matches_local)
>> event = CNF_RJCT;
>> if (!mesh_plink_free_count(sdata) ||
>> - (sta->llid != llid || sta->plid != plid))
>> + sta->llid != llid ||
>> + (sta->plid && sta->plid != plid))
>> event = CNF_IGNR;
>> else
>> event = CNF_ACPT;
>> @@ -1080,6 +1081,10 @@ mesh_process_plink_frame(struct ieee80211_sub_if_data *sdata,
>> goto unlock_rcu;
>> }
>>
>> + /* 802.11-2012 13.3.7.2 - update plid on CNF if not set */
>> + if (!sta->plid && event == CNF_ACPT)
>> + sta->plid = plid;
>> +
>> changed |= mesh_plink_fsm(sdata, sta, event);
>>
>> unlock_rcu:
>> --
>> 2.0.0.rc2
>>
>
> --
> Bob Copeland %% http://www.bobcopeland.com
> _______________________________________________
> Devel mailing list
> [email protected]
> http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel

2014-07-24 10:58:49

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: mesh_plink: handle confirm frames with new plid

On Sat, 2014-06-28 at 16:54 -0400, Bob Copeland wrote:
> [oops, +linux-wireless]
> On Sat, Jun 28, 2014 at 04:35:25PM -0400, Bob Copeland wrote:
> > The 802.11 standard says when processing a plink confirm
> > frame:
> >
> > "If the peerLinkID in the mesh peering instance has not been
> > set, the Local Link ID field of the Mesh Peering Confirm
> > request shall be copied into the peerLinkID in the mesh
> > peering instance."
> >
> > We were only doing this when receiving an open peering frame,
> > but it could happen that the open frame gets lost and so we
> > should handle this case rather than rejecting the confirm and
> > failing the whole peering process.

Applied.

johannes


2014-07-04 03:12:56

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: mesh_plink: handle confirm frames with new plid

Hi, Bob

Thanks for your explanation.

I am also thinking whether it is easy to replicate the mentioned
problem. It seems that the re-auth is working fine.

Regards,
Chun-Yeow

On Thu, Jul 3, 2014 at 11:10 PM, Bob Copeland <[email protected]> wrote:
> On Thu, Jul 03, 2014 at 10:28:37AM -0400, Bob Copeland wrote:
>> > What is the consequence if we don't handle this case? Is the peer
>> > going to do the re-auth again?
>> >
>> > Regards,
>> > Chun-Yeow
>>
>> It shouldn't be a big problem -- looking at 802.11-2012 figure 13-2:
>
> Actually, thinking about it more, a worse case is if B reaches
> ESTAB and then A reboots. Now, A sends an Open, gets back
> a Confirm that A will ignore (due to no plid). A will still
> timeout and send a Close frame to restart everything, but we
> would have to wait quite a while to re-establish the peering.
>
> --
> Bob Copeland %% http://www.bobcopeland.com

2014-07-03 14:28:41

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH 1/2] mac80211: mesh_plink: handle confirm frames with new plid

[reordered top-posting]

On Thu, Jul 03, 2014 at 08:36:28PM +0800, Chun-Yeow Yeoh wrote:
> >> "If the peerLinkID in the mesh peering instance has not been
> >> set, the Local Link ID field of the Mesh Peering Confirm
> >> request shall be copied into the peerLinkID in the mesh
> >> peering instance."
> >>
> Hi, Bob
>
> What is the consequence if we don't handle this case? Is the peer
> going to do the re-auth again?
>
> Regards,
> Chun-Yeow

It shouldn't be a big problem -- looking at 802.11-2012 figure 13-2:

Let's say station A is in OPN_SNT and station B is in OPN_RCVD,
but station A failed to get the Open frame from B.

When A gets a Confirm frame from B, it would ignore it (due to
missing plid), then it would resend Open on dot11MeshRetryTimeout.
Unfortunately, Confirm responses from B to that Open frame would
be ignored too.

However, B should also retry Open on dot11MeshRetryTimeout.
If any are successful, A moves to OPN_RCVD, then both have plids
and everything should be ok from then on.

In the worst case, plink timer on either station fires
dot11MeshMaxRetries times and peering instance closes. Then peering
can start over after leaving holding state and either station gets a
beacon.

So in sum, it just adds a bit of resiliency and means we don't have
to wait for a dot11MeshRetryTimeout in the case of one lost open
frame.

--
Bob Copeland %% http://www.bobcopeland.com