2012-03-21 03:56:43

by Chun-Yeow Yeoh

[permalink] [raw]
Subject: [PATCH] mac80211: fix the RANN propagation issues

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 <[email protected]>
---
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)

#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) {
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



2012-03-21 17:39:09

by Thomas Pedersen

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the RANN propagation issues

Hi,

>> 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 '>')?
>

I suggested using these macros to remain consistent with other places
in the code. Looking at them again, I don't see how "u32 < u32" , etc.
might fail. If no one has any insight into why this might be a bug,
just use ordinary conditionals for now. Later (if you feel like it),
you can submit a patch removing these macros.

>> ?#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

2012-03-21 06:40:10

by Javier Cardona

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix the RANN propagation issues

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 <[email protected]> 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 <[email protected]>
> ---
> ?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