2013-02-03 01:03:26

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 0/3] implement active mesh scanning

This set improves the mesh scanning functionality by:

- responding to broadcast or directed probe requests
- allowing the user to generate a directed mesh probe request, ie:
iw mesh0 scan ssid foo
on a mesh interface will send probe requests where the ssid "foo" has
been filled into the mesh ID IE.

Also patch 1 caches the mesh beacon in a struct beacon_data each time it
changes so probe responses can be quickly built. This is also convenient when
the TIM IE becomes needed for PS.

Applies on mac80211-next/master

Thomas Pedersen (3):
mac80211: cache mesh beacon
mac80211: generate mesh probe requests
mac80211: generate mesh probe responses

net/mac80211/cfg.c | 2 +
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/mesh.c | 199 +++++++++++++++++++++++++++++++++++++++++++-
net/mac80211/mesh.h | 3 +
net/mac80211/mesh_plink.c | 6 +-
net/mac80211/rx.c | 5 +-
net/mac80211/tx.c | 72 +++++-----------
net/mac80211/util.c | 1 +
8 files changed, 229 insertions(+), 60 deletions(-)

--
1.7.10.4



2013-02-04 17:36:51

by Johannes Berg

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


> +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();
> +
> + RCU_INIT_POINTER(ifmsh->beacon, NULL);
> + synchronize_rcu();

That doesn't seem right? Why force to NULL and synchronize, instead of
just building an update and overwriting the old beacon with the new,
using kfree_rcu() to get rid of the old afterwards? synchronize_rcu() is
quite expensive (might take hundreds of milliseconds.)

Also, doesn't this just leak the old one?

> + /* need an skb for IE builders to operate on */
> + skb = dev_alloc_skb(max(head_len, tail_len));

Heh. Might consider changing the IE builder functions?

