All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Wunderlich <sw@simonwunderlich.de>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] IBSS can't beacon after rejoin / regression in TSF syncing code?
Date: Mon, 27 Jan 2014 14:27:51 +0100	[thread overview]
Message-ID: <201401271427.51372.sw@simonwunderlich.de> (raw)
In-Reply-To: <21221.57261.792711.70974@gargle.gargle.HOWL>

Hello Sujith,

> Simon Wunderlich wrote:
> > we have found a regression in the IBSS creation/joining part of mac80211
> > which is appearently connected to the TSF-syncing patches introduced
> > last year[1]. It prevents beaconing of an adhoc member after rejoining a
> > cell when this cell is currently empty. The problem is present in at
> > least 3.10 and 3.13.
> > 
> > To reproduce, use two adhoc peers and let them join/leave in the
> > following order:
> > 
> > station 1: join
> > station 2: join
> > station 2: leave
> > station 1: leave
> > station 1: join
> > 
> > now we would expect that station 1 sends beacons, but it doesn't. After
> > inspecting the code, station 1 actually selected the "old" ibss network
> > and waits for a beacon to sync the tsf which is never received, as all
> > members already left the network. An easy workaround is to set the IBSS
> > creator always to true.
> 
> The race condition is that station-1 (the creator) removes station-2 only
> after a while, based on the expiration/inactive timer.
> 
> The small window that IEEE80211_IBSS_MERGE_INTERVAL introduces when
> ieee80211_ibss_disconnect() is called causes the race, since we assume that
> station-2 is still active and do not remove the BSS from cfg80211.
> 
> I am not sure why we have to keep the BSS around when we are leaving the
> network.
> 
> Is this patch the right approach ?

Thanks for the prompt answer!

Yeah, this patch works for my case. I'm not completely sure why we only unlink 
for this special case (no stations & bssid = zero), I don't see why it would 
hurt to always throw away that BSS and rescan on the next join?

I'm CCing Teemu, who introduced this roughly 3.5 years ago ("mac80211: remove 
BSS from cfg80211 list when leaving IBSS", 
5ea096c0c85e80335889539899af9a4717976e0b) , maybe he can explain it more.  I 
couldn't understand that from the commit message and the corresponding mail 
thread.

If we don't hear anything or there aren't any further objections, I think we 
can clean this patch and merge it. :)

Thanks a lot!
     Simon

