[PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Alina Friedrichsen]

[-- Attachment #1: Type: text/plain, Size: 1263 bytes --]

It's completely unnecessary to merge with same BSSID. Because here in our city networks we have many of the same BSSID (and SSID) with deferent timestamps, this causes problems.

See the bug report here:
http://wiki.villagetelco.org/index.php/Information_about_cell-id_splitting%2C_stuck_beacons%2C_and_failed_IBSS_merges!#The_phenomenon_of_IBSS-ID_cell_splits

Signed-off-by: Alina Friedrichsen <x-alina@gmx.net>

diff -urN compat-wireless-2009-01-05.orig/net/mac80211/mlme.c compat-wireless-2009-01-05.work/net/mac80211/mlme.c
--- compat-wireless-2009-01-05.orig/net/mac80211/mlme.c	2009-01-06 02:01:46.000000000 +0100
+++ compat-wireless-2009-01-05/net/mac80211/mlme.c	2009-01-06 02:05:22.000000000 +0100
@@ -1705,7 +1705,8 @@
 		       (unsigned long long)(rx_timestamp - beacon_timestamp),
 		       jiffies);
 #endif /* CONFIG_MAC80211_IBSS_DEBUG */
-		if (beacon_timestamp > rx_timestamp) {
+		if (beacon_timestamp > rx_timestamp &&
+		    memcmp(sdata->u.sta.bssid, mgmt->bssid, ETH_ALEN) != 0) {
 #ifdef CONFIG_MAC80211_IBSS_DEBUG
 			printk(KERN_DEBUG "%s: beacon TSF higher than "
 			       "local TSF - IBSS merge with BSSID %s\n",

-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

[-- Attachment #2: step3-dont-merge-with-the-same-bssid.patch --]
[-- Type: text/x-patch, Size: 730 bytes --]

diff -urN compat-wireless-2009-01-05.orig/net/mac80211/mlme.c compat-wireless-2009-01-05.work/net/mac80211/mlme.c
--- compat-wireless-2009-01-05.orig/net/mac80211/mlme.c	2009-01-06 02:01:46.000000000 +0100
+++ compat-wireless-2009-01-05/net/mac80211/mlme.c	2009-01-06 02:05:22.000000000 +0100
@@ -1705,7 +1705,8 @@
 		       (unsigned long long)(rx_timestamp - beacon_timestamp),
 		       jiffies);
 #endif /* CONFIG_MAC80211_IBSS_DEBUG */
-		if (beacon_timestamp > rx_timestamp) {
+		if (beacon_timestamp > rx_timestamp &&
+		    memcmp(sdata->u.sta.bssid, mgmt->bssid, ETH_ALEN) != 0) {
 #ifdef CONFIG_MAC80211_IBSS_DEBUG
 			printk(KERN_DEBUG "%s: beacon TSF higher than "
 			       "local TSF - IBSS merge with BSSID %s\n",

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Jouni Malinen]

On Tue, Jan 06, 2009 at 03:01:30AM +0100, Alina Friedrichsen wrote:

> It's completely unnecessary to merge with same BSSID. Because here in our city networks we have many of the same BSSID (and SSID) with deferent timestamps, this causes problems.

I agree that merge part (updating Beacon/ProbeRsp data) can be skipped
for same BSSID, but this change seems to break timesync by removing call
to local->ops->reset_tsf(). I don't think that that should be removed
unconditionally. I would like to know whether that operation is causing
any problems in citywide networks and if yes, it might be better to
provide a configuration option to allow the timesync for IBSS to be
disabled since not doing that could cause problems to power saving in
IBSS (not that it is really implemented that widely).

In other words, I would be fine if the calls to
ieee80211_sta_join_ibss() and ieee80211_ibss_add_sta() are replaced with
local->ops->reset_tsf() if BSSIDs are the same when
beacon_timestamp>rx_timestamp.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Alina Friedrichsen]

Hi Jouni!

> I agree that merge part (updating Beacon/ProbeRsp data) can be skippe=
d
> for same BSSID, but this change seems to break timesync by removing c=
all
> to local->ops->reset_tsf(). I don't think that that should be removed
> unconditionally.

Thanks for your comment.

> I would like to know whether that operation is causing
> any problems in citywide networks and if yes, it might be better to
> provide a configuration option to allow the timesync for IBSS to be
> disabled since not doing that could cause problems to power saving in
> IBSS (not that it is really implemented that widely).

As I see, only the sta_info_flush_delayed() call causes the problems. I=
 can only disable this call in the ieee80211_sta_join_ibss() function, =
if the BSSID is the same. Do you think this would be better?

> In other words, I would be fine if the calls to
> ieee80211_sta_join_ibss() and ieee80211_ibss_add_sta() are replaced w=
ith
> local->ops->reset_tsf() if BSSIDs are the same when
> beacon_timestamp>rx_timestamp.

This would be a way, too.

Regards
Alina

--=20
Sensationsangebot verl=E4ngert: GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K1308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Jouni Malinen]

On Fri, Jan 09, 2009 at 09:53:15PM +0100, Alina Friedrichsen wrote:

> As I see, only the sta_info_flush_delayed() call causes the problems. I can only disable this call in the ieee80211_sta_join_ibss() function, if the BSSID is the same. Do you think this would be better?

Yes, I think it would be better option than removing it and the reset_tsf()
call. Removing STA entries for other IBSS members in any merge case,
even if BSSID changes, does not sound correct in the first place. This
should really only be done if the SSID changes.

> > In other words, I would be fine if the calls to
> > ieee80211_sta_join_ibss() and ieee80211_ibss_add_sta() are replaced with
> > local->ops->reset_tsf() if BSSIDs are the same when
> > beacon_timestamp>rx_timestamp.
> 
> This would be a way, too.

Looking at ieee80211_sta_join_ibss(), most of the stuff there should not
really be needed if BSSID remains the same and we are only syncing our
TSF. reset_tsf() call should be enough for that. Other parts of this
function, apart from sta_info_flush_delayed(), should be run if BSSID
changes. sta_info_flush_delayed() should be included only if SSID
changes (or maybe if we join back to the same IBSS after a long time,
but I'm not too concerned about that corner case).

I think bit more cleanup here by splitting ieee80211_sta_join_ibss()
into pieces and only calling some of them in the merge/timesync case
would be the best solution. For the particular problem that you are
seeing, the minimal change would probably be just to make
sta_info_flush_delayed() call conditional on SSID change (I don't think
this would need to be based on BSSID change).
 
-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Alina Friedrichsen]

Hello Jouni!

> Yes, I think it would be better option than removing it and the
> reset_tsf()
> call. Removing STA entries for other IBSS members in any merge case,
> even if BSSID changes, does not sound correct in the first place. Thi=
s
> should really only be done if the SSID changes.
>=20
> [...]
>=20
> For the particular problem that you are
> seeing, the minimal change would probably be just to make
> sta_info_flush_delayed() call conditional on SSID change (I don't thi=
nk
> this would need to be based on BSSID change).

If the others agree, I can implement it in the end of the week. Or do y=
ou want it the other way?
I personally don't want change too much...?

Regards
Alina
--=20
Sensationsangebot verl=E4ngert: GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K1308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Jouni Malinen]

On Mon, Jan 12, 2009 at 05:15:34AM +0100, Alina Friedrichsen wrote:
> > For the particular problem that you are
> > seeing, the minimal change would probably be just to make
> > sta_info_flush_delayed() call conditional on SSID change (I don't think
> > this would need to be based on BSSID change).
> 
> If the others agree, I can implement it in the end of the week. Or do you want it the other way?
> I personally don't want change too much...?

Of course I would like to have the full cleanup in the long run, but I
have nothing against first doing the minimal change to fix the
identified issue.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] Fixed BSSID (timesync)

[Alina Friedrichsen]

Hi,

I'm looking now for a better implementation for the IBSS fixed BSSID st=
uff, that handles the TSF sync correctly. I think now having centralize=
d IBSS join function ist more easy to maintain, as many little function=
s. This function can look in the environment for the current state, so =
that it can see what to do for the current join.

So I have moved all my fixed BSSID stuff in this centralized function, =
so that the TSF will be reseted, even if a fixed BSSID is set.

I will allow a channel change too, if you have a fixed BSSID but not a =
fixed channel setting. If there is a other IBSS network with the same B=
SSID but in an other channel.
Like now a BSSID change is allowed, if you set a fixed channel but not =
a fixed BSSID.
I think this would be a more clean and consistent implementation.

What do you think about it?
I will post a beta patch tomorrow.

Regards
Alina

--=20
Sensationsangebot verl=E4ngert: GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K1308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fixed BSSID (timesync)

[Alina Friedrichsen]

Hi,

I'm looking now for a better implementation for the IBSS fixed BSSID st=
uff, that handles the TSF sync correctly. I think now having centralize=
d IBSS join function ist more easy to maintain, as many little function=
s. This function can look in the environment for the current state, so =
that it can see what to do for the current join.

So I have moved all my fixed BSSID stuff in this centralized function, =
so that the TSF will be reseted, even if a fixed BSSID is set.

I will allow a channel change too, if you have a fixed BSSID but not a =
fixed channel setting. If there is a other IBSS network with the same B=
SSID but in an other channel.
Like now a BSSID change is allowed, if you set a fixed channel but not =
a fixed BSSID.
I think this would be a more clean and consistent implementation.

What do you think about it?
I will post a beta patch tomorrow.

Regards
Alina

--=20
Sensationsangebot verl=E4ngert: GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K1308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Alina Friedrichsen]

