Hi,
this patch corrects some bugs of the mac80211 wireless driver framework related to setting a static BSSID in the ad-hoc mode (e.g. for mesh networks like OLSR).
This patch should correct the bug which was reported by Elektra in that wiki, too. Elektra, can you please test it?
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
Regards
Alina
--
Sensationsangebot verl?ngert: GMX FreeDSL - Telefonanschluss + DSL
f?r nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=OM.AD.PD003K1308T4569a
Hi Alina,
> > please use a bool for that
>
> Yes, but the called function is written in the old C style before C99, so I think I should use the same type.
I disagree, even if the function doesn't use the proper types
pervasively, we shouldn't be adding old-style stuff.
What Jouni pointed out still stands, you should separate the bugfix and
the 'disregard ibss join' feature into two patches.
johannes
Hi Johannes!
> I disagree, even if the function doesn't use the proper types
> pervasively, we shouldn't be adding old-style stuff.
Okay, I can use bool to if you want, too.
> What Jouni pointed out still stands, you should separate the bugfix and
> the 'disregard ibss join' feature into two patches.
As I say, the "the 'disregard ibss join' feature" is already implemented only not complete. If you want a driver, that do in some situations merges and some others don't, you can easily remove the line 1663. But I think it would nice, if the main driver implement it properly, not half.
For now we can only use the madwifi and an awful hacked proprietary 2.4er kernel broadcom driver for it. It would nice to have a clean free software implementation for it.
Regards
Alina
--
Psssst! Schon vom neuen GMX MultiMessenger geh?rt? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger
Hi Alina,
> #endif /* CONFIG_MAC80211_IBSS_DEBUG */
> if (beacon_timestamp > rx_timestamp) {
> + if (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",
> - sdata->dev->name, print_mac(mac, mgmt->bssid));
> + printk(KERN_DEBUG "%s: beacon TSF higher than "
> + "local TSF - IBSS merge with BSSID %s\n",
> + sdata->dev->name, print_mac(mac, mgmt->bssid));
> #endif
> - ieee80211_sta_join_ibss(sdata, &sdata->u.sta, bss);
> - ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
> + ieee80211_sta_join_ibss(sdata, &sdata->u.sta, bss);
> + ieee80211_ibss_add_sta(sdata, mgmt->bssid, mgmt->sa, supp_rates);
> + }
> }
Can you move the memcmp() into the other condition to avoid indenting
twice please? Also, it seems there needs to be a STA_BSSID_SET exclusion
somewhere above to avoid doing merges?
> @@ -2576,11 +2582,14 @@
> {
> struct ieee80211_if_sta *ifsta;
> int res;
> + int valid;
please use a bool for that
> ifsta = &sdata->u.sta;
> + valid = is_valid_ether_addr(bssid);
>
> if (memcmp(ifsta->bssid, bssid, ETH_ALEN) != 0) {
> - memcpy(ifsta->bssid, bssid, ETH_ALEN);
> + if(valid) memcpy(ifsta->bssid, bssid, ETH_ALEN);
> + else memset(ifsta->bssid, 0, ETH_ALEN);
> res = 0;
and indent these properly
I won't comment on the actual code right now, the IBSS code is pretty
much unknown to me and I don't use it.
johannes
On Tue, Dec 30, 2008 at 05:15:39PM +0100, Alina Friedrichsen wrote:
> This breaks nothing. I think if you set your BSSID manually, you really don't want that the driver changed it back. If you want this behavior, you don't set it.
But it _does_ break interoperability with standard compliant IBSS
implementations if the forced BSSID means that the STA does not merge
with another part of the IBSS when this should happen by the STA
changing its BSSID to match with the other part. If you do not want the
code behave as described for IBSS, use something else than IBSS..
> Handle BSSID splits in a mile width city mesh network with 500 or more nodes is impossible in a other way, I think. The standard was never written for that dimensions.
Agreed, but we should not claim that the behavior with hardcoded BSSID
would be standard IBSS. It is something different and we should not
confuse users by calling it the same. I would have nothing against
this type of optional feature if it were called something else than IBSS
(or "adhoc" which iwconfig uses for IBSS).
> Besides it fixes a bug that driver want to try to merge with the same BSSID, which is unnecessary and causes problems.
If that fix is separate from the change that introduces hardcoding of
the BSSID, it would be better to bring that in as a separate patch.
--
Jouni Malinen PGP id EFC895FA
Hi Johannes,
thanks for your comments. I'm not so familiar with your coding style for now. Here is the revised patch.
> Can you move the memcmp() into the other condition to avoid indenting
> twice please?
Done.
> Also, it seems there needs to be a STA_BSSID_SET exclusion
> somewhere above to avoid doing merges?
Yes this function is even called if you set a fixed BSSID, where you don't want any merges. Done in line 1663.
> please use a bool for that
Yes, but the called function is written in the old C style before C99, so I think I should use the same type.
> static inline int is_valid_ether_addr(const u8 *addr)
> and indent these properly
Done.
> I won't comment on the actual code right now, the IBSS code is pretty
> much unknown to me and I don't use it.
You only need it for mesh networks.
Regards
Alina
--
Sensationsangebot verl?ngert: GMX FreeDSL - Telefonanschluss + DSL
f?r nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=OM.AD.PD003K1308T4569a
Hi Jouni!
> But it _does_ break interoperability with standard compliant IBSS
> implementations if the forced BSSID means that the STA does not merge
> with another part of the IBSS when this should happen by the STA
> changing its BSSID to match with the other part.
But in the practice this is not a bug, it's an optional feature and you=
can mix nodes with fixed BSSID and nodes without, so that the backbone=
nodes (routers) force the rather client nodes (e.g. notebooks) to use =
this BSSID, so that the mesh network don't break into many pieces. That=
was happen many times before we use a fixed BSSID.
You don't need to use it, if you don't want it, but we must use it, or =
it will not work.
> Agreed, but we should not claim that the behavior with hardcoded BSSI=
D
> would be standard IBSS.
It's a little modified IBSS, that allow you to build bigger networks. N=
ot a huge change, that is completely incompatible with the standard.
> It is something different and we should not
> confuse users by calling it the same. I would have nothing against
> this type of optional feature if it were called something else than I=
BSS
> (or "adhoc" which iwconfig uses for IBSS).
I can call it Moonchild [0], if you want. ;)
I don't care on the name of it.
> If that fix is separate from the change that introduces hardcoding of
> the BSSID, it would be better to bring that in as a separate patch.
I don't introduce setting of a fixed BSSID with this patch, I only fix =
some bugs of the current implementation.
Regards
Alina
[0] The Neverending Story
--=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
On Tue, Dec 30, 2008 at 03:56:51AM +0100, Alina Friedrichsen wrote:
> this patch corrects some bugs of the mac80211 wireless driver framework related to setting a static BSSID in the ad-hoc mode (e.g. for mesh networks like OLSR).
This is not how IEEE 802.11 IBSS is supposed to work. If there are
issues in IBSS splits, merges, or timesync, I would consider fixes to
those to be preferred over hacks that hide the issues and cause
interoperability issues with standard compliant IBSS implementations.
Anyway, since I don't really care that much about IBSS to actually start
using lots of time to fix problems there, I would not object to this
type of change as long it does not change the default behavior (i.e., by
default, mac80211 should operate in standard compliant manner) and the
users are not mislead to enable such a hack while still believing that
their devices are operating in a standard IBSS.
--
Jouni Malinen PGP id EFC895FA
Hello Jouni!
> This is not how IEEE 802.11 IBSS is supposed to work. If there are
> issues in IBSS splits, merges, or timesync, I would consider fixes to
> those to be preferred over hacks that hide the issues and cause
> interoperability issues with standard compliant IBSS implementations.
This breaks nothing. I think if you set your BSSID manually, you really=
don't want that the driver changed it back. If you want this behavior,=
you don't set it.
Handle BSSID splits in a mile width city mesh network with 500 or more =
nodes is impossible in a other way, I think. The standard was never wri=
tten for that dimensions.
Besides it fixes a bug that driver want to try to merge with the same B=
SSID, which is unnecessary and causes problems.
See:
http://wiki.villagetelco.org/index.php/Information_about_cell-id_splitt=
ing%2C_stuck_beacons%2C_and_failed_IBSS_merges!#The_phenomenon_of_IBSS-=
ID_cell_splits
> Anyway, since I don't really care that much about IBSS
We need it much here in the "Freifunk" mesh network of Berlin.
> to actually start
> using lots of time to fix problems there, I would not object to this
> type of change as long it does not change the default behavior (i.e.,=
by
> default, mac80211 should operate in standard compliant manner) and th=
e
> users are not mislead to enable such a hack while still believing tha=
t
> their devices are operating in a standard IBSS.
ACK
Regards
Alina
--=20
Psssst! Schon vom neuen GMX MultiMessenger geh=F6rt? Der kann`s mit all=
en: http://www.gmx.net/de/go/multimessenger
[dropping openwrt list, it ignores non-subscribers anyway]
On Thu, 2009-01-01 at 23:41 +0100, Alina Friedrichsen wrote:
> Hello,
>
> here is a new version of this patch, that fixes a bug that manual
> channel setting in the ad-hoc (IBSS) mode doesn't work properly, too.
>
> It would nice, if it can be committed, before it's out of sync.
You need to provide a proper changelog that explains the changes, a
proper subject (don't mention any drivers) and a signed-off-by, see
http://wireless.kernel.org/en/developers/Documentation/SubmittingPatches
please.
johannes
Hello,
here is a new version of this patch, that fixes a bug that manual channel setting in the ad-hoc (IBSS) mode doesn't work properly, too.
It would nice, if it can be committed, before it's out of sync.
Regards
Alina
--
Psssst! Schon vom neuen GMX MultiMessenger geh?rt? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger