This patch set is addressing some issues found in the current 802.11s implementation,
specifically when using hostap mpm.
It's aligning the beacon format and handling some corner cases.
Maital Hahn (2):
mac80211: mesh: flush stations before beacons are stopped
mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting
Meirav Kama (2):
mac80211: mesh: fixed HT ies in beacon template
mac80211: sta_info: max_peers reached falsely
net/mac80211/cfg.c | 1 +
net/mac80211/mesh.c | 46 ++++++++++++++++++++++++++++++++++++++++------
net/mac80211/mesh_hwmp.c | 42 +++++++++++++++++++++++++-----------------
net/mac80211/sta_info.c | 14 ++++++++++++++
net/mac80211/util.c | 3 ---
net/wireless/mesh.c | 2 +-
6 files changed, 81 insertions(+), 27 deletions(-)
--
2.9.0
On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
>
> net/mac80211/mesh.c | 33 ++++++++++++++++++++++++++++++++-
> net/mac80211/util.c | 3 ---
> net/wireless/mesh.c | 2 +-
That's not a good patch - one change is mac80211 and the other
cfg80211.
> - .ht_opmode = IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED,
> + .ht_opmode = IEEE80211_HT_OP_MODE_PROTECTION_NONE,
>
How are you planning to comply with 802.11 now?
The HT Protection field in a mesh STA may be set to no protection mode
only if
— All STAs detected in the primary or the secondary channel are HT
STAs, and
— All mesh STA members of this MBSS that are one-hop neighbors of the
transmitting mesh STA are either:
— 20/40 MHz HT mesh STAs in a 20/40 MHz MBSS, or
— 20 MHz HT mesh STAs in a 20 MHz MBSS.
johannes
On Tue, Jun 28, 2016 at 15:56:21, Bob Copeland wrote:
> linux- [email protected]; [email protected]; Kama, Meirav
> Subject: Re: [PATCH 4/4] mac80211: sta_info: max_peers reached falsely
>
> On Tue, Jun 28, 2016 at 02:13:07PM +0300, Yaniv Machani wrote:
> > From: Meirav Kama <[email protected]>
> >
> > Issue happened when receiving delete_sta command without changing
> > plink_state from ESTAB to HOLDING before.
> > When receiving delete_sta command for mesh interface verify
> > plink_state is not ESTAB and if so, decrease plink count and update
> > beacon.
>
> This should be fixed already (and properly) by the patch
> "mac80211: Fix mesh estab links counting" -- please let us know if you
> have a case that's still broken with that fix.
>
Thanks Bob,
Will be dropped.
Yaniv
> --
> Bob Copeland %% http://bobcopeland.com/
Hi Yaniv,
On Tue, Jun 28, 2016 at 9:13 PM, Yaniv Machani <[email protected]> wrote:
> This patch set is addressing some issues found in the current 802.11s implementation,
> specifically when using hostap mpm.
> It's aligning the beacon format and handling some corner cases.
>
> Maital Hahn (2):
> mac80211: mesh: flush stations before beacons are stopped
> mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting
>
> Meirav Kama (2):
> mac80211: mesh: fixed HT ies in beacon template
> mac80211: sta_info: max_peers reached falsely
Patches that you send must be signed off by you, not ack'd by you.
I.e.
From: Random Developer <[email protected]>
.....
Signed-off-by: Random Developer <[email protected]>
Signed-off-by: Patch Sender <[email protected]>
Thanks,
--
Julian Calaby
Email: [email protected]
Profile: http://www.google.com/profiles/julian.calaby/
On Tue, Jun 28, 2016 at 02:13:07PM +0300, Yaniv Machani wrote:
> From: Meirav Kama <[email protected]>
>
> Issue happened when receiving delete_sta command without
> changing plink_state from ESTAB to HOLDING before.
> When receiving delete_sta command for mesh interface
> verify plink_state is not ESTAB and if so, decrease
> plink count and update beacon.
This should be fixed already (and properly) by the patch
"mac80211: Fix mesh estab links counting" -- please let us
know if you have a case that's still broken with that fix.
--
Bob Copeland %% http://bobcopeland.com/
From: Maital Hahn <[email protected]>
Some drivers (e.g. wl18xx) expect that the last stage in the
de-initialization process will be stopping the beacons, similar to ap.
Update ieee80211_stop_mesh() flow accordingly.
Signed-off-by: Maital Hahn <[email protected]>
Acked-by: Yaniv Machani <[email protected]>
---
net/mac80211/mesh.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 21b1fdf..9214bc1 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -896,20 +896,22 @@ void ieee80211_stop_mesh(struct ieee80211_sub_if_data *sdata)
netif_carrier_off(sdata->dev);
+ /* flush STAs and mpaths on this iface */
+ sta_info_flush(sdata);
+ mesh_path_flush_by_iface(sdata);
+
/* stop the beacon */
ifmsh->mesh_id_len = 0;
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);
+
+ /* remove beacon */
bcn = rcu_dereference_protected(ifmsh->beacon,
lockdep_is_held(&sdata->wdev.mtx));
RCU_INIT_POINTER(ifmsh->beacon, NULL);
kfree_rcu(bcn, rcu_head);
- /* flush STAs and mpaths on this iface */
- sta_info_flush(sdata);
- mesh_path_flush_by_iface(sdata);
-
/* free all potentially still buffered group-addressed frames */
local->total_ps_buffered -= skb_queue_len(&ifmsh->ps.bc_buf);
skb_queue_purge(&ifmsh->ps.bc_buf);
--
2.9.0
From: Meirav Kama <[email protected]>
Issue happened when receiving delete_sta command without
changing plink_state from ESTAB to HOLDING before.
When receiving delete_sta command for mesh interface
verify plink_state is not ESTAB and if so, decrease
plink count and update beacon.
Signed-off-by: Meirav Kama <[email protected]>
Acked-by: Yaniv Machani <[email protected]>
---
net/mac80211/sta_info.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index 76b737d..1ce6320 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1009,11 +1009,25 @@ int sta_info_destroy_addr_bss(struct ieee80211_sub_if_data *sdata,
{
struct sta_info *sta;
int ret;
+#ifdef CONFIG_MAC80211_MESH
+ bool dec_links = false;
+#endif
mutex_lock(&sdata->local->sta_mtx);
sta = sta_info_get_bss(sdata, addr);
+#ifdef CONFIG_MAC80211_MESH
+ if (sdata->vif.type == NL80211_IFTYPE_MESH_POINT &&
+ sta->mesh->plink_state == NL80211_PLINK_ESTAB)
+ dec_links = true;
+#endif
ret = __sta_info_destroy(sta);
mutex_unlock(&sdata->local->sta_mtx);
+#ifdef CONFIG_MAC80211_MESH
+ if (dec_links) {
+ mesh_plink_dec_estab_count(sdata);
+ ieee80211_mbss_info_change_notify(sdata, BSS_CHANGED_BEACON);
+ }
+#endif
return ret;
}
--
2.9.0
From: Meirav Kama <[email protected]>
There are several values in HT info elements of mesh beacon (built by the
mac80211) that are incorrect.
To fix them:
1. mac80211 will check configuration from cfg and will build accordingly.
2. changes made in mesh default values.
Signed-off-by: Meirav Kama <[email protected]>
Acked-by: Yaniv Machani <[email protected]>
---
net/mac80211/mesh.c | 33 ++++++++++++++++++++++++++++++++-
net/mac80211/util.c | 3 ---
net/wireless/mesh.c | 2 +-
3 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 1f5be54..1b63b11 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -423,6 +423,8 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
enum nl80211_band band = ieee80211_get_sdata_band(sdata);
struct ieee80211_supported_band *sband;
u8 *pos;
+ u16 cap;
+
sband = local->hw.wiphy->bands[band];
if (!sband->ht_cap.ht_supported ||
@@ -431,11 +433,40 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_10)
return 0;
+ /* determine capability flags */
+ cap = sband->ht_cap.cap;
+
+ /* if channel width is 20MHz - configure HT capab accordingly*/
+ if (sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_20) {
+ cap &= ~IEEE80211_HT_CAP_SUP_WIDTH_20_40;
+ cap &= ~IEEE80211_HT_CAP_DSSSCCK40;
+ }
+
+ /* set SM PS mode properly */
+ cap &= ~IEEE80211_HT_CAP_SM_PS;
+ switch (sdata->smps_mode) {
+ case IEEE80211_SMPS_AUTOMATIC:
+ case IEEE80211_SMPS_NUM_MODES:
+ WARN_ON(1);
+ case IEEE80211_SMPS_OFF:
+ cap |= WLAN_HT_CAP_SM_PS_DISABLED <<
+ IEEE80211_HT_CAP_SM_PS_SHIFT;
+ break;
+ case IEEE80211_SMPS_STATIC:
+ cap |= WLAN_HT_CAP_SM_PS_STATIC <<
+ IEEE80211_HT_CAP_SM_PS_SHIFT;
+ break;
+ case IEEE80211_SMPS_DYNAMIC:
+ cap |= WLAN_HT_CAP_SM_PS_DYNAMIC <<
+ IEEE80211_HT_CAP_SM_PS_SHIFT;
+ break;
+ }
+
if (skb_tailroom(skb) < 2 + sizeof(struct ieee80211_ht_cap))
return -ENOMEM;
pos = skb_put(skb, 2 + sizeof(struct ieee80211_ht_cap));
- ieee80211_ie_build_ht_cap(pos, &sband->ht_cap, sband->ht_cap.cap);
+ ieee80211_ie_build_ht_cap(pos, &sband->ht_cap, cap);
return 0;
}
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 42bf0b6..5375a82 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -2349,10 +2349,7 @@ u8 *ieee80211_ie_build_ht_oper(u8 *pos, struct ieee80211_sta_ht_cap *ht_cap,
ht_oper->operation_mode = cpu_to_le16(prot_mode);
ht_oper->stbc_param = 0x0000;
- /* It seems that Basic MCS set and Supported MCS set
- are identical for the first 10 bytes */
memset(&ht_oper->basic_set, 0, 16);
- memcpy(&ht_oper->basic_set, &ht_cap->mcs, 10);
return pos + sizeof(struct ieee80211_ht_operation);
}
diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index fa2066b..ac19a19 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -70,7 +70,7 @@ const struct mesh_config default_mesh_config = {
.dot11MeshGateAnnouncementProtocol = false,
.dot11MeshForwarding = true,
.rssi_threshold = MESH_RSSI_THRESHOLD,
- .ht_opmode = IEEE80211_HT_OP_MODE_PROTECTION_NONHT_MIXED,
+ .ht_opmode = IEEE80211_HT_OP_MODE_PROTECTION_NONE,
.dot11MeshHWMPactivePathToRootTimeout = MESH_PATH_TO_ROOT_TIMEOUT,
.dot11MeshHWMProotInterval = MESH_ROOT_INTERVAL,
.dot11MeshHWMPconfirmationInterval = MESH_ROOT_CONFIRMATION_INTERVAL,
--
2.9.0
On Tue, Jun 28, 2016 at 02:13:06PM +0300, Yaniv Machani wrote:
> From: Meirav Kama <[email protected]>
>
> There are several values in HT info elements of mesh beacon (built by the
> mac80211) that are incorrect.
Would be good to enumerate the problems here.
> To fix them:
> 1. mac80211 will check configuration from cfg and will build accordingly.
> 2. changes made in mesh default values.
What is wrong with the defaults?
> sband = local->hw.wiphy->bands[band];
> if (!sband->ht_cap.ht_supported ||
> @@ -431,11 +433,40 @@ int mesh_add_ht_cap_ie(struct ieee80211_sub_if_data *sdata,
> sdata->vif.bss_conf.chandef.width == NL80211_CHAN_WIDTH_10)
> return 0;
>
> + /* determine capability flags */
> + cap = sband->ht_cap.cap;
There is some weird whitespace here (space instead of tabs for the
comment).
--
Bob Copeland %% http://bobcopeland.com/
From: Maital Hahn <[email protected]>
Once receiving a CLOSE action frame from the disconnecting peer,
flush all entries in the path table which has this peer as the
next hop.
In addition, upon receiving a packet, if next hop is not found,
trigger PERQ immidiatly, instead of just putting it in the queue.
Signed-off-by: Maital Hahn <[email protected]>
Acked-by: Yaniv Machani <[email protected]>
---
net/mac80211/cfg.c | 1 +
net/mac80211/mesh.c | 3 ++-
net/mac80211/mesh_hwmp.c | 42 +++++++++++++++++++++++++-----------------
3 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 0c12e40..f876ef7 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1011,6 +1011,7 @@ static void sta_apply_mesh_params(struct ieee80211_local *local,
if (sta->mesh->plink_state == NL80211_PLINK_ESTAB)
changed = mesh_plink_dec_estab_count(sdata);
sta->mesh->plink_state = params->plink_state;
+ mesh_path_flush_by_nexthop(sta);
ieee80211_mps_sta_status_update(sta);
changed |= ieee80211_mps_set_sta_local_pm(sta,
diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c
index 9214bc1..1f5be54 100644
--- a/net/mac80211/mesh.c
+++ b/net/mac80211/mesh.c
@@ -159,7 +159,8 @@ void mesh_sta_cleanup(struct sta_info *sta)
if (!sdata->u.mesh.user_mpm) {
changed |= mesh_plink_deactivate(sta);
del_timer_sync(&sta->mesh->plink_timer);
- }
+ } else
+ mesh_path_flush_by_nexthop(sta);
/* make sure no readers can access nexthop sta from here on */
mesh_path_flush_by_nexthop(sta);
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 8f9c3bd..9783d49 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -19,7 +19,7 @@
#define MAX_PREQ_QUEUE_LEN 64
-static void mesh_queue_preq(struct mesh_path *, u8);
+static void mesh_queue_preq(struct mesh_path *, u8, bool);
static inline u32 u32_field_get(const u8 *preq_elem, int offset, bool ae)
{
@@ -830,7 +830,8 @@ static void hwmp_rann_frame_process(struct ieee80211_sub_if_data *sdata,
mhwmp_dbg(sdata,
"time to refresh root mpath %pM\n",
orig_addr);
- mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH);
+ mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH,
+ false);
mpath->last_preq_to_root = jiffies;
}
@@ -925,7 +926,7 @@ void mesh_rx_path_sel_frame(struct ieee80211_sub_if_data *sdata,
* Locking: the function must be called from within a rcu read lock block.
*
*/
-static void mesh_queue_preq(struct mesh_path *mpath, u8 flags)
+static void mesh_queue_preq(struct mesh_path *mpath, u8 flags, bool immediate)
{
struct ieee80211_sub_if_data *sdata = mpath->sdata;
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
@@ -964,18 +965,24 @@ static void mesh_queue_preq(struct mesh_path *mpath, u8 flags)
++ifmsh->preq_queue_len;
spin_unlock_bh(&ifmsh->mesh_preq_queue_lock);
- if (time_after(jiffies, ifmsh->last_preq + min_preq_int_jiff(sdata)))
+ if (immediate) {
ieee80211_queue_work(&sdata->local->hw, &sdata->work);
+ } else {
+ if (time_after(jiffies,
+ ifmsh->last_preq + min_preq_int_jiff(sdata))) {
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
- else if (time_before(jiffies, ifmsh->last_preq)) {
- /* avoid long wait if did not send preqs for a long time
- * and jiffies wrapped around
- */
- ifmsh->last_preq = jiffies - min_preq_int_jiff(sdata) - 1;
- ieee80211_queue_work(&sdata->local->hw, &sdata->work);
- } else
- mod_timer(&ifmsh->mesh_path_timer, ifmsh->last_preq +
- min_preq_int_jiff(sdata));
+ } else if (time_before(jiffies, ifmsh->last_preq)) {
+ /* avoid long wait if did not send preqs for a long time
+ * and jiffies wrapped around
+ */
+ ifmsh->last_preq = jiffies -
+ min_preq_int_jiff(sdata) - 1;
+ ieee80211_queue_work(&sdata->local->hw, &sdata->work);
+ } else
+ mod_timer(&ifmsh->mesh_path_timer, ifmsh->last_preq +
+ min_preq_int_jiff(sdata));
+ }
}
/**
@@ -1110,7 +1117,7 @@ int mesh_nexthop_resolve(struct ieee80211_sub_if_data *sdata,
}
if (!(mpath->flags & MESH_PATH_RESOLVING))
- mesh_queue_preq(mpath, PREQ_Q_F_START);
+ mesh_queue_preq(mpath, PREQ_Q_F_START, true);
if (skb_queue_len(&mpath->frame_queue) >= MESH_FRAME_QUEUE_LEN)
skb_to_free = skb_dequeue(&mpath->frame_queue);
@@ -1157,8 +1164,9 @@ int mesh_nexthop_lookup(struct ieee80211_sub_if_data *sdata,
msecs_to_jiffies(sdata->u.mesh.mshcfg.path_refresh_time)) &&
ether_addr_equal(sdata->vif.addr, hdr->addr4) &&
!(mpath->flags & MESH_PATH_RESOLVING) &&
- !(mpath->flags & MESH_PATH_FIXED))
- mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH);
+ !(mpath->flags & MESH_PATH_FIXED)) {
+ mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH, false);
+ }
next_hop = rcu_dereference(mpath->next_hop);
if (next_hop) {
@@ -1192,7 +1200,7 @@ void mesh_path_timer(unsigned long data)
mpath->discovery_timeout *= 2;
mpath->flags &= ~MESH_PATH_REQ_QUEUED;
spin_unlock_bh(&mpath->state_lock);
- mesh_queue_preq(mpath, 0);
+ mesh_queue_preq(mpath, 0, false);
} else {
mpath->flags &= ~(MESH_PATH_RESOLVING |
MESH_PATH_RESOLVED |
--
2.9.0
On Tue, Jun 28, 2016 at 02:13:05PM +0300, Yaniv Machani wrote:
> From: Maital Hahn <[email protected]>
>
> Once receiving a CLOSE action frame from the disconnecting peer,
> flush all entries in the path table which has this peer as the
> next hop.
Please address the user-visible behavior in your commit messages.
Does it crash? Does it send frames to an invalid peer? Do
frames get dropped?
> In addition, upon receiving a packet, if next hop is not found,
> trigger PERQ immidiatly, instead of just putting it in the queue.
"PREQ"
Please split this into a separate patch that we can review
separately (and also give the "why" in the commit log).
> Signed-off-by: Maital Hahn <[email protected]>
> Acked-by: Yaniv Machani <[email protected]>
> ---
> net/mac80211/cfg.c | 1 +
> net/mac80211/mesh.c | 3 ++-
> net/mac80211/mesh_hwmp.c | 42 +++++++++++++++++++++++++-----------------
> 3 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 0c12e40..f876ef7 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1011,6 +1011,7 @@ static void sta_apply_mesh_params(struct ieee80211_local *local,
> if (sta->mesh->plink_state == NL80211_PLINK_ESTAB)
> changed = mesh_plink_dec_estab_count(sdata);
> sta->mesh->plink_state = params->plink_state;
> + mesh_path_flush_by_nexthop(sta);
This isn't necessary, caller should already be doing
mesh_path_flush_by_nexthop() in every case I could see. Besides it
cannot be done under plink lock.
> +++ b/net/mac80211/mesh.c
> @@ -159,7 +159,8 @@ void mesh_sta_cleanup(struct sta_info *sta)
> if (!sdata->u.mesh.user_mpm) {
> changed |= mesh_plink_deactivate(sta);
> del_timer_sync(&sta->mesh->plink_timer);
> - }
> + } else
> + mesh_path_flush_by_nexthop(sta);
And this is already fixed in mac80211-next.
--
Bob Copeland %% http://bobcopeland.com/
On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> From: Maital Hahn <[email protected]>
>
> Some drivers (e.g. wl18xx) expect that the last stage in the
> de-initialization process will be stopping the beacons, similar to
> ap. Update ieee80211_stop_mesh() flow accordingly.
>
How well have you tested that with other drivers?
Changing behaviour to something a single driver desires isn't
necessarily the best thing to do, since there always are multiple
drivers.
If you're able to demonstrate that it works with the other drivers I'm
willing to take that - the change makes sense after all, and it seems
drivers must support this ordering since peers are also removed
dynamically... But still. Don't just make a change like that without
even giving any indication why you think it's fine for other drivers!
johannes
On Tue, Jun 28, 2016 at 15:27:47, Bob Copeland wrote:
> linux- [email protected]; [email protected]; Hahn, Maital
> Subject: Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a
> mesh peer is disconnecting
>
> On Tue, Jun 28, 2016 at 02:13:05PM +0300, Yaniv Machani wrote:
> > From: Maital Hahn <[email protected]>
> >
> > Once receiving a CLOSE action frame from the disconnecting peer,
> > flush all entries in the path table which has this peer as the next hop.
>
> Please address the user-visible behavior in your commit messages.
> Does it crash? Does it send frames to an invalid peer? Do frames get dropped?
>
Hi Bob,
It was a crash, apparently already fixed by your patches some time ago.
I'll remove that part and resend the 2nd part (with some more 'why', and less typos..)
> > In addition, upon receiving a packet, if next hop is not found,
> > trigger PERQ immidiatly, instead of just putting it in the queue.
>
> "PREQ"
>
> Please split this into a separate patch that we can review separately
> (and also give the "why" in the commit log).
>
> > @@ -1011,6 +1011,7 @@ static void sta_apply_mesh_params(struct
> ieee80211_local *local,
> > if (sta->mesh->plink_state == NL80211_PLINK_ESTAB)
> > changed =
> mesh_plink_dec_estab_count(sdata);
> > sta->mesh->plink_state = params->plink_state;
> > + mesh_path_flush_by_nexthop(sta);
>
> This isn't necessary, caller should already be doing
> mesh_path_flush_by_nexthop() in every case I could see. Besides it
> cannot be done under plink lock.
>
I believe this was fixed in your patch "mac80211: mesh: flush paths outside of plink lock"
There is probably no need in that on the latest as well.
Thanks,
Yaniv
T24gV2VkLCBKdW4gMjksIDIwMTYgYXQgMTA6MTQ6MTksIEpvaGFubmVzIEJlcmcgd3JvdGU6DQo+
IENjOiBIYWhuLCBNYWl0YWwNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAxLzRdIG1hYzgwMjExOiBt
ZXNoOiBmbHVzaCBzdGF0aW9ucyBiZWZvcmUgYmVhY29ucyANCj4gYXJlIHN0b3BwZWQNCj4gDQo+
IE9uIFR1ZSwgMjAxNi0wNi0yOCBhdCAxNDoxMyArMDMwMCwgWWFuaXYgTWFjaGFuaSB3cm90ZToN
Cj4gPiBGcm9tOiBNYWl0YWwgSGFobiA8bWFpdGFsbUB0aS5jb20+DQo+ID4NCj4gPiBTb21lIGRy
aXZlcnMgKGUuZy4gd2wxOHh4KSBleHBlY3QgdGhhdCB0aGUgbGFzdCBzdGFnZSBpbiB0aGUgDQo+
ID4gZGUtaW5pdGlhbGl6YXRpb24gcHJvY2VzcyB3aWxsIGJlIHN0b3BwaW5nIHRoZSBiZWFjb25z
LCBzaW1pbGFyIHRvIGFwLg0KPiA+IFVwZGF0ZSBpZWVlODAyMTFfc3RvcF9tZXNoKCkgZmxvdyBh
Y2NvcmRpbmdseS4NCj4gPg0KPiBIb3cgd2VsbCBoYXZlIHlvdSB0ZXN0ZWQgdGhhdCB3aXRoIG90
aGVyIGRyaXZlcnM/DQo+IA0KDQpTb3JyeSBmb3IgdGhlIGRlbGF5ZWQgcmVzcG9uc2UgKEkndmUg
YmVlbiBvdXQpIGFuZCB0aGFua3MgZm9yIHlvdXIgY29tbWVudHMsDQpJIGhhdmUgdGVzdGVkIGl0
IHdpdGggUlQzNTcyIGFzIHdlbGwsIGFuZCBkaWRuJ3Qgc2VlIGFueSBpc3N1ZS4NCkknbGwgdXBk
YXRlIHRoZSBjb21tZW50IHRvIHJlZmxlY3QgdGhhdC4NCg0KVGhhbmtzLA0KWWFuaXYNCg0KPiBD
aGFuZ2luZyBiZWhhdmlvdXIgdG8gc29tZXRoaW5nIGEgc2luZ2xlIGRyaXZlciBkZXNpcmVzIGlz
bid0IA0KPiBuZWNlc3NhcmlseSB0aGUgYmVzdCB0aGluZyB0byBkbywgc2luY2UgdGhlcmUgYWx3
YXlzIGFyZSBtdWx0aXBsZSBkcml2ZXJzLg0KPiANCj4gSWYgeW91J3JlIGFibGUgdG8gZGVtb25z
dHJhdGUgdGhhdCBpdCB3b3JrcyB3aXRoIHRoZSBvdGhlciBkcml2ZXJzIEknbSANCj4gd2lsbGlu
ZyB0byB0YWtlIHRoYXQgLSB0aGUgY2hhbmdlIG1ha2VzIHNlbnNlIGFmdGVyIGFsbCwgYW5kIGl0
IHNlZW1zIA0KPiBkcml2ZXJzIG11c3Qgc3VwcG9ydCB0aGlzIG9yZGVyaW5nIHNpbmNlIHBlZXJz
IGFyZSBhbHNvIHJlbW92ZWQgDQo+IGR5bmFtaWNhbGx5Li4uIEJ1dCBzdGlsbC4gRG9uJ3QganVz
dCBtYWtlIGEgY2hhbmdlIGxpa2UgdGhhdCB3aXRob3V0IA0KPiBldmVuIGdpdmluZyBhbnkgaW5k
aWNhdGlvbiB3aHkgeW91IHRoaW5rIGl0J3MgZmluZSBmb3Igb3RoZXIgZHJpdmVycyENCj4gDQo+
IGpvaGFubmVzDQoNCg0K
On Wed, Jul 13, 2016 at 10:11:25AM +0000, Machani, Yaniv wrote:
> > > Some drivers (e.g. wl18xx) expect that the last stage in the
> > > de-initialization process will be stopping the beacons, similar to ap.
> > > Update ieee80211_stop_mesh() flow accordingly.
> > >
> > How well have you tested that with other drivers?
> >
>
> Sorry for the delayed response (I've been out) and thanks for your comments,
> I have tested it with RT3572 as well, and didn't see any issue.
> I'll update the comment to reflect that.
I'll give this a test on ath10k and wcn36xx as they are the ones most
likely to care about ordering.
--
Bob Copeland %% http://bobcopeland.com/
On Wed, Jul 13, 2016 at 16:33:38, Bob Copeland wrote:
> linux- [email protected]; [email protected]; Hahn, Maital
> Subject: Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons
> are stopped
>
> On Wed, Jul 13, 2016 at 10:11:25AM +0000, Machani, Yaniv wrote:
> > > > Some drivers (e.g. wl18xx) expect that the last stage in the
> > > > de-initialization process will be stopping the beacons, similar to ap.
> > > > Update ieee80211_stop_mesh() flow accordingly.
> > > >
> > > How well have you tested that with other drivers?
> > >
> >
> > Sorry for the delayed response (I've been out) and thanks for your
> > comments, I have tested it with RT3572 as well, and didn't see any issue.
> > I'll update the comment to reflect that.
>
> I'll give this a test on ath10k and wcn36xx as they are the ones most
> likely to care about ordering.
>
Thank you,
Yaniv
> --
> Bob Copeland %% http://bobcopeland.com/
T24gV2VkLCBKdW4gMjksIDIwMTYgYXQgMTA6MTc6MzUsIEpvaGFubmVzIEJlcmcgd3JvdGU6DQo+
IENjOiBLYW1hLCBNZWlyYXYNCj4gU3ViamVjdDogUmU6IFtQQVRDSCAzLzRdIG1hYzgwMjExOiBt
ZXNoOiBmaXhlZCBIVCBpZXMgaW4gYmVhY29uIA0KPiB0ZW1wbGF0ZQ0KPiANCj4gT24gVHVlLCAy
MDE2LTA2LTI4IGF0IDE0OjEzICswMzAwLCBZYW5pdiBNYWNoYW5pIHdyb3RlOg0KPiA+DQo+ID4g
wqBuZXQvbWFjODAyMTEvbWVzaC5jIHwgMzMgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr
KystDQo+ID4gwqBuZXQvbWFjODAyMTEvdXRpbC5jIHzCoMKgMyAtLS0NCj4gPiDCoG5ldC93aXJl
bGVzcy9tZXNoLmMgfMKgwqAyICstDQo+IA0KPiBUaGF0J3Mgbm90IGEgZ29vZCBwYXRjaCAtIG9u
ZSBjaGFuZ2UgaXMgbWFjODAyMTEgYW5kIHRoZSBvdGhlciBjZmc4MDIxMS4NCj4gDQo+ID4gLQku
aHRfb3Btb2RlID0NCj4gSUVFRTgwMjExX0hUX09QX01PREVfUFJPVEVDVElPTl9OT05IVF9NSVhF
RCwNCj4gPiArCS5odF9vcG1vZGUgPSBJRUVFODAyMTFfSFRfT1BfTU9ERV9QUk9URUNUSU9OX05P
TkUsDQo+ID4NCj4gSG93IGFyZSB5b3UgcGxhbm5pbmcgdG8gY29tcGx5IHdpdGggODAyLjExIG5v
dz8NCg0KR29vZCBwb2ludCwgdGhpcyBjaGFuZ2VkIHNob3VsZCBiZSByZW1vdmVkLg0KVGhlIHJl
YXNvbiBmb3IgdGhpcyBjaGFuZ2Ugd2FzIHRoYXQgd2UndmUgbm90aWNlZCBhIGRpZmZlcmVuY2Ug
YmV0d2VlbiBtZXNoIGJlYWNvbiAoYnVpbHQgYnkgdGhlIG1hYzgwMjExKSBhbmQgbWVzaCBhY3Rp
b25zIChidWlsdCBieSB0aGUgc3VwcGxpY2FudCkgaW4gdGhlIEhUIGluZm9ybWF0aW9uIElFLg0K
SW4gYmVhY29ucyB0aGUgSFQgb3BlcmF0aW9uYWwgbW9kZSBpcyBNaXhlZCBNb2RlICgweDExKSAg
d2hpbGUgaW4gYWN0aW9ucyBpdCBpcyBOb25lICgweDAwKS4gDQpBZnRlciBhIHNlY29uZCBsb29r
LCBpdCBzZWVtcyB0aGF0IGl0J3MgdGhlIFN1cHBsaWNhbnQgdGhhdCBkb2Vzbid0IHNldCB0aGUg
ZGVmYXVsdCB2YWx1ZSBjb3JyZWN0bHkuDQoNCldlJ2xsIHNlbmQgYW4gdXBkYXRlZCBwYXRjaCBm
b3IgaXQuDQpUaGFua3MsDQpZYW5pdg0KDQo+IA0KPiBUaGUgSFQgUHJvdGVjdGlvbiBmaWVsZCBp
biBhIG1lc2ggU1RBIG1heSBiZSBzZXQgdG8gbm8gcHJvdGVjdGlvbiBtb2RlIA0KPiBvbmx5IGlm
IOKAlCBBbGwgU1RBcyBkZXRlY3RlZCBpbiB0aGUgcHJpbWFyeSBvciB0aGUgc2Vjb25kYXJ5IGNo
YW5uZWwgDQo+IGFyZSBIVA0KPiDCoCBTVEFzLCBhbmQNCj4g4oCUIEFsbCBtZXNoIFNUQSBtZW1i
ZXJzIG9mIHRoaXMgTUJTUyB0aGF0IGFyZSBvbmUtaG9wIG5laWdoYm9ycyBvZiB0aGUNCj4gwqAg
dHJhbnNtaXR0aW5nIG1lc2ggU1RBIGFyZSBlaXRoZXI6DQo+IMKgIOKAlCAyMC80MCBNSHogSFQg
bWVzaCBTVEFzIGluIGEgMjAvNDAgTUh6IE1CU1MsIG9yDQo+IMKgIOKAlCAyMCBNSHogSFQgbWVz
aCBTVEFzIGluIGEgMjAgTUh6IE1CU1MuDQo+IA0KPiBqb2hhbm5lcw0KDQoNCg==
On Wed, 2016-07-13 at 10:11 +0000, Machani, Yaniv wrote:
> On Wed, Jun 29, 2016 at 10:14:19, Johannes Berg wrote:
> > Cc: Hahn, Maital
> > Subject: Re: [PATCH 1/4] mac80211: mesh: flush stations before
> > beacons
> > are stopped
> >
> > On Tue, 2016-06-28 at 14:13 +0300, Yaniv Machani wrote:
> > > From: Maital Hahn <[email protected]>
> > >
> > > Some drivers (e.g. wl18xx) expect that the last stage in the
> > > de-initialization process will be stopping the beacons, similar
> > > to ap.
> > > Update ieee80211_stop_mesh() flow accordingly.
> > >
> > How well have you tested that with other drivers?
> >
>
> Sorry for the delayed response (I've been out) and thanks for your
> comments,
> I have tested it with RT3572 as well, and didn't see any issue.
> I'll update the comment to reflect that.
>
I'm actually reasonably sure that it *must* work, similiar to the AP
mode change, since it's always valid to remove stations while the the
mesh beacon is still active.
I hoped you'd actually figure out that line of reasoning and put it
into the commit message :)
johannes