[aodvv2-discuss] Re: Fwd: [manet] A few more AODVv2 comments
- From: Victoria Mercieca <vmercieca0@xxxxxxxxx>
- To: "aodvv2-discuss@xxxxxxxxxxxxx" <aodvv2-discuss@xxxxxxxxxxxxx>
- Date: Tue, 24 Mar 2015 15:47:54 +0000
Hi Charlie,
In response to the 8a update and the algorithms in Appendix A, here's some
changes to make:
================
MetricType related:
================
In the list of rteMsg parameters, I'd suggest moving the metricType
parameter to after the targSeqNum.
In Generate_RREQ:
outRREQ.metricType := if not DEFAULT_METRIC_TYPE,
metric type needed by application
can be moved to just above the origAddrMetric bit. - could remove the "if
not default metric type"?
if (outRREQ.metricType != DEFAULT_METRIC_TYPE)
{ /* Build MetricType Message TLV */
metricMsgTlv.value := outRREQ.metricType
}
should be removed, and replaced with (lower down
where metricAddrBlkTlv.value is set)
metricAddrBlkTlv.typeExtension := outRREQ.MetricType
In Regenerate_RREQ, the same two points apply.
In Generate_RREP:
if (rte.metricType != DEFAULT_METRIC_TYPE)
outRREP.metricType := rte.metricType
can be moved lower to just above the targAddrMetric.- could remove the "if
not default metric type"?
if (outRREP.metricType != DEFAULT_METRIC_TYPE)
{ /* Build MetricType Message TLV */
metricMsgTlv.value := outRREP.metricType
}
can be removed and replaced with (lower down where metricAddrBlkTlv.value
is set)
metricAddrBlkTlv.typeExtension := outRREP.MetricType
In Regenerate_RREP, the same two points apply.
In A.4 about RERR, do we still use metric type the same way, i.e. one
metric type per message? if so this looks fine as is.
/* optional to include outRERR.metricType */ should be removed? If caused
by undeliverable packet, there was no route, so we have no metric type to
put in.
if (outRERR.metricType != DEFAULT_METRIC_TYPE)
{ /* Build MetricType Message TLV */
metricMsgTlv.value := outRERR.metricType
}
should be removed and replaced with (lower down)
/* Build Metric Address Block TLV containing MetricType as type extension
but no value */
metricAddrBlkTlv.typeExtension := outRREP.metricType
In A.4.3, for Regenerte_RERR, the same point applies.
I think that's all!
==========================
Non-MetricType related:
==========================
In A.1.1 we have if (rte exists) then create route...oops!
To match earlier sections we should put loopfree within the "rule from 8.1"
code and update that to say if seqnum is the same, AND (if cost is less, OR
(if cost is same or greater and LoopFree is true)) then update route entry.
In Fetch_Route_Table_Entry, we compare to "MetricType" but the m should not
be capitalised, since metricType (lower case m) is what is passed to this
function.
For LoopFree, we should probably replace R2 and R1 with AdvRte and Rte for
consistency
In Update_Rte_Msg_Table, we should be storing metric too under "if (entry
does not exist)":
entry.Metric := rteMsg.origAddrMetric(for RREQ) or
rteMsg.targAddrMetric(for RREP)
We also have "rteMsg.OrigSeqNum" and "rteMsg.TargSeqNum" where the o and t
should be lower case to match the definitions at the start of Appendix A.
Although it seems that the definition of rteMsg parameters is inconsistent
with the definition of AdvRte and Route parameters, perhaps they should all
be capitalised throughout the appendix? And maybe we should define
parameters of the RteMsgTable entries up there too?
If you want:
The RteMsg table entries have the following parameters as described in
Section 6.5:
RteMsgTableEntry.MessageType
RteMsgTableEntry.OrigAddr
RteMsgTableEntry.TargAddr
RteMsgTableEntry.OrigSeqNum (RREQ only)
RteMsgTableEntry.TargSeqNum (RREP only)
RteMsgTableEntry.MetricType
RteMsgTableEntry.Metric
RteMsgTableEntry.Timestamp
In A.3.3 we need an extra line between metricAddrBlkTlv.value :=
outRREP.targAddrMetric and the validity time line.
In A.4 we should be consistent with the other sections using capital
letters for parameters.
In A.4.1 we should somehow stick in that the RERR isn't generated if only
Idle routes are affected. Or just remove
OR
(rte.State == Idle AND ENABLE_IDLE_IN_RERR)
for now? or after the foreach (rte in route table)
add:
/* repeat for-loop for Idle routes if num-broken-addr > 0 AND
ENABLE_IDLE_IN_RERR */
By the way, a quick google search for "aodvv2 issue tracker" showed up a
lot of our discussion list emails!
Regards,
Vicky.
On Fri, Mar 20, 2015 at 12:48 AM, Charlie Perkins <
charles.perkins@xxxxxxxxxxxxx> wrote:
> Hello Vicky and all,
>
> Attached, please find a revised text incorporating most of the suggested
> changes. I have to leave right now, so unfortunately I have not proofread
> the results but I wanted to send it out to you. I did not change the
> pseudocode in the Appendix, which needs to be done.
>
> I am sick. Bad timing. I hope to be well before stepping onto the
> airplane
> tomorrow.
>
> Regards,
> Charlie P.
>
>
> On 3/19/2015 4:52 AM, Victoria Mercieca wrote:
>
>
>
>>
>>
>> This could be changed to say "Lower layer trigger that a link is no
>> longer bidirectional"
>>
>> or "Lower layer triggers, for example message reception or link status
> notifications".
> The fact we are receiving L2 frames could be seen as a confirmation of
> bidirectionality?
>
>
> Done.
>
>
>>
>> I would have explicitly said NHDP HELLO message, not just message later
>> in that section (though it is the only NHDP message).
>>
>>
>> That enlargement fits well in Section 4 during the discussion where NHDP
>> is mentioned, and I put it there.
>>
>> Easily added in the list though?
>
>
> I didn't think it fit nicely into that list, but on your recommendation I
> put it there also.
>
>
>> Blacklist removal seems to be MAY at one point, and SHOULD at another.
>>
>> Fixed.
>>
>> Also that’s discussing things that trigger additions to blacklists (e.g.
>> failure to receive RREP_Ack) and removals due to timeouts. Might there not
>> also be specific reasons to remove from blacklists (e.g. receiving NHDP
>> HELLO that indicates that if using that option)?
>>
>> Yes, this is a good point.
>>
>> True, we have already put in that if RREP_Ack is received from a
> router on the blacklist, it is removed from the blacklist. Maybe in Section
> 6.2 where we discuss adding to the blacklist if we receive notification of
> a non-bidirectional link, we also mention "If any of these triggers signal
> that bidirectional connectivity exists to a blacklisted router, the router
> can be immediately removed from the blacklist." or something similar?
>
>
> Done.
>
>
> It seems rather a lack of progress that we are still at the only form
>> of metric that an implementation will know what to do with is a hop count.
>>
>> That's not true. It is known how to use other metrics, and in fact
>> specifications for AODV were written for the other metrics. However those
>> specifications have been considered to be out of scope for the base
>> document.
>>
>> Well, almost. Most of the discussion of metrics appears to require only
>> an increasing function. Except the penultimate bullet in the first list in
>> Section 8.1 appears to be explicitly additive. Once you decide on additive,
>> you can make the whole metrics decision much more open.
>>
>> Having gone to the trouble of including mechanism for alternative
>> metrics, it would seem a shame to make such a severe restriction.
>>
>>
>> Is it enough to say that the current draft will support any additive
> metric / increasing metric, using Cost(R) = AdvRte.Metric + Cost(L), where
> Cost(L) > 0, but also that LoopFree may be further optimised for other
> metrics? Not mentioning specific metric types but making it extra clear
> that its possible with the current spec to use a different metric. Note -
> our Section 7.4 doesnt actually say a different LoopFree may also be
> required.
>
>
>
> I’d go with Henning’s suggestion to fix a single data type (32 bit
>> integer perhaps) which would replace the second paragraph of 7.4 and make
>> things much more easily extensible.
>>
>>
>> A single data type for metric cannot accurately represent the costs of
>> the routes. All you can get is monotonicity. Or maybe I misunderstand?
>>
>> Section 8.1, from the numbered points onwards, appears to give three
>> descriptions of how you handle competing routes. One is good, if completely
>> rigorous. Two is fine if the other one is more human readable (and possibly
>> belongs in an earlier section). Three appears more than is wanted. (I also
>> note the use of both AND and && in the second.) Note that I haven’t
>> actually checked if all three are consistent (that’s not trivial when they
>> switch between formulae and terms like stale) let alone if they work. (I
>> recall someone has done some work on the latter point, is that
>> published/referenceable?)
>>
>> I agree with this, and perhaps the pseudo-code version is better left
> in the appendix in A.1.1? And we may not need the summary at the end of 8.1?
> We do have inconsistency with where LoopFree is checked. It is step 3 in
> section 8.1, which means it is currently not checked if the advertised
> route is lower cost, and is only checked if its repairing an invalid route.
>
>
> Well, actually, that's pretty much right. I'm not sure about deleting the
> summary.
> Any summary can be viewed as duplicative. However, there are people in
> the world
> who like to get an overall understanding of what's going on, even at the
> cost of a
> few sentences.
>
> But in the appendix we check LoopFree first before even checking
> SeqNum. I suppose the correct thing would be to check it after determining
> the incoming information is not stale.
>
>
> Not really, because if the information is not stale and the cost is less
> we want to
> use the route without calling LoopFree().
>
>
>
> Suggestions:
>
> 8.1:
> Switch step 2 and 3.
> Add in to the new Step 2: "If LoopFree returns true, continue to step 3."
> The final point in the new Step 3 should be: "If the advertised route's
> cost is the same or greater than the stored route, but the stored route's
> state is Invalid, the advertised route can be used to repair the Invalid
> route, since it has been proven LoopFree in step 2."
> Remove the rest of the section. At least remove the pseudo-code since this
> is slightly inconsistent as is, and is present in a more accurate form in
> the appendix.
>
> A.1.1:
>
> /* rules from 8.1 */
>
> if (AdvRte.SeqNum < Route.SeqNum) /* advertised route is stale */
>
> return null;
>
> if (!LoopFree(advRte, rte))
> { /* incoming route cannot be guaranteed loop free */
> return null;
> }
> if (
> (advRte.SeqNum > Route.SeqNum) /* stored route is stale, use
> advRte */
> OR
> ((advRte.SeqNum == Route.SeqNum) /* same SeqNum */
> AND
> [(advRte.Cost < Route.Metric) /* advRte is better */
>
> OR
> ((Route.State == Invalid))] /* advRte can repair stored */
>
> {
> Update_Route_Table_Entry (rte, advRte);
> }
>
>
>
> One publication is cited. Another publication about AODVv1 would still
>> be valid and could be cited.
>>
>> Section 8.3 first paragraph I think would be better separating out AODVv2
>> from IP. Whether to forward a packet is a decision made consulting IP’s
>> table, not AODVv2’s use of invalid. I think that means you should indicate
>> that invalid routes should be struck from IP’s table.
>>
>> Well, not exactly, because invalid routes still contain valid sequence
>> number information.
>>
>> So we have a local table which includes all the routes, including
> Invalid ones, and we insert or remove entries in the main IP routing table
> depending on state, so that the IP routing table contains all the AODVv2
> routing table's Active and Idle routes but not the Invalid ones.
>
>
> That is not enforced by the specification. In our implementation we
> modified the
> IP route table, and it is by no means incorrect to do so.
>
>
>
>>
>> Third paragraph in 8.3 precursor table appears from left field. (Unless
>> I’ve missed an earlier reference, I haven’t done an electronic search).
>> While postponing details is fine, the fact that there is such a thing,
>> related to a route, and might matter, wants flagging up earlier. At the
>> least indicated as a possible addition in Section 6.1.
>>
>>
>> The precursors are indeed mentioned as an optional part of the route
>> table entry. I added a cross reference in section 6.1 also. What more
>> should be done? It was earlier thought that specification details about
>> optional features should be in the optional section. For myself, I think
>> the feature is important enough to belong in the main body of the text, but
>> I had earlier been requested to describe it as optional ...
>>
>> Maybe to soften it, change to "Implementations which support the
> optional feature of precursor lists as described in Section 12.2 MUST also
> expunge the route's precursor list at the time that the route itself is
> expunged."? Or is this an implementation detail we dont really need to
> put here? We could have a Delete_Route_Table_Entry in the appendix where we
> stress that the precursors list should be expunged?
>
>
> I don't really see what's wrong with keeping normative text about
> precursors in
> the section about precursors, and can't understand moving it to an
> appendix.
>
>
>
>>
>> Section 8.4 invalid route may be expunged. What’s the intended status of
>> the least recently used comment? Is that a may within a may, or a should
>> within a may (you may do this, if you do, you should do it that way), or
>> just a suggestion?
>>
>>
>> It is a MAY and a SHOULD within the MAY.
>>
>> Does this make that clearer: "Any Invalid route MAY be expunged, but
> the least recently used Invalid route SHOULD be expunged first."?
>
>
> I used a slightly modified version of your text. But
> really the existing text was pretty unambiguous.
>
>
>
>
>>
>> The binary exponential backoff on page 25 needs more about what is and is
>> not proposed (and how its parameters – not specified – relate to other
>> parametesr).
>>
>>
>> Here is the text from RFC 3561:
>>
>> To reduce congestion in a network, repeated attempts by a source node
>> at route discovery for a single destination MUST utilize a binary
>> exponential backoff. The first time a source node broadcasts a RREQ,
>> it waits NET_TRAVERSAL_TIME milliseconds for the reception of a RREP.
>> If a RREP is not received within that time, the source node sends a
>> new RREQ. When calculating the time to wait for the RREP after
>> sending the second RREQ, the source node MUST use a binary
>> exponential backoff. Hence, the waiting time for the RREP
>> corresponding to the second RREQ is 2 * NET_TRAVERSAL_TIME
>> milliseconds. If a RREP is not received within this time period,
>> another RREQ may be sent, up to RREQ_RETRIES additional attempts
>> after the first RREQ. For each additional attempt, the waiting time
>> for the RREP is multiplied by 2, so that the time conforms to a
>> binary exponential backoff.
>>
>>
>> Would that be sufficient?
>>
>> "SHOULD utilize a binary exponential backoff as described in RFC 3561." ?
>
> Or:
>
> "SHOULD utilize a binary exponential backoff where the wait time is doubled
> for each retry based on an initial value of RREQ_WAIT_TIME."
>
>
> SHOULD utilize a
> binary exponential backoff, as described in <xref target="RFC3561"/>,
> where the initial wait time is RREQ_WAIT_TIME and the
> wait time is doubled for each retry based.
>
>
>>
>> I’m not at all sure about the packet queuing comments. Do we have good
>> experience here? Is affixed buffer right? Is that “usually positive”
>> comment known to be so? I know enough about buffering to know I don’t know
>> enough about buffering, and that there are some counterintuitive results
>> in the field.
>>
>>
>> I have had quite a bit of experience with buffering. This draft is not
>> the place to review those results,
>> but one thing for sure is that TCP session initiation packets ought to be
>> buffered.
>>
>>
>> I haven’t read section 9 in any detail yet, but I will note that figure 1
>> is really about message contents, not message structure (or format).
>>
>>
>> The contents of the message are organized according to the structure in
>> the figure. We could mix up the contents in any order, and they
>> would still truthfully be the contents. The ordering of the contents
>> follows the structure as shown. I'm not sure how to make this clearer.
>>
>>
>> I agree with you, it mirrors what the RFC5444 message will look like, and
>> this is why we explain the messages in this order. Maybe using the word
>> "format" makes it seem like this should be more specific, but it's for
>> RFC5444 to handle the specifics of the format. We could replace "have the
>> following general format:" with "have the following contents:" and it would
>> appease Chris here without really making much difference? The reader gets
>> the idea of the message structure anyway from the way we've written the rest
>> of the section.
>
>
> First, let's try replacing "general format" by "general structure" and see
> if he likes it.
>
>
>
Other related posts:
