2013-12-03 13:48:28

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v6 0/5] CSA beacon count changes

Hi,

This is a new version of my patchset. Now it's rebased on the latest
mac80211-next, so all the locking changes are included. I hope the
warnings observed with mesh and IBSS go away now that the locks are
working properly.

Simon and Chun-Yeow, it would be great if you could re-test this
updated version and let me know how it goes.

Thanks!

Luca.

Luciano Coelho (5):
mac80211: refactor ieee80211_ibss_process_chanswitch()
mac80211: align ieee80211_ibss_csa_beacon() with
ieee80211_csa_beacon()
mac80211: refactor ieee80211_mesh_process_chanswitch()
mac80211: align ieee80211_mesh_csa_beacon() with
ieee80211_csa_beacon()
mac80211: only set CSA beacon when at least one beacon must be
transmitted

include/net/mac80211.h | 10 ++--
net/mac80211/cfg.c | 120 +++++++++++++++++++++++++++++++++++----------
net/mac80211/ibss.c | 78 +++++++----------------------
net/mac80211/ieee80211_i.h | 11 +++--
net/mac80211/mesh.c | 78 ++++++-----------------------
net/mac80211/tx.c | 19 +++----
net/mac80211/util.c | 1 -
7 files changed, 148 insertions(+), 169 deletions(-)

--
1.8.4.2



2013-12-03 13:48:28

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v6 1/5] mac80211: refactor ieee80211_ibss_process_chanswitch()

Refactor ieee80211_ibss_process_chanswitch() to use
ieee80211_channel_switch() and avoid code duplication.

Change-Id: I265a12c7f825dc20535bad1197a81437310d0086
Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/cfg.c | 4 ++--
net/mac80211/ibss.c | 58 +++++++---------------------------------------
net/mac80211/ieee80211_i.h | 2 ++
3 files changed, 12 insertions(+), 52 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 754069c..b5bc27e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3048,8 +3048,8 @@ unlock:
sdata_unlock(sdata);
}

-static int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
- struct cfg80211_csa_settings *params)
+int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_csa_settings *params)
{
struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
struct ieee80211_local *local = sdata->local;
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 0f1fb5d..3514aab 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -784,18 +784,10 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
struct cfg80211_csa_settings params;
struct ieee80211_csa_ie csa_ie;
struct ieee80211_if_ibss *ifibss = &sdata->u.ibss;
- struct ieee80211_chanctx_conf *chanctx_conf;
- struct ieee80211_chanctx *chanctx;
enum nl80211_channel_type ch_type;
- int err, num_chanctx;
+ int err;
u32 sta_flags;

- if (sdata->vif.csa_active)
- return true;
-
- if (!sdata->vif.bss_conf.ibss_joined)
- return false;
-
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_5:
@@ -826,9 +818,6 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.count = csa_ie.count;
params.chandef = csa_ie.chandef;

- if (ifibss->chandef.chan->band != params.chandef.chan->band)
- goto disconnect;
-
switch (ifibss->chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
case NL80211_CHAN_WIDTH_20:
@@ -884,29 +873,6 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,
params.radar_required = true;
}

- rcu_read_lock();
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
- if (!chanctx_conf) {
- rcu_read_unlock();
- goto disconnect;
- }
-
- /* don't handle for multi-VIF cases */
- chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
- if (chanctx->refcount > 1) {
- rcu_read_unlock();
- goto disconnect;
- }
- num_chanctx = 0;
- list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
- num_chanctx++;
-
- if (num_chanctx > 1) {
- rcu_read_unlock();
- goto disconnect;
- }
- rcu_read_unlock();
-
/* all checks done, now perform the channel switch. */
ibss_dbg(sdata,
"received channel switch announcement to go to channel %d MHz\n",
@@ -914,19 +880,9 @@ ieee80211_ibss_process_chanswitch(struct ieee80211_sub_if_data *sdata,

params.block_tx = !!csa_ie.mode;

- ieee80211_ibss_csa_beacon(sdata, &params);
- sdata->csa_radar_required = params.radar_required;
-
- if (params.block_tx)
- ieee80211_stop_queues_by_reason(&sdata->local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
-
- sdata->csa_chandef = params.chandef;
- sdata->vif.csa_active = true;
-
- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, &params.chandef);
+ if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
+ &params))
+ goto disconnect;

ieee80211_ibss_csa_mark_radar(sdata);

@@ -962,7 +918,8 @@ ieee80211_rx_mgmt_spectrum_mgmt(struct ieee80211_sub_if_data *sdata,
if (len < required_len)
return;

- ieee80211_ibss_process_chanswitch(sdata, elems, false);
+ if (!sdata->vif.csa_active)
+ ieee80211_ibss_process_chanswitch(sdata, elems, false);
}

static void ieee80211_rx_mgmt_deauth_ibss(struct ieee80211_sub_if_data *sdata,
@@ -1143,7 +1100,8 @@ static void ieee80211_rx_bss_info(struct ieee80211_sub_if_data *sdata,
goto put_bss;

/* process channel switch */
- if (ieee80211_ibss_process_chanswitch(sdata, elems, true))
+ if (sdata->vif.csa_active ||
+ ieee80211_ibss_process_chanswitch(sdata, elems, true))
goto put_bss;

/* same BSSID */
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 32bae21..a7b4d55 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1442,6 +1442,8 @@ void ieee80211_handle_roc_started(struct ieee80211_roc_work *roc);

/* channel switch handling */
void ieee80211_csa_finalize_work(struct work_struct *work);
+int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
+ struct cfg80211_csa_settings *params);

/* interface handling */
int ieee80211_iface_init(void);
--
1.8.4.2


2013-12-03 13:48:29

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v6 3/5] mac80211: refactor ieee80211_mesh_process_chanswitch()

Refactor ieee80211_mesh_process_chanswitch() to use
ieee80211_channel_switch() and avoid code duplication.

Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/cfg.c | 10 +++++--
net/mac80211/ieee80211_i.h | 6 +++-
net/mac80211/mesh.c | 68 ++++++++++------------------------------------
net/mac80211/util.c | 1 -
4 files changed, 27 insertions(+), 58 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index b5bc27e..790c15d 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3156,9 +3156,15 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
params->chandef.chan->band)
return -EINVAL;

