This patch fixes a bug in hwmp_preq_frame_process where the wrong metric
can be used when forwarding a PREQ. This happens because the code uses
the same metric variable to record the value of the metric to the source
of the PREQ and the value of the metric to the target of the PREQ.
This comes into play when both reply and forward are set which happens
when IEEE80211_PREQ_PROACTIVE_PREP_FLAG is set and when MP_F_DO | MP_F_RF
is set. The original code had a special case to handle the first case
but not the second.
The patch uses distinct variables for the two metrics which makes the
code flow much clearer and removes the need to restore the original
value of metric when forwarding.
Signed-off-by: Alexis Green <[email protected]>
CC: Jesse Jones <[email protected]>
---
diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
index 214e63b..bb93170 100644
--- a/net/mac80211/mesh_hwmp.c
+++ b/net/mac80211/mesh_hwmp.c
@@ -510,14 +510,14 @@ static u32 hwmp_route_info_get(struct
ieee80211_sub_if_data *sdata,
static void hwmp_preq_frame_process(struct ieee80211_sub_if_data *sdata,
struct ieee80211_mgmt *mgmt,
- const u8 *preq_elem, u32 metric)
+ const u8 *preq_elem, u32 orig_metric)
{
struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
struct mesh_path *mpath = NULL;
const u8 *target_addr, *orig_addr;
const u8 *da;
u8 target_flags, ttl, flags;
- u32 orig_sn, target_sn, lifetime, orig_metric;
+ u32 orig_sn, target_sn, lifetime, target_metric;
bool reply = false;
bool forward = true;
bool root_is_gate;
@@ -528,7 +528,6 @@ static void hwmp_preq_frame_process(struct
ieee80211_sub_if_data *sdata,
target_sn = PREQ_IE_TARGET_SN(preq_elem);
orig_sn = PREQ_IE_ORIG_SN(preq_elem);
target_flags = PREQ_IE_TARGET_F(preq_elem);
- orig_metric = metric;
/* Proactive PREQ gate announcements */
flags = PREQ_IE_FLAGS(preq_elem);
root_is_gate = !!(flags & RANN_FLAG_IS_GATE);
@@ -539,7 +538,7 @@ static void hwmp_preq_frame_process(struct
ieee80211_sub_if_data *sdata,
mhwmp_dbg(sdata, "PREQ is for us\n");
forward = false;
reply = true;
- metric = 0;
+ target_metric = 0;
if (time_after(jiffies, ifmsh->last_sn_update +
net_traversal_jiffies(sdata)) ||
time_before(jiffies, ifmsh->last_sn_update)) {
@@ -556,7 +555,7 @@ static void hwmp_preq_frame_process(struct
ieee80211_sub_if_data *sdata,
reply = true;
target_addr = sdata->vif.addr;
target_sn = ++ifmsh->sn;
- metric = 0;
+ target_metric = 0;
ifmsh->last_sn_update = jiffies;
}
if (root_is_gate)
@@ -574,7 +573,7 @@ static void hwmp_preq_frame_process(struct
ieee80211_sub_if_data *sdata,
} else if ((!(target_flags & MP_F_DO)) &&
(mpath->flags & MESH_PATH_ACTIVE)) {
reply = true;
- metric = mpath->metric;
+ target_metric = mpath->metric;
target_sn = mpath->sn;
if (target_flags & MP_F_RF)
target_flags |= MP_F_DO;
@@ -593,7 +592,8 @@ static void hwmp_preq_frame_process(struct
ieee80211_sub_if_data *sdata,
mesh_path_sel_frame_tx(MPATH_PREP, 0, orig_addr,
orig_sn, 0, target_addr,
target_sn, mgmt->sa, 0, ttl,
- lifetime, metric, 0, sdata);
+ lifetime, target_metric,
+ 0, sdata);
} else {
ifmsh->mshstats.dropped_frames_ttl++;
}
@@ -619,13 +619,12 @@ static void hwmp_preq_frame_process(struct
ieee80211_sub_if_data *sdata,
if (flags & IEEE80211_PREQ_PROACTIVE_PREP_FLAG) {
target_addr = PREQ_IE_TARGET_ADDR(preq_elem);
target_sn = PREQ_IE_TARGET_SN(preq_elem);
- metric = orig_metric;
}
mesh_path_sel_frame_tx(MPATH_PREQ, flags, orig_addr,
orig_sn, target_flags, target_addr,
target_sn, da, hopcount, ttl, lifetime,
- metric, preq_id, sdata);
+ orig_metric, preq_id, sdata);
if (!is_multicast_ether_addr(da))
ifmsh->mshstats.fwded_unicast++;
else
Resubmitting as v2 due to mangled white spaces.
On Mon, Jun 8, 2015 at 10:28 AM, Jesse Jones <[email protected]> wrote:
> I took a look at the spec too and also didn't see mention of the MP_F_RF
> flag. Section 13.10.9.3 for example talks about PREQ handling and in case E2
> talks about sending replies back. It says to do that if target only is not
> set which makes sense: MP_F_RF and MP_F_DO are exclusive options so it's
> silly to use two different flags for them.
>
> But for the purposes of this patch that is a moot issue: the semantics of
> the function don't change and the original code was both confusing and using
> the wrong value for metric for the not target only, intermediate station
> case.
>
> -- Jesse
>
> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Yeoh Chun-Yeow
> Sent: Monday, June 08, 2015 1:55 AM
> To: [email protected]
> Subject: Re: [PATCH] mac80211: Fix a case of incorrect metric used when
> forwarding a PREQ
>
> As mentioned in Section 13.10.9.3 Case E2, the PREQ forwarding should use
> metric "as received + own metric toward transmitter of received PREQ". So
> you are right.
>
> Just MP_F_RF bit is no longer use. After replying with PREP, forwarding the
> PREQ by setting the IEEE80211_PREQ_TO_FLAG in the per target flags.
>
> ---
> Chun-Yeow
>
> On Mon, Jun 8, 2015 at 4:07 PM, Yeoh Chun-Yeow <[email protected]>
> wrote:
>> Hi, Alexis Green
>>
>> I just take a look back on the code before implementing the Proactive
>> PREQ for PREP. For the case of reply and forward (target_flags &
>> MP_F_RF), the previous metric used is the one which resolved from
>> mpath->metric.
>>
>> Refer here:
>> https://github.com/o11s/open80211s/blob/o11s-0.4.2/net/mac80211/mesh_h
>> wmp.c
>>
>> But to say that, I cannpt confirm whether this is correct. The
>> standard 802.11-2012 has no longer specified bit-1 MP_F_RF (Refer
>> section 8.4.2.115 per target flag). So maybe it is not relevant
>> anymore. We may need to remove as well.
>>
>> ----
>> Chun-Yeow
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected] More majordomo info at
> http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
I took a look at the spec too and also didn't see mention of the MP_F_RF
flag. Section 13.10.9.3 for example talks about PREQ handling and in case E2
talks about sending replies back. It says to do that if target only is not
set which makes sense: MP_F_RF and MP_F_DO are exclusive options so it's
silly to use two different flags for them.
But for the purposes of this patch that is a moot issue: the semantics of
the function don't change and the original code was both confusing and using
the wrong value for metric for the not target only, intermediate station
case.
-- Jesse
-----Original Message-----
From: [email protected]
[mailto:[email protected]] On Behalf Of Yeoh Chun-Yeow
Sent: Monday, June 08, 2015 1:55 AM
To: [email protected]
Subject: Re: [PATCH] mac80211: Fix a case of incorrect metric used when
forwarding a PREQ
As mentioned in Section 13.10.9.3 Case E2, the PREQ forwarding should use
metric "as received + own metric toward transmitter of received PREQ". So
you are right.
Just MP_F_RF bit is no longer use. After replying with PREP, forwarding the
PREQ by setting the IEEE80211_PREQ_TO_FLAG in the per target flags.
---
Chun-Yeow
On Mon, Jun 8, 2015 at 4:07 PM, Yeoh Chun-Yeow <[email protected]>
wrote:
> Hi, Alexis Green
>
> I just take a look back on the code before implementing the Proactive
> PREQ for PREP. For the case of reply and forward (target_flags &
> MP_F_RF), the previous metric used is the one which resolved from
> mpath->metric.
>
> Refer here:
> https://github.com/o11s/open80211s/blob/o11s-0.4.2/net/mac80211/mesh_h
> wmp.c
>
> But to say that, I cannpt confirm whether this is correct. The
> standard 802.11-2012 has no longer specified bit-1 MP_F_RF (Refer
> section 8.4.2.115 per target flag). So maybe it is not relevant
> anymore. We may need to remove as well.
>
> ----
> Chun-Yeow
Hi, Alexis Green
I just take a look back on the code before implementing the Proactive
PREQ for PREP. For the case of reply and forward (target_flags &
MP_F_RF), the previous metric used is the one which resolved from
mpath->metric.
Refer here: https://github.com/o11s/open80211s/blob/o11s-0.4.2/net/mac80211/mesh_hwmp.c
But to say that, I cannpt confirm whether this is correct. The
standard 802.11-2012 has no longer specified bit-1 MP_F_RF (Refer
section 8.4.2.115 per target flag). So maybe it is not relevant
anymore. We may need to remove as well.
----
Chun-Yeow
As mentioned in Section 13.10.9.3 Case E2, the PREQ forwarding should
use metric "as received + own metric toward transmitter of received
PREQ". So you are right.
Just MP_F_RF bit is no longer use. After replying with PREP,
forwarding the PREQ by setting the IEEE80211_PREQ_TO_FLAG in the per
target flags.
---
Chun-Yeow
On Mon, Jun 8, 2015 at 4:07 PM, Yeoh Chun-Yeow <[email protected]> wrote:
> Hi, Alexis Green
>
> I just take a look back on the code before implementing the Proactive
> PREQ for PREP. For the case of reply and forward (target_flags &
> MP_F_RF), the previous metric used is the one which resolved from
> mpath->metric.
>
> Refer here: https://github.com/o11s/open80211s/blob/o11s-0.4.2/net/mac80211/mesh_hwmp.c
>
> But to say that, I cannpt confirm whether this is correct. The
> standard 802.11-2012 has no longer specified bit-1 MP_F_RF (Refer
> section 8.4.2.115 per target flag). So maybe it is not relevant
> anymore. We may need to remove as well.
>
> ----
> Chun-Yeow