Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1432869 > unrolled thread

[PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped Yaniv Machani <yanivma@ti.com> - 2016-06-28 13:20 +0200
    [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting Yaniv Machani <yanivma@ti.com> - 2016-06-28 13:20 +0200
      Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh  peer is disconnecting Bob Copeland <me@bobcopeland.com> - 2016-06-28 14:30 +0200
        RE: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh  peer is disconnecting "Machani, Yaniv" <yanivma@ti.com> - 2016-06-28 16:50 +0200
    Re: [PATCH 1/4] mac80211: mesh: flush stations before beacons are  stopped Johannes Berg <johannes@sipsolutions.net> - 2016-06-29 09:20 +0200

#1432869 — [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped

From: Maital Hahn <maitalm@ti.com>

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 <maitalm@ti.com>
Acked-by: Yaniv Machani <yanivma@ti.com>
---
 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

[toc] | [next] | [standalone]

#1432871 — [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting

From: Maital Hahn <maitalm@ti.com>

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 <maitalm@ti.com>
Acked-by: Yaniv Machani <yanivma@ti.com>
---
 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

[toc] | [prev] | [next] | [standalone]

#1432939 — 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 <maitalm@ti.com>
> 
> 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 <maitalm@ti.com>
> Acked-by: Yaniv Machani <yanivma@ti.com>
> ---
>  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/

[toc] | [prev] | [next] | [standalone]

#1433036 — RE: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting

On Tue, Jun 28, 2016 at 15:27:47, Bob Copeland wrote:
> linux- wireless@vger.kernel.org; netdev@vger.kernel.org; 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 <maitalm@ti.com>
> >
> > 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
 

[toc] | [prev] | [next] | [standalone]

#1433502 — 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 <maitalm@ti.com>
> 
> 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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web