- err = ieee80211_mesh_csa_beacon(sdata, params, true);
- if (err < 0)
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
+
+ err = ieee80211_mesh_csa_beacon(sdata, params,
+ (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
+ if (err < 0) {
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
return err;
+ }
break;
#endif
default:
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index a7b4d55..cf38a74 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -611,7 +611,11 @@ struct ieee80211_if_mesh {
struct ps_data ps;
/* Channel Switching Support */
struct mesh_csa_settings __rcu *csa;
- bool chsw_init;
+ enum {
+ IEEE80211_MESH_CSA_ROLE_NONE,
+ IEEE80211_MESH_CSA_ROLE_INIT,
+ IEEE80211_MESH_CSA_ROLE_REPEATER,
+ } csa_role;
u8 chsw_ttl;
u16 pre_value;
};
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 330d1f7..980cc12 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -685,7 +685,7 @@ ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
*pos++ = csa->settings.count;
*pos++ = WLAN_EID_CHAN_SWITCH_PARAM;
*pos++ = 6;
- if (ifmsh->chsw_init) {
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT) {
*pos++ = ifmsh->mshcfg.dot11MeshTTL;
*pos |= WLAN_EID_CHAN_SWITCH_PARAM_INITIATOR;
} else {
@@ -853,19 +853,11 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
{
struct cfg80211_csa_settings params;
struct ieee80211_csa_ie csa_ie;
- struct ieee80211_chanctx_conf *chanctx_conf;
- struct ieee80211_chanctx *chanctx;
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
enum ieee80211_band band = ieee80211_get_sdata_band(sdata);
- int err, num_chanctx;
+ int err;
u32 sta_flags;

- if (sdata->vif.csa_active)
- return true;
-
- if (!ifmsh->mesh_id)
- return false;
-
sta_flags = IEEE80211_STA_DISABLE_VHT;
switch (sdata->vif.bss_conf.chandef.width) {
case NL80211_CHAN_WIDTH_20_NOHT:
@@ -890,10 +882,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
params.chandef = csa_ie.chandef;
params.count = csa_ie.count;

- if (sdata->vif.bss_conf.chandef.chan->band !=
- params.chandef.chan->band)
- return false;
-
if (!cfg80211_chandef_usable(sdata->local->hw.wiphy, &params.chandef,
IEEE80211_CHAN_DISABLED)) {
sdata_info(sdata,
@@ -916,25 +904,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
return false;
}

- rcu_read_lock();
- chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
- if (!chanctx_conf)
- goto failed_chswitch;
-
- /* don't handle for multi-VIF cases */
- chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
- if (chanctx->refcount > 1)
- goto failed_chswitch;
-
- num_chanctx = 0;
- list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
- num_chanctx++;
-
- if (num_chanctx > 1)
- goto failed_chswitch;
-
- rcu_read_unlock();
-
mcsa_dbg(sdata,
"received channel switch announcement to go to channel %d MHz\n",
params.chandef.chan->center_freq);
@@ -945,27 +914,16 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
else
ifmsh->chsw_ttl = 0;

- if (ifmsh->chsw_ttl > 0)
- if (ieee80211_mesh_csa_beacon(sdata, &params, false) < 0)
- return false;
-
- sdata->csa_radar_required = params.radar_required;
-
- if (params.block_tx)
- ieee80211_stop_queues_by_reason(&sdata->local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ if (ifmsh->chsw_ttl == 0)
+ return false;

- sdata->csa_chandef = params.chandef;
- sdata->vif.csa_active = true;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_REPEATER;

- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, &params.chandef);
+ if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
+ &params) < 0)
+ return false;

return true;
-failed_chswitch:
- rcu_read_unlock();
- return false;
}

static void
@@ -1075,7 +1033,8 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
ifmsh->sync_ops->rx_bcn_presp(sdata,
stype, mgmt, &elems, rx_status);

- if (!ifmsh->chsw_init)
+ if (ifmsh->csa_role != IEEE80211_MESH_CSA_ROLE_INIT &&
+ !sdata->vif.csa_active)
ieee80211_mesh_process_chnswitch(sdata, &elems, true);
}

@@ -1086,7 +1045,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
int ret = 0;

/* Reset the TTL value and Initiator flag */
- ifmsh->chsw_init = false;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
ifmsh->chsw_ttl = 0;

/* Remove the CSA and MCSP elements from the beacon */
@@ -1200,7 +1159,8 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata,

ifmsh->pre_value = pre_value;

- if (!ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
+ if (!sdata->vif.csa_active &&
+ !ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
mcsa_dbg(sdata, "Failed to process CSA action frame");
return;
}
@@ -1355,7 +1315,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
mesh_rmc_init(sdata);
ifmsh->last_preq = jiffies;
ifmsh->next_perr = jiffies;
- ifmsh->chsw_init = false;
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
/* Allocate all mesh structures when creating the first mesh interface. */
if (!mesh_allocated)
ieee80211s_init();
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 06265d7..f153ab4 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2487,7 +2487,6 @@ int ieee80211_send_action_csa(struct ieee80211_sub_if_data *sdata,
ifmsh->pre_value++;
put_unaligned_le16(ifmsh->pre_value, pos);/* Precedence Value */
pos += 2;
- ifmsh->chsw_init = true;
}

ieee80211_tx_skb(sdata, skb);
--
1.8.4.2


2013-12-05 02:05:58

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH v6 3/5] mac80211: refactor ieee80211_mesh_process_chanswitch()

Sorry, should be patch v5.

On Wed, Dec 4, 2013 at 5:36 PM, Yeoh Chun-Yeow <[email protected]> wrote:
> Hi, Luciano
>
> This patch applies clean on mac80211-next, but to able to test the
> mesh channel switch, patch entitled "mac80211: fix the mesh channel
> switch support" is required.
>
> So I have used the patch v4 for the testing instead of this one.
>
> ----
> Chun-Yeow
>
> On Tue, Dec 3, 2013 at 9:48 PM, Luciano Coelho <[email protected]> wrote:
>> Refactor ieee80211_mesh_process_chanswitch() to use
>> ieee80211_channel_switch() and avoid code duplication.
>>
>> Signed-off-by: Luciano Coelho <[email protected]>
>> ---
>> net/mac80211/cfg.c | 10 +++++--
>> net/mac80211/ieee80211_i.h | 6 +++-
>> net/mac80211/mesh.c | 68 ++++++++++------------------------------------
>> net/mac80211/util.c | 1 -
>> 4 files changed, 27 insertions(+), 58 deletions(-)
>>
>> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
>> index b5bc27e..790c15d 100644
>> --- a/net/mac80211/cfg.c
>> +++ b/net/mac80211/cfg.c
>> @@ -3156,9 +3156,15 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
>> params->chandef.chan->band)
>> return -EINVAL;
>>
>> - err = ieee80211_mesh_csa_beacon(sdata, params, true);
>> - if (err < 0)
>> + if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
>> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
>> +
>> + err = ieee80211_mesh_csa_beacon(sdata, params,
>> + (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
>> + if (err < 0) {
>> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
>> return err;
>> + }
>> break;
>> #endif
>> default:
>> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
>> index a7b4d55..cf38a74 100644
>> --- a/net/mac80211/ieee80211_i.h
>> +++ b/net/mac80211/ieee80211_i.h
>> @@ -611,7 +611,11 @@ struct ieee80211_if_mesh {
>> struct ps_data ps;
>> /* Channel Switching Support */
>> struct mesh_csa_settings __rcu *csa;
>> - bool chsw_init;
>> + enum {
>> + IEEE80211_MESH_CSA_ROLE_NONE,
>> + IEEE80211_MESH_CSA_ROLE_INIT,
>> + IEEE80211_MESH_CSA_ROLE_REPEATER,
>> + } csa_role;
>> u8 chsw_ttl;
>> u16 pre_value;
>> };
>> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
>> index 330d1f7..980cc12 100644
>> --- a/net/mac80211/mesh.c
>> +++ b/net/mac80211/mesh.c
>> @@ -685,7 +685,7 @@ ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
>> *pos++ = csa->settings.count;
>> *pos++ = WLAN_EID_CHAN_SWITCH_PARAM;
>> *pos++ = 6;
>> - if (ifmsh->chsw_init) {
>> + if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT) {
>> *pos++ = ifmsh->mshcfg.dot11MeshTTL;
>> *pos |= WLAN_EID_CHAN_SWITCH_PARAM_INITIATOR;
>> } else {
>> @@ -853,19 +853,11 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
>> {
>> struct cfg80211_csa_settings params;
>> struct ieee80211_csa_ie csa_ie;
>> - struct ieee80211_chanctx_conf *chanctx_conf;
>> - struct ieee80211_chanctx *chanctx;
>> struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>> enum ieee80211_band band = ieee80211_get_sdata_band(sdata);
>> - int err, num_chanctx;
>> + int err;
>> u32 sta_flags;
>>
>> - if (sdata->vif.csa_active)
>> - return true;
>> -
>> - if (!ifmsh->mesh_id)
>> - return false;
>> -
>> sta_flags = IEEE80211_STA_DISABLE_VHT;
>> switch (sdata->vif.bss_conf.chandef.width) {
>> case NL80211_CHAN_WIDTH_20_NOHT:
>> @@ -890,10 +882,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
>> params.chandef = csa_ie.chandef;
>> params.count = csa_ie.count;
>>
>> - if (sdata->vif.bss_conf.chandef.chan->band !=
>> - params.chandef.chan->band)
>> - return false;
>> -
>> if (!cfg80211_chandef_usable(sdata->local->hw.wiphy, &params.chandef,
>> IEEE80211_CHAN_DISABLED)) {
>> sdata_info(sdata,
>> @@ -916,25 +904,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
>> return false;
>> }
>>
>> - rcu_read_lock();
>> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> - if (!chanctx_conf)
>> - goto failed_chswitch;
>> -
>> - /* don't handle for multi-VIF cases */
>> - chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
>> - if (chanctx->refcount > 1)
>> - goto failed_chswitch;
>> -
>> - num_chanctx = 0;
>> - list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
>> - num_chanctx++;
>> -
>> - if (num_chanctx > 1)
>> - goto failed_chswitch;
>> -
>> - rcu_read_unlock();
>> -
>> mcsa_dbg(sdata,
>> "received channel switch announcement to go to channel %d MHz\n",
>> params.chandef.chan->center_freq);
>> @@ -945,27 +914,16 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
>> else
>> ifmsh->chsw_ttl = 0;
>>
>> - if (ifmsh->chsw_ttl > 0)
>> - if (ieee80211_mesh_csa_beacon(sdata, &params, false) < 0)
>> - return false;
>> -
>> - sdata->csa_radar_required = params.radar_required;
>> -
>> - if (params.block_tx)
>> - ieee80211_stop_queues_by_reason(&sdata->local->hw,
>> - IEEE80211_MAX_QUEUE_MAP,
>> - IEEE80211_QUEUE_STOP_REASON_CSA);
>> + if (ifmsh->chsw_ttl == 0)
>> + return false;
>>
>> - sdata->csa_chandef = params.chandef;
>> - sdata->vif.csa_active = true;
>> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_REPEATER;
>>
>> - ieee80211_bss_info_change_notify(sdata, err);
>> - drv_channel_switch_beacon(sdata, &params.chandef);
>> + if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
>> + &params) < 0)
>> + return false;
>>
>> return true;
>> -failed_chswitch:
>> - rcu_read_unlock();
>> - return false;
>> }
>>
>> static void
>> @@ -1075,7 +1033,8 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
>> ifmsh->sync_ops->rx_bcn_presp(sdata,
>> stype, mgmt, &elems, rx_status);
>>
>> - if (!ifmsh->chsw_init)
>> + if (ifmsh->csa_role != IEEE80211_MESH_CSA_ROLE_INIT &&
>> + !sdata->vif.csa_active)
>> ieee80211_mesh_process_chnswitch(sdata, &elems, true);
>> }
>>
>> @@ -1086,7 +1045,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
>> int ret = 0;
>>
>> /* Reset the TTL value and Initiator flag */
>> - ifmsh->chsw_init = false;
>> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
>> ifmsh->chsw_ttl = 0;
>>
>> /* Remove the CSA and MCSP elements from the beacon */
>> @@ -1200,7 +1159,8 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata,
>>
>> ifmsh->pre_value = pre_value;
>>
>> - if (!ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
>> + if (!sdata->vif.csa_active &&
>> + !ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
>> mcsa_dbg(sdata, "Failed to process CSA action frame");
>> return;
>> }
>> @@ -1355,7 +1315,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
>> mesh_rmc_init(sdata);
>> ifmsh->last_preq = jiffies;
>> ifmsh->next_perr = jiffies;
>> - ifmsh->chsw_init = false;
>> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
>> /* Allocate all mesh structures when creating the first mesh interface. */
>> if (!mesh_allocated)
>> ieee80211s_init();
>> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
>> index 06265d7..f153ab4 100644
>> --- a/net/mac80211/util.c
>> +++ b/net/mac80211/util.c
>> @@ -2487,7 +2487,6 @@ int ieee80211_send_action_csa(struct ieee80211_sub_if_data *sdata,
>> ifmsh->pre_value++;
>> put_unaligned_le16(ifmsh->pre_value, pos);/* Precedence Value */
>> pos += 2;
>> - ifmsh->chsw_init = true;
>> }
>>
>> ieee80211_tx_skb(sdata, skb);
>> --
>> 1.8.4.2
>>

2013-12-03 13:48:29

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v6 4/5] mac80211: align ieee80211_mesh_csa_beacon() with ieee80211_csa_beacon()

The return value of ieee80211_mesh_csa_beacon is not aligned with the
return value of ieee80211_csa_beacon() and
ieee80211_ibss_csa_beacon(). For consistency and to be able to use
both functions with similar code, change ieee80211_mesh_csa_beacon()
not to send the bss changed notification itself, but return what has
changed so the caller can send the notification instead.

Change-Id: If4c111fd874a3e17a5df98d306f7f1c8fcaa6a1a
Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/mesh.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 980cc12..5476ad9 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1089,12 +1089,10 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
return ret;
}

- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
-
if (csa_action)
ieee80211_send_action_csa(sdata, csa_settings);

- return 0;
+ return BSS_CHANGED_BEACON;
}

static int mesh_fwd_csa_frame(struct ieee80211_sub_if_data *sdata,
--
1.8.4.2


2013-12-05 10:35:28

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] mac80211: only set CSA beacon when at least one beacon must be transmitted

> Actually, I just looked at the specs again and the count value for mesh
> is completely different. Instead of specifying the number of TBTTs
> until the switch happens, it specifies the time in TUs until the switch
> happens and 0 means at any time after the frame was sent (the same as
> with nonmesh). In our implementation, we just decrease the count on
> every beacon, which is wrong in the mesh case.

Yes, you are absolutely correct.

>
> The count implementation for mesh is bogus. We need to fix that.
>
> What if, for now, I just fix the oops because of the rcu_derefence when
> the rcu is not initialized

Sure

> and we fix the count implementation later?

Hmm, even with unit of 100TU, we can't use the current count approach
since the default TBTT for mesh is 1000 TU.

----
Chun-Yeow

2013-12-03 13:48:30

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v6 5/5] mac80211: only set CSA beacon when at least one beacon must be transmitted

