On Fri, Apr 4, 2014 at 11:39 AM, Thomas Pedersen <[email protected]> wrote:
> On Thu, Apr 3, 2014 at 9:57 PM, Colleen Twitty <[email protected]> wrote:
>> [PATCH] Update target SN when a new origin SN is RX'ed.
>>
>> Before possibly updating the mpath towards a station in
>> hwmp_route_info_get, get the SN from the mpath table.
>> Then pass this on to the function that processes PREQ's so
>> that we can use this information to update the target SN
>> if the origin SN has been updated.
>
> Why? What bug does this patch fix?
>
>> Signed-off-by: Colleen Twitty <[email protected]>
>> ---
>> This patch is meant to solve the some issue as Bob's, but with a different
>> approach.  This removes considering net_traversal_jiffies in updating the
>> sequence number.
>
> Does that mean we can get rid of net_traversal_jiffies entirely then?
> PREQs are already gated by dot11MeshHWMPpreqMinInterval, so it seems
> kind of redundant now.

No, they are used correctly in limiting the increment rate of a STA's
origin SN.  I think we still need them there.

>>  net/mac80211/mesh_hwmp.c | 39 +++++++++++++++++++++++++++++++--------
>>  1 file changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/net/mac80211/mesh_hwmp.c b/net/mac80211/mesh_hwmp.c
>> index f951468..505e180 100644
>> --- a/net/mac80211/mesh_hwmp.c
>> +++ b/net/mac80211/mesh_hwmp.c
>> @@ -511,7 +511,8 @@ 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 metric,
>> +                                   bool chk_old_sn, u32 old_sn)
>>  {
>>         struct ieee80211_if_mesh *ifmsh = &sdata->u.mesh;
>>         struct mesh_path *mpath = NULL;
>> @@ -541,12 +542,9 @@ static void hwmp_preq_frame_process(struct 
>> ieee80211_sub_if_data *sdata,
>>                 forward = false;
>>                 reply = true;
>>                 metric = 0;
>> -               if (time_after(jiffies, ifmsh->last_sn_update +
>> -                                       net_traversal_jiffies(sdata)) ||
>> -                   time_before(jiffies, ifmsh->last_sn_update)) {
>> -                       target_sn = ++ifmsh->sn;
>> -                       ifmsh->last_sn_update = jiffies;
>> -               }
>> +       if (chk_old_sn && SN_LT(old_sn, orig_sn))
>> +               ifmsh->sn = max(ifmsh->sn, target_sn)+1;
>> +       target_sn = ifmsh->sn;
>>         } else if (is_broadcast_ether_addr(target_addr) &&
>>                    (target_flags & IEEE80211_PREQ_TO_FLAG)) {
>>                 rcu_read_lock();
>> @@ -855,7 +853,11 @@ void mesh_rx_path_sel_frame(struct 
>> ieee80211_sub_if_data *sdata,
>>         struct ieee802_11_elems elems;
>>         size_t baselen;
>>         u32 last_hop_metric;
>> +       const u8 *orig_addr;
>>         struct sta_info *sta;
>> +       bool chk_old_sn = false;
>> +       u32 old_sn;
>> +       struct mesh_path *mpath = NULL;
>>
>>         /* need action_code */
>>         if (len < IEEE80211_MIN_ACTION_SIZE + 1)
>> @@ -877,11 +879,32 @@ void mesh_rx_path_sel_frame(struct 
>> ieee80211_sub_if_data *sdata,
>>                 if (elems.preq_len != 37)
>>                         /* Right now we support just 1 destination and no AE 
>> */
>>                         return;
>> +
>> +               orig_addr = PREQ_IE_ORIG_ADDR(elems.preq);
>> +
>> +               /* Sec. 13.10.8.3 HWMP sequence numbering calls for the 
>> target
>> +               * HWMP seq number to be incremented after each processed 
>> PREQ,
>> +               * but that would result in all but the last PREP metric info
>> +               * to be ignored.  Instead, we only increment the target SN
>> +               * when we detect that the originator SN has changed.  This
>> +               * results in all the PREPs generated in response to a single
>> +               * PREQ to have the same seq num, and, therefore, for the
>> +               * metrics for the different paths to be taken into account.
>> +               */
>> +               rcu_read_lock();
>> +               mpath = mesh_path_lookup(sdata, orig_addr);
>> +               if (mpath) {
>> +                       chk_old_sn = true;
>> +                       old_sn = mpath->sn;
>> +               }
>> +               rcu_read_unlock();
>
> hwmp_preq_frame_process() already does a mesh_path_lookup(), so why do
> it out here? If you get rid of this you could save the parameter bloat
> as well.

Yes, hwmp_preq_fame_process does a mesh-path_lookup.  However, it also
may update the information in the mpath table, and I am trying to
detect if the origin SN has changed. I am doing this lookup in order
to get the SN before it is (maybe) updated.  I think the next step,
assuming this is the behavior we want for updating the target sequence
number, is to restructure some of the mpath code to avoid this.  Can
you see a more straightfoward path here?

>>                 last_hop_metric = hwmp_route_info_get(sdata, mgmt, 
>> elems.preq,
>>                                                       MPATH_PREQ);
>>                 if (last_hop_metric)
>>                         hwmp_preq_frame_process(sdata, mgmt, elems.preq,
>> -                                               last_hop_metric);
>> +                                               last_hop_metric, chk_old_sn,
>> +                                               old_sn);
>>         }
>>         if (elems.prep) {
>>                 if (elems.prep_len != 31)
>> --
>> 1.8.4.3
>>
>> _______________________________________________
>> Devel mailing list
>> [email protected]
>> http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel
> _______________________________________________
> Devel mailing list
> [email protected]
> http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel
_______________________________________________
Devel mailing list
[email protected]
http://lists.open80211s.org/cgi-bin/mailman/listinfo/devel

Reply via email to