diff mbox

[v2,4/4] mac80211: disconnect iface if CSA unexpectedly fails

Message ID 1395408675-26013-5-git-send-email-michal.kazior@tieto.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Michal Kazior March 21, 2014, 1:31 p.m. UTC
It doesn't make much sense to leave a cripled
interface running.

As a side effect this will unblock tx queues with
CSA reason immediately after failure instead of
until after userspace requests interface to stop.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Luca Coelho March 24, 2014, 2:55 p.m. UTC | #1
On Fri, 2014-03-21 at 14:31 +0100, Michal Kazior wrote:
> It doesn't make much sense to leave a cripled
> interface running.
> 
> As a side effect this will unblock tx queues with
> CSA reason immediately after failure instead of
> until after userspace requests interface to stop.
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  net/mac80211/cfg.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 65768ef..64e152d 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -3088,7 +3088,7 @@ static int ieee80211_set_after_csa_beacon(struct ieee80211_sub_if_data *sdata,
>  	return 0;
>  }
>  
> -static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
> +static int ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>  {
>  	struct ieee80211_local *local = sdata->local;
>  	u32 changed = 0;
> @@ -3100,7 +3100,7 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>  	sdata->radar_required = sdata->csa_radar_required;
>  	err = ieee80211_vif_change_channel(sdata, &changed);
>  	if (WARN_ON(err < 0))
> -		return;
> +		return err;

Now you propagate the error up, do we still need the WARN_ON here?

[...]
> @@ -3123,6 +3125,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
>  		container_of(work, struct ieee80211_sub_if_data,
>  			     csa_finalize_work);
>  	struct ieee80211_local *local = sdata->local;
> +	int err;
>  
>  	sdata_lock(sdata);
>  	mutex_lock(&local->mtx);
> @@ -3134,7 +3137,13 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
>  	if (!ieee80211_sdata_running(sdata))
>  		goto unlock;
>  
> -	ieee80211_csa_finalize(sdata);
> +	err = ieee80211_csa_finalize(sdata);
> +	if (err) {
> +		sdata_info(sdata, "failed to finalize CSA, disconnecting\n");
> +		cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev);
> +		goto unlock;
> +	}
> +

This is neat. :) Maybe it's possible to do the same thing in mlme.c and
get rid of the connection_drop_work?

--
Luca.

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Kazior March 25, 2014, 7:46 a.m. UTC | #2
On 24 March 2014 15:55, Luca Coelho <luca@coelho.fi> wrote:
> On Fri, 2014-03-21 at 14:31 +0100, Michal Kazior wrote:
>> It doesn't make much sense to leave a cripled
>> interface running.
>>
>> As a side effect this will unblock tx queues with
>> CSA reason immediately after failure instead of
>> until after userspace requests interface to stop.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>> ---
>>  net/mac80211/cfg.c | 17 +++++++++++++----
>>  1 file changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>> index 65768ef..64e152d 100644
>> --- a/net/mac80211/cfg.c
>> +++ b/net/mac80211/cfg.c
>> @@ -3088,7 +3088,7 @@ static int ieee80211_set_after_csa_beacon(struct ieee80211_sub_if_data *sdata,
>>       return 0;
>>  }
>>
>> -static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>> +static int ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>>  {
>>       struct ieee80211_local *local = sdata->local;
>>       u32 changed = 0;
>> @@ -3100,7 +3100,7 @@ static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>>       sdata->radar_required = sdata->csa_radar_required;
>>       err = ieee80211_vif_change_channel(sdata, &changed);
>>       if (WARN_ON(err < 0))
>> -             return;
>> +             return err;
>
> Now you propagate the error up, do we still need the WARN_ON here?

Good point.


> [...]
>> @@ -3123,6 +3125,7 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
>>               container_of(work, struct ieee80211_sub_if_data,
>>                            csa_finalize_work);
>>       struct ieee80211_local *local = sdata->local;
>> +     int err;
>>
>>       sdata_lock(sdata);
>>       mutex_lock(&local->mtx);
>> @@ -3134,7 +3137,13 @@ void ieee80211_csa_finalize_work(struct work_struct *work)
>>       if (!ieee80211_sdata_running(sdata))
>>               goto unlock;
>>
>> -     ieee80211_csa_finalize(sdata);
>> +     err = ieee80211_csa_finalize(sdata);
>> +     if (err) {
>> +             sdata_info(sdata, "failed to finalize CSA, disconnecting\n");
>> +             cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev);
>> +             goto unlock;
>> +     }
>> +
>
> This is neat. :) Maybe it's possible to do the same thing in mlme.c and
> get rid of the connection_drop_work?

I don't think it's necessary now.


Micha?
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 65768ef..64e152d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3088,7 +3088,7 @@  static int ieee80211_set_after_csa_beacon(struct ieee80211_sub_if_data *sdata,
 	return 0;
 }
 
-static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
+static int ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
 {
 	struct ieee80211_local *local = sdata->local;
 	u32 changed = 0;
@@ -3100,7 +3100,7 @@  static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
 	sdata->radar_required = sdata->csa_radar_required;
 	err = ieee80211_vif_change_channel(sdata, &changed);
 	if (WARN_ON(err < 0))
-		return;
+		return err;
 
 	if (!local->use_chanctx) {
 		local->_oper_chandef = sdata->csa_chandef;
@@ -3111,10 +3111,12 @@  static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
 
 	err = ieee80211_set_after_csa_beacon(sdata, &changed);
 	if (err)
-		return;
+		return err;
 
 	ieee80211_bss_info_change_notify(sdata, changed);
 	cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
+
+	return 0;
 }
 
 void ieee80211_csa_finalize_work(struct work_struct *work)
@@ -3123,6 +3125,7 @@  void ieee80211_csa_finalize_work(struct work_struct *work)
 		container_of(work, struct ieee80211_sub_if_data,
 			     csa_finalize_work);
 	struct ieee80211_local *local = sdata->local;
+	int err;
 
 	sdata_lock(sdata);
 	mutex_lock(&local->mtx);
@@ -3134,7 +3137,13 @@  void ieee80211_csa_finalize_work(struct work_struct *work)
 	if (!ieee80211_sdata_running(sdata))
 		goto unlock;
 
-	ieee80211_csa_finalize(sdata);
+	err = ieee80211_csa_finalize(sdata);
+	if (err) {
+		sdata_info(sdata, "failed to finalize CSA, disconnecting\n");
+		cfg80211_stop_iface(local->hw.wiphy, &sdata->wdev);
+		goto unlock;
+	}
+
 	if (!ieee80211_csa_needs_block_tx(local))
 		ieee80211_wake_queues_by_reason(&local->hw,
 					IEEE80211_MAX_QUEUE_MAP,