> +static int
> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
> +{
> + struct ieee80211_sub_if_data *sdata;
> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
> +
> + rcu_read_lock();
> + kfree(rcu_dereference(ifmsh->beacon));
> + rcu_read_unlock();
> +
> + if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {

The warning is probably not a good idea since it's really for allocation
failures only which already print long messages.

> + mpl_dbg(sdata, "couldn't rebuild mesh beacon, stopping!\n");
> + ieee80211_stop_mesh(sdata);

I'm not sure that's such a good idea? Nothing in userspace would expect
to randomly stop the mesh.

> + return -1;

Why not have a proper error code and propagate it properly? :)

johannes


2013-02-04 18:16:38

by Thomas Pedersen

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

On Mon, Feb 4, 2013 at 10:03 AM, Johannes Berg
<[email protected]> wrote:
> On Mon, 2013-02-04 at 09:58 -0800, Thomas Pedersen wrote:
>> On Mon, Feb 4, 2013 at 9:38 AM, Johannes Berg <[email protected]> wrote:
>> > On Sat, 2013-02-02 at 17:02 -0800, Thomas Pedersen wrote:
>> >
>> >> - pos = skb_put(skb, ie_ssid_len);
>> >> - *pos++ = WLAN_EID_SSID;
>> >> + if (ieee80211_vif_is_mesh(vif)) {
>> >> + pos = skb_put(skb, 2 + 2 + ssid_len);
>> >> + *pos++ = WLAN_EID_SSID;
>> >> + *pos++ = 0;
>> >> + /* NOTE: mesh ID will be out of order */
>> >
>> > Why put it out of order?
>> >
>> > Also I'm not convinced that it's a good idea to translate "SSID" from
>> > the userspace API to "mesh ID" silently? Might make more sense to have
>> > those separately maybe? I mean, it seems reasonable to even think you
>> > might scan for a mesh network when you're not a mesh interface, for
>> > example?
>>
>> Yeah that makes more sense, but will obviously require more work.
>> I'll just drop this for now.
>
> I guess the other question is if you actually want this at all. I mean,
> if you just do patch 1 and 3, then unless you want to actively scan for
> multiple networks you can just include the mesh ID in the IE parameter.
> Seems for many purposes that could be acceptable already. Now I'm not
> saying that you shouldn't put it into the kernel, there certainly could
> be value in that, just saying that in terms of effort that might be
> easier?

Yeah, allowing userspace to specify the mesh ID IEs would be easy for now.

Thanks,
--
Thomas

2013-02-04 18:12:55

by Johannes Berg

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

On Mon, 2013-02-04 at 10:09 -0800, Thomas Pedersen wrote:
> On Mon, Feb 4, 2013 at 9:37 AM, Johannes Berg <[email protected]> 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();
> >> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
> >> + band = chanctx_conf->def.chan->band;
> >> + rcu_read_unlock();
> >> +
> >> + RCU_INIT_POINTER(ifmsh->beacon, NULL);
> >> + synchronize_rcu();
> >
> > That doesn't seem right? Why force to NULL and synchronize, instead of
> > just building an update and overwriting the old beacon with the new,
> > using kfree_rcu() to get rid of the old afterwards? synchronize_rcu() is
> > quite expensive (might take hundreds of milliseconds.)
>
> OK, I just copied this from the IBSS code. Overwriting the old one
> makes sense though.

What? I should check that out. OTOH, IBSS code never really changes its
beacon during operation, I think, so it might not matter much there?

> >> + /* need an skb for IE builders to operate on */
> >> + skb = dev_alloc_skb(max(head_len, tail_len));
> >
> > Heh. Might consider changing the IE builder functions?
>
> Some IEs are variable length though, so I like how they append (and
> can check for space) the right length to skb->len.

Fair enough. It seemed a bit odd but I'm not worried about it.

> >> + mpl_dbg(sdata, "couldn't rebuild mesh beacon, stopping!\n");
> >> + ieee80211_stop_mesh(sdata);
> >
> > I'm not sure that's such a good idea? Nothing in userspace would expect
> > to randomly stop the mesh.
>
> So if rebuilding failed, just continue with the old beacon?

That seems acceptable to me. You're probably running into bigger issues
if allocating a little bit of memory here already fails.

johannes


2013-02-03 01:03:28

by Thomas Pedersen

[permalink] [raw]
Subject: [PATCH 1/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 | 2 +
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/mesh.c | 136 +++++++++++++++++++++++++++++++++++++++++++-
net/mac80211/mesh.h | 3 +
net/mac80211/mesh_plink.c | 6 +-
net/mac80211/tx.c | 57 +++----------------
6 files changed, 151 insertions(+), 54 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 661b878..f3ca170 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1777,6 +1777,8 @@ static int ieee80211_update_mesh_config(struct wiphy *wiphy,
if (_chg_mesh_attr(NL80211_MESHCONF_HWMP_CONFIRMATION_INTERVAL, mask))
conf->dot11MeshHWMPconfirmationInterval =
nconf->dot11MeshHWMPconfirmationInterval;
+ /* need to update the IEs as well */
+ 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 c9c66de..48efad8 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -581,6 +581,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 f920da1..e08abc0 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -547,7 +547,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));
@@ -598,6 +598,136 @@ 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();
+
+ RCU_INIT_POINTER(ifmsh->beacon, NULL);
+ synchronize_rcu();
+
+ 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) +
+ 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_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;
+ sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
+
+ rcu_read_lock();
+ kfree(rcu_dereference(ifmsh->beacon));
+ rcu_read_unlock();
+
+ if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {
+ mpl_dbg(sdata, "couldn't rebuild mesh beacon, stopping!\n");
+ ieee80211_stop_mesh(sdata);
+ return -1;
+ }
+ return 0;
+}
+
+int 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 -1;
+ ieee80211_bss_info_change_notify(sdata, changed);
+ return 0;
+}
+
void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
@@ -629,7 +759,8 @@ void ieee80211_start_mesh(struct ieee80211_sub_if_data *sdata)
sdata->vif.bss_conf.basic_rates =
ieee80211_mandatory_rates(local, band);

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

netif_carrier_on(sdata->dev);
}
@@ -646,6 +777,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(ifmsh->beacon);

/* flush STAs and mpaths on this iface */
sta_info_flush(sdata);
diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h
index aff3015..e6377ec 100644
--- a/net/mac80211/mesh.h
+++ b/net/mac80211/mesh.h
@@ -241,6 +241,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() */
+int ieee80211_mbss_info_change_notify(struct ieee80211_sub_if_data *sdata,
+ u32 changed);

/* Mesh paths */
int mesh_nexthop_lookup(struct sk_buff *skb,
diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c
index 6787d69..4708170 100644
--- a/net/mac80211/mesh_plink.c
+++ b/net/mac80211/mesh_plink.c
@@ -224,7 +224,7 @@ void mesh_plink_deactivate(struct sta_info *sta)
sta->reason);
spin_unlock_bh(&sta->lock);

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

static int mesh_plink_frame_tx(struct ieee80211_sub_if_data *sdata,
@@ -647,7 +647,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);
+ ieee80211_mbss_info_change_notify(sdata, changed);
}


