2013-02-11 21:08:43

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 1/3] mac80211: consolidate MBSS change notification

A few mesh utility functions will call
ieee80211_bss_info_change_notify(), and then the caller
might notify the driver of the same change again. Avoid
this redundancy by propagating the BSS changes and
generally calling bss_info_change_notify() once per
change.

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/cfg.c | 19 +++++++++++--------
net/mac80211/mesh.c | 2 +-
net/mac80211/mesh.h | 10 +++++-----
net/mac80211/mesh_plink.c | 33 ++++++++++++++++++---------------
net/mac80211/mesh_ps.c | 24 ++++++++++++++++++------
5 files changed, 53 insertions(+), 35 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 8f6b593..c5a6704 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1261,8 +1261,8 @@ static int sta_apply_parameters(struct ieee80211_local *local,

if (ieee80211_vif_is_mesh(&sdata->vif)) {
#ifdef CONFIG_MAC80211_MESH
+ u32 changed = 0;
if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED) {
- u32 changed = 0;

switch (params->plink_state) {
case NL80211_PLINK_ESTAB:
@@ -1272,8 +1272,8 @@ static int sta_apply_parameters(struct ieee80211_local *local,
sta->plink_state = params->plink_state;

ieee80211_mps_sta_status_update(sta);
- ieee80211_mps_set_sta_local_pm(sta,
- sdata->u.mesh.mshcfg.power_mode);
+ changed |= ieee80211_mps_set_sta_local_pm(sta,
+ sdata->u.mesh.mshcfg.power_mode);
break;
case NL80211_PLINK_LISTEN:
case NL80211_PLINK_BLOCKED:
@@ -1287,26 +1287,29 @@ static int sta_apply_parameters(struct ieee80211_local *local,
sta->plink_state = params->plink_state;

ieee80211_mps_sta_status_update(sta);
- ieee80211_mps_local_status_update(sdata);
+ changed |=
+ ieee80211_mps_local_status_update(sdata);
break;
default:
/* nothing */
break;
}
- ieee80211_bss_info_change_notify(sdata, changed);
} else {
switch (params->plink_action) {
case PLINK_ACTION_OPEN:
- mesh_plink_open(sta);
+ changed |= mesh_plink_open(sta);
break;
case PLINK_ACTION_BLOCK:
- mesh_plink_block(sta);
+ changed |= mesh_plink_block(sta);
break;
}
}

if (params->local_pm)
- ieee80211_mps_set_sta_local_pm(sta, params->local_pm);
+ changed |=
+ ieee80211_mps_set_sta_local_pm(sta,
+ params->local_pm);
+ ieee80211_bss_info_change_notify(sdata, changed);
#endif
}

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 2bf0158..0adec3d 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -675,7 +675,7 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
sdata->vif.bss_conf.basic_rates =
ieee80211_mandatory_rates(local, band);

- ieee80211_mps_local_status_update(sdata);
+ changed |= ieee80211_mps_local_status_update(sdata);

ieee80211_bss_info_change_notify(sdata, changed);

diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 3b9d862..7ad035f 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -245,9 +245,9 @@ void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh);
const struct ieee80211_mesh_sync_ops *ieee80211_mesh_sync_ops_get(u8 method);

/* mesh power save */
-void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata);
-void ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
- enum nl80211_mesh_power_mode pm);
+u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata);
+u32 ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
+ enum nl80211_mesh_power_mode pm);
void ieee80211_mps_set_frame_flags(struct ieee80211_sub_if_data *sdata,
struct sta_info *sta,
struct ieee80211_hdr *hdr);
@@ -289,8 +289,8 @@ bool mesh_peer_accepts_plinks(struct ieee802_11_elems *ie);
u32 mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata);
void mesh_plink_broken(struct sta_info *sta);
u32 mesh_plink_deactivate(struct sta_info *sta);
-int mesh_plink_open(struct sta_info *sta);
-void mesh_plink_block(struct sta_info *sta);
+u32 mesh_plink_open(struct sta_info *sta);
+u32 mesh_plink_block(struct sta_info *sta);
void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt, size_t len,
struct ieee80211_rx_status *rx_status);
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index 56c9b31..a4c7a7e 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -202,7 +202,7 @@ static u32 __mesh_plink_deactivate(struct sta_info *sta)
mesh_path_flush_by_nexthop(sta);

ieee80211_mps_sta_status_update(sta);
- ieee80211_mps_local_status_update(sdata);
+ changed |= ieee80211_mps_local_status_update(sdata);

return changed;
}
@@ -494,6 +494,7 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
struct ieee802_11_elems *elems)
{
struct sta_info *sta;
+ u32 changed = 0;

sta = mesh_sta_info_get(sdata, hw_addr, elems);
if (!sta)
@@ -504,11 +505,12 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
sdata->u.mesh.accepting_plinks &&
sdata->u.mesh.mshcfg.auto_open_plinks &&
rssi_threshold_check(sta, sdata))
- mesh_plink_open(sta);
+ changed = mesh_plink_open(sta);

ieee80211_mps_frame_release(sta, elems);
out:
rcu_read_unlock();
+ ieee80211_bss_info_change_notify(sdata, changed);
}

static void mesh_plink_timer(unsigned long data)
@@ -621,13 +623,14 @@ static inline void mesh_plink_timer_set(struct sta_info *sta, int timeout)
add_timer(&sta->plink_timer);
}

