Hello Vicky,
My bits of follow-up is inserted inline below...
On 7/23/2015 7:33 AM, Victoria Mercieca wrote:
Finally - some feedback!! Some comments inline...:
On Thu, Jul 23, 2015 at 11:56 AM, Dearlove, Christopher (UK) <chris.dearlove@xxxxxxxxxxxxxx <mailto:chris.dearlove@xxxxxxxxxxxxxx>> wrote:
“Version 2” doesn’t appear in the draft title. (Not sure why only
just noticed that.)
So...currently it is: "Ad Hoc On-Demand Distance Vector (AODVv2) Routing"
Should it be "Ad Hoc On-Demand Distance Vector Version 2 (AODVv2) Routing"
or "Ad Hoc On-Demand Distance Vector (AODVv2) Routing Version 2"
or "Ad Hoc On-Demand Distance Vector Routing Version 2 (AODVv2)"
or "Ad Hoc On-Demand Distance Vector Version 2 Routing (AODVv2)"
Section 1, what’s the difference between loop avoidance and loop
freedom?
Loop freedom is the state of having no loops. Loop avoidance is how you ensure that. I guess this bit :
AODVv2 compares route metrics in a way that ensures loop avoidance.
AODVv2 also uses sequence numbers to assure loop freedom, enabling
identification of stale routing information so that it can be
discarded.
can be reworded to:
To ensure loop freedom, AODVv2 uses sequence numbers to identify stale routing information, and compares route metrics to determine if advertised routes could form loops.
sound ok?
Section 2 is missing IAR. (Haven’t checked this section in detail,
just noticed that.)
Have added this into the Terminology section:
"Internet AODVv2 Router
An AODVv2 router with an interface to the Internet."
Throughout: inconsistency in using RFC 5444 and [RFC5444].
Changed to a reference throughout - will display as [RFC5444]. Except in section headings and in algorithm names in the appendix.
Section 3. AODVv2 doesn’t support some things that OLSRv2 was
required to support, in particular using same address on multiple
interfaces (with a limitation that made that practical) as some
wanted unnumbered interfaces (Stan IIRC).
Section 3 para 4 says route requests that can’t be confirmed
bidirectional should be ignored. Why should and not must?
Changed to MUST.
Throughout: Much inconsistency in use of SHOULD vs should and MUST
vs must. (Former in normative sections, latter or avoid
otherwise.) Haven’t noticed for MAY/may but worth checking.
Any feedback on these would be helpful at some point, if anyone gets time to go through it.
Section 4.2 last paragraph. I would have thought this was a MUST NOT.
Section 4.3 second paragraph should (SHOULD) or MUST? I would have
thought latter, and I think some later text assumes latter. Also
later in section.
bit about neighbors which cant confirm adjacenty MUST be marked as blacklisted (changed to a MUST).
Section 4.4 para 3 can probably should be a 2119 word.
Section 4.4 sequence numbers wrap, so at the least a comment that
indicates greater/less is according to that should be included.
(An algorithm that properly handles the 0 omission would help
implementers.)
Anyone like to suggest text? I cant think of anything that explains this nicely.
In order to ascertain that information about a destination is not
stale, the node compares its current numerical value for the sequence
number with that obtained from the incoming AODV message. This
comparison MUST be done using signed 32-bit arithmetic, this is
necessary to accomplish sequence number rollover. If the result of
subtracting the currently stored sequence number from the value of
the incoming sequence number is less than zero, then the information
related to that destination in the AODV message MUST be discarded,
since that information is stale compared to the node's currently
stored information.
Section 4.5 Is this table conceptual like that in 4.6? (Later in
section uses word comparable that wouldn’t be my choice.)
Section 4.6 has a broken paragraph. (Could easily be fixed in -11,
haven’t checked.)
Fixed now, not in 11.
Section 5, para 2, use of “or” seems wrong.
removed reference to RFC6551 since this is in 12.3 anyway.
Section 5.1. I don’t like that non-hop-count metrics aren’t
described (as hop count really isn’t good enough in reality) and
that adding a new metric type means adding code. The latter is a
design issue, so there is no right/wrong here.
Section 5.3. I don’t understand why a stored invalid route can’t
be overwritten by a new higher metric route. (Sequence number,
that I understand.)
This is the loopfree section... well... if we have a newer seqnum, the route is automatically seen as not forming a loop. so we dont check loopfree... so if we look at a case when we do check loopfree, the seqnum of the advertised route must have been the same as the seqnum of our existing invalid route. If AdvRte has a worse metric than current Route, we cant guarantee it's loop free and shouldnt use it. It doesnt matter if our route is invalid... we could be installing a route which formed a loop, now assuming its a valid route, and send traffic down that route, only for it to go round and round.
Section 6.2 is where I think bidirectionality becomes MUST, so
earlier SHOULDs may be wrong.
Section 6.2 states that if some other mechanism is present routes
can be known to be bidirectional. That’s OK (I think). But then to
say that RREP_ACKs can be omitted is wrong, given that various
places describe consequences of missing RREP_ACKs, without
considering this case. I think anything other than always sending
RREP_ACKs is not appropriate.
I think we're still being misunderstood here.
Maybe we should say "in that case, *requesting *acknowldegment of a RREP ... is unnecessary".
It's not that we wouldnt send the RREP_Ack, it's that we wouldnt request it.
Section 6.3 para 3 is I think unsupportable, to say that it’s
outside AODVv2 who regenerates messages. This is essential for
compatible interoperability.
Section 6.4 page 23 para 2 (ignoring incomplete). I can see
“(albeit usually positive)” being challenged.
Anyone know what is meant by "ignoring incomplete"?
Section 6.4 page 23 para 3 why fixed buffer? And as this is
supposed to be IP’s job, why specify?
Section 6.4 page 23 para 4 using [RFC 3561] in this way as a
SHOULD makes it a normative reference, which is a downref, and
it’s buried somewhere in that reference without an indication.
Should be pulled out and included in this document.
Section 6.4 penultimate para last line, why SHOULD and not MUST?
changed to a MUST
Section 6.5.2 made me realise a major omission. This is all about
AODVv2’s tables. But at some point those need copying to IP’s
routing table (as additions/updates/removals) and that’s not
discussed, and it’s not listed in Appendix A. Also all the text in
the document uses the AODVv2 tables, which is fine when within
AODVv2, but of course any IP packet unicast (I think multiple
unicast is noted as an alternative to multicast – and there’s a
comment on future use of unicast) would use the IP table, so would
need to get that timing right. (Futureproofing in the latter case,
but the multiple unicast may need thought.)
This cropped up in a previous discussion on this list...
Section 6.6 point 3, the equal case. I think it’s obvious, but
should say “equal or better”. (If that’s wrong, it’s not obvious.)
May occur in other places.
change to say "equal or better"
Section 6.7.1 Active the do A or B at the least should indicate
preference.
is this in regard to the "expunge or mark as invalid"? what is the preference, when a timed route expires?
Section 6.7.1 about using idle and active routes to forward
packets, this is also where that is done by IP, so it should be
about idle and active routes being in IP’s table.
Section 6.7.1 there’s a mention of inferior matching routes. Not
sure what’s meant.
changed to "any matching routes with metric values worse than this route"
Figures are described as message structure. They are message
content. Structure is 5444. (As worded there’s a sort of
suggestion of an alternative structure, but this isn’t that, as
there’s no indication how optional fields would be handled.)
changed to "contents" because this comment was made before.
Section 7.1.3, that situations where RREQs are not regenerated are
not declared makes me very uneasy.
Section 12. I’m not sure it’s up to this document to specify TBD
numbers. I also know from experience that IANA won’t be happy with
this presentation.
They need it spelled out in detail, add this entry to this table,
and so on. Look at the various OLSRv2 documents for details
(especially later ones).
Already existing TLVs shouldn’t be in the IANA section – this is
instructions to IANA what to do.
If needed here, put in another section. Also why allocate a new
SEQ_NUM TLV, I think the existing sequence number TLV (the version
named (COMPLETE) – I’d just say that once somewhere) will do.
Section 12.4, I’d add an unspecified value, along the RFC 7188 line.
Section 13, para 4, what’s said is true, but it’s not as simple as
this makes it appear. (And there are issues of information leakage
in other layers.)
Section 13, use of sequence number as timestamp is good. Should
think about how fast 16 bits wraps and if that’s an issue. (Not
saying it is, just food for thought.)
Section 13, specifying TIMESTAMP data, suggest indicate algorithm
number.
I think Section 13 means that RFC 7182 should be a normative
reference.
Appendix A I think should be about two way interaction with IP,
and is missing IP routing table update. It’s also very
underspecified. For example the ability to queue packets has
requirements to add packet to queue, to flush queue. Is it IP’s
responsibility to release queue when a route appears?
Appendix D needs to have its normativeness indicated. Is the
description in the main body (which as I note I haven’t reviewed)
complete and normative, and this is example? (That would handle if
there’s any mismatch.) Otherwise things get messy.