Hello Jouni!

> I agree that merge part (updating Beacon/ProbeRsp data) can be skippe=
d
> for same BSSID, but this change seems to break timesync by removing c=
all
> to local->ops->reset_tsf(). I don't think that that should be removed
> unconditionally.

I have looked in the Source of ath5k and ath9k. It seens to me that the=
 TSF sync is done in the low-level driver and/or the hardware directly.=
 So I see no reason to set it back to zero, if you have a dummy merge t=
o the same BSSID and channel. (E.g. caused by TSF overflows.) I think i=
t should only done if you change the network, so BSSID and/or channel i=
s deferent.

Any doubts?

Regards
Alina

--=20
Sensationsangebot verl=E4ngert: GMX FreeDSL - Telefonanschluss + DSL=20
f=FCr nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=3DOM.AD.PD003K1308T45=
69a
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Jouni Malinen]

On Sat, Jan 17, 2009 at 11:12:25PM +0100, Alina Friedrichsen wrote:
> > I agree that merge part (updating Beacon/ProbeRsp data) can be skipped
> > for same BSSID, but this change seems to break timesync by removing call
> > to local->ops->reset_tsf(). I don't think that that should be removed
> > unconditionally.

> I have looked in the Source of ath5k and ath9k. It seens to me that the TSF sync is done in the low-level driver and/or the hardware directly. So I see no reason to set it back to zero, if you have a dummy merge to the same BSSID and channel. (E.g. caused by TSF overflows.) I think it should only done if you change the network, so BSSID and/or channel is deferent.