-int mesh_plink_open(struct sta_info *sta)
+u32 mesh_plink_open(struct sta_info *sta)
{
__le16 llid;
struct ieee80211_sub_if_data *sdata = sta->sdata;
+ u32 changed;

if (!test_sta_flag(sta, WLAN_STA_AUTH))
- return -EPERM;
+ return 0;

spin_lock_bh(&sta->lock);
get_random_bytes(&llid, 2);
@@ -635,7 +638,7 @@ int mesh_plink_open(struct sta_info *sta)
if (sta->plink_state != NL80211_PLINK_LISTEN &&
sta->plink_state != NL80211_PLINK_BLOCKED) {
spin_unlock_bh(&sta->lock);
- return -EBUSY;
+ return 0;
}
sta->plink_state = NL80211_PLINK_OPN_SNT;
mesh_plink_timer_set(sta, sdata->u.mesh.mshcfg.dot11MeshRetryTimeout);
@@ -645,15 +648,15 @@ int mesh_plink_open(struct sta_info *sta)
sta->sta.addr);

/* set the non-peer mode to active during peering */
- ieee80211_mps_local_status_update(sdata);
+ changed = ieee80211_mps_local_status_update(sdata);

- return mesh_plink_frame_tx(sdata, WLAN_SP_MESH_PEERING_OPEN,
- sta->sta.addr, llid, 0, 0);
+ mesh_plink_frame_tx(sdata, WLAN_SP_MESH_PEERING_OPEN,
+ sta->sta.addr, llid, 0, 0);
+ return changed;
}

-void mesh_plink_block(struct sta_info *sta)
+u32 mesh_plink_block(struct sta_info *sta)
{
- struct ieee80211_sub_if_data *sdata = sta->sdata;
u32 changed;

spin_lock_bh(&sta->lock);
@@ -661,7 +664,7 @@ void mesh_plink_block(struct sta_info *sta)
sta->plink_state = NL80211_PLINK_BLOCKED;
spin_unlock_bh(&sta->lock);

- ieee80211_bss_info_change_notify(sdata, changed);
+ return changed;
}


@@ -882,7 +885,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
mshcfg->dot11MeshRetryTimeout);

/* set the non-peer mode to active during peering */
- ieee80211_mps_local_status_update(sdata);
+ changed |= ieee80211_mps_local_status_update(sdata);

