[PATCH] mac80211: mesh STAs only process mesh beacons

[PATCH] mac80211: mesh STAs only process mesh beacons

[Thomas Pedersen]

Before, a mesh STA would execute some code on behalf of AP or IBSS
beacons. Since the mesh stack currently does not consider anything but
other mesh STAs interesting, limit processing to just these and save a
little overhead.

Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
---
 net/mac80211/mesh.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index c1e6a5c..41c2d26 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -681,8 +681,12 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
 	ieee802_11_parse_elems(mgmt->u.probe_resp.variable, len - baselen,
 			       &elems);
 
-	/* ignore beacons from secure mesh peers if our security is off */
-	if (elems.rsn_len && sdata->u.mesh.security == IEEE80211_MESH_SEC_NONE)
+	/* ignore non-mesh or secure / unsecure mismatch */
+	if ((!elems.mesh_id || !elems.mesh_config) ||
+	    (elems.rsn &&
+	     sdata->u.mesh.security == IEEE80211_MESH_SEC_NONE) ||
+	    (!elems.rsn &&
+	     sdata->u.mesh.security != IEEE80211_MESH_SEC_NONE))
 		return;
 
 	if (elems.ds_params && elems.ds_params_len == 1)
@@ -695,8 +699,7 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
 	if (!channel || channel->flags & IEEE80211_CHAN_DISABLED)
 		return;
 
-	if (elems.mesh_id && elems.mesh_config &&
-	    mesh_matches_local(sdata, &elems))
+	if (mesh_matches_local(sdata, &elems))
 		mesh_neighbour_update(sdata, mgmt->sa, &elems);
 
 	if (ifmsh->sync_ops)
-- 
1.7.9.5

Re: [PATCH] mac80211: mesh STAs only process mesh beacons

[Johannes Berg]

On Sat, 2012-10-20 at 19:03 -0700, Thomas Pedersen wrote:
> Before, a mesh STA would execute some code on behalf of AP or IBSS
> beacons. Since the mesh stack currently does not consider anything but
> other mesh STAs interesting, limit processing to just these and save a
> little overhead.

I can apply this, but I think while you're at it the function deserves a
bit more thought:

> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>

> -	if (elems.mesh_id && elems.mesh_config &&
> -	    mesh_matches_local(sdata, &elems))
> +	if (mesh_matches_local(sdata, &elems))
>  		mesh_neighbour_update(sdata, mgmt->sa, &elems);
>  
>  	if (ifmsh->sync_ops)

Why would you want to invoke sync_ops when the mesh is somebody else's
mesh, i.e. doesn't match local?

johannes

Re: [PATCH] mac80211: mesh STAs only process mesh beacons

[Thomas Pedersen]

On Mon, Oct 22, 2012 at 6:32 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Sat, 2012-10-20 at 19:03 -0700, Thomas Pedersen wrote:
>> Before, a mesh STA would execute some code on behalf of AP or IBSS
>> beacons. Since the mesh stack currently does not consider anything but
>> other mesh STAs interesting, limit processing to just these and save a
>> little overhead.
>
> I can apply this, but I think while you're at it the function deserves a
> bit more thought:
>
>> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
>
>> -     if (elems.mesh_id && elems.mesh_config &&
>> -         mesh_matches_local(sdata, &elems))
>> +     if (mesh_matches_local(sdata, &elems))
>>               mesh_neighbour_update(sdata, mgmt->sa, &elems);
>>
>>       if (ifmsh->sync_ops)
>
> Why would you want to invoke sync_ops when the mesh is somebody else's
> mesh, i.e. doesn't match local?

Right now we only allocate a sta_info if mesh_matches_local, so
sync_ops should simply fail its sta_info lookup and return.

In the future we might want to allocate station entries for non-peers
as well since it is useful to track the Toffset of non-peer neighbors
for beacon collision avoidance and scheduling MCCAOP interference
periods. This is also required by the standard FWIW.

Thomas

Re: [PATCH] mac80211: mesh STAs only process mesh beacons

[Johannes Berg]

On Tue, 2012-10-23 at 10:49 -0700, Thomas Pedersen wrote:
> On Mon, Oct 22, 2012 at 6:32 AM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Sat, 2012-10-20 at 19:03 -0700, Thomas Pedersen wrote:
> >> Before, a mesh STA would execute some code on behalf of AP or IBSS
> >> beacons. Since the mesh stack currently does not consider anything but
> >> other mesh STAs interesting, limit processing to just these and save a
> >> little overhead.
> >
> > I can apply this, but I think while you're at it the function deserves a
> > bit more thought:
> >
> >> Signed-off-by: Thomas Pedersen <thomas@cozybit.com>
> >
> >> -     if (elems.mesh_id && elems.mesh_config &&
> >> -         mesh_matches_local(sdata, &elems))
> >> +     if (mesh_matches_local(sdata, &elems))
> >>               mesh_neighbour_update(sdata, mgmt->sa, &elems);
> >>
> >>       if (ifmsh->sync_ops)
> >
> > Why would you want to invoke sync_ops when the mesh is somebody else's
> > mesh, i.e. doesn't match local?
> 
> Right now we only allocate a sta_info if mesh_matches_local, so
> sync_ops should simply fail its sta_info lookup and return.
> 
> In the future we might want to allocate station entries for non-peers
> as well since it is useful to track the Toffset of non-peer neighbors
> for beacon collision avoidance and scheduling MCCAOP interference
> periods. This is also required by the standard FWIW.

Ok, fair enough. FWIW, Antonio is also working on tracking non-peer
stations (not as station entries, but separately) so maybe you should
collaborate.

Applied your patch.

johannes