A beacon should never have a Channel Switch Announcement information
element with a count of 0, because a count of 1 means switch just
before the next beacon. So, if a count of 0 was valid in a beacon, it
would have been transmitted in the next channel already, which is
useless. A CSA count equal to zero is only meaningful in action
frames or probe_responses.

Fix the ieee80211_csa_is_complete() and ieee80211_update_csa()
functions accordingly.

With a CSA count of 0, we won't transmit any CSA beacons, because the
switch will happen before the next TBTT. To avoid extra work and
potential confusion in the drivers, complete the CSA immediately,
instead of waiting for the driver to call ieee80211_csa_finish().

To keep things simpler, we also switch immediately when the CSA count
is 1, while in theory we should delay the switch until just before the
next TBTT.

Additionally, move the ieee80211_csa_finish() function to cfg.c,
where it makes more sense.

Signed-off-by: Luciano Coelho <[email protected]>
---
include/net/mac80211.h | 10 ++--
net/mac80211/cfg.c | 114 ++++++++++++++++++++++++++++++++++-----------
net/mac80211/ibss.c | 6 ---
net/mac80211/ieee80211_i.h | 3 +-
net/mac80211/mesh.c | 6 +--
net/mac80211/tx.c | 19 +++-----
6 files changed, 102 insertions(+), 56 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 73d99bc..a72d408 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2717,11 +2717,13 @@ enum ieee80211_roc_type {
* @channel_switch_beacon: Starts a channel switch to a new channel.
* Beacons are modified to include CSA or ECSA IEs before calling this
* function. The corresponding count fields in these IEs must be
- * decremented, and when they reach zero the driver must call
+ * decremented, and when they reach 1 the driver must call
* ieee80211_csa_finish(). Drivers which use ieee80211_beacon_get()
* get the csa counter decremented by mac80211, but must check if it is
- * zero using ieee80211_csa_is_complete() after the beacon has been
+ * 1 using ieee80211_csa_is_complete() after the beacon has been
* transmitted and then call ieee80211_csa_finish().
+ * If the CSA count starts as zero or 1, this function will not be called,
+ * since there won't be any time to beacon before the switch anyway.
*
* @join_ibss: Join an IBSS (on an IBSS interface); this is called after all
* information in bss_conf is set up and the beacon can be retrieved. A
@@ -3416,13 +3418,13 @@ static inline struct sk_buff *ieee80211_beacon_get(struct ieee80211_hw *hw,
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
*
* After a channel switch announcement was scheduled and the counter in this
- * announcement hit zero, this function must be called by the driver to
+ * announcement hits 1, this function must be called by the driver to
* notify mac80211 that the channel can be changed.
*/
void ieee80211_csa_finish(struct ieee80211_vif *vif);

/**
- * ieee80211_csa_is_complete - find out if counters reached zero
+ * ieee80211_csa_is_complete - find out if counters reached 1
* @vif: &struct ieee80211_vif pointer from the add_interface callback.
*
* This function returns whether the channel switch counters reached zero.
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 790c15d..0ebce55 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1,4 +1,4 @@
-/*
+ /*
* mac80211 configuration hooks for cfg80211
*
* Copyright 2006-2010 Johannes Berg <[email protected]>
@@ -2982,21 +2982,19 @@ cfg80211_beacon_dup(struct cfg80211_beacon_data *beacon)
return new_beacon;
}

-void ieee80211_csa_finalize_work(struct work_struct *work)
+void ieee80211_csa_finish(struct ieee80211_vif *vif)
{
- struct ieee80211_sub_if_data *sdata =
- container_of(work, struct ieee80211_sub_if_data,
- csa_finalize_work);
- struct ieee80211_local *local = sdata->local;
- int err, changed = 0;
+ struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);

- sdata_lock(sdata);
- /* AP might have been stopped while waiting for the lock. */
- if (!sdata->vif.csa_active)
- goto unlock;
+ ieee80211_queue_work(&sdata->local->hw,
+ &sdata->csa_finalize_work);
+}
+EXPORT_SYMBOL(ieee80211_csa_finish);

- if (!ieee80211_sdata_running(sdata))
- goto unlock;
+static void ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
+{
+ struct ieee80211_local *local = sdata->local;
+ int err, changed = 0;

sdata->radar_required = sdata->csa_radar_required;
err = ieee80211_vif_change_channel(sdata, &changed);
@@ -3048,6 +3046,29 @@ unlock:
sdata_unlock(sdata);
}

+void ieee80211_csa_finalize_work(struct work_struct *work)
+{
+ struct ieee80211_sub_if_data *sdata =
+ container_of(work, struct ieee80211_sub_if_data,
+ csa_finalize_work);
+
+ sdata_lock(sdata);
+ /* AP might have been stopped while waiting for the lock. */
+ if (!sdata->vif.csa_active)
+ goto unlock;
+
+ if (!ieee80211_sdata_running(sdata))
+ goto unlock;
+
+ if (!ieee80211_sdata_running(sdata))
+ return;
+
+ ieee80211_csa_finalize(sdata);
+
+unlock:
+ sdata_unlock(sdata);
+}
+
int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_csa_settings *params)
{
@@ -3056,7 +3077,7 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
struct ieee80211_chanctx_conf *chanctx_conf;
struct ieee80211_chanctx *chanctx;
struct ieee80211_if_mesh __maybe_unused *ifmsh;
- int err, num_chanctx;
+ int err, num_chanctx, changed = 0;

lockdep_assert_held(&sdata->wdev.mtx);

@@ -3097,19 +3118,40 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,

switch (sdata->vif.type) {
case NL80211_IFTYPE_AP:
- sdata->csa_counter_offset_beacon =
- params->counter_offset_beacon;
- sdata->csa_counter_offset_presp = params->counter_offset_presp;
sdata->u.ap.next_beacon =
cfg80211_beacon_dup(&params->beacon_after);
if (!sdata->u.ap.next_beacon)
return -ENOMEM;

+ /*
+ * With a count of 0, we don't have to wait for any
+ * TBTT before switching, so complete the CSA
+ * immediately. In theory, with a count == 1 we
+ * should delay the switch until just before the next
+ * TBTT, but that would complicate things so we switch
+ * immediately too. If we would delay the switch
+ * until the next TBTT, we would have to set the probe
+ * response here.
+ *
+ * TODO: A channel switch with count <= 1 without
+ * sending a CSA action frame is kind of useless,
+ * because the clients won't know we're changing
+ * channels. The action frame must be implemented
+ * either here or in the userspace.
+ */
+ if (params->count <= 1)
+ break;
+
+ sdata->csa_counter_offset_beacon =
+ params->counter_offset_beacon;
+ sdata->csa_counter_offset_presp = params->counter_offset_presp;
err = ieee80211_assign_beacon(sdata, &params->beacon_csa);
if (err < 0) {
kfree(sdata->u.ap.next_beacon);
return err;
}
+ changed |= err;
+
break;
case NL80211_IFTYPE_ADHOC:
if (!sdata->vif.bss_conf.ibss_joined)
@@ -3137,9 +3179,16 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
params->chandef.chan->band)
return -EINVAL;

- err = ieee80211_ibss_csa_beacon(sdata, params);
- if (err < 0)
- return err;
+ /* see comments and TODO in the NL80211_IFTYPE_AP block */
+ if (params->count > 1) {
+ err = ieee80211_ibss_csa_beacon(sdata, params);
+ if (err < 0)
+ return err;
+ changed |= err;
+ }
+
+ ieee80211_send_action_csa(sdata, params);
+
break;
#ifdef CONFIG_MAC80211_MESH
case NL80211_IFTYPE_MESH_POINT:
@@ -3159,12 +3208,18 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;

- err = ieee80211_mesh_csa_beacon(sdata, params,
- (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
- if (err < 0) {
- ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
- return err;
+ if (params->count > 1) {
+ err = ieee80211_mesh_csa_beacon(sdata, params);
+ if (err < 0) {
+ ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
+ return err;
+ }
+ changed |= err;
}
+
+ if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT)
+ ieee80211_send_action_csa(sdata, params);
+
break;
#endif
default:
@@ -3181,8 +3236,13 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
sdata->csa_chandef = params->chandef;
sdata->vif.csa_active = true;

- ieee80211_bss_info_change_notify(sdata, err);
- drv_channel_switch_beacon(sdata, &params->chandef);
+ if (changed) {
+ ieee80211_bss_info_change_notify(sdata, changed);
+ drv_channel_switch_beacon(sdata, &params->chandef);
+ } else {
+ /* if the beacon didn't change, we can finalize immediately */
+ ieee80211_csa_finalize(sdata);
+ }

return 0;
}
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 23e035f..595902c 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -516,12 +516,6 @@ int ieee80211_ibss_csa_beacon(struct ieee80211_sub_if_data *sdata,
if (old_presp)
kfree_rcu(old_presp, rcu_head);

- /* it might not send the beacon for a while. send an action frame
- * immediately to announce the channel switch.
- */
- if (csa_settings)
- ieee80211_send_action_csa(sdata, csa_settings);
-
return BSS_CHANGED_BEACON;
out:
return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index cf38a74..9b5f1bc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1400,8 +1400,7 @@ void ieee80211_mesh_work(struct ieee80211_sub_if_data *sdata);
void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
struct sk_buff *skb);
int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_csa_settings *csa_settings,
- bool csa_action);
+ struct cfg80211_csa_settings *csa_settings);
int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata);