Some hardware designs require the reset_tsf call to allow the hardware
to get back in sync with the TSF based on received timestamp values in
some cases. I would expect this to be used in ath9k and ath5k. This does
not mean that the timestamp would be reset to zero in actual Beacon
frames, i.e., this is just something done internally in the
driver/firmware/hardware and the need for reset_tsf() is to just provide
the notification to the driver when this special case is needed.

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Alina Friedrichsen]

Hello Jouni!

> Some hardware designs require the reset_tsf call to allow the hardwar=
e
> to get back in sync with the TSF based on received timestamp values i=
n
> some cases. I would expect this to be used in ath9k and ath5k. This d=
oes
> not mean that the timestamp would be reset to zero in actual Beacon
> frames, i.e., this is just something done internally in the
> driver/firmware/hardware and the need for reset_tsf() is to just prov=
ide
> the notification to the driver when this special case is needed.

Okay, thanks for your notice. Than I think the dummy merge detection co=
de should in the ieee80211_sta_join_ibss() function and if so only call=
 reset_tsf()?!
Should it make any other thinks too in this case?

Regards
Alina

--=20
Psssst! Schon vom neuen GMX MultiMessenger geh=F6rt? Der kann`s mit all=
en: http://www.gmx.net/de/go/multimessenger
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Fixed BSSID step 3: Don't merge with the same BSSID

[Jouni Malinen]

On Tue, Jan 20, 2009 at 07:56:35PM +0100, Alina Friedrichsen wrote:

> Okay, thanks for your notice. Than I think the dummy merge detection code should in the ieee80211_sta_join_ibss() function and if so only call reset_tsf()?!
> Should it make any other thinks too in this case?

That is probably enough, but I have to admit I have not gone through all
the details here. It certainly should not need to remove existing STA
entries or reconfigure beacon data etc. that may have been there in the
past.

-- 
Jouni Malinen                                            PGP id EFC895FA