All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bob Copeland <me@bobcopeland.com>
To: Yaniv Machani <yanivma@ti.com>
Cc: linux-kernel@vger.kernel.org,
	Johannes Berg <johannes@sipsolutions.net>,
	"David S . Miller" <davem@davemloft.net>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	Maital Hahn <maitalm@ti.com>
Subject: Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting
Date: Tue, 28 Jun 2016 08:27:47 -0400	[thread overview]
Message-ID: <20160628122747.GA7757@localhost> (raw)
In-Reply-To: <20160628111307.8784-3-yanivma@ti.com>

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/

WARNING: multiple messages have this Message-ID (diff)
From: Bob Copeland <me-aXfl/3sk2vNUbtYUoyoikg@public.gmane.org>
To: Yaniv Machani <yanivma-l0cyMroinI0@public.gmane.org>
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Johannes Berg <johannes-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>,
	"David S . Miller"
	<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Maital Hahn <maitalm-l0cyMroinI0@public.gmane.org>
Subject: Re: [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting
Date: Tue, 28 Jun 2016 08:27:47 -0400	[thread overview]
Message-ID: <20160628122747.GA7757@localhost> (raw)
In-Reply-To: <20160628111307.8784-3-yanivma-l0cyMroinI0@public.gmane.org>

On Tue, Jun 28, 2016 at 02:13:05PM +0300, Yaniv Machani wrote:
> From: Maital Hahn <maitalm-l0cyMroinI0@public.gmane.org>
> 
> 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-l0cyMroinI0@public.gmane.org>
> Acked-by: Yaniv Machani <yanivma-l0cyMroinI0@public.gmane.org>
> ---
>  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/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2016-06-28 12:28 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-28 11:13 [PATCH 0/4] Mesh mpm fixes and enhancements Yaniv Machani
2016-06-28 11:13 ` [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped Yaniv Machani
2016-06-28 11:13   ` [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting Yaniv Machani
2016-06-28 11:13     ` [PATCH 3/4] mac80211: mesh: fixed HT ies in beacon template Yaniv Machani
2016-06-28 11:13       ` [PATCH 4/4] mac80211: sta_info: max_peers reached falsely Yaniv Machani
2016-06-28 12:56         ` Bob Copeland
2016-06-28 13:05           ` Machani, Yaniv
2016-06-28 14:06       ` [PATCH 3/4] mac80211: mesh: fixed HT ies in beacon template Bob Copeland
2016-06-29  7:17       ` Johannes Berg
2016-06-29  7:17         ` Johannes Berg
2016-07-13 11:15         ` Machani, Yaniv
2016-07-13 11:15           ` Machani, Yaniv
2016-07-13 11:15           ` Machani, Yaniv
2016-06-28 12:27     ` Bob Copeland [this message]
2016-06-28 12:27       ` [PATCH 2/4] mac80211/cfg: mesh: fix healing time when a mesh peer is disconnecting Bob Copeland
2016-06-28 14:43       ` Machani, Yaniv
2016-06-29  7:14   ` [PATCH 1/4] mac80211: mesh: flush stations before beacons are stopped Johannes Berg
2016-07-13 10:11     ` Machani, Yaniv
2016-07-13 10:11       ` Machani, Yaniv
2016-07-13 10:11       ` Machani, Yaniv
2016-07-13 13:33       ` Bob Copeland
2016-07-13 19:54         ` Machani, Yaniv
2016-08-01  9:38       ` Johannes Berg
2016-06-29  4:43 ` [PATCH 0/4] Mesh mpm fixes and enhancements Julian Calaby

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160628122747.GA7757@localhost \
    --to=me@bobcopeland.com \
    --cc=davem@davemloft.net \
    --cc=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=maitalm@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=yanivma@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.