Confusion about hwmp_rann_frame_process

[e1001925]

Dear open80211s team,

I am very confused about something inside hwmp_rann_frame_process function. I know this is a solid project and most probably I misunderstood, but still...

Here below is part of the hwmp_rann_frame_process function for your convenience:

    metric_txsta = airtime_link_metric_get(local, sta); //Ok we get current metric

    mpath = mesh_path_lookup(sdata, orig_addr);
    if (!mpath) {
        mpath = mesh_path_add(sdata, orig_addr);
        if (IS_ERR(mpath)) {
            rcu_read_unlock();
            sdata->u.mesh.mshstats.dropped_frames_no_route++;
            return;
        }
    }

    if (!(SN_LT(mpath->sn, orig_sn)) &&
        !(mpath->sn == orig_sn && metric < mpath->rann_metric)) { //Why compare without adding metric_txsta?
        rcu_read_unlock();
        return;
    }

    if ((!(mpath->flags & (MESH_PATH_ACTIVE | MESH_PATH_RESOLVING)) ||
         (time_after(jiffies, mpath->last_preq_to_root +
                  root_path_confirmation_jiffies(sdata)) ||
         time_before(jiffies, mpath->last_preq_to_root))) &&
         !(mpath->flags & MESH_PATH_FIXED) && (ttl != 0)) {
        mhwmp_dbg(sdata,
              "time to refresh root mpath %pM\n",
              orig_addr);
        mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH);//hey we haven't set rann_snd_addr
        mpath->last_preq_to_root = jiffies;
    }

    mpath->sn = orig_sn;
    mpath->rann_metric = metric + metric_txsta; //Now we record after compare?
    mpath->is_root = true;
    /* Recording RANNs sender address to send individually
     * addressed PREQs destined for root mesh STA */
    memcpy(mpath->rann_snd_addr, mgmt->sa, ETH_ALEN); //Like metric, we record after we called mesh_queue_preq

The first question is about metric, we use airtime_link_metric_get to get current airtime, however we compare in this place: _metric < mpath->rannmetric without adding the result metric_txsta , then we add it and record at the end. Will it cause any problem and why not add this? At least, IMHO airtime_link_metric_get seems can be called after all these "return" code, since in many occasions the result is not truly used before return.

The other question is about rann_snd_addr is recorded after mesh_queue_preq function be called. I see preq queued here has to wait for mesh_path_start_discovery function, but how to guarantee the correct sequence about updating rann_snd_addr.

Could anyone please show me some hints?

Many thanks.

[chunyeow]

On Mon, Nov 13, 2017 at 7:52 PM, Michael65535 notifications@github.com wrote:

Dear open80211s team,

I am very confused about something inside hwmp_rann_frame_process function. I know this is a solid project and most probably I misunderstood, but still...

Here below is part of the hwmp_rann_frame_process function for your convenience:

metric_txsta = airtime_link_metric_get(local, sta); //Ok we get current metric

Get the airtime own link metric toward the transmitting mesh STA.

mpath = mesh_path_lookup(sdata, orig_addr); if (!mpath) { mpath = mesh_path_add(sdata, orig_addr); if (IS_ERR(mpath)) { rcu_read_unlock(); sdata->u.mesh.mshstats.dropped_frames_no_route++; return; } }

if (!(SN_LT(mpath->sn, orig_sn)) && !(mpath->sn == orig_sn && metric < mpath->rann_metric)) { //Why compare without adding metric_txsta? rcu_read_unlock(); return; }

Please refer the section 13.10.12.4.2 of IEEE 802.11-2012, the comparison is based on "updated path metric is worse than previous path metric". If you further refer to Table 13-26, the forwarded RANN content for link metric is "As received ⊕ own link metric toward the transmitting mesh STA". So you need to compare without adding the metric_txsta, otherwise it will be added twice.

if ((!(mpath->flags & (MESH_PATH_ACTIVE | MESH_PATH_RESOLVING)) || (time_after(jiffies, mpath->last_preq_to_root + root_path_confirmation_jiffies(sdata)) || time_before(jiffies, mpath->last_preq_to_root))) && !(mpath->flags & MESH_PATH_FIXED) && (ttl != 0)) { mhwmp_dbg(sdata, "time to refresh root mpath %pM\n", orig_addr); mesh_queue_preq(mpath, PREQ_Q_F_START | PREQ_Q_F_REFRESH);//hey we haven't set rann_snd_addr mpath->last_preq_to_root = jiffies; }

mpath->sn = orig_sn; mpath->rann_metric = metric + metric_txsta; //Now we record after compare? mpath->is_root = true; /* Recording RANNs sender address to send individually

• addressed PREQs destined for root mesh STA */ memcpy(mpath->rann_snd_addr, mgmt->sa, ETH_ALEN); //Like metric, we record after we called mesh_queue_preq

The first question is about metric, we use airtime_link_metric_get to get current airtime, however we compare in this place: metric < mpath->rann_metric without adding the result metric_txsta , then we add it and record at the end. Will it cause any problem and why not add this? At least, IMHO airtime_link_metric_get seems can be called after all these "return" code, since in many condition its not truly used before return.

As discussed above. Please refer to the IEEE 802.11-2012 to get clearer picture.

The other question is about rann_snd_addr is recorded after mesh_queue_preq function be called. I see preq queued here has to wait for mesh_path_start_discovery function, but how to guarantee the correct sequence about updating rann_snd_addr

Since it is under RCU-protected section, this should be fine.

---

Chun-Yeow

[e1001925]

Hi chunyeow,

Thanks for your kind reply! I am clear for most of your answers. But I checked the spec, and my confusion is still about contents of "updated path metric".

Since previous path metric(mpath->rann_metric) is the sum of metric before this node(metric) and this node(metric_txsta), now when a fresh RANN is received, is it fair to just compare them without adding my current airtime metric? Could you please provide more explanations about "otherwise it will be added twice“？

[chunyeow]

On Tue, Nov 14, 2017 at 10:56 AM, Michael65535 notifications@github.com wrote:

Hi chunyeow,

Thanks for your kind reply! I am clear for most of your answers. But I checked the spec, and my confusion is still about contents of "updated path metric".

Since previous path metric(mpath->rann_metric) is the sum of metric before this node(metric) and this node(metric_txsta), now when a fresh RANN is received, is it fair to just compare them without adding my current airtime metric? Could you please provide some explanation about "otherwise it will be added twice“？

Yes, you are right. After reviewing the code again, I think that we need to add the own link metric toward the transmitting mesh STA. Please apply the attached patch for your Linux code base.

Thanks for reporting.

---

Chun-Yeow

[e1001925]

Hi chunyeow,

Glad to know that. Since in my environment it triggered frequently and forward RANN with worse metric.. Thanks for your confirmation.

[chunyeow]

Thanks for reporting.

---

Chun-Yeow

On Wed, Nov 15, 2017 at 11:10 AM, Michael65535 notifications@github.com wrote:

Hi chunyeow,

Glad to know that. Since in my environment it triggered frequently and forward RANN with worse metric.. Thanks for your confirmation.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/o11s/open80211s/issues/69#issuecomment-344472666, or mute the thread https://github.com/notifications/unsubscribe-auth/ABBewgKa3-nDMTmpkQ7KY5tBW6ZVDnLpks5s2lYhgaJpZM4QbpgU .