@@ -1065,5 +1065,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 a2cb6a3..01dc001 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2424,66 +2424,25 @@ 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 */
- 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);
+ bcn->head_len +
+ 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);
- 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_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_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);
+ /* TODO: add TIM */
+ memcpy(skb_put(skb, bcn->tail_len), bcn->tail, bcn->tail_len);
} else {
WARN_ON(1);
goto out;
--
1.7.10.4


2013-02-03 01:03:30

by Thomas Pedersen

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

A mesh probe request must include the mesh ID IE and a
wildcard SSID IE. Add this and transpose the SSID scan
list into mesh IDs for mesh interfaces.

Also allow the user to specify a mesh ID IE in the user
scan IEs, since additional SSID IEs seem to be allowed.

Signed-off-by: Thomas Pedersen <[email protected]>
---
net/mac80211/tx.c | 15 +++++++++++----
net/mac80211/util.c | 1 +
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 01dc001..9f3e9ba 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2622,11 +2622,18 @@ struct sk_buff *ieee80211_probereq_get(struct ieee80211_hw *hw,
memcpy(hdr->addr2, vif->addr, ETH_ALEN);
eth_broadcast_addr(hdr->addr3);

- pos = skb_put(skb, ie_ssid_len);
- *pos++ = WLAN_EID_SSID;
+ if (ieee80211_vif_is_mesh(vif)) {
+ pos = skb_put(skb, 2 + 2 + ssid_len);
+ *pos++ = WLAN_EID_SSID;
+ *pos++ = 0;
+ /* NOTE: mesh ID will be out of order */
+ *pos++ = WLAN_EID_MESH_ID;
+ } else {
+ pos = skb_put(skb, 2 + ssid_len);
+ *pos++ = WLAN_EID_SSID;
+ }
*pos++ = ssid_len;
- if (ssid_len)
- memcpy(pos, ssid, ssid_len);
+ memcpy(pos, ssid, ssid_len);
pos += ssid_len;

return skb;
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 139ad9b..aeb0f88 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1181,6 +1181,7 @@ int ieee80211_build_preq_ies(struct ieee80211_local *local, u8 *buffer,
if (ie && ie_len) {
static const u8 before_ht[] = {
WLAN_EID_SSID,
+ WLAN_EID_MESH_ID,
WLAN_EID_SUPP_RATES,
WLAN_EID_REQUEST,
WLAN_EID_EXT_SUPP_RATES,
--
1.7.10.4


2013-02-04 17:58:21

by Thomas Pedersen

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

On Mon, Feb 4, 2013 at 9:38 AM, Johannes Berg <[email protected]> wrote:
> On Sat, 2013-02-02 at 17:02 -0800, Thomas Pedersen wrote:
>
>> - pos = skb_put(skb, ie_ssid_len);
>> - *pos++ = WLAN_EID_SSID;
>> + if (ieee80211_vif_is_mesh(vif)) {
>> + pos = skb_put(skb, 2 + 2 + ssid_len);
>> + *pos++ = WLAN_EID_SSID;
>> + *pos++ = 0;
>> + /* NOTE: mesh ID will be out of order */
>
> Why put it out of order?
>
> Also I'm not convinced that it's a good idea to translate "SSID" from
> the userspace API to "mesh ID" silently? Might make more sense to have
> those separately maybe? I mean, it seems reasonable to even think you
> might scan for a mesh network when you're not a mesh interface, for
> example?

Yeah that makes more sense, but will obviously require more work.
I'll just drop this for now.

--
Thomas

2013-02-04 18:10:15

by Thomas Pedersen

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

On Mon, Feb 4, 2013 at 9:37 AM, Johannes Berg <[email protected]> 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();
>> + chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf);
>> + band = chanctx_conf->def.chan->band;
>> + rcu_read_unlock();
>> +
>> + RCU_INIT_POINTER(ifmsh->beacon, NULL);
>> + synchronize_rcu();
>
> That doesn't seem right? Why force to NULL and synchronize, instead of
> just building an update and overwriting the old beacon with the new,
> using kfree_rcu() to get rid of the old afterwards? synchronize_rcu() is
> quite expensive (might take hundreds of milliseconds.)

OK, I just copied this from the IBSS code. Overwriting the old one
makes sense though.

> Also, doesn't this just leak the old one?

ieee80211_mesh_rebuild_beacon() will free it, but maybe that doesn't make sense.

>> + /* need an skb for IE builders to operate on */
>> + skb = dev_alloc_skb(max(head_len, tail_len));
>
> Heh. Might consider changing the IE builder functions?

Some IEs are variable length though, so I like how they append (and
can check for space) the right length to skb->len.

>> +static int
>> +ieee80211_mesh_rebuild_beacon(struct ieee80211_if_mesh *ifmsh)
>> +{
>> + struct ieee80211_sub_if_data *sdata;
>> + sdata = container_of(ifmsh, struct ieee80211_sub_if_data, u.mesh);
>> +
>> + rcu_read_lock();
>> + kfree(rcu_dereference(ifmsh->beacon));
>> + rcu_read_unlock();
>> +
>> + if (WARN_ON(ieee80211_mesh_build_beacon(ifmsh))) {
>
> The warning is probably not a good idea since it's really for allocation
> failures only which already print long messages.

OK.

>> + mpl_dbg(sdata, "couldn't rebuild mesh beacon, stopping!\n");
>> + ieee80211_stop_mesh(sdata);
>
> I'm not sure that's such a good idea? Nothing in userspace would expect
> to randomly stop the mesh.

So if rebuilding failed, just continue with the old beacon?

>> + return -1;
>
> Why not have a proper error code and propagate it properly? :)

OK.

--
Thomas

2013-02-03 01:03:33

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 e08abc0..697ebc1 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -802,6 +802,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,
@@ -891,6 +951,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 a190895..226ae06 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2677,8 +2677,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-04 17:38:18

by Johannes Berg

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

On Sat, 2013-02-02 at 17:02 -0800, Thomas Pedersen wrote:

> - pos = skb_put(skb, ie_ssid_len);
> - *pos++ = WLAN_EID_SSID;
> + if (ieee80211_vif_is_mesh(vif)) {
> + pos = skb_put(skb, 2 + 2 + ssid_len);
> + *pos++ = WLAN_EID_SSID;
> + *pos++ = 0;
> + /* NOTE: mesh ID will be out of order */

Why put it out of order?

Also I'm not convinced that it's a good idea to translate "SSID" from
the userspace API to "mesh ID" silently? Might make more sense to have
those separately maybe? I mean, it seems reasonable to even think you
might scan for a mesh network when you're not a mesh interface, for
example?

johannes


2013-02-04 18:05:08

by Thomas Pedersen

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

On Mon, Feb 4, 2013 at 9:38 AM, Johannes Berg <[email protected]> wrote:
> On Sat, 2013-02-02 at 17:02 -0800, Thomas Pedersen wrote:
>
>> - pos = skb_put(skb, ie_ssid_len);
>> - *pos++ = WLAN_EID_SSID;
>> + if (ieee80211_vif_is_mesh(vif)) {
>> + pos = skb_put(skb, 2 + 2 + ssid_len);
>> + *pos++ = WLAN_EID_SSID;
>> + *pos++ = 0;
>> + /* NOTE: mesh ID will be out of order */
>
> Why put it out of order?
>
> Also I'm not convinced that it's a good idea to translate "SSID" from
> the userspace API to "mesh ID" silently? Might make more sense to have
> those separately maybe? I mean, it seems reasonable to even think you
> might scan for a mesh network when you're not a mesh interface, for
> example?

Yeah that makes more sense, but will obviously require more work.
I'll just drop this for now.

--
Thomas

2013-02-04 18:03:22

by Johannes Berg

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

On Mon, 2013-02-04 at 09:58 -0800, Thomas Pedersen wrote:
> On Mon, Feb 4, 2013 at 9:38 AM, Johannes Berg <[email protected]> wrote:
> > On Sat, 2013-02-02 at 17:02 -0800, Thomas Pedersen wrote:
> >
> >> - pos = skb_put(skb, ie_ssid_len);
> >> - *pos++ = WLAN_EID_SSID;
> >> + if (ieee80211_vif_is_mesh(vif)) {
> >> + pos = skb_put(skb, 2 + 2 + ssid_len);
> >> + *pos++ = WLAN_EID_SSID;
> >> + *pos++ = 0;
> >> + /* NOTE: mesh ID will be out of order */
> >
> > Why put it out of order?
> >
> > Also I'm not convinced that it's a good idea to translate "SSID" from
> > the userspace API to "mesh ID" silently? Might make more sense to have
> > those separately maybe? I mean, it seems reasonable to even think you
> > might scan for a mesh network when you're not a mesh interface, for
> > example?
>
> Yeah that makes more sense, but will obviously require more work.
> I'll just drop this for now.

I guess the other question is if you actually want this at all. I mean,
if you just do patch 1 and 3, then unless you want to actively scan for
multiple networks you can just include the mesh ID in the IE parameter.
Seems for many purposes that could be acceptable already. Now I'm not
saying that you shouldn't put it into the kernel, there certainly could
be value in that, just saying that in terms of effort that might be
easier?

johannes