2010-09-11 01:23:29

by Steve deRosier

[permalink] [raw]
Subject: mesh beacons broken on HEAD for libertas_tf

I found an issue with recent changes to mac80211 that break mesh
beacons at least for libertas_tf.

I say "recent" in that the exact same libertas_tf code works fine in a
2.6.34-rc5-wl kernel but not in head.

ieee80211_do_open() calls ieee80211_start_mesh() which calls
ieee80211_bss_info_change_notify() to turn on beaconing.
ieee80211_bss_info_change_notify() checks that state is
SDATA_STATE_RUNNING before it enables beacons.

Problem is, SDATA_STATE_RUNNING doesn't get set until right before
ieee80211_do_open() exits, long after ieee80211_start_mesh() gets
called and tries to enable beaconing.

The SDATA_STATE_RUNNING stuff was added in commit 34d4bc4d about two weeks ago.

The basic symptom is that the commands go down to the firmware to
setup the beaconing, but instead of beaconing being enabled, it's
disabled in the command call.

I can set the bit right before the start mesh call and all works as
expected. But, I'm sure this isn't the correct change.

I see four general strategies:
1. Move the setting of SDATA_STATE_RUNNING to earlier in the function.
This is obviously problematic, having to unset it on errors or so on.
2. Remove the check in ieee80211_bss_info_change_notify(). Of course,
this was probably put here for a reason.
3. Put an exception to the check in ieee80211_bss_info_change_notify()
so that if vif->type is mesh, that we don't check the
SDATA_STATE_RUNNING bit. Fairly straight forward and I know how to do
that.
4. Somehow delay the mesh setup from setting up beacons till after the
ieee80211_do_open() call finishes. I'm not sure how to accomplish
this in a sensible way.

Does anyone have any suggestions?

Thanks,
- Steve


2010-09-14 13:13:53

by Johannes Berg

[permalink] [raw]
Subject: Re: mesh beacons broken on HEAD for libertas_tf

On Fri, 2010-09-10 at 18:23 -0700, Steve deRosier wrote:

> 1. Move the setting of SDATA_STATE_RUNNING to earlier in the function.
> This is obviously problematic, having to unset it on errors or so on.

Yeah, that's why I did it this way mostly.

> 3. Put an exception to the check in ieee80211_bss_info_change_notify()
> so that if vif->type is mesh, that we don't check the
> SDATA_STATE_RUNNING bit. Fairly straight forward and I know how to do
> that.

I see you posted a patch for this. I don't think it's a great change,
and I think you may also need to check netif_running() which is true
earlier in the function, but not while the interface is really down.

> 4. Somehow delay the mesh setup from setting up beacons till after the
> ieee80211_do_open() call finishes. I'm not sure how to accomplish
> this in a sensible way.

That would be best, really. The main issue is that the way mesh is
configured is really sad -- you set parameters before bringing up the
interface and then they take effect. It'd be _much_ nicer if it behaved
like IBSS in cfg80211, so that you can set parameters to "join" a mesh
and then you could "leave" a mesh again without setting the interface
down, etc. Some backward compat code might be needed in cfg80211 so if
you set an interface up while it's already configured, these things take
effect already ...

johannes