spin_unlock_bh(&sta->lock);
mesh_plink_frame_tx(sdata,
@@ -978,7 +981,7 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
mpl_dbg(sdata, "Mesh plink with %pM ESTABLISHED\n",
sta->sta.addr);
ieee80211_mps_sta_status_update(sta);
- ieee80211_mps_set_sta_local_pm(sta,
+ changed |= ieee80211_mps_set_sta_local_pm(sta,
mshcfg->power_mode);
break;
default:
@@ -1020,8 +1023,8 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
WLAN_SP_MESH_PEERING_CONFIRM,
sta->sta.addr, llid, plid, 0);
ieee80211_mps_sta_status_update(sta);
- ieee80211_mps_set_sta_local_pm(sta,
- mshcfg->power_mode);
+ changed |= ieee80211_mps_set_sta_local_pm(sta,
+ mshcfg->power_mode);
break;
default:
spin_unlock_bh(&sta->lock);
diff --git a/net/mac80211/mesh_ps.c b/net/mac80211/mesh_ps.c
index b677962..aa83eac 100644
--- a/net/mac80211/mesh_ps.c
+++ b/net/mac80211/mesh_ps.c
@@ -74,14 +74,17 @@ static void mps_qos_null_tx(struct sta_info *sta)
* @sdata: local mesh subif
*
* sets the non-peer power mode and triggers the driver PS (re-)configuration
+ * Return BSS_CHANGED_BEACON if a beacon update is necessary.
*/
-void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
+u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
struct sta_info *sta;
bool peering = false;
int light_sleep_cnt = 0;
int deep_sleep_cnt = 0;
+ u32 changed = 0;
+ enum nl80211_mesh_power_mode nonpeer_pm;

rcu_read_lock();
list_for_each_entry_rcu(sta, &sdata->local->sta_list, list) {
@@ -115,17 +118,25 @@ void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
*/
if (peering) {
mps_dbg(sdata, "setting non-peer PM to active for peering\n");
- ifmsh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
+ nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
} else if (light_sleep_cnt || deep_sleep_cnt) {
mps_dbg(sdata, "setting non-peer PM to deep sleep\n");
- ifmsh->nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
+ nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
} else {
mps_dbg(sdata, "setting non-peer PM to user value\n");
- ifmsh->nonpeer_pm = ifmsh->mshcfg.power_mode;
+ nonpeer_pm = ifmsh->mshcfg.power_mode;
}

+ if (ifmsh->nonpeer_pm != nonpeer_pm ||
+ ifmsh->ps_peers_light_sleep != light_sleep_cnt ||
+ ifmsh->ps_peers_deep_sleep != deep_sleep_cnt)
+ changed = BSS_CHANGED_BEACON;
+
+ ifmsh->nonpeer_pm = nonpeer_pm;
ifmsh->ps_peers_light_sleep = light_sleep_cnt;
ifmsh->ps_peers_deep_sleep = deep_sleep_cnt;
+
+ return changed;
}

/**
@@ -133,8 +144,9 @@ void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
*
* @sta: mesh STA
* @pm: the power mode to set
+ * Return BSS_CHANGED_BEACON if a beacon update is in order.
*/
-void ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
+u32 ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
enum nl80211_mesh_power_mode pm)
{
struct ieee80211_sub_if_data *sdata = sta->sdata;
@@ -151,7 +163,7 @@ void ieee80211_mps_set_sta_local_pm(struct sta_info *sta,
if (sta->plink_state == NL80211_PLINK_ESTAB)
mps_qos_null_tx(sta);

- ieee80211_mps_local_status_update(sdata);
+ return ieee80211_mps_local_status_update(sdata);
}

/**
--
1.7.10.4



2013-02-11 21:08:48

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 3/3] mac80211: generate mesh probe responses

Mesh interfaces will now respond to any broadcast (or
matching directed mesh) probe requests with a probe
response.

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/mesh.c | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
net/mac80211/rx.c | 5 ++--
2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index ffbe1d5..0bb7754 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -862,6 +862,66 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
sdata->u.mesh.timers_running = 0;
}

+static void
+ieee80211_mesh_rx_probe_req(struct ieee80211_sub_if_data *sdata,
+ struct ieee80211_mgmt *mgmt, size_t len)
+{
+ struct ieee80211_local *local = sdata->local;
+ struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
+ struct sk_buff *presp;
+ struct beacon_data *bcn;
+ struct ieee80211_mgmt *hdr;
+ struct ieee802_11_elems elems;
+ size_t baselen;
+ u8 *pos, *end;
+
+ end = ((u8 *) mgmt) + len;
+ pos = mgmt->u.probe_req.variable;
+ baselen = (u8 *) pos - (u8 *) mgmt;
+ if (baselen > len)
+ return;
+
+ ieee802_11_parse_elems(pos, len - baselen, &elems);
+
+ /* 802.11-2012 10.1.4.3.2 */
+ if ((!ether_addr_equal(mgmt->da, sdata->vif.addr) &&
+ !is_broadcast_ether_addr(mgmt->da)) ||
+ elems.ssid_len != 0)
+ return;
+
+ if (elems.mesh_id_len != 0 &&
+ (elems.mesh_id_len != ifmsh->mesh_id_len ||
+ memcmp(elems.mesh_id, ifmsh->mesh_id, ifmsh->mesh_id_len))) {
+ mpl_dbg(sdata, "ignoring probe request for different MBSS\n");
+ return;
+ }
+
+ rcu_read_lock();
+ bcn = rcu_dereference(ifmsh->beacon);
+
+ if (!bcn)
+ goto out;
+
+ presp = dev_alloc_skb(local->tx_headroom +
+ bcn->head_len + bcn->tail_len);
+ if (WARN_ON(!presp))
+ goto out;
+
+ skb_reserve(presp, local->tx_headroom);
+ memcpy(skb_put(presp, bcn->head_len), bcn->head, bcn->head_len);
+ memcpy(skb_put(presp, bcn->tail_len), bcn->tail, bcn->tail_len);
+ hdr = (struct ieee80211_mgmt *) presp->data;
+ hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
+ IEEE80211_STYPE_PROBE_RESP);
+ memcpy(hdr->da, mgmt->sa, ETH_ALEN);
+ mpl_dbg(sdata, "sending probe resp. to %pM\n", hdr->da);
+ IEEE80211_SKB_CB(presp)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
+ ieee80211_tx_skb(sdata, presp);
+out:
+ rcu_read_unlock();
+ return;
+}
+
static void ieee80211_mesh_rx_bcn_presp(struct ieee80211_sub_if_data *sdata,
u16 stype,
struct ieee80211_mgmt *mgmt,
@@ -951,6 +1011,9 @@ void ieee80211_mesh_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
ieee80211_mesh_rx_bcn_presp(sdata, stype, mgmt, skb->len,
rx_status);
break;
+ case IEEE80211_STYPE_PROBE_REQ:
+ ieee80211_mesh_rx_probe_req(sdata, mgmt, skb->len);
+ break;
case IEEE80211_STYPE_ACTION:
ieee80211_mesh_rx_mgmt_action(sdata, mgmt, skb->len, rx_status);
break;
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index b5f1bba..34aa975 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2692,8 +2692,9 @@ ieee80211_rx_h_mgmt(struct ieee80211_rx_data *rx)
return RX_DROP_MONITOR;
break;
case cpu_to_le16(IEEE80211_STYPE_PROBE_REQ):
- /* process only for ibss */
- if (sdata->vif.type != NL80211_IFTYPE_ADHOC)
+ /* process only for ibss and mesh */
+ if (sdata->vif.type != NL80211_IFTYPE_ADHOC &&
+ sdata->vif.type != NL80211_IFTYPE_MESH_POINT)
return RX_DROP_MONITOR;
break;
default:
--
1.7.10.4


2013-02-12 17:56:40

by Marco Porsch

[permalink] [raw]
Subject: Re: [PATCH 1/3] mac80211: consolidate MBSS change notification

On 02/11/2013 10:07 PM, Thomas Pedersen wrote:
> A few mesh utility functions will call
> ieee80211_bss_info_change_notify(), and then the caller
> might notify the driver of the same change again. Avoid
> this redundancy by propagating the BSS changes and
> generally calling bss_info_change_notify() once per
> change.
>
> Signed-off-by: Thomas Pedersen <[email protected]>
> ---
> net/mac80211/cfg.c | 19 +++++++++++--------
> net/mac80211/mesh.c | 2 +-
> net/mac80211/mesh.h | 10 +++++-----
> net/mac80211/mesh_plink.c | 33 ++++++++++++++++++---------------
> net/mac80211/mesh_ps.c | 24 ++++++++++++++++++------
> 5 files changed, 53 insertions(+), 35 deletions(-)

[...]

> diff --git a/net/mac80211/mesh_ps.c b/net/mac80211/mesh_ps.c
> index b677962..aa83eac 100644
> --- a/net/mac80211/mesh_ps.c
> +++ b/net/mac80211/mesh_ps.c
> @@ -74,14 +74,17 @@ static void mps_qos_null_tx(struct sta_info *sta)
> * @sdata: local mesh subif
> *
> * sets the non-peer power mode and triggers the driver PS (re-)configuration
> + * Return BSS_CHANGED_BEACON if a beacon update is necessary.
> */
> -void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
> +u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
> {
> struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
> struct sta_info *sta;
> bool peering = false;
> int light_sleep_cnt = 0;
> int deep_sleep_cnt = 0;
> + u32 changed = 0;
> + enum nl80211_mesh_power_mode nonpeer_pm;
>
> rcu_read_lock();
> list_for_each_entry_rcu(sta, &sdata->local->sta_list, list) {
> @@ -115,17 +118,25 @@ void ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata)
> */
> if (peering) {
> mps_dbg(sdata, "setting non-peer PM to active for peering\n");
> - ifmsh->nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
> + nonpeer_pm = NL80211_MESH_POWER_ACTIVE;
> } else if (light_sleep_cnt || deep_sleep_cnt) {
> mps_dbg(sdata, "setting non-peer PM to deep sleep\n");
> - ifmsh->nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
> + nonpeer_pm = NL80211_MESH_POWER_DEEP_SLEEP;
> } else {
> mps_dbg(sdata, "setting non-peer PM to user value\n");
> - ifmsh->nonpeer_pm = ifmsh->mshcfg.power_mode;
> + nonpeer_pm = ifmsh->mshcfg.power_mode;
> }
>
> + if (ifmsh->nonpeer_pm != nonpeer_pm ||
> + ifmsh->ps_peers_light_sleep != light_sleep_cnt ||
> + ifmsh->ps_peers_deep_sleep != deep_sleep_cnt)
> + changed = BSS_CHANGED_BEACON;

Here it only affects the beacon if the number of light/deep sleep peers
changed from/to zero or non-zero.
The following should avoid some unnecessary updates:

!ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt ||
!ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt)

> +
> + ifmsh->nonpeer_pm = nonpeer_pm;
> ifmsh->ps_peers_light_sleep = light_sleep_cnt;
> ifmsh->ps_peers_deep_sleep = deep_sleep_cnt;
> +
> + return changed;
> }
>
> /**


2013-02-11 21:08:46

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 2/3] mac80211: cache mesh beacon

Previously, the entire mesh beacon would be generated each
time the beacon timer fired. Instead generate a beacon
head and tail (so the TIM can easily be inserted when mesh
power save is on) when starting a mesh or the MBSS
parameters change.

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/cfg.c | 5 +-
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/mesh.c | 145 +++++++++++++++++++++++++++++++++++++++++++-
net/mac80211/mesh.h | 3 +
net/mac80211/mesh_plink.c | 4 +-
net/mac80211/tx.c | 61 +++----------------
6 files changed, 159 insertions(+), 60 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c5a6704..dd70568 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1802,11 +1802,10 @@ static int ieee80211_update_mesh_config(struct wiphy *wiphy,
conf->power_mode = nconf->power_mode;
ieee80211_mps_local_status_update(sdata);
}
- if (_chg_mesh_attr(NL80211_MESHCONF_AWAKE_WINDOW, mask)) {
+ if (_chg_mesh_attr(NL80211_MESHCONF_AWAKE_WINDOW, mask))
conf->dot11MeshAwakeWindowDuration =
nconf->dot11MeshAwakeWindowDuration;
- ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON);
- }
+ ieee80211_mbss_info_change_notify(sdata, BSS_CHANGED_BEACON);
return 0;
}

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 5635dfc..5f71ebe 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -578,6 +578,7 @@ struct ieee80211_if_mesh {
u32 mesh_seqnum;
bool accepting_plinks;
int num_gates;
+ struct beacon_data __rcu *beacon;
const u8 *ie;
u8 ie_len;
enum {
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 0adec3d..ffbe1d5 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -171,7 +171,7 @@ void mesh_sta_cleanup(struct sta_info *sta)
}

if (changed)
- ieee80211_bss_info_change_notify(sdata, changed);
+ ieee80211_mbss_info_change_notify(sdata, changed);
}

int mesh_rmc_init(struct ieee80211_sub_if_data *sdata)
@@ -593,7 +593,7 @@ static void ieee80211_mesh_housekeeping(struct ieee80211_sub_if_data *sdata,
mesh_path_expire(sdata);

changed = mesh_accept_plinks_update(sdata);
- ieee80211_bss_info_change_notify(sdata, changed);
+ ieee80211_mbss_info_change_notify(sdata, changed);

mod_timer(&ifmsh->housekeeping_timer,
round_jiffies(jiffies + IEEE80211_MESH_HOUSEKEEPING_INTERVAL));
@@ -644,6 +644,140 @@ void ieee80211_mesh_restart(struct ieee80211_sub_if_data *sdata)
}
#endif

+static int
+ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
+{
+ struct beacon_data *bcn;
+ int head_len, tail_len;
+ struct sk_buff *skb;
+ struct ieee80211_mgmt *mgmt;
+ struct ieee80211_chanctx_conf *chanctx_conf;
+ enum ieee80211_band band;
+ u8 *pos;
+ struct ieee80211_sub_if_data *sdata;
+ int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
+ sizeof(mgmt->u.beacon);
+
+ sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
+ rcu_read_lock();
+ chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
+ band = chanctx_conf->def.chan->band;
+ rcu_read_unlock();
+
+ head_len = hdr_len +
+ 2 + /* NULL SSID */
+ 2 + 8 + /* supported rates */
+ 2 + 3; /* DS params */
+ tail_len = 2 + (IEEE80211_MAX_SUPP_RATES - 8) +
+ 2 + sizeof(struct ieee80211_ht_cap) +
+ 2 + sizeof(struct ieee80211_ht_operation) +
+ 2 + ifmsh->mesh_id_len +
+ 2 + sizeof(struct ieee80211_meshconf_ie) +
+ 2 + sizeof(__le16) + /* awake window */
+ ifmsh->ie_len;
+
+ bcn = kzalloc(sizeof(*bcn) + head_len + tail_len, GFP_KERNEL);
+ /* need an skb for IE builders to operate on */
+ skb = dev_alloc_skb(max(head_len, tail_len));
+
+ if (!bcn || !skb)
+ goto out_free;
+
+ /*
+ * pointers go into the block we allocated,
+ * memory is | beacon_data | head | tail |
+ */
+ bcn->head = ((u8 *) bcn) + sizeof(*bcn);
+
+ /* fill in the head */
+ mgmt = (struct ieee80211_mgmt *) skb_put(skb, hdr_len);
+ memset(mgmt, 0, hdr_len);
+ mgmt->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
+ IEEE80211_STYPE_BEACON);
+ eth_broadcast_addr(mgmt->da);
+ memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
+ memcpy(mgmt->bssid, sdata->vif.addr, ETH_ALEN);
+ mgmt->u.beacon.beacon_int =
+ cpu_to_le16(sdata->vif.bss_conf.beacon_int);
+ mgmt->u.beacon.capab_info |= cpu_to_le16(
+ sdata->u.mesh.security ? WLAN_CAPABILITY_PRIVACY : 0);
+
+ pos = skb_put(skb, 2);
+ *pos++ = WLAN_EID_SSID;
+ *pos++ = 0x0;
+
+ if (ieee80211_add_srates_ie(sdata, skb, true, band) ||
+ mesh_add_ds_params_ie(skb, sdata)) {
+ mpl_dbg(sdata, "couldn't add beacon head IEs\n");
+ goto out_free;
+ }
+ bcn->head_len = skb->len;
+ memcpy(bcn->head, skb->data, bcn->head_len);
+
+ /* now the tail */
+ skb_trim(skb, 0);
+ bcn->tail = bcn->head + bcn->head_len;
+
+ if (ieee80211_add_ext_srates_ie(sdata, skb, true, band) ||
+ mesh_add_rsn_ie(skb, sdata) ||
+ mesh_add_ht_cap_ie(skb, sdata) ||
+ mesh_add_ht_oper_ie(skb, sdata) ||
+ mesh_add_meshid_ie(skb, sdata) ||
+ mesh_add_meshconf_ie(skb, sdata) ||
+ mesh_add_awake_window_ie(skb, sdata) ||
+ mesh_add_vendor_ies(skb, sdata)) {
+ mpl_dbg(sdata, "couldn't add beacon tail IEs!\n");
+ goto out_free;
+ }
+ bcn->tail_len = skb->len;
+ memcpy(bcn->tail, skb->data, bcn->tail_len);
+
+ dev_kfree_skb(skb);
+ rcu_assign_pointer(ifmsh->beacon, bcn);
+ return 0;
+out_free:
+ kfree(bcn);
+ dev_kfree_skb(skb);
+ return -ENOMEM;
+}
+
+static int
+ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
+{
+ struct ieee80211_sub_if_data *sdata;
+ struct beacon_data *old_bcn;
+ int ret;
+ sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
+
+ rcu_read_lock();
+ old_bcn = rcu_dereference(ifmsh->beacon);
+ ret = ieee80211_mesh_build_beacon(ifmsh);
+ if (ret) {
+ mpl_dbg(sdata, "couldn't rebuild mesh beacon! %d\n", ret);
+ /* just reuse old beacon */
+ goto out;
+ }
+
+ kfree_rcu(old_bcn, rcu_head);
+out:
+ rcu_read_unlock();
+ return ret;
+}
+
+void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
+ u32 changed)
+{
+ if (sdata->vif.bss_conf.enable_beacon &&
+ (changed & (BSS_CHANGED_BEACON |
+ BSS_CHANGED_HT |
+ BSS_CHANGED_BASIC_RATES |
+ BSS_CHANGED_BEACON_INT)))
+ if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
+ return;
+ ieee80211_bss_info_change_notify(sdata, changed);
+ return;
+}
+
void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
@@ -677,6 +811,11 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)

