| Message ID | 1411760105-18614-7-git-send-email-luca@coelho.fi (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 26 September 2014 21:35, Luca Coelho <luca@coelho.fi> wrote: > From: Luciano Coelho <luciano.coelho@intel.com> > > Instead of immediately reopening the queues (in case of block_tx), > calling the post_channel_switch operation and sending the > notification, wait for the first beacon on the new channel. This > makes sure that we don't lose packets if the AP/GO is not on the new > channel yet. > > Signed-off-by: Luciano Coelho <luciano.coelho@intel.com> [...] Just a few nitpicks from me: > +static void ieee80211_chswitch_post_beacon(struct ieee80211_sub_if_data *sdata) > +{ > + struct ieee80211_local *local = sdata->local; > + struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; > + int ret; > + > + sdata_assert_lock(sdata); > I was thinking about WARN_ON(!sdata->vif.csa_active) here. csa_active should be set in all cases if csa_waiting_bcn is set, no? > - /* XXX: wait for a beacon first? */ > if (sdata->csa_block_tx) { > ieee80211_wake_vif_queues(local, sdata, > IEEE80211_QUEUE_STOP_REASON_CSA); > sdata->csa_block_tx = false; > } > > + sdata->vif.csa_active = false; > + ifmgd->csa_waiting_bcn = false; > + > ret = drv_post_channel_switch(sdata); > if (ret) { > sdata_info(sdata, > "driver post channel switch failed, disconnecting\n"); > ieee80211_queue_work(&local->hw, > &ifmgd->csa_connection_drop_work); > - goto out; > + return; > } > <---- here > - ieee80211_sta_reset_beacon_monitor(sdata); > - ieee80211_sta_reset_conn_monitor(sdata); > - > -out: > - mutex_unlock(&local->chanctx_mtx); > - mutex_unlock(&local->mtx); > - sdata_unlock(sdata); > } Is that an empty line I before final `}`? 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
Sorry for the delay in replying. On Mon, 2014-09-29 at 11:02 +0200, Michal Kazior wrote: > On 26 September 2014 21:35, Luca Coelho <luca@coelho.fi> wrote: > > From: Luciano Coelho <luciano.coelho@intel.com> > > > > Instead of immediately reopening the queues (in case of block_tx), > > calling the post_channel_switch operation and sending the > > notification, wait for the first beacon on the new channel. This > > makes sure that we don't lose packets if the AP/GO is not on the new > > channel yet. > > > > Signed-off-by: Luciano Coelho <luciano.coelho@intel.com> > [...] > > Just a few nitpicks from me: > > > +static void ieee80211_chswitch_post_beacon(struct ieee80211_sub_if_data *sdata) > > +{ > > + struct ieee80211_local *local = sdata->local; > > + struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; > > + int ret; > > + > > + sdata_assert_lock(sdata); > > > > I was thinking about WARN_ON(!sdata->vif.csa_active) here. csa_active > should be set in all cases if csa_waiting_bcn is set, no? Good idea, csa_active must still be true otherwise we may get into trouble. I'll add the WARN_ON. > > > - /* XXX: wait for a beacon first? */ > > if (sdata->csa_block_tx) { > > ieee80211_wake_vif_queues(local, sdata, > > IEEE80211_QUEUE_STOP_REASON_CSA); > > sdata->csa_block_tx = false; > > } > > > > + sdata->vif.csa_active = false; > > + ifmgd->csa_waiting_bcn = false; > > + > > ret = drv_post_channel_switch(sdata); > > if (ret) { > > sdata_info(sdata, > > "driver post channel switch failed, disconnecting\n"); > > ieee80211_queue_work(&local->hw, > > &ifmgd->csa_connection_drop_work); > > - goto out; > > + return; > > } > > <---- here > > - ieee80211_sta_reset_beacon_monitor(sdata); > > - ieee80211_sta_reset_conn_monitor(sdata); > > - > > -out: > > - mutex_unlock(&local->chanctx_mtx); > > - mutex_unlock(&local->mtx); > > - sdata_unlock(sdata); > > } > > Is that an empty line I before final `}`? Yep, good catch, I'll fix it. -- 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
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index a9cc491..78d6121 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -434,6 +434,8 @@ struct ieee80211_if_managed { unsigned int flags; + bool csa_waiting_bcn; + bool beacon_crc_valid; u32 beacon_crc; diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c index af23722..e469b33 100644 --- a/net/mac80211/iface.c +++ b/net/mac80211/iface.c @@ -842,6 +842,8 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata, sdata_lock(sdata); mutex_lock(&local->mtx); sdata->vif.csa_active = false; + if (sdata->vif.type == NL80211_IFTYPE_STATION) + sdata->u.mgd.csa_waiting_bcn = false; if (sdata->csa_block_tx) { ieee80211_wake_vif_queues(local, sdata, IEEE80211_QUEUE_STOP_REASON_CSA); diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c index 2e29cc3..d996126 100644 --- a/net/mac80211/mlme.c +++ b/net/mac80211/mlme.c @@ -1001,31 +1001,43 @@ static void ieee80211_chswitch_work(struct work_struct *work) /* XXX: shouldn't really modify cfg80211-owned data! */ ifmgd->associated->channel = sdata->csa_chandef.chan; - sdata->vif.csa_active = false; + ifmgd->csa_waiting_bcn = true; + + ieee80211_sta_reset_beacon_monitor(sdata); + ieee80211_sta_reset_conn_monitor(sdata); + +out: + mutex_unlock(&local->chanctx_mtx); + mutex_unlock(&local->mtx); + sdata_unlock(sdata); +} + +static void ieee80211_chswitch_post_beacon(struct ieee80211_sub_if_data *sdata) +{ + struct ieee80211_local *local = sdata->local; + struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; + int ret; + + sdata_assert_lock(sdata); - /* XXX: wait for a beacon first? */ if (sdata->csa_block_tx) { ieee80211_wake_vif_queues(local, sdata, IEEE80211_QUEUE_STOP_REASON_CSA); sdata->csa_block_tx = false; } + sdata->vif.csa_active = false; + ifmgd->csa_waiting_bcn = false; + ret = drv_post_channel_switch(sdata); if (ret) { sdata_info(sdata, "driver post channel switch failed, disconnecting\n"); ieee80211_queue_work(&local->hw, &ifmgd->csa_connection_drop_work); - goto out; + return; } - ieee80211_sta_reset_beacon_monitor(sdata); - ieee80211_sta_reset_conn_monitor(sdata); - -out: - mutex_unlock(&local->chanctx_mtx); - mutex_unlock(&local->mtx); - sdata_unlock(sdata); } void ieee80211_chswitch_done(struct ieee80211_vif *vif, bool success) @@ -1943,6 +1955,7 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata, ieee80211_vif_release_channel(sdata); sdata->vif.csa_active = false; + ifmgd->csa_waiting_bcn = false; if (sdata->csa_block_tx) { ieee80211_wake_vif_queues(local, sdata, IEEE80211_QUEUE_STOP_REASON_CSA); @@ -2191,6 +2204,7 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata) true, frame_buf); mutex_lock(&local->mtx); sdata->vif.csa_active = false; + ifmgd->csa_waiting_bcn = false; if (sdata->csa_block_tx) { ieee80211_wake_vif_queues(local, sdata, IEEE80211_QUEUE_STOP_REASON_CSA); @@ -3215,6 +3229,9 @@ static void ieee80211_rx_mgmt_beacon(struct ieee80211_sub_if_data *sdata, } } + if (ifmgd->csa_waiting_bcn) + ieee80211_chswitch_post_beacon(sdata); + if (ncrc == ifmgd->beacon_crc && ifmgd->beacon_crc_valid) return; ifmgd->beacon_crc = ncrc; @@ -3687,11 +3704,12 @@ static void ieee80211_sta_bcn_mon_timer(unsigned long data) struct ieee80211_sub_if_data *sdata = (struct ieee80211_sub_if_data *) data; struct ieee80211_local *local = sdata->local; + struct ieee80211_if_managed *ifmgd = &sdata->u.mgd; if (local->quiescing) return; - if (sdata->vif.csa_active) + if (sdata->vif.csa_active && !ifmgd->csa_waiting_bcn) return; sdata->u.mgd.connection_loss = false; @@ -3709,7 +3727,7 @@ static void ieee80211_sta_conn_mon_timer(unsigned long data) if (local->quiescing) return; - if (sdata->vif.csa_active) + if (sdata->vif.csa_active && !ifmgd->csa_waiting_bcn) return; ieee80211_queue_work(&local->hw, &ifmgd->monitor_work);