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]>
---
v2:
revert back to plain conditionals (Javier and Thomas)
net/mac80211/mesh.h | 2 ++
net/mac80211/mesh_hwmp.c | 20 ++++++++++++++++----
2 files changed, 18 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..14bb1bd 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -732,11 +732,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 +750,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 +763,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 +790,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 ((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
On Thu, Mar 22, 2012 at 11:58 AM, Marek Lindner <[email protected]> wrote:
> On Thursday, March 22, 2012 17:31:33 Javier Cardona wrote:
>> >> ?#define SN_GT(x, y) ((long) (y) - (long) (x) < 0)
>> >> ?#define SN_LT(x, y) ((long) (x) - (long) (y) < 0)
>> >
>> > Your macros tried to address the problem but casting your sequence number
>> > to long also breaks the wrap around.
>>
>> Ah, thanks for your help. ?I guess we do need those macros after all
>> but they'd have to be re-written as
>>
>> #define SN_LT(x, y) ((s32)(x - y) < 0)
>> #define SN_GT(x, y) ((s32)(x - y) > 0)
>
> No, you need unsigned values for this arithmetic to work (which is why long
> also fails).
The macro is always used on u32 values, so the arithmetic would be
unsigned in this case.
Cheers,
Javier
On Thu, Mar 22, 2012 at 5:47 PM, Yeoh Chun-Yeow <[email protected]> wrote:
> "In the section 11C.9.8.3 HWMP sequence numbering, Comparing HWMP
> sequence numbers is done using a circular modulo 232 comparison."
That would be section 13.10.8.3 in the ratified version of the
standard. And it's modulo 2?? (2^32), not 232 :)
Cheers,
Javier
On Thursday, March 22, 2012 20:13:28 Javier Cardona wrote:
> On Thu, Mar 22, 2012 at 11:58 AM, Marek Lindner <[email protected]>
wrote:
> > On Thursday, March 22, 2012 17:31:33 Javier Cardona wrote:
> >> >> #define SN_GT(x, y) ((long) (y) - (long) (x) < 0)
> >> >> #define SN_LT(x, y) ((long) (x) - (long) (y) < 0)
> >> >
> >> > Your macros tried to address the problem but casting your sequence
> >> > number to long also breaks the wrap around.
> >>
> >> Ah, thanks for your help. I guess we do need those macros after all
> >> but they'd have to be re-written as
> >>
> >> #define SN_LT(x, y) ((s32)(x - y) < 0)
> >> #define SN_GT(x, y) ((s32)(x - y) > 0)
> >
> > No, you need unsigned values for this arithmetic to work (which is why
> > long also fails).
>
> The macro is always used on u32 values, so the arithmetic would be
> unsigned in this case.
Ok - then you should be on the safe side.
Regards,
Marek
Hi,
I am not familiar with the 802.11s code and could be totally wrong here but ..
On Thursday, March 22, 2012 13:56:08 Chun-Yeow Yeoh wrote:
> + if ((mpath->sn < orig_sn || (mpath->sn == orig_sn &&
.. this sequence number handling looks broken to me. What happens when the
sequence number wraps around ? One could be smaller than the other and still
be newer.
I know this isn't really what your patch changes.
> #define SN_GT(x, y) ((long) (y) - (long) (x) < 0)
> #define SN_LT(x, y) ((long) (x) - (long) (y) < 0)
Your macros tried to address the problem but casting your sequence number to
long also breaks the wrap around.
Regards,
Marek
Hi, Marek
Thanks. Appreciate your comments. HWMP sequence number is 32bits or 4
octets. That's why it is defined as u32.
Hi, Javier
Thanks for your explanation and clarification.
I will resubmit the patch for the changes.
"In the section 11C.9.8.3 HWMP sequence numbering, Comparing HWMP
sequence numbers is done using a circular modulo 232 comparison."
Regards,
Chun-Yeow
On Thu, Mar 22, 2012 at 5:56 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]>
Acked-by: Javier Cardona <[email protected]>
> ---
> v2:
> revert back to plain conditionals (Javier and Thomas)
>
> ?net/mac80211/mesh.h ? ? ?| ? ?2 ++
> ?net/mac80211/mesh_hwmp.c | ? 20 ++++++++++++++++----
> ?2 files changed, 18 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..14bb1bd 100644
> --- a/net/mac80211/mesh_hwmp.c
> +++ b/net/mac80211/mesh_hwmp.c
> @@ -732,11 +732,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 +750,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 +763,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 +790,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 ((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
>
--
Javier Cardona
cozybit Inc.
http://www.cozybit.com
Hi Marek,
On Thu, Mar 22, 2012 at 3:33 AM, Marek Lindner <[email protected]> wrote:
>
> Hi,
>
> I am not familiar with the 802.11s code and could be totally wrong here but ..
>
>
> On Thursday, March 22, 2012 13:56:08 Chun-Yeow Yeoh wrote:
>> + ? ? ? if ((mpath->sn < orig_sn || (mpath->sn == orig_sn &&
>
> .. this sequence number handling looks broken to me. What happens when the
> sequence number wraps around ? One could be smaller than the other and still
> be newer.
> I know this isn't really what your patch changes.
>
>
>> ?#define SN_GT(x, y) ((long) (y) - (long) (x) < 0)
>> ?#define SN_LT(x, y) ((long) (x) - (long) (y) < 0)
>
> Your macros tried to address the problem but casting your sequence number to
> long also breaks the wrap around.
Ah, thanks for your help. I guess we do need those macros after all
but they'd have to be re-written as
#define SN_LT(x, y) ((s32)(x - y) < 0)
#define SN_GT(x, y) ((s32)(x - y) > 0)
And since this is just to handle wrap arounds, there is no need for
the new SN_EQ that was suggested earlier.
Cheers,
Javier
On Thursday, March 22, 2012 17:31:33 Javier Cardona wrote:
> >> #define SN_GT(x, y) ((long) (y) - (long) (x) < 0)
> >> #define SN_LT(x, y) ((long) (x) - (long) (y) < 0)
> >
> > Your macros tried to address the problem but casting your sequence number
> > to long also breaks the wrap around.
>
> Ah, thanks for your help. I guess we do need those macros after all
> but they'd have to be re-written as
>
> #define SN_LT(x, y) ((s32)(x - y) < 0)
> #define SN_GT(x, y) ((s32)(x - y) > 0)
No, you need unsigned values for this arithmetic to work (which is why long
also fails).
> And since this is just to handle wrap arounds, there is no need for
> the new SN_EQ that was suggested earlier.
Sounds reasonable to me.
Regards,
Marek