Return-path: Received: from mail-ob0-f174.google.com ([209.85.214.174]:50720 "EHLO mail-ob0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752162Ab2CUGkK convert rfc822-to-8bit (ORCPT ); Wed, 21 Mar 2012 02:40:10 -0400 Received: by obbeh20 with SMTP id eh20so505480obb.19 for ; Tue, 20 Mar 2012 23:40:09 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1332333182-11941-1-git-send-email-yeohchunyeow@gmail.com> References: <1332333182-11941-1-git-send-email-yeohchunyeow@gmail.com> From: Javier Cardona Date: Tue, 20 Mar 2012 23:39:49 -0700 Message-ID: (sfid-20120321_074014_951175_86B06E67) Subject: Re: [PATCH] mac80211: fix the RANN propagation issues To: Chun-Yeow Yeoh Cc: linux-wireless@vger.kernel.org, johannes@sipsolutions.net, thomas@cozybit.com, linville@tuxdriver.com, devel@lists.open80211s.org Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-wireless-owner@vger.kernel.org List-ID: Yeoh, Looks good from here. Just one question (inline), which is probably more for Thomas than you... On Wed, Mar 21, 2012 at 5:33 AM, Chun-Yeow Yeoh wrote: > This patch is intended to solve the follwing issues in RANN propagation: > [1] The interval in propagated RANN should be based on the interval of received RANN. > [2] The aggregated path metric for propagated RANN is as received plus own link metric > ? ?towards the transmitting mesh STA (not root mesh STA). > [3] The comparison of path metric for RANN with same sequence number should be done > ? ?before deciding whether to propagate or not. > > Signed-off-by: Chun-Yeow Yeoh > --- > ?net/mac80211/mesh.h ? ? ?| ? ?2 ++ > ?net/mac80211/mesh_hwmp.c | ? 21 +++++++++++++++++---- > ?2 files changed, 19 insertions(+), 4 deletions(-) > > diff --git a/net/mac80211/mesh.h b/net/mac80211/mesh.h > index 8d53b71..adf1be3 100644 > --- a/net/mac80211/mesh.h > +++ b/net/mac80211/mesh.h > @@ -86,6 +86,7 @@ enum mesh_deferred_task_flags { > ?* mpath itself. ?No need to take this lock when adding or removing > ?* an mpath to a hash bucket on a path table. > ?* @rann_snd_addr: the RANN sender address > + * @rann_metric: the aggregated path metric towards the root node > ?* @is_root: the destination station of this path is a root node > ?* @is_gate: the destination station of this path is a mesh gate > ?* > @@ -112,6 +113,7 @@ struct mesh_path { > ? ? ? ?enum mesh_path_flags flags; > ? ? ? ?spinlock_t state_lock; > ? ? ? ?u8 rann_snd_addr[ETH_ALEN]; > + ? ? ? u32 rann_metric; > ? ? ? ?bool is_root; > ? ? ? ?bool is_gate; > ?}; > diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c > index f80a9e3..fcfebc7 100644 > --- a/net/mac80211/mesh_hwmp.c > +++ b/net/mac80211/mesh_hwmp.c > @@ -88,6 +88,7 @@ static inline u32 u16_field_get(u8 *preq_elem, int offset, bool ae) > ?#define MSEC_TO_TU(x) (x*1000/1024) > ?#define SN_GT(x, y) ((long) (y) - (long) (x) < 0) > ?#define SN_LT(x, y) ((long) (x) - (long) (y) < 0) > +#define SN_EQ(x, y) ((long) (x) - (long) (y) == 0) I never understood the point of these macros so I'm not too happy about seeing another one creep in :) Can someone explain why we need them? Why not just use good old-fashioned '==' (and '<' or '>')? > ?#define net_traversal_jiffies(s) \ > ? ? ? ?msecs_to_jiffies(s->u.mesh.mshcfg.dot11MeshHWMPnetDiameterTraversalTime) > @@ -732,11 +733,12 @@ static void hwmp_rann_frame_process(struct ieee80211_sub_if_data *sdata, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct ieee80211_rann_ie *rann) > ?{ > ? ? ? ?struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh; > + ? ? ? struct ieee80211_local *local = sdata->local; > + ? ? ? struct sta_info *sta; > ? ? ? ?struct mesh_path *mpath; > ? ? ? ?u8 ttl, flags, hopcount; > ? ? ? ?u8 *orig_addr; > - ? ? ? u32 orig_sn, metric; > - ? ? ? u32 interval = ifmsh->mshcfg.dot11MeshHWMPRannInterval; > + ? ? ? u32 orig_sn, metric, metric_txsta, interval; > ? ? ? ?bool root_is_gate; > > ? ? ? ?ttl = rann->rann_ttl; > @@ -749,6 +751,7 @@ static void hwmp_rann_frame_process(struct ieee80211_sub_if_data *sdata, > ? ? ? ?root_is_gate = !!(flags & RANN_FLAG_IS_GATE); > ? ? ? ?orig_addr = rann->rann_addr; > ? ? ? ?orig_sn = le32_to_cpu(rann->rann_seq); > + ? ? ? interval = le32_to_cpu(rann->rann_interval); > ? ? ? ?hopcount = rann->rann_hopcount; > ? ? ? ?hopcount++; > ? ? ? ?metric = le32_to_cpu(rann->rann_metric); > @@ -761,6 +764,14 @@ static void hwmp_rann_frame_process(struct ieee80211_sub_if_data *sdata, > ? ? ? ? ? ? ? ? ? ? ? ?orig_addr, mgmt->sa, root_is_gate); > > ? ? ? ?rcu_read_lock(); > + ? ? ? sta = sta_info_get(sdata, mgmt->sa); > + ? ? ? if (!sta) { > + ? ? ? ? ? ? ? rcu_read_unlock(); > + ? ? ? ? ? ? ? return; > + ? ? ? } > + > + ? ? ? metric_txsta = airtime_link_metric_get(local, sta); > + > ? ? ? ?mpath = mesh_path_lookup(orig_addr, sdata); > ? ? ? ?if (!mpath) { > ? ? ? ? ? ? ? ?mesh_path_add(orig_addr, sdata); > @@ -780,14 +791,16 @@ static void hwmp_rann_frame_process(struct ieee80211_sub_if_data *sdata, > ? ? ? ? ? ? ? ?mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH); > ? ? ? ?} > > - ? ? ? if (mpath->sn < orig_sn && ifmsh->mshcfg.dot11MeshForwarding) { > + ? ? ? if ((SN_LT(mpath->sn, orig_sn) || (SN_EQ(mpath->sn, orig_sn) && > + ? ? ? ? ?metric < mpath->rann_metric)) && ifmsh->mshcfg.dot11MeshForwarding) { i.e. if ((mpath->sn < orig_sn || (mpath->sn == orig_sn && metric < mpath->rann_metric)) && ifmsh->mshcfg.dot11MeshForwarding) { > ? ? ? ? ? ? ? ?mesh_path_sel_frame_tx(MPATH_RANN, flags, orig_addr, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? cpu_to_le32(orig_sn), > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0, NULL, 0, broadcast_addr, > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hopcount, ttl, cpu_to_le32(interval), > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cpu_to_le32(metric + mpath->metric), > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cpu_to_le32(metric + metric_txsta), > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0, sdata); > ? ? ? ? ? ? ? ?mpath->sn = orig_sn; > + ? ? ? ? ? ? ? mpath->rann_metric = metric + metric_txsta; > ? ? ? ?} > > ? ? ? ?/* Using individually addressed PREQ for root node */ > -- > 1.7.0.4 > Thanks, Javier