changed |= ieee80211_mps_local_status_update(sdata);

+ if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {
+ ieee80211_stop_mesh(sdata);
+ return;
+ }
+
ieee80211_bss_info_change_notify(sdata, changed);

netif_carrier_on(sdata->dev);
@@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
sdata->vif.bss_conf.enable_beacon = false;
clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
+ kfree_rcu(ifmsh->beacon, rcu_head);

/* flush STAs and mpaths on this iface */
sta_info_flush(sdata);
@@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
skb_queue_head_init(&ifmsh->ps.bc_buf);
spin_lock_init(&ifmsh->mesh_preq_queue_lock);
spin_lock_init(&ifmsh->sync_offset_lock);
+ RCU_INIT_POINTER(ifmsh->beacon, NULL);

sdata->vif.bss_conf.bssid = zero_addr;
}
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index 7ad035f..3ce00ba 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -243,6 +243,9 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata);
void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata);
void ieee80211_mesh_root_setup(struct ieee80211_if_mesh *ifmsh);
const struct ieee80211_mesh_sync_ops *ieee80211_mesh_sync_ops_get(u8 method);
+/* wrapper for ieee80211_bss_info_change_notify() */
+void ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
+ u32 changed);

/* mesh power save */
u32 ieee80211_mps_local_status_update(struct ieee80211_sub_if_data *sdata);
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index a4c7a7e..13983b6 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -510,7 +510,7 @@ void mesh_neighbour_update(struct ieee80211_sub_if_data *sdata,
ieee80211_mps_frame_release(sta, elems);
out:
rcu_read_unlock();
- ieee80211_bss_info_change_notify(sdata, changed);
+ ieee80211_mbss_info_change_notify(sdata, changed);
}

