Hi Charlie, couple of comments:
On Wed, Aug 19, 2015 at 8:40 PM, Charlie Perkins <
charles.perkins@xxxxxxxxxxxxx> wrote:
Hello folks,
We really do need to send out some discussion on the mailing list for
Chris and then for the other comments as well.
Here is a distillation of relevant discussion on the aodvv2-discuss list.
Please let me know if you have any comments.
On Thu, Jul 23, 2015 at 11:56 AM, Dearlove, Christopher (UK) <
<chris.dearlove@xxxxxxxxxxxxxx>chris.dearlove@xxxxxxxxxxxxxx> wrote:
“Version 2” doesn’t appear in the draft title. (Not sure why only just
noticed that.)
How about "Ad Hoc On-Demand Distance Vector Routing Version 2 (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, justAdded this into the Terminology section:
noticed that.)
"Internet AODVv2 Router
An AODVv2 router with an interface to the Internet."
But the IAR should also be allowed to interface to other external networks
that are not "the Internet", so the name should be changed to External
Network Access Router (ENAR).
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).
As best I can tell, this seems to be a consequence of OLSR's requirement
to know the complete topology.
so is it a requirement for AODVv2?
"Neighboring routers which cannot confirm adjacency when requested MUST be
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.
We have fixed some of these but specific suggestions would be very helpful.
Section 4.2 last paragraph. I would have thought this was a MUST NOT.
Has been changed accordingly...
Now it says that neighbors which cannot confirm adjacency MUST be marked
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.
as blacklisted (changed to a MUST).
Actual text:
Done.
Section 4.4 sequence numbers wrap, so at the least a comment thatHere's what RFC 3561 says:
indicates greater/less is according to that should be included. (An
algorithm that properly handles the 0 omission would help implementers.)
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.
We have used that, except to replace "32-bit" by "16-bit". I am mystified
why this was omitted from AODVv2, but no one ever noticed it before.
Section 4.5 Is this table conceptual like that in 4.6? (Later in section
uses word comparable that wouldn’t be my choice.)
Yes, the table is conceptual. Comparisons can be done on conceptual table
entries as well. Nevertheless, the wording has been changed to avoid use
of that word.
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.)
It can be overwritten as long as LoopFree() returns TRUE. How about the
following text as a clarification:
"The LoopFree function for the HopCount metric is derived from the fact
that route cost
increases with number of hops. Therefore, an advertised route with higher
cost than the
corresponding stored route could include the stored route as a
sub-section. Replacing the
stored route with the advertised route could form a routing loop. LoopFree
returns FALSE to
indicate that an advertised route with higher cost is not to be used to
update a stored
route. In the case where the stored route is Invalid, it is possible that
the advertised
route includes the stored route and came from a router which did not yet
receive notification
of the route becoming Invalid, so the advertised route should not be used
in case it forms
a loop."
Added to say: "a loop to a broken route" at the end.
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.
I agree it's time to develop new metrics, but those should be in separate
documents. I'll try to make one before the next IETF.
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.
Better: "in that case, *requesting *acknowldegment of a RREP ... is
unnecessary".
It's not that we wouldn't send the RREP_Ack, it's that we wouldn't 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.
I don't really see what's wrong with it. Using some method (e.g.,
maintaining a connected dominating set for message regeneration) to reduce
multicast flooding overhead is clearly an excellent way to improve
performance as the size of the network grows. AODV does not have to
specify which method should be used. As long as the messages can reach all
of the multicast receivers, interoperability will be maintained no matter
which method is chosen. There are some methods identified in RFC 6621 which
would work, and yet we should not make a normative reference to that
document.
If you agree with value of the performance improvement, and that the
choice of method would not affect interoperability, then do you have any
suggestion about how to improve the wording?
Since most of the world's traffic is over TCP, it's pretty clear that
Section 6.4 page 23 para 2 (ignoring incomplete). I can see “(albeit
usually positive)” being challenged.
buffering is usually positive. Nevertheless, "albeit usually positive" can
be removed, since we state what the effects are. If another document
specifying buffering behavior is wanted, that doesn't belong in AODVv2. We
could replace "The packet buffer is configured" by "The packet buffer MAY
be configured". This does not affect interoperability; following the
suggestion but buffering DOES affect proper operation of data transmission
(i.e., the purpose of having a routing protocol in the first place). The
relaxed mandate would enable conformant implementations that did not need
the performance improvement that might be available from buffering.
already removed "albeit usually positive".
Section 6.4 page 23 para 3 why fixed buffer? And as this is supposed to be"To reduce congestion in a network, repeated
IP’s job, why specify?
IP never had to do this before, because IP never had to work with reactive
route establishment.
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.
We will reword to avoid the apparent downref. Do you have a suggestion
for this?
It now says this:
"remains Active until its expiration time, after which it MUST become
Section 6.4 penultimate para last line, why SHOULD and not MUST?Check. Good catch!
user space implementation has other advantages so that many people will
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.)
For any such conditions as this, kernel implementation is better. But
prefer to not worry about it.
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"
The preference should be determined by memory requirement. If there's no
Section 6.7.1 Active the do A or B at the least should indicate
preference.
problem with storage, it's better to keep the route table information. Now
that passage says:
"remains Active until its expiration time, after which it SHOULD be marked
as
Invalid and maintained for its sequence number information, but MAY be
expunged if memory is constrained."
actually it says:
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.
changed to "any matching routes with metric values worse than this route"
Section 6.7.1 there’s a mention of inferior matching routes. Not sure
what’s meant.
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".
Section 7.1.3, that situations where RREQs are not regenerated are not
declared makes me very uneasy.
Section 7.1.3 is about regeneration. We could add something like "A
router may avoid regenerating a RREQ, for instance if it is heavily loaded
or low on energy and therefore unwilling..."
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.
This is a reasonable comment, and the IANA text is being restructured
accordingly. For instance, mention of VALIDITY_TIME TLV will be removed
from the IANA 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.
Not sure about what "COMPLETE" means, but the suggestion to use an
existing sequence number TLV is reasonable. Notably, there was not such a
TLV back when the AODVv2 SEQ_NUM TLV was specified. I think that Address
Block TLV type 6 with type extension 0 will work, at some additional
expense per message.
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.)
I didn't really understand this. Could you say more?
Section 13, specifying TIMESTAMP data, suggest indicate algorithm number.
Do you mean for the comparison?
I think Section 13 means that RFC 7182 should be a normative reference.
Yes, I think that would be very appropriate.
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?
That's one way to do it, for sure. Did you want separate itemized bullets
for the two queuing operations you mentioned?
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.
The title of the appendix indicates that they are examples, but we will
add a statement that the appendix is non-normative. We could even say
"Example (Non-Normative) Algorithms for AODVv2 Operations".
Regards,
Charlie P.
