[PATCH 1/2] mac80211: Improved IBSS merges

[PATCH 1/2] mac80211: Improved IBSS merges

[Benoit Papillault]

According to $11.1.4 from IEEE 802.11-2007, TSF is reset (drv_reset_tsf)
on MLME-JOIN, ie only when a new IBSS is created or if an existing IBSS
is joined, but not on IBSS merges.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/ibss.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 423befa..9328395 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -77,9 +77,6 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	u32 bss_change;
 	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
 
-	/* Reset own TSF to allow time synchronization work. */
-	drv_reset_tsf(local);
-
 	skb = ifibss->skb;
 	rcu_assign_pointer(ifibss->presp, NULL);
 	synchronize_rcu();
@@ -579,6 +576,8 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
 	else
 		sdata->drop_unencrypted = 0;
 
+	drv_reset_tsf(local);
+
 	__ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
 				  ifibss->channel, 3, /* first two are basic */
 				  capability, 0);
@@ -634,6 +633,7 @@ static void ieee80211_sta_find_ibss(struct ieee80211_sub_if_data *sdata)
 		       " based on configured SSID\n",
 		       sdata->name, cbss->bssid);
 
+		drv_reset_tsf(local);
 		ieee80211_sta_join_ibss(sdata, bss);
 		ieee80211_rx_bss_put(local, bss);
 		return;
-- 
1.5.6.5

[PATCH 2/2] mac80211: Improved IBSS merges even if BSSID is the same

[Benoit Papillault]

According to IEEE 802.11, we should merge even if the BSSID from the
received beacon is the same as our since merge can update more things
that BSSID itself.

This is specifically true for ath9k where the merge updates TBTT timers
as a side effect and this is indeed needed if the TSF has been updated
(if TSF is updated, we need to update TBTT timers as well).

However, it could cause continuous merges if the RX timestamp or
drv_get_tsf() is not accurate or not implemented.

Signed-off-by: Benoit Papillault <benoit.papillault@free.fr>
---
 net/mac80211/ibss.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 9328395..5f343ed 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -372,10 +372,6 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
 				sdata->u.ibss.ssid_len))
 		goto put_bss;
 
-	/* same BSSID */
-	if (memcmp(cbss->bssid, sdata->u.ibss.bssid, ETH_ALEN) == 0)
-		goto put_bss;
-
 	if (rx_status->flag & RX_FLAG_TSFT) {
 		/*
 		 * For correct IBSS merging we need mactime; since mactime is
-- 
1.5.6.5

Re: [PATCH 2/2] mac80211: Improved IBSS merges even if BSSID is the same

[Johannes Berg]

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

On Mon, 2010-02-15 at 00:33 +0100, Benoit Papillault wrote:
> According to IEEE 802.11, we should merge even if the BSSID from the
> received beacon is the same as our since merge can update more things
> that BSSID itself.
> 
> This is specifically true for ath9k where the merge updates TBTT
> timers
> as a side effect and this is indeed needed if the TSF has been updated
> (if TSF is updated, we need to update TBTT timers as well).
> 
> However, it could cause continuous merges if the RX timestamp or
> drv_get_tsf() is not accurate or not implemented.

It can't actually. Please do your own research on things like that
before posting a patch :)

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 2/2] mac80211: Improved IBSS merges even if BSSID is the same

[Benoit PAPILLAULT]

Johannes Berg a écrit :
> On Mon, 2010-02-15 at 00:33 +0100, Benoit Papillault wrote:
>   
>> According to IEEE 802.11, we should merge even if the BSSID from the
>> received beacon is the same as our since merge can update more things
>> that BSSID itself.
>>
>> This is specifically true for ath9k where the merge updates TBTT
>> timers
>> as a side effect and this is indeed needed if the TSF has been updated
>> (if TSF is updated, we need to update TBTT timers as well).
>>
>> However, it could cause continuous merges if the RX timestamp or
>> drv_get_tsf() is not accurate or not implemented.
>>     
>
> It can't actually. Please do your own research on things like that
> before posting a patch :)
>
> johannes
>   
For Zydas chipset for instance, the TSF is not merged at the hardware 
level (I sent another patch to fix that). As such, if we receive a 
beacon with a higher TSF and did a merge at the software level, then on 
the next beacon, our own TSF has moved by the same delta as the value in 
the received beacon, so the beacon TSF is still higher than our own TSF, 
thus merging again (since we now accept merge with the same BSSID).

For the not implemented case (ie local->ops->get_tsf is null), 
drv_get_tsf() will returns -1 and then no merge could happen. You are 
right. I put part of our discussion into the changelog, mostly to 
acknowledge your toughts without checking myself. My bad!

Regards,
Benoit

Re: [PATCH 2/2] mac80211: Improved IBSS merges even if BSSID is the same

[Johannes Berg]

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

On Mon, 2010-02-15 at 00:33 +0100, Benoit Papillault wrote:
> According to IEEE 802.11, we should merge even if the BSSID from the
> received beacon is the same as our since merge can update more things
> that BSSID itself.
> 
> This is specifically true for ath9k where the merge updates TBTT
> timers
> as a side effect and this is indeed needed if the TSF has been updated
> (if TSF is updated, we need to update TBTT timers as well).
> 
> However, it could cause continuous merges if the RX timestamp or
> drv_get_tsf() is not accurate or not implemented.

The latter (not implemented) case doesn't actually. I don't just want
you to edit the changelog based on my thoughts, I would prefer if you
actually looked at the code to get your changelog.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: [PATCH 1/2] mac80211: Improved IBSS merges

[Jouni Malinen]

On Mon, Feb 15, 2010 at 12:33:54AM +0100, Benoit Papillault wrote:
> According to $11.1.4 from IEEE 802.11-2007, TSF is reset (drv_reset_tsf)
> on MLME-JOIN, ie only when a new IBSS is created or if an existing IBSS
> is joined, but not on IBSS merges.

While that may indeed be what the standard says, drv_reset_tsf() may be
needed to allow the driver to actually learn the new timestamp correctly
in the case of merge. Have you verified that the drivers that support
IBSS do indeed update their timestamps without that call in case there
is a large difference in the timestamp values that the separate IBSSes
used before the merge?

-- 
Jouni Malinen                                            PGP id EFC895FA

Re: [PATCH 1/2] mac80211: Improved IBSS merges

[Johannes Berg]

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

On Mon, 2010-02-15 at 15:40 +0200, Jouni Malinen wrote:
> On Mon, Feb 15, 2010 at 12:33:54AM +0100, Benoit Papillault wrote:
> > According to $11.1.4 from IEEE 802.11-2007, TSF is reset
> (drv_reset_tsf)
> > on MLME-JOIN, ie only when a new IBSS is created or if an existing
> IBSS
> > is joined, but not on IBSS merges.
> 
> While that may indeed be what the standard says, drv_reset_tsf() may
> be
> needed to allow the driver to actually learn the new timestamp
> correctly
> in the case of merge. Have you verified that the drivers that support
> IBSS do indeed update their timestamps without that call in case there
> is a large difference in the timestamp values that the separate IBSSes
> used before the merge?

Good point. Also, it shouldn't hurt to always reset it, so what's the
point?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]