> 
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index 771080e..e1688cd 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -688,17 +688,18 @@ static int ieee80211_sta_active_ibss(struct
> ieee80211_sub_if_data *sdata) return active;
>  }
> 
> -static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata)
> +static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata,
> bool leave) {
>  	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
>  	struct ieee80211_local *local = sdata->local;
>  	struct cfg80211_bss *cbss;
>  	struct beacon_data *presp;
>  	struct sta_info *sta;
> -	int active_ibss;
> +	int active_ibss = 0;
>  	u16 capability;
> 
> -	active_ibss = ieee80211_sta_active_ibss(sdata);
> +	if (!leave)
> +		active_ibss = ieee80211_sta_active_ibss(sdata);
> 
>  	if (!active_ibss && !is_zero_ether_addr(ifibss->bssid)) {
>  		capability = WLAN_CAPABILITY_IBSS;
> @@ -765,7 +766,7 @@ static void ieee80211_csa_connection_drop_work(struct
> work_struct *work)
> 
>  	sdata_lock(sdata);
> 
> -	ieee80211_ibss_disconnect(sdata);
> +	ieee80211_ibss_disconnect(sdata, false);
>  	synchronize_rcu();
>  	skb_queue_purge(&sdata->skb_queue);
> 
> @@ -1721,7 +1722,7 @@ int ieee80211_ibss_leave(struct ieee80211_sub_if_data
> *sdata) {
>  	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
> 
> -	ieee80211_ibss_disconnect(sdata);
> +	ieee80211_ibss_disconnect(sdata, true);
>  	ifibss->ssid_len = 0;
>  	memset(ifibss->bssid, 0, ETH_ALEN);
> 
> 
> Sujith

WARNING: multiple messages have this Message-ID (diff)
From: Simon Wunderlich <sw@simonwunderlich.de>
To: Sujith Manoharan <sujith@msujith.org>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>,
	"ath9k-devel@lists.ath9k.org" <ath9k-devel@lists.ath9k.org>,
	linux-wireless@vger.kernel.org,
	Antonio Quartulli <antonio@meshcoding.com>,
	Teemu Paasikivi <ext-teemu.3.paasikivi@nokia.com>
Subject: Re: IBSS can't beacon after rejoin / regression in TSF syncing code?
Date: Mon, 27 Jan 2014 14:27:51 +0100	[thread overview]
Message-ID: <201401271427.51372.sw@simonwunderlich.de> (raw)
In-Reply-To: <21221.57261.792711.70974@gargle.gargle.HOWL>

Hello Sujith,

> Simon Wunderlich wrote:
> > we have found a regression in the IBSS creation/joining part of mac80211
> > which is appearently connected to the TSF-syncing patches introduced
> > last year[1]. It prevents beaconing of an adhoc member after rejoining a
> > cell when this cell is currently empty. The problem is present in at
> > least 3.10 and 3.13.
> > 
> > To reproduce, use two adhoc peers and let them join/leave in the
> > following order:
> > 
> > station 1: join
> > station 2: join
> > station 2: leave
> > station 1: leave
> > station 1: join
> > 
> > now we would expect that station 1 sends beacons, but it doesn't. After
> > inspecting the code, station 1 actually selected the "old" ibss network
> > and waits for a beacon to sync the tsf which is never received, as all
> > members already left the network. An easy workaround is to set the IBSS
> > creator always to true.
> 
> The race condition is that station-1 (the creator) removes station-2 only
> after a while, based on the expiration/inactive timer.
> 
> The small window that IEEE80211_IBSS_MERGE_INTERVAL introduces when
> ieee80211_ibss_disconnect() is called causes the race, since we assume that
> station-2 is still active and do not remove the BSS from cfg80211.
> 
> I am not sure why we have to keep the BSS around when we are leaving the
> network.
> 
> Is this patch the right approach ?

Thanks for the prompt answer!

Yeah, this patch works for my case. I'm not completely sure why we only unlink 
for this special case (no stations & bssid = zero), I don't see why it would 
hurt to always throw away that BSS and rescan on the next join?

I'm CCing Teemu, who introduced this roughly 3.5 years ago ("mac80211: remove 
BSS from cfg80211 list when leaving IBSS", 
5ea096c0c85e80335889539899af9a4717976e0b) , maybe he can explain it more.  I 
couldn't understand that from the commit message and the corresponding mail 
thread.

If we don't hear anything or there aren't any further objections, I think we 
can clean this patch and merge it. :)

Thanks a lot!
     Simon

> 
> diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
> index 771080e..e1688cd 100644
> --- a/net/mac80211/ibss.c
> +++ b/net/mac80211/ibss.c
> @@ -688,17 +688,18 @@ static int ieee80211_sta_active_ibss(struct
> ieee80211_sub_if_data *sdata) return active;
>  }
> 
> -static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata)
> +static void ieee80211_ibss_disconnect(struct ieee80211_sub_if_data *sdata,
> bool leave) {
>  	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
>  	struct ieee80211_local *local = sdata->local;
>  	struct cfg80211_bss *cbss;
>  	struct beacon_data *presp;
>  	struct sta_info *sta;
> -	int active_ibss;
> +	int active_ibss = 0;
>  	u16 capability;
> 
> -	active_ibss = ieee80211_sta_active_ibss(sdata);
> +	if (!leave)
> +		active_ibss = ieee80211_sta_active_ibss(sdata);
> 
>  	if (!active_ibss && !is_zero_ether_addr(ifibss->bssid)) {
>  		capability = WLAN_CAPABILITY_IBSS;
> @@ -765,7 +766,7 @@ static void ieee80211_csa_connection_drop_work(struct
> work_struct *work)
> 
>  	sdata_lock(sdata);
> 
> -	ieee80211_ibss_disconnect(sdata);
> +	ieee80211_ibss_disconnect(sdata, false);
>  	synchronize_rcu();
>  	skb_queue_purge(&sdata->skb_queue);
> 
> @@ -1721,7 +1722,7 @@ int ieee80211_ibss_leave(struct ieee80211_sub_if_data
> *sdata) {
>  	struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
> 
> -	ieee80211_ibss_disconnect(sdata);
> +	ieee80211_ibss_disconnect(sdata, true);
>  	ifibss->ssid_len = 0;
>  	memset(ifibss->bssid, 0, ETH_ALEN);
> 
> 
> Sujith

  reply	other threads:[~2014-01-27 13:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-24 17:26 [ath9k-devel] IBSS can't beacon after rejoin / regression in TSF syncing code? Simon Wunderlich
2014-01-24 17:26 ` Simon Wunderlich
2014-01-27  4:25 ` [ath9k-devel] " Sujith Manoharan
2014-01-27  4:25   ` Sujith Manoharan
2014-01-27 13:27   ` Simon Wunderlich [this message]
2014-01-27 13:27     ` Simon Wunderlich
2014-01-27 13:34     ` [ath9k-devel] " Simon Wunderlich
2014-01-27 13:34       ` Simon Wunderlich
2014-01-28  1:41     ` [ath9k-devel] " Sujith Manoharan
2014-01-28  1:41       ` Sujith Manoharan
2014-01-28  7:29       ` [ath9k-devel] " Antonio Quartulli
2014-01-28  7:29         ` Antonio Quartulli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201401271427.51372.sw@simonwunderlich.de \
    --to=sw@simonwunderlich.de \
    --cc=ath9k-devel@lists.ath9k.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.