* [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case @ 2016-06-19 20:51 Jouni Malinen 2016-06-21 19:09 ` Johannes Berg 2016-06-28 10:40 ` Johannes Berg 0 siblings, 2 replies; 5+ messages in thread From: Jouni Malinen @ 2016-06-19 20:51 UTC (permalink / raw) To: Johannes Berg; +Cc: linux-wireless If a user space program (e.g., wpa_supplicant) deletes a STA entry that is currently in NL80211_PLINK_ESTAB state, the number of established plinks counter was not decremented and this could result in rejecting new plink establishment before really hitting the real maximum plink limit. For !user_mpm case, this decrementation is handled by mesh_plink_deactive(). Fix this by decrementing estab_plinks on STA deletion (mesh_sta_cleanup() gets called from there) so that the counter has a correct value and the Beacon frame advertisement in Mesh Configuration element shows the proper value for capability to accept additional peers. Signed-off-by: Jouni Malinen <j@w1.fi> --- net/mac80211/mesh.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index 21b1fdf..6a1603b 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -148,14 +148,17 @@ u32 mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata) void mesh_sta_cleanup(struct sta_info *sta) { struct ieee80211_sub_if_data *sdata = sta->sdata; - u32 changed; + u32 changed = 0; /* * maybe userspace handles peer allocation and peering, but in either * case the beacon is still generated by the kernel and we might need * an update. */ - changed = mesh_accept_plinks_update(sdata); + if (sdata->u.mesh.user_mpm && + sta->mesh->plink_state == NL80211_PLINK_ESTAB) + changed |= mesh_plink_dec_estab_count(sdata); + changed |= mesh_accept_plinks_update(sdata); if (!sdata->u.mesh.user_mpm) { changed |= mesh_plink_deactivate(sta); del_timer_sync(&sta->mesh->plink_timer); -- 1.9.1 -- Jouni Malinen PGP id EFC895FA ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case 2016-06-19 20:51 [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case Jouni Malinen @ 2016-06-21 19:09 ` Johannes Berg 2016-06-21 22:26 ` Bob Copeland 2016-06-28 10:40 ` Johannes Berg 1 sibling, 1 reply; 5+ messages in thread From: Johannes Berg @ 2016-06-21 19:09 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless > */ > - changed = mesh_accept_plinks_update(sdata); > + if (sdata->u.mesh.user_mpm && > + sta->mesh->plink_state == NL80211_PLINK_ESTAB) > + changed |= mesh_plink_dec_estab_count(sdata); > + changed |= mesh_accept_plinks_update(sdata); > if (!sdata->u.mesh.user_mpm) { > changed |= mesh_plink_deactivate(sta); > del_timer_sync(&sta->mesh->plink_timer); > Does it have to be done before the mesh_accept_plinks_update()? If not, you should put it with the existing u.mesh.user_mpm check. If yes, then the code is further buggy since only mesh_plink_deactivate() will call it when the kernel MPM is used. johannes ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case 2016-06-21 19:09 ` Johannes Berg @ 2016-06-21 22:26 ` Bob Copeland 2016-06-21 22:29 ` Bob Copeland 0 siblings, 1 reply; 5+ messages in thread From: Bob Copeland @ 2016-06-21 22:26 UTC (permalink / raw) To: Johannes Berg; +Cc: Jouni Malinen, linux-wireless On Tue, Jun 21, 2016 at 09:09:10PM +0200, Johannes Berg wrote: > > > */ > > - changed = mesh_accept_plinks_update(sdata); > > + if (sdata->u.mesh.user_mpm && > > + sta->mesh->plink_state == NL80211_PLINK_ESTAB) > > + changed |= mesh_plink_dec_estab_count(sdata); > > + changed |= mesh_accept_plinks_update(sdata); > > if (!sdata->u.mesh.user_mpm) { > > changed |= mesh_plink_deactivate(sta); > > del_timer_sync(&sta->mesh->plink_timer); > > > > Does it have to be done before the mesh_accept_plinks_update()? > > If not, you should put it with the existing u.mesh.user_mpm check. If > yes, then the code is further buggy since only mesh_plink_deactivate() > will call it when the kernel MPM is used. Looks further buggy, so perhaps this untested patch would work, i.e. move the accepting-plinks change closer to the decrement, and push the user_mpm check down into mesh_plink_deactivate to just avoid sending the peering frames. [There's also a bit there for power saving that we likely want to keep for secure networks.] Then again maybe accepting_plinks flag should just be computed when used instead of tracking the state separately. --- net/mac80211/mesh.c | 18 ++---------------- net/mac80211/mesh_plink.c | 16 ++++++++++++---- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/net/mac80211/mesh.c b/net/mac80211/mesh.c index 21b1fdf..3c150f8 100644 --- a/net/mac80211/mesh.c +++ b/net/mac80211/mesh.c @@ -148,22 +148,8 @@ u32 mesh_accept_plinks_update(struct ieee80211_sub_if_data *sdata) void mesh_sta_cleanup(struct sta_info *sta) { struct ieee80211_sub_if_data *sdata = sta->sdata; - u32 changed; - - /* - * maybe userspace handles peer allocation and peering, but in either - * case the beacon is still generated by the kernel and we might need - * an update. - */ - changed = mesh_accept_plinks_update(sdata); - if (!sdata->u.mesh.user_mpm) { - changed |= mesh_plink_deactivate(sta); - del_timer_sync(&sta->mesh->plink_timer); - } - - /* make sure no readers can access nexthop sta from here on */ - mesh_path_flush_by_nexthop(sta); - synchronize_net(); + u32 changed = mesh_plink_deactivate(sta); + del_timer_sync(&sta->mesh->plink_timer); if (changed) ieee80211_mbss_info_change_notify(sdata, changed); diff --git a/net/mac80211/mesh_plink.c b/net/mac80211/mesh_plink.c index 79f2a0a..69ac7a8 100644 --- a/net/mac80211/mesh_plink.c +++ b/net/mac80211/mesh_plink.c @@ -349,6 +349,8 @@ static u32 __mesh_plink_deactivate(struct sta_info *sta) changed = mesh_plink_dec_estab_count(sdata); sta->mesh->plink_state = NL80211_PLINK_BLOCKED; + changed |= mesh_accept_plinks_update(sdata); + ieee80211_mps_sta_status_update(sta); changed |= ieee80211_mps_set_sta_local_pm(sta, NL80211_MESH_POWER_UNKNOWN); @@ -370,13 +372,19 @@ u32 mesh_plink_deactivate(struct sta_info *sta) spin_lock_bh(&sta->mesh->plink_lock); changed = __mesh_plink_deactivate(sta); - sta->mesh->reason = WLAN_REASON_MESH_PEER_CANCELED; - mesh_plink_frame_tx(sdata, sta, WLAN_SP_MESH_PEERING_CLOSE, - sta->sta.addr, sta->mesh->llid, sta->mesh->plid, - sta->mesh->reason); + + if (sdata->u.mesh.user_mpm) { + sta->mesh->reason = WLAN_REASON_MESH_PEER_CANCELED; + mesh_plink_frame_tx(sdata, sta, WLAN_SP_MESH_PEERING_CLOSE, + sta->sta.addr, sta->mesh->llid, sta->mesh->plid, + sta->mesh->reason); + } spin_unlock_bh(&sta->mesh->plink_lock); mesh_path_flush_by_nexthop(sta); + /* make sure no readers can access nexthop sta from here on */ + synchronize_net(); + return changed; } -- 2.9.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case 2016-06-21 22:26 ` Bob Copeland @ 2016-06-21 22:29 ` Bob Copeland 0 siblings, 0 replies; 5+ messages in thread From: Bob Copeland @ 2016-06-21 22:29 UTC (permalink / raw) To: Johannes Berg; +Cc: Jouni Malinen, linux-wireless On Tue, Jun 21, 2016 at 06:26:02PM -0400, Bob Copeland wrote: > Looks further buggy, so perhaps this untested patch would work, i.e. ^^^^^^^^ (still) > @@ -370,13 +372,19 @@ u32 mesh_plink_deactivate(struct sta_info *sta) > > spin_lock_bh(&sta->mesh->plink_lock); > changed = __mesh_plink_deactivate(sta); > - sta->mesh->reason = WLAN_REASON_MESH_PEER_CANCELED; > - mesh_plink_frame_tx(sdata, sta, WLAN_SP_MESH_PEERING_CLOSE, > - sta->sta.addr, sta->mesh->llid, sta->mesh->plid, > - sta->mesh->reason); > + > + if (sdata->u.mesh.user_mpm) { should be !sdata->u.mesh.user_mpm :) > + sta->mesh->reason = WLAN_REASON_MESH_PEER_CANCELED; > + mesh_plink_frame_tx(sdata, sta, WLAN_SP_MESH_PEERING_CLOSE, > + sta->sta.addr, sta->mesh->llid, sta->mesh->plid, > + sta->mesh->reason); > + } > spin_unlock_bh(&sta->mesh->plink_lock); > mesh_path_flush_by_nexthop(sta); > > + /* make sure no readers can access nexthop sta from here on */ > + synchronize_net(); > + > return changed; > } > > -- > 2.9.0 > -- Bob Copeland %% http://bobcopeland.com/ ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case 2016-06-19 20:51 [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case Jouni Malinen 2016-06-21 19:09 ` Johannes Berg @ 2016-06-28 10:40 ` Johannes Berg 1 sibling, 0 replies; 5+ messages in thread From: Johannes Berg @ 2016-06-28 10:40 UTC (permalink / raw) To: Jouni Malinen; +Cc: linux-wireless On Sun, 2016-06-19 at 23:51 +0300, Jouni Malinen wrote: > If a user space program (e.g., wpa_supplicant) deletes a STA entry > that > is currently in NL80211_PLINK_ESTAB state, the number of established > plinks counter was not decremented and this could result in rejecting > new plink establishment before really hitting the real maximum plink > limit. For !user_mpm case, this decrementation is handled by > mesh_plink_deactive(). > > Fix this by decrementing estab_plinks on STA deletion > (mesh_sta_cleanup() gets called from there) so that the counter has a > correct value and the Beacon frame advertisement in Mesh > Configuration > element shows the proper value for capability to accept additional > peers. > Applied. johannes ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-06-28 10:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-06-19 20:51 [PATCH] mac80211: Fix mesh estab_plinks counting in STA removal case Jouni Malinen 2016-06-21 19:09 ` Johannes Berg 2016-06-21 22:26 ` Bob Copeland 2016-06-21 22:29 ` Bob Copeland 2016-06-28 10:40 ` Johannes Berg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).