/* scan/BSS handling */
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 5476ad9..e604b9e 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -1064,8 +1064,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
}

int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
- struct cfg80211_csa_settings *csa_settings,
- bool csa_action)
+ struct cfg80211_csa_settings *csa_settings)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
struct mesh_csa_settings *tmp_csa_settings;
@@ -1089,9 +1088,6 @@ int ieee80211_mesh_csa_beacon(struct ieee80211_sub_if_data *sdata,
return ret;
}

- if (csa_action)
- ieee80211_send_action_csa(sdata, csa_settings);
-
return BSS_CHANGED_BEACON;
}

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6d59e21..b86a679 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2397,15 +2397,6 @@ static int ieee80211_beacon_add_tim(struct ieee80211_sub_if_data *sdata,
return 0;
}

-void ieee80211_csa_finish(struct ieee80211_vif *vif)
-{
- struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
-
- ieee80211_queue_work(&sdata->local->hw,
- &sdata->csa_finalize_work);
-}
-EXPORT_SYMBOL(ieee80211_csa_finish);
-
static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata,
struct beacon_data *beacon)
{
@@ -2434,8 +2425,12 @@ static void ieee80211_update_csa(struct ieee80211_sub_if_data *sdata,
if (WARN_ON(counter_offset_beacon >= beacon_data_len))
return;

- /* warn if the driver did not check for/react to csa completeness */
- if (WARN_ON(beacon_data[counter_offset_beacon] == 0))
+ /* Warn if the driver did not check for/react to csa
+ * completeness. A beacon with CSA counter set to 0 should
+ * never occur, because a counter of 1 means switch just
+ * before the next beacon.
+ */
+ if (WARN_ON(beacon_data[counter_offset_beacon] == 1))
return;

beacon_data[counter_offset_beacon]--;
@@ -2501,7 +2496,7 @@ bool ieee80211_csa_is_complete(struct ieee80211_vif *vif)
if (WARN_ON(counter_beacon > beacon_data_len))
goto out;

- if (beacon_data[counter_beacon] == 0)
+ if (beacon_data[counter_beacon] == 1)
ret = true;
out:
rcu_read_unlock();
--
1.8.4.2


2013-12-04 09:45:10

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] mac80211: only set CSA beacon when at least one beacon must be transmitted

On Wed, Dec 4, 2013 at 5:35 PM, Yeoh Chun-Yeow <[email protected]> wrote:
>> #ifdef CONFIG_MAC80211_MESH
>> case NL80211_IFTYPE_MESH_POINT:
>> @@ -3159,12 +3208,18 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
>> if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
>> ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
>>
>> - err = ieee80211_mesh_csa_beacon(sdata, params,
>> - (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
>> - if (err < 0) {
>> - ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
>> - return err;
>> + if (params->count > 1) {
>> + err = ieee80211_mesh_csa_beacon(sdata, params);
>> + if (err < 0) {
>> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
>> + return err;
>> + }
>> + changed |= err;
>> }
>
> Since the ieee80211_mesh_csa_beacon is not called, the kfree_rcu of
> tmp_csa_settings in ieee80211_mesh_finish_csa has caused kernel oops.
>
> Anyway, I would like to suggest if the params->count is less than 1,
> the mesh channel switch is not triggered since there maybe mesh STA
> located two hops away from the initiator.
>
> if (params->count > 1) {
> .....
> } else
> return -EINVAL;

maybe try to push the checking of params->count earlier, such as follow:

if (params->count < 2)
return -EINVAL;

----
Chun-Yeow

2013-12-03 13:48:29

by Luciano Coelho

[permalink] [raw]
Subject: [PATCH v6 2/5] mac80211: align ieee80211_ibss_csa_beacon() with ieee80211_csa_beacon()

The return value of ieee80211_ibss_csa_beacon is not aligned with the
return value of ieee80211_csa_beacon(). For consistency and to be
able to use both functions with similar code, change
ieee80211_ibss_csa_beacon() not to send the bss changed notification
itself, but return what has changed so the caller can send the
notification instead.

Signed-off-by: Luciano Coelho <[email protected]>
---
net/mac80211/ibss.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index 3514aab..23e035f 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -522,7 +522,7 @@ int ieee80211_ibss_csa_beacon(struct ieee80211_sub_if_data *sdata,
if (csa_settings)
ieee80211_send_action_csa(sdata, csa_settings);

- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
+ return BSS_CHANGED_BEACON;
out:
return ret;
}
@@ -559,11 +559,17 @@ int ieee80211_ibss_finish_csa(struct ieee80211_sub_if_data *sdata)

/* generate the beacon */
err = ieee80211_ibss_csa_beacon(sdata, NULL);
- sdata_unlock(sdata);
if (err < 0)
- return err;
+ goto out;

- return 0;
+ if (err) {
+ ieee80211_bss_info_change_notify(sdata, err);
+ err = 0;
+ }
+out:
+ sdata_unlock(sdata);
+
+ return err;
}

void ieee80211_ibss_stop(struct ieee80211_sub_if_data *sdata)
--
1.8.4.2


2013-12-05 09:03:07

by Luciano Coelho

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] mac80211: only set CSA beacon when at least one beacon must be transmitted

T24gV2VkLCAyMDEzLTEyLTA0IGF0IDE3OjQ1ICswODAwLCBZZW9oIENodW4tWWVvdyB3cm90ZToN
Cj4gT24gV2VkLCBEZWMgNCwgMjAxMyBhdCA1OjM1IFBNLCBZZW9oIENodW4tWWVvdyA8eWVvaGNo
dW55ZW93QGdtYWlsLmNvbT4gd3JvdGU6DQo+ID4+ICAjaWZkZWYgQ09ORklHX01BQzgwMjExX01F
U0gNCj4gPj4gICAgICAgICBjYXNlIE5MODAyMTFfSUZUWVBFX01FU0hfUE9JTlQ6DQo+ID4+IEBA
IC0zMTU5LDEyICszMjA4LDE4IEBAIGludCBpZWVlODAyMTFfY2hhbm5lbF9zd2l0Y2goc3RydWN0
IHdpcGh5ICp3aXBoeSwgc3RydWN0IG5ldF9kZXZpY2UgKmRldiwNCj4gPj4gICAgICAgICAgICAg
ICAgIGlmIChpZm1zaC0+Y3NhX3JvbGUgPT0gSUVFRTgwMjExX01FU0hfQ1NBX1JPTEVfTk9ORSkN
Cj4gPj4gICAgICAgICAgICAgICAgICAgICAgICAgaWZtc2gtPmNzYV9yb2xlID0gSUVFRTgwMjEx
X01FU0hfQ1NBX1JPTEVfSU5JVDsNCj4gPj4NCj4gPj4gLSAgICAgICAgICAgICAgIGVyciA9IGll
ZWU4MDIxMV9tZXNoX2NzYV9iZWFjb24oc2RhdGEsIHBhcmFtcywNCj4gPj4gLSAgICAgICAgICAg
ICAgICAgICAgICAgKGlmbXNoLT5jc2Ffcm9sZSA9PSBJRUVFODAyMTFfTUVTSF9DU0FfUk9MRV9J
TklUKSk7DQo+ID4+IC0gICAgICAgICAgICAgICBpZiAoZXJyIDwgMCkgew0KPiA+PiAtICAgICAg
ICAgICAgICAgICAgICAgICBpZm1zaC0+Y3NhX3JvbGUgPSBJRUVFODAyMTFfTUVTSF9DU0FfUk9M
RV9OT05FOw0KPiA+PiAtICAgICAgICAgICAgICAgICAgICAgICByZXR1cm4gZXJyOw0KPiA+PiAr
ICAgICAgICAgICAgICAgaWYgKHBhcmFtcy0+Y291bnQgPiAxKSB7DQo+ID4+ICsgICAgICAgICAg
ICAgICAgICAgICAgIGVyciA9IGllZWU4MDIxMV9tZXNoX2NzYV9iZWFjb24oc2RhdGEsIHBhcmFt
cyk7DQo+ID4+ICsgICAgICAgICAgICAgICAgICAgICAgIGlmIChlcnIgPCAwKSB7DQo+ID4+ICsg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgaWZtc2gtPmNzYV9yb2xlID0gSUVFRTgwMjEx
X01FU0hfQ1NBX1JPTEVfTk9ORTsNCj4gPj4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICByZXR1cm4gZXJyOw0KPiA+PiArICAgICAgICAgICAgICAgICAgICAgICB9DQo+ID4+ICsgICAg
ICAgICAgICAgICAgICAgICAgIGNoYW5nZWQgfD0gZXJyOw0KPiA+PiAgICAgICAgICAgICAgICAg
fQ0KPiA+DQo+ID4gU2luY2UgdGhlIGllZWU4MDIxMV9tZXNoX2NzYV9iZWFjb24gaXMgbm90IGNh
bGxlZCwgdGhlIGtmcmVlX3JjdSBvZg0KPiA+IHRtcF9jc2Ffc2V0dGluZ3MgaW4gaWVlZTgwMjEx
X21lc2hfZmluaXNoX2NzYSBoYXMgY2F1c2VkIGtlcm5lbCBvb3BzLg0KDQpVZ2gsIHJpZ2h0LCBt
eSBiYWQuDQoNCg0KDQo+ID4gQW55d2F5LCBJIHdvdWxkIGxpa2UgdG8gc3VnZ2VzdCBpZiB0aGUg
cGFyYW1zLT5jb3VudCBpcyBsZXNzIHRoYW4gMSwNCj4gPiB0aGUgbWVzaCBjaGFubmVsIHN3aXRj
aCBpcyBub3QgdHJpZ2dlcmVkIHNpbmNlIHRoZXJlIG1heWJlIG1lc2ggU1RBDQo+ID4gbG9jYXRl
ZCB0d28gaG9wcyBhd2F5IGZyb20gdGhlIGluaXRpYXRvci4NCj4gPg0KPiA+IGlmIChwYXJhbXMt
PmNvdW50ID4gMSkgew0KPiA+IC4uLi4uDQo+ID4gfSBlbHNlDQo+ID4gcmV0dXJuIC1FSU5WQUw7
DQo+IA0KPiBtYXliZSB0cnkgdG8gcHVzaCB0aGUgY2hlY2tpbmcgb2YgcGFyYW1zLT5jb3VudCBl
YXJsaWVyLCBzdWNoIGFzIGZvbGxvdzoNCj4gDQo+IGlmIChwYXJhbXMtPmNvdW50IDwgMikNCj4g
ICAgICAgcmV0dXJuIC1FSU5WQUw7DQoNCkFjdHVhbGx5LCBJIGp1c3QgbG9va2VkIGF0IHRoZSBz
cGVjcyBhZ2FpbiBhbmQgdGhlIGNvdW50IHZhbHVlIGZvciBtZXNoDQppcyBjb21wbGV0ZWx5IGRp
ZmZlcmVudC4gIEluc3RlYWQgb2Ygc3BlY2lmeWluZyB0aGUgbnVtYmVyIG9mIFRCVFRzDQp1bnRp
bCB0aGUgc3dpdGNoIGhhcHBlbnMsIGl0IHNwZWNpZmllcyB0aGUgdGltZSBpbiBUVXMgdW50aWwg
dGhlIHN3aXRjaA0KaGFwcGVucyBhbmQgMCBtZWFucyBhdCBhbnkgdGltZSBhZnRlciB0aGUgZnJh
bWUgd2FzIHNlbnQgKHRoZSBzYW1lIGFzDQp3aXRoIG5vbm1lc2gpLiAgSW4gb3VyIGltcGxlbWVu
dGF0aW9uLCB3ZSBqdXN0IGRlY3JlYXNlIHRoZSBjb3VudCBvbg0KZXZlcnkgYmVhY29uLCB3aGlj
aCBpcyB3cm9uZyBpbiB0aGUgbWVzaCBjYXNlLg0KDQpUaGUgY291bnQgaW1wbGVtZW50YXRpb24g
Zm9yIG1lc2ggaXMgYm9ndXMuICBXZSBuZWVkIHRvIGZpeCB0aGF0Lg0KDQpXaGF0IGlmLCBmb3Ig
bm93LCBJIGp1c3QgZml4IHRoZSBvb3BzIGJlY2F1c2Ugb2YgdGhlIHJjdV9kZXJlZmVuY2Ugd2hl
bg0KdGhlIHJjdSBpcyBub3QgaW5pdGlhbGl6ZWQgYW5kIHdlIGZpeCB0aGUgY291bnQgaW1wbGVt
ZW50YXRpb24gbGF0ZXI/DQoNClRoYW5rcyBhIGxvdCBmb3IgdGVzdGluZyENCg0KLS0NCkNoZWVy
cywNCkx1Y2EuDQo=

2013-12-04 09:35:58

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH v6 5/5] mac80211: only set CSA beacon when at least one beacon must be transmitted

> #ifdef CONFIG_MAC80211_MESH
> case NL80211_IFTYPE_MESH_POINT:
> @@ -3159,12 +3208,18 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
> if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
> ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
>
> - err = ieee80211_mesh_csa_beacon(sdata, params,
> - (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
> - if (err < 0) {
> - ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
> - return err;
> + if (params->count > 1) {
> + err = ieee80211_mesh_csa_beacon(sdata, params);
> + if (err < 0) {
> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
> + return err;
> + }
> + changed |= err;
> }

Since the ieee80211_mesh_csa_beacon is not called, the kfree_rcu of
tmp_csa_settings in ieee80211_mesh_finish_csa has caused kernel oops.

Anyway, I would like to suggest if the params->count is less than 1,
the mesh channel switch is not triggered since there maybe mesh STA
located two hops away from the initiator.

if (params->count > 1) {
.....
} else
return -EINVAL;




> +
> + if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT)
> + ieee80211_send_action_csa(sdata, params);
> +
> break;

----
Chun-Yeow

2013-12-04 09:36:18

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: Re: [PATCH v6 3/5] mac80211: refactor ieee80211_mesh_process_chanswitch()

Hi, Luciano

This patch applies clean on mac80211-next, but to able to test the
mesh channel switch, patch entitled "mac80211: fix the mesh channel
switch support" is required.

So I have used the patch v4 for the testing instead of this one.

----
Chun-Yeow

On Tue, Dec 3, 2013 at 9:48 PM, Luciano Coelho <[email protected]> wrote:
> Refactor ieee80211_mesh_process_chanswitch() to use
> ieee80211_channel_switch() and avoid code duplication.
>
> Signed-off-by: Luciano Coelho <[email protected]>
> ---
> net/mac80211/cfg.c | 10 +++++--
> net/mac80211/ieee80211_i.h | 6 +++-
> net/mac80211/mesh.c | 68 ++++++++++------------------------------------
> net/mac80211/util.c | 1 -
> 4 files changed, 27 insertions(+), 58 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index b5bc27e..790c15d 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -3156,9 +3156,15 @@ int ieee80211_channel_switch(struct wiphy *wiphy, struct net_device *dev,
> params->chandef.chan->band)
> return -EINVAL;
>
> - err = ieee80211_mesh_csa_beacon(sdata, params, true);
> - if (err < 0)
> + if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_NONE)
> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_INIT;
> +
> + err = ieee80211_mesh_csa_beacon(sdata, params,
> + (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT));
> + if (err < 0) {
> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
> return err;
> + }
> break;
> #endif
> default:
> diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
> index a7b4d55..cf38a74 100644
> --- a/net/mac80211/ieee80211_i.h
> +++ b/net/mac80211/ieee80211_i.h
> @@ -611,7 +611,11 @@ struct ieee80211_if_mesh {
> struct ps_data ps;
> /* Channel Switching Support */
> struct mesh_csa_settings __rcu *csa;
> - bool chsw_init;
> + enum {
> + IEEE80211_MESH_CSA_ROLE_NONE,
> + IEEE80211_MESH_CSA_ROLE_INIT,
> + IEEE80211_MESH_CSA_ROLE_REPEATER,
> + } csa_role;
> u8 chsw_ttl;
> u16 pre_value;
> };
> diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
> index 330d1f7..980cc12 100644
> --- a/net/mac80211/mesh.c
> +++ b/net/mac80211/mesh.c
> @@ -685,7 +685,7 @@ ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
> *pos++ = csa->settings.count;
> *pos++ = WLAN_EID_CHAN_SWITCH_PARAM;
> *pos++ = 6;
> - if (ifmsh->chsw_init) {
> + if (ifmsh->csa_role == IEEE80211_MESH_CSA_ROLE_INIT) {
> *pos++ = ifmsh->mshcfg.dot11MeshTTL;
> *pos |= WLAN_EID_CHAN_SWITCH_PARAM_INITIATOR;
> } else {
> @@ -853,19 +853,11 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
> {
> struct cfg80211_csa_settings params;
> struct ieee80211_csa_ie csa_ie;
> - struct ieee80211_chanctx_conf *chanctx_conf;
> - struct ieee80211_chanctx *chanctx;
> struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
> enum ieee80211_band band = ieee80211_get_sdata_band(sdata);
> - int err, num_chanctx;
> + int err;
> u32 sta_flags;
>
> - if (sdata->vif.csa_active)
> - return true;
> -
> - if (!ifmsh->mesh_id)
> - return false;
> -
> sta_flags = IEEE80211_STA_DISABLE_VHT;
> switch (sdata->vif.bss_conf.chandef.width) {
> case NL80211_CHAN_WIDTH_20_NOHT:
> @@ -890,10 +882,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
> params.chandef = csa_ie.chandef;
> params.count = csa_ie.count;
>
> - if (sdata->vif.bss_conf.chandef.chan->band !=
> - params.chandef.chan->band)
> - return false;
> -
> if (!cfg80211_chandef_usable(sdata->local->hw.wiphy, &params.chandef,
> IEEE80211_CHAN_DISABLED)) {
> sdata_info(sdata,
> @@ -916,25 +904,6 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
> return false;
> }
>
> - rcu_read_lock();
> - chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> - if (!chanctx_conf)
> - goto failed_chswitch;
> -
> - /* don't handle for multi-VIF cases */
> - chanctx = container_of(chanctx_conf, struct ieee80211_chanctx, conf);
> - if (chanctx->refcount > 1)
> - goto failed_chswitch;
> -
> - num_chanctx = 0;
> - list_for_each_entry_rcu(chanctx, &sdata->local->chanctx_list, list)
> - num_chanctx++;
> -
> - if (num_chanctx > 1)
> - goto failed_chswitch;
> -
> - rcu_read_unlock();
> -
> mcsa_dbg(sdata,
> "received channel switch announcement to go to channel %d MHz\n",
> params.chandef.chan->center_freq);
> @@ -945,27 +914,16 @@ ieee80211_mesh_process_chnswitch(struct ieee80211_sub_if_data *sdata,
> else
> ifmsh->chsw_ttl = 0;
>
> - if (ifmsh->chsw_ttl > 0)
> - if (ieee80211_mesh_csa_beacon(sdata, &params, false) < 0)
> - return false;
> -
> - sdata->csa_radar_required = params.radar_required;
> -
> - if (params.block_tx)
> - ieee80211_stop_queues_by_reason(&sdata->local->hw,
> - IEEE80211_MAX_QUEUE_MAP,
> - IEEE80211_QUEUE_STOP_REASON_CSA);
> + if (ifmsh->chsw_ttl == 0)
> + return false;
>
> - sdata->csa_chandef = params.chandef;
> - sdata->vif.csa_active = true;
> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_REPEATER;
>
> - ieee80211_bss_info_change_notify(sdata, err);
> - drv_channel_switch_beacon(sdata, &params.chandef);
> + if (ieee80211_channel_switch(sdata->local->hw.wiphy, sdata->dev,
> + &params) < 0)
> + return false;
>
> return true;
> -failed_chswitch:
> - rcu_read_unlock();
> - return false;
> }
>
> static void
> @@ -1075,7 +1033,8 @@ static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
> ifmsh->sync_ops->rx_bcn_presp(sdata,
> stype, mgmt, &elems, rx_status);
>
> - if (!ifmsh->chsw_init)
> + if (ifmsh->csa_role != IEEE80211_MESH_CSA_ROLE_INIT &&
> + !sdata->vif.csa_active)
> ieee80211_mesh_process_chnswitch(sdata, &elems, true);
> }
>
> @@ -1086,7 +1045,7 @@ int ieee80211_mesh_finish_csa(struct ieee80211_sub_if_data *sdata)
> int ret = 0;
>
> /* Reset the TTL value and Initiator flag */
> - ifmsh->chsw_init = false;
> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
> ifmsh->chsw_ttl = 0;
>
> /* Remove the CSA and MCSP elements from the beacon */
> @@ -1200,7 +1159,8 @@ static void mesh_rx_csa_frame(struct ieee80211_sub_if_data *sdata,
>
> ifmsh->pre_value = pre_value;
>
> - if (!ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
> + if (!sdata->vif.csa_active &&
> + !ieee80211_mesh_process_chnswitch(sdata, &elems, false)) {
> mcsa_dbg(sdata, "Failed to process CSA action frame");
> return;
> }
> @@ -1355,7 +1315,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
> mesh_rmc_init(sdata);
> ifmsh->last_preq = jiffies;
> ifmsh->next_perr = jiffies;
> - ifmsh->chsw_init = false;
> + ifmsh->csa_role = IEEE80211_MESH_CSA_ROLE_NONE;
> /* Allocate all mesh structures when creating the first mesh interface. */
> if (!mesh_allocated)
> ieee80211s_init();
> diff --git a/net/mac80211/util.c b/net/mac80211/util.c
> index 06265d7..f153ab4 100644
> --- a/net/mac80211/util.c
> +++ b/net/mac80211/util.c
> @@ -2487,7 +2487,6 @@ int ieee80211_send_action_csa(struct ieee80211_sub_if_data *sdata,
> ifmsh->pre_value++;
> put_unaligned_le16(ifmsh->pre_value, pos);/* Precedence Value */
> pos += 2;
> - ifmsh->chsw_init = true;
> }
>
> ieee80211_tx_skb(sdata, skb);
> --
> 1.8.4.2
>