static void mesh_plink_timer(unsigned long data)
@@ -1092,5 +1092,5 @@ void mesh_rx_plink_frame(struct ieee80211_sub_if_data *sdata, struct ieee80211_m
rcu_read_unlock();

if (changed)
- ieee80211_bss_info_change_notify(sdata, changed);
+ ieee80211_mbss_info_change_notify(sdata, changed);
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 2ef0e19..1da61fd 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2431,71 +2431,26 @@ struct sk_buff *ieee80211_beacon_get_tim(struct ieee80211_hw *hw,
hdr->frame_control = cpu_to_le16(IEEE80211_FTYPE_MGMT |
IEEE80211_STYPE_BEACON);
} else if (ieee80211_vif_is_mesh(&sdata->vif)) {
- struct ieee80211_mgmt *mgmt;
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
- u8 *pos;
- int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
- sizeof(mgmt->u.beacon);
+ struct beacon_data *bcn = rcu_dereference(ifmsh->beacon);

-#ifdef CONFIG_MAC80211_MESH
- if (!sdata->u.mesh.mesh_id_len)
+ if (!bcn)
goto out;
-#endif

if (ifmsh->sync_ops)
ifmsh->sync_ops->adjust_tbtt(
sdata);

skb = dev_alloc_skb(local->tx_headroom +
- hdr_len +
- 2 + /* NULL SSID */
- 2 + 8 + /* supported rates */
- 2 + 3 + /* DS params */
+ bcn->head_len +
256 + /* TIM IE */
- 2 + (IEEE80211_MAX_SUPP_RATES - 8) +
- 2 + sizeof(struct ieee80211_ht_cap) +
- 2 + sizeof(struct ieee80211_ht_operation) +
- 2 + sdata->u.mesh.mesh_id_len +
- 2 + sizeof(struct ieee80211_meshconf_ie) +
- sdata->u.mesh.ie_len +
- 2 + sizeof(__le16)); /* awake window */
+ bcn->tail_len);
if (!skb)
goto out;
-
- skb_reserve(skb, local->hw.extra_tx_headroom);
- mgmt = (struct ieee80211_mgmt *) skb_put(skb, hdr_len);
- memset(mgmt, 0, hdr_len);
- mgmt->frame_control =
- cpu_to_le16(IEEE80211_FTYPE_MGMT | IEEE80211_STYPE_BEACON);
- eth_broadcast_addr(mgmt->da);
- memcpy(mgmt->sa, sdata->vif.addr, ETH_ALEN);
- memcpy(mgmt->bssid, sdata->vif.addr, ETH_ALEN);
- ieee80211_mps_set_frame_flags(sdata, NULL, (void *) mgmt);
- mgmt->u.beacon.beacon_int =
- cpu_to_le16(sdata->vif.bss_conf.beacon_int);
- mgmt->u.beacon.capab_info |= cpu_to_le16(
- sdata->u.mesh.security ? WLAN_CAPABILITY_PRIVACY : 0);
-
- pos = skb_put(skb, 2);
- *pos++ = WLAN_EID_SSID;
- *pos++ = 0x0;
-
- band = chanctx_conf->def.chan->band;
-
- if (ieee80211_add_srates_ie(sdata, skb, true, band) ||
- mesh_add_ds_params_ie(skb, sdata) ||
- ieee80211_beacon_add_tim(sdata, &ifmsh->ps, skb) ||
- ieee80211_add_ext_srates_ie(sdata, skb, true, band) ||
- mesh_add_rsn_ie(skb, sdata) ||
- mesh_add_ht_cap_ie(skb, sdata) ||
- mesh_add_ht_oper_ie(skb, sdata) ||
- mesh_add_meshid_ie(skb, sdata) ||
- mesh_add_meshconf_ie(skb, sdata) ||
- mesh_add_awake_window_ie(skb, sdata) ||
- mesh_add_vendor_ies(skb, sdata)) {
- pr_err("o11s: couldn't add ies!\n");
- goto out;
- }
+ skb_reserve(skb, local->tx_headroom);
+ memcpy(skb_put(skb, bcn->head_len), bcn->head, bcn->head_len);
+ ieee80211_beacon_add_tim(sdata, &ifmsh->ps, skb);
+ memcpy(skb_put(skb, bcn->tail_len), bcn->tail, bcn->tail_len);
} else {
WARN_ON(1);
goto out;
--
1.7.10.4


2013-02-11 21:21:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: generate mesh probe responses

On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:

> + if (elems.mesh_id_len != 0 &&
> + (elems.mesh_id_len != ifmsh->mesh_id_len ||
> + memcmp(elems.mesh_id, ifmsh->mesh_id, ifmsh->mesh_id_len))) {
> + mpl_dbg(sdata, "ignoring probe request for different MBSS\n");

> + presp = dev_alloc_skb(local->tx_headroom +
> + bcn->head_len + bcn->tail_len);
> + if (WARN_ON(!presp))
> + goto out;

same as before.


> + mpl_dbg(sdata, "sending probe resp. to %pM\n", hdr->da);

I also think you should probably drop both mpl_dbg() statements as
they're likely to just be noise for any other debugging when you
actually use this?

johannes


2013-02-11 21:17:56

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/3] mac80211: cache mesh beacon

On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:

> +static int
> +ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
> +{
> + struct beacon_data *bcn;
> + int head_len, tail_len;
> + struct sk_buff *skb;
> + struct ieee80211_mgmt *mgmt;
> + struct ieee80211_chanctx_conf *chanctx_conf;
> + enum ieee80211_band band;
> + u8 *pos;
> + struct ieee80211_sub_if_data *sdata;
> + int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
> + sizeof(mgmt->u.beacon);
> +
> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> + rcu_read_lock();

This is weird since you already did it outside the function?

> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> + band = chanctx_conf->def.chan->band;

could be NULL? I guess not at this point though.

> +static int
> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
> +{
> + struct ieee80211_sub_if_data *sdata;
> + struct beacon_data *old_bcn;
> + int ret;
> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> +
> + rcu_read_lock();
> + old_bcn = rcu_dereference(ifmsh->beacon);
> + ret = ieee80211_mesh_build_beacon(ifmsh);

This looks totally wrong. You must protect the assignment to
ifmsg->beacon by some lock, so then you don't need the rcu_read_lock()
here since you're under that lock, so this should be
rcu_dereference_protected(..., lockdep_is_held(whatever_lock));

> + if (sdata->vif.bss_conf.enable_beacon &&
> + (changed & (BSS_CHANGED_BEACON |
> + BSS_CHANGED_HT |
> + BSS_CHANGED_BASIC_RATES |
> + BSS_CHANGED_BEACON_INT)))
> + if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
> + return;

Does that return make any sense?

> changed |= ieee80211_mps_local_status_update(sdata);
>
> + if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {

no warning for memory allocation failures please

> @@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
> sdata->vif.bss_conf.enable_beacon = false;
> clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
> ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
> + kfree_rcu(ifmsh->beacon, rcu_head);

I think you should set it to NULL first, just so it's clearer.

> @@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
> skb_queue_head_init(&ifmsh->ps.bc_buf);
> spin_lock_init(&ifmsh->mesh_preq_queue_lock);
> spin_lock_init(&ifmsh->sync_offset_lock);
> + RCU_INIT_POINTER(ifmsh->beacon, NULL);

Isn't everything initialized to 0/null?

johannes


2013-02-11 21:34:17

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 2/3] mac80211: cache mesh beacon

On Mon, 2013-02-11 at 13:24 -0800, Thomas Pedersen wrote:

> >> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> >> + rcu_read_lock();
> >
> > This is weird since you already did it outside the function?
>
> Yes, but we shouldn't rely on the caller creating an RCU read section?

I don't really see a problem with that, but the other locking issue
means that you need this anyway.

> >> +static int
> >> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
> >> +{
> >> + struct ieee80211_sub_if_data *sdata;
> >> + struct beacon_data *old_bcn;
> >> + int ret;
> >> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> >> +
> >> + rcu_read_lock();
> >> + old_bcn = rcu_dereference(ifmsh->beacon);
> >> + ret = ieee80211_mesh_build_beacon(ifmsh);
> >
> > This looks totally wrong. You must protect the assignment to
> > ifmsg->beacon by some lock, so then you don't need the rcu_read_lock()
> > here since you're under that lock, so this should be
> > rcu_dereference_protected(..., lockdep_is_held(whatever_lock));
>
> OK, I guess we better protect assignment by some lock then :)

I'm sure there's some lock already? Otherwise doing mesh operations from
userspace and the workqueue would probably be quite racy?

> >> + if (sdata->vif.bss_conf.enable_beacon &&
> >> + (changed & (BSS_CHANGED_BEACON |
> >> + BSS_CHANGED_HT |
> >> + BSS_CHANGED_BASIC_RATES |
> >> + BSS_CHANGED_BEACON_INT)))
> >> + if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
> >> + return;
> >
> > Does that return make any sense?
>
> The alternative is to keep notifying the driver. I just wanted to stop
> everything since we're out of memory, but we can keep calling
> bss_info_change_notify() is you think that makes more sense.

Either way is fine to me.

> >> @@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
> >> sdata->vif.bss_conf.enable_beacon = false;
> >> clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
> >> ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
> >> + kfree_rcu(ifmsh->beacon, rcu_head);
> >
> > I think you should set it to NULL first, just so it's clearer.
>
> For who? It seems there is no need in this path, but OK.

You don't have any synchronize_rcu() here so how can you be sure there's
not someone, say in a tasklet, using ifmsh->beacon at this point?

> >> @@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
> >> skb_queue_head_init(&ifmsh->ps.bc_buf);
> >> spin_lock_init(&ifmsh->mesh_preq_queue_lock);
> >> spin_lock_init(&ifmsh->sync_offset_lock);
> >> + RCU_INIT_POINTER(ifmsh->beacon, NULL);
> >
> > Isn't everything initialized to 0/null?
>
> Yep, I wanted any needed RCU magic to happen here though.

I don't think there's any RCU magic, particularly not with
RCU_INIT_POINTER, but I don't mind the assignment much :)

johannes


2013-02-11 21:24:56

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH 2/3] mac80211: cache mesh beacon

On Mon, Feb 11, 2013 at 1:17 PM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:
>
>> +static int
>> +ieee80211_mesh_build_beacon(struct ieee80211_if_mesh *ifmsh)
>> +{
>> + struct beacon_data *bcn;
>> + int head_len, tail_len;
>> + struct sk_buff *skb;
>> + struct ieee80211_mgmt *mgmt;
>> + struct ieee80211_chanctx_conf *chanctx_conf;
>> + enum ieee80211_band band;
>> + u8 *pos;
>> + struct ieee80211_sub_if_data *sdata;
>> + int hdr_len = offsetof(struct ieee80211_mgmt, u.beacon) +
>> + sizeof(mgmt->u.beacon);
>> +
>> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
>> + rcu_read_lock();
>
> This is weird since you already did it outside the function?

Yes, but we shouldn't rely on the caller creating an RCU read section?

>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> + band = chanctx_conf->def.chan->band;
>
> could be NULL? I guess not at this point though.

Yeah it will have been set.

>> +static int
>> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
>> +{
>> + struct ieee80211_sub_if_data *sdata;
>> + struct beacon_data *old_bcn;
>> + int ret;
>> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
>> +
>> + rcu_read_lock();
>> + old_bcn = rcu_dereference(ifmsh->beacon);
>> + ret = ieee80211_mesh_build_beacon(ifmsh);
>
> This looks totally wrong. You must protect the assignment to
> ifmsg->beacon by some lock, so then you don't need the rcu_read_lock()
> here since you're under that lock, so this should be
> rcu_dereference_protected(..., lockdep_is_held(whatever_lock));

OK, I guess we better protect assignment by some lock then :)

>> + if (sdata->vif.bss_conf.enable_beacon &&
>> + (changed & (BSS_CHANGED_BEACON |
>> + BSS_CHANGED_HT |
>> + BSS_CHANGED_BASIC_RATES |
>> + BSS_CHANGED_BEACON_INT)))
>> + if (ieee80211_mesh_rebuild_beacon(&sdata->u.mesh))
>> + return;
>
> Does that return make any sense?

The alternative is to keep notifying the driver. I just wanted to stop
everything since we're out of memory, but we can keep calling
bss_info_change_notify() is you think that makes more sense.

>> changed |= ieee80211_mps_local_status_update(sdata);
>>
>> + if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {
>
> no warning for memory allocation failures please

OK.

>> @@ -694,6 +833,7 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
>> sdata->vif.bss_conf.enable_beacon = false;
>> clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
>> ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED);
>> + kfree_rcu(ifmsh->beacon, rcu_head);
>
> I think you should set it to NULL first, just so it's clearer.

For who? It seems there is no need in this path, but OK.

>> @@ -883,6 +1023,7 @@ void ieee80211_mesh_init_sdata(struct ieee80211_sub_if_data *sdata)
>> skb_queue_head_init(&ifmsh->ps.bc_buf);
>> spin_lock_init(&ifmsh->mesh_preq_queue_lock);
>> spin_lock_init(&ifmsh->sync_offset_lock);
>> + RCU_INIT_POINTER(ifmsh->beacon, NULL);
>
> Isn't everything initialized to 0/null?

Yep, I wanted any needed RCU magic to happen here though.

--
Thomas

2013-02-12 18:23:43

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH 1/3] mac80211: consolidate MBSS change notification

On Tue, Feb 12, 2013 at 9:56 AM, Marco Porsch <[email protected]> wrote:
> On 02/11/2013 10:07 PM, Thomas Pedersen wrote:
>> + if (ifmsh->nonpeer_pm != nonpeer_pm ||
>> + ifmsh->ps_peers_light_sleep != light_sleep_cnt ||
>> + ifmsh->ps_peers_deep_sleep != deep_sleep_cnt)
>> + changed = BSS_CHANGED_BEACON;
>
>
> Here it only affects the beacon if the number of light/deep sleep peers
> changed from/to zero or non-zero.
> The following should avoid some unnecessary updates:
>
> !ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt ||
> !ifmsh->ps_peers_deep_sleep != !deep_sleep_cnt)

Ah nice. I'll add this to the v2, not checking deep_sleep_cnt twice of course :)

--
Thomas

2013-02-11 22:11:45

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH 3/3] mac80211: generate mesh probe responses

On Mon, Feb 11, 2013 at 1:20 PM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2013-02-11 at 13:07 -0800, Thomas Pedersen wrote:
>
>> + if (elems.mesh_id_len != 0 &&
>> + (elems.mesh_id_len != ifmsh->mesh_id_len ||
>> + memcmp(elems.mesh_id, ifmsh->mesh_id, ifmsh->mesh_id_len))) {
>> + mpl_dbg(sdata, "ignoring probe request for different MBSS\n");
>
>> + presp = dev_alloc_skb(local->tx_headroom +
>> + bcn->head_len + bcn->tail_len);
>> + if (WARN_ON(!presp))
>> + goto out;
>
> same as before.
>
>
>> + mpl_dbg(sdata, "sending probe resp. to %pM\n", hdr->da);
>
> I also think you should probably drop both mpl_dbg() statements as
> they're likely to just be noise for any other debugging when you
> actually use this?

OK. I'll replace them with sdata_info() then.

--
Thomas