Subject: Re: [manet] draft-ietf-manet-aodvv2-15 review

From: Charlie Perkins <[email-masked]>
Date: Mon, 2 May 2016 22:16:48 -0700

Hello Jiazi,

A bit of follow-up below...

On 4/28/2016 7:53 AM, Jiazi YI wrote:
> Hi,
>
> Here is my preliminary review on aodvv2-15. There are still some 
> points that I'm not sure that I have fully understood -- I will get 
> back to them if I had time.
>
> A summary of my review:
>
> 1. The specification lacks of a "big picture" of the protocol 
> functioning. People prefer to have a general idea of the whole 
> protocol at the very beginning. This is particularly important for 
> readers who are not familiar with reactive protocol in general.
> Some of the sections can be reorganized a bit to improve the 
> readability. For example, with the current structure, the readers need 
> to reach section 6.6 (Route discovery) before they can really touch 
> the core function of the protocol.

As with many documents, there is a need to establish definitions and 
data structures before specifying the details of the control protocol 
interactions.

>
> 2. As an experimental protocol, the part related to experiments needs 
> to be improved. IMO, at least the following information must be provided:
>        - The motivation of this experimental document
>        - The experiments to be conducted
>        - What kind of feedback is expected to reported.

We will provide some indication for useful experiments.

>
>
> 3. There are some redundant processing procedures in the draft. On the 
> other hand, some connections between the sections seem to be missing. 
> Sometimes it's hard to build connection between the message processing 
> and related information bases.

If you could be more specific, that would help.  I know that there has 
been a lot of effort to provide exactly this kind of continuity, so I 
don't know what is missing.

>
> 4. The specification uses lots of multicast messages. While multicast 
> RREQ is inevitable, the necessity of multicast RREP/RERR is not clear. 
> It bring issues related to control message overhead and security.

Multicast RERR is useful when many precursors need to receive the 
message ASAP.  The justification for multicast RREP is more detailed, 
but I think on this point there is definitely room for discussion.  Even 
for multicast RERR, serial unicast is preferable whenever the number of 
precursors is small.

>
> 5. The interaction with RIB: The route state maintenance requires 
> "/LocalRoute.State MUST be checked before IP packet forwarding and 
> before any operation based on LocalRoute.State./". Having such check 
> for each data packet might be heavy load for the system if the routing 
> daemon runs in userspace (because of context switch). While 
> interaction with RIB is inevitable, we want to reduce it as much as 
> possible -- per data packet operation is too much.

This is an implementation detail.  Systems can be built with RIB and FIB 
co-located, which would be my preference.

>
> 6. There are too many "MAY" and "SHOULD" in the document without 
> elaboration. Some of which might cause interoperability issues or even 
> loops.

Do you suggest an elaboration after every single RFC 2119 term?

> Please check the details inline:
>
> *section 1, overview*
>
>        The Ad hoc On-Demand Distance Vector Version 2 (AODVv2) protocol
>        enables dynamic,*self-starting*, multihop routing
>
>
> The term "self-starting" is very vague. It makes people think of 
> auto-configuration. And I didn't see such kind of feature is 
> supported. I would avoid using it.

The protocol is indeed self-starting, and previous commenters never 
suggested confusion with auto-configuration.  I can't think of another 
term that denotes that the routing table is built dynamically without 
necessarily requiring any preconfigured route table entries.  Can you 
suggest a better term?

>
>     AODVv2 is an Experimental protocol.
>      ...
>
> I think much more information is required here:
>
> 1) Because RFC 3561 has already been experimented for 13 years, as its 
> version 2, AODVv2 should summarize what kind of experiences have been 
> obtained in the past 13 years, and justify the design choices made in 
> the current draft (like use of RFC5444, removing some of the features, 
> etc)

I never heard of a rule like this - that if a protocol is going to 
Experimental, we need to describe the experiments that have already been 
done.

However, the use of RFC 5444 was not justified by any experiment, but 
instead mandated by RFC 5498. I am surprised that you request 
justification for removing the features, since you were among the people 
who requested removing them.  This was done to satisfy that request.

>
> 2) The motivation and experiments to be conducted need to be 
> elaborated. It's even necessary to have a separate section/subsection 
> for it. Some experiment points that come to my mind without much 
> thinking: support of multiple interfaces, support of gateways, use of 
> different metrics, the security model, etc..

These are reasonable experiments and should be mentioned in a section 
for that purpose.

>
>        The basic operations of the AODVv2 protocol are route discovery and
>        route maintenance...
>
>
> It's necessary to give an overview of the basic functioning of the 
> protocol, rather than simply saying "use RREQ/RREP to carry 
> information and build routes". Because after this, the draft jumps 
> directly into the protocol details.
> It's extremely important for the readers who are not familiar with 
> reactive protocol.

Which aspects of reactive protocol operation do you think should be 
mentioned?  Perhaps the longer the description gets, the less likely it 
is that people will read and digest it.

>
> *section 2, Terminology*
>
> Please pay attention to the style of the terms. Most of the time, 
> style like AckReq is used, but sometimes RERR_Gen is used.
> Good to be consistent.
>
>        Unreachable Address
>           An address reported in a Route Error message, as described in
>           Section 7.4.1.
>
>
> need to explain what's the address for exactly, rather than giving a 
> pointer to a big section.

It seems difficult to imagine that someone would see the term 
"Unreachable Address" and be unable to determine that the address would 
be used in the Route Error message to notify other AODVv2 nodes that the 
address is unreachable.  That is exactly what the address is used for, 
and seems intuitive to me.  Do you have any suggestion about how to 
remedy the lack?

>
> *section 3, applicability statement*
> *
> *
>
>        AODVv2 handles a wide variety of mobility and traffic patterns by
>        determining routes on-demand...
>
>
> It necessary to explicitly call out what kind of traffic patterns that 
> reactive protocols are not suitable for, such as lots of routers send 
> data packets to instantaneous and multiple destinations. If that 
> happens, what's the consequence (broadcasting storm, etc).

We can mention that AODVv2 supports traffic patterns for there are a lot 
of source/destination pairs that do not need routes to each other.  
Sending data to multiple destinations at the same time sounds like 
multicast, which is out of scope.  I don't know about instantaneous 
destinations...?

>        The routing algorithm in AODVv2 MAY be operated at layers other
>     than
>        the network layer, using layer-appropriate addresses.
>
>
> Considering the deep coupling between AODVv2 and IP, I don't see how 
> it can work. This should be removed.

Well, it's been done at layer 2.  It was used for 802.11s.  It's been 
done for Bluetooth.  Of course in those situations you wouldn't use the 
IP address, but that's a pretty simple translation to make.

>
>
> 4.2 router client set
>
>        RouterClient.Cost
>           The cost associated with reaching this address or address range.
>
>
> Both "metric" and "cost" are used in the draft. If they are the same, 
> please pick a term and use it consistently.

They are not the same.  Some people have been using metrics that measure 
benefit, not cost.  I don't like those metrics, myself. Both terms are 
needed.

>
>     The AODVv2 router adding the Router Client MUST wait for
>        any existing routing information about this Router Client to be
>        purged from the network, i.e., at least MAX_SEQNUM_LIFETIME
>     since the
>        last SeqNum update on the router which is removing this Router
>        Client. 
>
>
> I'm not sure that I parsed this sentence correctly, but the 
> adding/removing of router client seems to be more an external 
> operation and out of the control of routing protocol itself.

It's just the routing information about the client in other places of 
the network, not anything related to an external operation.  Is there 
something specific that is confusing here?

>
> 4.3 neighbor set
>
>
>      Neighbor.State ... There are three possible states: Confirmed,
>     Unknown, and
>           Blacklisted.  Unknown is the initial state.
>
>
> I think the "Unknown" is a bit confusing here. It's actually something 
> we already know -- a state that "we heard from that node, but 
> bi-directionality is not confirmed".
> I think "Heard" is a more appropriate term to be used here.

That's O.K. with me - perhaps other people have some feedback here...?

>
> 4.4 sequence numbers
>
> I think the comparison in wrap-around cases should be specified here.

Doesn't it work just to describe how subtraction works?  If the 
wrap-around cases are not exceptions, do they really need to be 
specified?  To me that sounds like a long and uninformative rathole.

>
> 4.5 Local route set
>
>
>     Alternatively, implementations
>        MAY choose to modify the Routing Information Base directly.
>
>
> This should be either more precise (in which condition, how we can 
> modify the RIB?), or be removed.

I think this is an attempt to point out that there are a lot of 
implementation strategies.

>
>        Multiple valid routes for the same address and prefix length
>     but for
>        different metric types may exist in the Local Route Set, but the
>        decision of which of these routes to install in the Routing
>        Information Base to use for forwarding is outside the scope of
>        AODVv2.
>
>
> It's OK, but maybe some rules must be specified and followed. For 
> example, the metric used must be the same: if a packet is forwarded 
> using metric A at router X, and then metric B at router Y, it's a 
> obvious source of loops.

I don't think this is possible for monotonically increasing metrics.

>
> 4.6.  Multicast Route Message Set
>
> Since this set is mainly for duplicate packet detection, I think some 
> of the fields are kind of redundant. For example, an RREQ/RREP message 
> can be identified by its originator's address and sequence number, why 
> 6 fields (OrigAddr, OrigFprefixLen, OrigSeqNum, TargAddr, 
> TargPrefixLen, TargSeqNum) are necessary here.

Because the originator can have two route discovery operations happening 
at the same time.

>
> *section 5 Metrics*
> *
> *
>
>        In Route Request messages, the metric describes the cost of the
>     route
>        from OrigAddr (and any other addresses included in the prefix
>     length
>        of RREQ_Gen's Router Client entry for OrigAddr) to the router
>     sending
>        the Route Request. 
>
>
> So the metric carried in the RREQ is the metric from Router Client to 
> the RREQ_Gen plus the metric from RREQ_Gen to the intermediate router.
> In the Router Client Set, we have only one field for 
> RouterClient.Cost. On the other hand, "AODVv2 supports different 
> metrics".
>
>           routerClient.cost             AODVv2 metric
> client-----------------------RREQ_gen---------------
>
> What if those two metrics are of different types? This is quite 
> possible because the two links might use different medium.

We should indeed allow for multiple metric types for any particular 
Router Client.  The quoted passage does not specifically prevent that, 
but I agree it should be made clearer.

>
>
>
> section 6.2 (Next hop monitoring) and section 6.3 (Neighbor set 
> update) are very related. One is about the function, and one is about 
> the corresponding operation. The following sections don't follow this 
> pattern -- why not making them to one?

This is worthwhile to explore, but I am not sure it could be done and 
finished this week.  Anyway, different people might like different 
organizations and the one in the draft is not wrong. Could we put this 
as a second priority and look at it a little later?

>
> section 6.7.1 (Evaluating route information) and section 6.8 
> (Suppressing redundant messages using Multicast Route message set) 
> seem to have some overlapped operations. They all need to check the 
> uniqueness of the messages, compare the metrics and stop processing 
> when duplication is detected. Although they are applied to different 
> information bases, they are doing the same thing.

There is a certain amount of overlap because there aren't all that many 
data elements to start with.  However, trying to structure an algorithm 
to use the minimum number of lines of code can start to make spaghetti 
code which is even harder to understand.  The purposes are different 
enough that I don't agree they are doing the same thing.  If this is not 
clear, then I can try to make an example but that would take another 
hour that I don't have until later this week.

>
> section 6.9 (suppressing redundant RERR messages using the Route Error 
> set) should obviously a part of section 6.10.2 (reporting invalid 
> routes), but it's a separate section with higher hierarchy.
>
> more details inline:
>
>        If such an entry does not exist, a new entry is created as
>     described
>        in Section 6.3. *While the value of Neighbor.State is Unknown,
>        acknowledgement of RREP messages sent to that neighbor MUST be
>        requested*. ...
>
>
> I think we need to precise this is one for the case of RREP transmission.

Sounds reasonable, thanks!

>
> section 6.5.  Message Transmission
>
>
>        To implement the congestion control, a queue length is set.  If the
>        queue is full, in order to queue a new message, a message of lower
>        priority must be removed from the queue.  If this is not possible,
>        the new message MUST be discarded.  The queue should be sorted in
>        order of message priority. 
>
>
> I think it's more implementation details rather than specification.

We were requested to specify this.  I agree it is implementation detail 
and that should be made clear.

> section 6.7.1
>
>        4.  Compare route costs
>            *  If AdvRte is better than all matching LocalRoutes, it SHOULD
>               be used to update the Local Route Set because it offers
>               improvement. *If it is not used to update the Local
>     Route Set,
>               the existing non-optimal LocalRoute will continue to be
>     used,
>               causing data traffic to use a non-optimal route.*
>
>
> If some routers made the update, and some routers choose not updating, 
> I'm afraid there will be loops in the network.

A route that was previously loop free does not suddenly become a looping 
route in this situation.

I'll try to respond to the rest of your comments tomorrow.  Thanks for 
the careful review.

Regards,
Charlie P.

---

From: "Dearlove, Christopher (UK)" <[email-masked]>
Date: Tue, 3 May 2016 08:55:43 +0000

Just one comment for now.

Jiazi:
> It's OK, but maybe some rules must be specified and followed. For example, the metric used must be the same: if a packet is forwarded using metric A at router X, and then metric B at router Y, it's a obvious source of loops.

> I don't think this is possible for monotonically increasing metrics.

Simple case, A, B, C all connected. Metrics are symmetric. For metric X: AB=3, AC=BC=1. For metric Y: BC=3, AB=AC=1.

Packet source A, destination B. A uses metric X. It sees the best route as A-C-B and forwards to C. However C is using metric Y, and sees the best route as C-A-B and forwards to A. Loop.

That I think is what Jiazi is meaning. Of course the metrics don't have to be the same in that, for example, if when accumulating the route metric A -B-C if A-B is a delay metric and B-C is a reliability metric, adding them together makes no sense and won't produce great routes, but won't loop.

********************************************************************
This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender.
You should not copy it or use it for any purpose nor disclose or
distribute its contents to any other person.
********************************************************************

---

From: Charlie Perkins <[email-masked]>
Date: Tue, 3 May 2016 08:39:34 -0700

Hello Chris,

Yes, you are right.  I was misunderstanding what Jiazi said, thinking 
"metric value" when he meant "metric type".

Regards,
Charlie P.

On 5/3/2016 1:55 AM, Dearlove, Christopher (UK) wrote:
> _______________________________________________
> manet mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/manet
>

---

From: Victoria Mercieca <[email-masked]>
Date: Wed, 4 May 2016 00:19:50 +0100

Hi Jiazi,

Thank you for your review. I have a few comments to add, inline marked with
"VM>".

On Thu, Apr 28, 2016 at 11:50 PM, Lotte Steenbrink <
[email-masked]> wrote:

> Hi Jiazi,
> thank you for your detailed review. A few comments and questions inline.
> (I wanted to get this reply out the door as soon as I could, so I/we will
> follow up regarding the yet unresponded to items later)
>
> Am 28.04.2016 um 16:53 schrieb Jiazi YI <[email-masked]>:
>
> Hi,
>
> Here is my preliminary review on aodvv2-15. There are still some points
> that I'm not sure that I have fully understood -- I will get back to them
> if I had time.
>
> A summary of my review:
>
> 1. The specification lacks of a "big picture" of the protocol functioning.
> People prefer to have a general idea of the whole protocol at the very
> beginning. This is particularly important for readers who are not familiar
> with reactive protocol in general.
> Some of the sections can be reorganized a bit to improve the readability.
> For example, with the current structure, the readers need to reach section
> 6.6 (Route discovery) before they can really touch the core function of the
> protocol.
>
> 2. As an experimental protocol, the part related to experiments needs to
> be improved. IMO, at least the following information must be provided:
> Sometimes it's hard to build connection between the message processing and
> related information bases.
> I think this is partially due to the lack of implementation, or the
> implementation experience is not reported to the author team?
>
> 4. The specification uses lots of multicast messages. While multicast RREQ
> is inevitable, the necessity of multicast RREP/RERR is not clear. It bring
> issues related to control message overhead and security.
>
> 5. The interaction with RIB: The route state maintenance requires "*LocalRoute.State
> MUST be checked before IP packet forwarding and before any operation based
> on LocalRoute.State.*". Having such check for each data packet might be
> heavy load for the system if the routing daemon runs in userspace (because
> of context switch). While interaction with RIB is inevitable, we want to
> reduce it as much as possible -- per data packet operation is too much.
>
>
> Hmmm. Yeah. To me it would make more sense to say "if you have a RIB, you
> MUST update it any time LocalRoute.State changes [so instead of the polling
> described above, you’d let the changes „trickle up“ and avid constant
> context switches]. If you don’t have a RIB and LocalRoute is consulted
> directly when forwarding a packet [this would’ve been the case in RIOT a
> few releases ago.. but there’s no kernel vs user space there either, so you
> wouldn’t have the expensive context switches], you MUST check
> LocalRoute.State before you forward to make sure the route is still valid.“
> ...
>
>
VM> That statement: "*LocalRoute.State MUST be checked before IP packet
forwarding * ".... I think is for the case where you don't have timers
running to handle timeouts. so before you do anything which depends on
route state, you need to check what the state should be. i.e., has the
route gone from idle to invalid. However, we also rely on knowledge of
whether a route is being used to keep it in Active state, so there's
another place we might want a per-packet operation.

> 8. security issues. There are discussions going on in different threads,
> so I won't repeat them here. I think the current regeneration-based
> approach is complex to secure and hard for future extensions.
>
>
> We’ve gotten rid of that approach as of now. :)
>
> I haven't checked the security considerations section in detail yet. It's
> important to get the big direction right first.
>
>>    enables dynamic,* self-starting*, multihop routing
>
>
> auto-configuration. And I didn't see such kind of feature is supported. I
> would avoid using it.
>
>
> Makes sense. I’ve removed the term.
>
>
> obtained in the past 13 years, and justify the design choices made in the
> current draft (like use of RFC5444, removing some of the features, etc)
>
> 2) The motivation and experiments to be conducted need to be elaborated.
> It's even necessary to have a separate section/subsection for it. Some
> experiment points that come to my mind without much thinking: support of
> multiple interfaces, support of gateways, use of different metrics, the
> security model, etc..
>
>
> Fair enough.  Thank you for the detailed pointers, I’ll write something up
> (unless my co-authors beat me to it ;) ) and get back to you, if that’s
> okay?
>
>
> protocol, rather than simply saying "use RREQ/RREP to carry information and
> build routes". Because after this, the draft jumps directly into the
> protocol details.
> Good point. we’ve removed „The basic operations of the AODVv2 protocol are
> route discovery and route maintenance.“, since that sentence occurs in the
> first paragraph already, and we’ve substituted the „An AODVv2 router is
> configured to perform route discovery…“ paragraph with:
>
>    The basic protocol mechanisms are as follows.  Since AODVv2 is a
>    reactive protocol, route discovery is initiated only when a route to
>    the target is needed (i.e. when a router' client wants to send data).
>    AODVv2 does this with the help of a Route Request (RREQ) and Route
>    Reply (RREP) cycle: an RREQ is distributed across the network until
>    it arrives at the target.  When forwarding an RREQ, all routers
>    across the network store the neighbor they've received the RREQ from,
>    memorizing a possible route back to the originator of the RREQ.  When
>    the target receives the RREQ, it answers with an RREP, which then
>    travels back to the originator along the path memorized by the
>    intermediate routers.  A metric value is included within the messages
>    to record the cost of the route.  AODVv2 uses sequence numbers to
>    identify stale routing information, and compares route metric values
>    to determine if advertised routes could form loops.
>
> we’ve also slightly updated the language of the next pararaph:
>
>    Route maintenance includes confirming bidirectionality of links to
>    next hop AODVv2 routers and issuing Route Error (RERR) messages
>    informing other routers of broken links.  It also includes reacting
>    to received Route Error messages, and extending and enforcing route
>    timeouts.
>
> What do you think?
>
>
>
> *section 2, Terminology*
>
> Please pay attention to the style of the terms. Most of the time, style
> like AckReq is used, but sometimes RERR_Gen is used.
> Good to be consistent.
>
>
> I think I’ve pointed out a similar issue too (in spring 2014, I think?),
> but I think I recall the conclusion being that while the draft should stick
> to CamelCase wherever it can, renaming RERR_Gen to RERRGen would have a
> negative impact on readability, so there are some exceptions to the rule…
>
>
>    Unreachable Address
> We had a longer explanation there but Justin said we should shorten it, so
> we did that and added the pointer… :D
>
>
> reactive protocols are not suitable for, such as lots of routers send data
> packets to instantaneous and multiple destinations. If that happens, what's
> the consequence (broadcasting storm, etc).
>
>
> Good point. We’ve reworded the paragraph to..
>
>    determining routes on-demand.  In networks with a large number of
>    routers, AODVv2 is best suited for relatively sparse traffic
>    scenarios where each router forwards IP packets to a small percentage
>    of other AODVv2 routers in the network.  In this case fewer routes
>    are needed, and therefore less control traffic is produced.  In large
>    networks with very frequent or bursty traffic, AODVv2 control
>    messages may cause a broadcast storm, overwhelming the network with
>    control messages and preventing routes from being established.  This
>    especially applies to networks with point-to-point or point-to-
>    multipoint traffic.  In this case, the transmission priorities
>    described in Section 6.5 prioritize route maintenance traffic over
>    route discovery traffic.
>
>    Data packets may be buffered until a route to their destination is
>    available, as described in Section 6.6.
>
> Does that help?
>
>
>
>>    AODVv2 provides for message integrity and security against replay
>>    attacks by using integrity check values, timestamps and sequence
>>    numbers, as described in Section 12.  If security associations can be
>>    established, encryption can be used for AODVv2 messages to ensure
>>    that only trusted routers participate in routing operations.
>
>
> We need to call out the hop-by-hop security model used, and the network
> MUST apply necessary mechanisms to make sure that only trusted routers can
> participate in routing operations.
>
>
> We’re currently working on getting rid of the hop by hop trust model in
> favor of end to end security. (I believe the thread is "Re: On
> Forwarding-and-regeneration (was: Re: draft-ietf-manet-aodvv2-13 review - a
> couple of big ticket Items)“?)
>
>
>
>>    The routing algorithm in AODVv2 MAY be operated at layers other than
> Considering the deep coupling between AODVv2 and IP, I don't see how it
> can work. This should be removed.
>
> *section 4 data structures*
>
> 4.1 Interface set
>
>>
>> The InterfaceSet is a conceptual data structure which contains
>>    information about all interfaces available to AODVv2.
>> ...
>> Otherwise the InterfaceSet MAY be empty.
>
>
> I'm not sure the sec MAY be empty -- a router contains at least one
> interface, no?
>
>
> Yes, but if it knows it only has one and it has access to this information
> elsewhere and this won’t change, it might as well save the bytes of stating
> this again… Iirc the idea was to give implementations for constrained
> devices a bit of leeway there.
>
>
> 4.2 router client set
>
>>    RouterClient.Cost
> Cost refers to the result of the Cost(L) function for the link to the
> RouterClient. Would it make more sense it this entry was called LinkCost?
>
>
>
>>    purged from the network, i.e., at least MAX_SEQNUM_LIFETIME since the
> adding/removing of router client seems to be more an external operation and
> out of the control of routing protocol itself.
>
>
> …But even when it has no influence on when/whether a router is
> added/removed, shouldn’t it still be able to deal with the consequences? So
> if an AODVv2 outer is assigned a client that has previously been attached
> to a different router, it should make sure it doesn’t confuse the network
> (which is achieved by waiting >= MAX_SEQNUM_LIFETIME before fully treating
> it as its own client)?
>
>
> 4.3 neighbor set
>
>>
>>  Neighbor.State ... There are three possible states: Confirmed, Unknown,
>> and
> I think the "Unknown" is a bit confusing here. It's actually something we
> already know -- a state that "we heard from that node, but
> Tod determine staleness,
> typo
> Whoopsie, thanks
> This should be either more precise (in which condition, how we can modify
> the RIB?), or be removed.
>
>
> Isn’t the how really implementation specific?
>
>
>
>
>>    Multiple valid routes for the same address and prefix length but for
> It's OK, but maybe some rules must be specified and followed. For example,
> the metric used must be the same: if a packet is forwarded using metric A
> at router X, and then metric B at router Y, it's a obvious source of loops.
>
>
> Of course, but that’s an issue of properly establishing routes, which
> should be specified farther along in the draft, shouldn’t it? The purpose
> of this paragraph is to say „you can have multiple valid routes (each
> following one metric type) to the same destination, but we won’t specify
> how to pick which metric type is most important to you (i.e. which route
> you trust the most).“
>
>
> Since this set is mainly for duplicate packet detection, I think some of
> the fields are kind of redundant. For example, an RREQ/RREP message can be
> identified by its originator's address and sequence number, why 6 fields
> (OrigAddr, OrigFprefixLen, OrigSeqNum, TargAddr, TargPrefixLen, TargSeqNum)
> are necessary here.
>
> VM> I think this is right, the "targ"-related fields might well be
irrelevant, especially since we only put RREQ in that table now.  Not that
important to change at this stage, I dont think, but maybe if theres a
tidy-up of the draft in future.

> *section 5 Metrics*
>
>    In Route Request messages, the metric describes the cost of the route
>>    from OrigAddr (and any other addresses included in the prefix length
>>    of RREQ_Gen's Router Client entry for OrigAddr) to the router sending
>>    the Route Request.
>
>
> So the metric carried in the RREQ is the metric from Router Client to the
> RREQ_Gen plus the metric from RREQ_Gen to the intermediate router.
> In the Router Client Set, we have only one field for RouterClient.Cost. On
> the other hand, "AODVv2 supports different metrics".
>
> What if those two metrics are of different types? This is quite possible
> because the two links might use different medium.
>
> VM> Good point. I guess the router client entries need a number of costs,
one for each metric type that will be supported.
can we do something like
RouterClient.Costs = a number of {MetricType, Metric Value} pairs.

And also maybe something like this?:
"The RouterClient.Cost is the initial metric value used in a route message
originated at this router. A Cost SHOULD be available for each MetricType
configured for use. However, consider the case where a route with metric
type MetricType is requested. If, at RREQ_Gen, there is no corresponding
Cost value for this MetricType in the Router Client entry for OrigAddr, the
value of OrigMetric is set to zero in the RREQ. Similarly, if the RREQ
received at RREP_Gen requests a route of a MetricType for which there is no
Cost value in the Router Client entry for TargAddr, the value of TargMetric
is set to zero in the RREP."

>
> *section 6 AODVv2 protocol Operations*
>
> *General comments: *
>
> I think this section is hard to read and needs some reconstruction.
> Generally speaking, people prefer begin from the important operations to
> have a big picture/general idea of the protocol functioning, and then the
> details.
> With the current organization, one needs to read till section 6.6 (Route
> discovery, retries and buffering) before he/she could reach the core
> functioning of the protocol.
>
> I don't quite get the current organization of sections either. For example:
>
> section 6.2 (Next hop monitoring) and section 6.3 (Neighbor set update)
> are very related. One is about the function, and one is about the
> corresponding operation. The following sections don't follow this pattern
> -- why not making them to one?
>
> section 6.7.1 (Evaluating route information) and section 6.8 (Suppressing
> redundant messages using Multicast Route message set) seem to have some
> overlapped operations. They all need to check the uniqueness of the
> messages, compare the metrics and stop processing when duplication is
> detected. Although they are applied to different information bases, they
> are doing the same thing.
>
> set) should obviously a part of section 6.10.2 (reporting invalid routes),
> but it's a separate section with higher hierarchy.
>
> VM> In that case the same would apply for use of the Multicast Route
Message Set to stop redundant RREQs then? My opinion is that it's ok to
have suppression as a protocol operation, and to refer to it within the
message processing section.

> more details inline:
>
>    If such an entry does not exist, a new entry is created as described
>>    in Section 6.3.
>>
>> *While the value of Neighbor.State is Unknown,   acknowledgement of RREP
>> messages sent to that neighbor MUST be   requested*. ...
>
>
> section 6.3
>    If an external mechanism reports a link as broken, the Neighbor Set
>>    entry SHOULD be removed.
>
>
> I don't know why SHOULD, not MUST is used here. In fact, this is the only
> place that I found that a Neighbor Set entry to be removed -- considering
> the network is with constrained devices in a dynamic environment, isn't a
> timer-based removing mechanisms is more adapted?
>
>
>>   an RREP which answers a RREQ sent within RREQ_WAIT_TIME over the
>>       same interface as Neighbor.Interface
>
>
> I'm not a native English speaker, so I'm not sure about it should be "a
> RREP" or "an RREP". But I think there is no reason to use them differently
> for RREQ/RREP? Need to check the consistency in the whole draft.
>
>
> The „a RREQ“ is probably my fault, since I find that notation more
> intuitive as well, but we’ve decided that it should be „an RREQ“, because
> you’d pronounce it „arr-reck“, which warrants an „an“ in front of it.
> Thanks for the hint, I’ve checked the draft and found more of these errors,
> all fixed now.
>
>
> section 6.4.  Interaction with the Forwarding Plane
>
>    o  A route has been updated: update the corresponding route in the
>>       Routing Information Base.
>
>
> Do we need to pay attention to the "multi-metrics" issue here? To support
> multi-metrics, do we need special fields in the RIB? If so, we need to call
> it out in the applicability statement.
>
>
> I’d guess it depends where you’re choosing one route over the other? If
> the RIB supports multiple metrics and can sort it out that’s great, if not
> you might want to decide which route to propagate on to the RIB based on
> its metric, or you might want to „translate“ metrics into differently
> prioritized routes in the RIB? (if that makes sense, it should probably be
> mentioned in 4.5.  Local Route Set though)
>
>
> section 6.6
>> The constants used inSection 6.7.1 this section are
>>    defined in Section 10.
>
>
> misplaced reference
>
>
> Oh, thanks. Fixed.
>
>
> section 6.7.1
>
>    4.  Compare route costs
>>           improvement.
>> *If it is not used to update the Local Route Set,          the existing
>> non-optimal LocalRoute will continue to be used,          causing data
>> traffic to use a non-optimal route.*
>
>
> If some routers made the update, and some routers choose not updating, I'm
> afraid there will be loops in the network.
>
> VM> Agree, it would be stupid not to update. We've made this change.

>        *  If AdvRte is worse and LocalRoute is valid, ignore AdvRte for
>>           further processing.  AdvRte MUST NOT be used to update the
>>           Local Route Set because it does not offer any improvement.
>>        *  If AdvRte is not better (i.e., it is worse or equal) but
>>           LocalRoute is Invalid, AdvRte SHOULD be used to update the
>>           Local Route Set because it can safely repair the existing
>>           Invalid LocalRoute.
>
>
> shouldn’t worse AdvRte is already filtered by the LoopFree function in the
> previous step 3?
>
> section 6.7.2.  Applying Route Updates
>
>> If there is no existing matching route,
>>    AdvRte allows a corresponding RREP to be sent.
>
>
> I don't quite get why this relevant to RREP sending here.
>
>
> Yeah… Honestly I don’t get this one either. I’d just delete it? Anybody
> against that?
>
> VM> Also happy to delete this sentence.

>
>
>>    7.  If an existing LocalRoute.State changed from Invalid or
>>        Unconfirmed to become Idle, any matching Unconfirmed LocalRoute
>>        with worse metric value SHOULD be expunged.
>>    8.  If an existing LocalRoute was updated with a better metric value,
>>        any matching Unconfirmed LocalRoute with worse metric value
>>        SHOULD be expunged.
>>    9.  If this update results in LocalRoute.State of Active or Idle,
>>        which matches a route request which is still in progress, the
>>        associated route request retry timers SHOULD be cancelled.
>
>
> why "SHOULD", not "MUST" here?
>
> VM> I agree, I think this should be a MUST.

> 6.8.  Suppressing Redundant Messages Using the Multicast Route Message
>       Set
>
> the processes defined in this section overlap with section 6.7.1 (bullet
> 1-2-3-4)
>
> section 6.10 Local route set maintenance
>
>
>>    During normal operation, AODVv2 does not require any explicit
>>    timeouts to manage the lifetime of a route.  At any time, any
>>    LocalRoute MAY be examined and updated according to the rules below.
>>    If timers are not used to prompt updates of LocalRoute.State
>>
>> *, the   LocalRoute.State MUST be checked before IP packet forwarding
>> and   before any operation based on LocalRoute.State*.
>>
>
> I don't think this is an efficient approach. If the routing daemon runs in
> user space (which is true in most cases), this normally means context
> switch. This is inevitable for reactive protocols, but doing so per packet
> seems to be expensive.
>
>
> see above :)
>
>
>
> *section 7 AODVv2 Protocol Messages*
>
> general comments:
>   - the message regeneration, rather than the message forwarding, plus
> different optional fields make the protocol hard to secure
>
>
> fixed now
>
>   - the tradeoff of using multicast RREP and RERR need to be carefully
> considered.
>   - the tradeoff of using ValidityTime needs to be carefully considered
>
>
> We’ve removed ValidityTime just now
>
>
> section 7.1
>
>
>>    PrefixLengthList
>>       Contains OrigPrefixLen, i.e., the length, in bits, of the prefix
>>       associated with the Router Client entry which includes OrigAddr.
>>       If omitted, the prefix length is equal to OrigAddr's address
>>       length in bits.
>
>
> it seems that only one prefix length is needed, why "list"?
>
>
>    ValidityTime
>>       The length of time that the message sender is willing to offer a
>>       route toward OrigAddr (and any other addresses included in the
>>       given prefix length).  Omitted if no time limit is imposed.
>
>
> the necessity of using ValidityTime has been raised here
> https://www.ietf.org/mail-archive/web/manet/current/msg18852.html
>
>
>>    5.  Include MetricType and set the type accordingly
>>    6.  Set OrigMetric := RouterClient.Cost for the Router Client entry
>>        which includes OrigAddr
>
>
> What if the metric type of the RouterClient.Cost is different from the
> metric type to be used in the route discovery? In fact, this is quite
> possible because they probably use different types of medium.
>
> VM> good point. I would say that the metric type requested should be one
of the ones for which we have a routerclient.cost. See also my comment
above about how there should be a cost per metrictype that can be used, and
how to handle it if there isnt.

>  However, the regenerated RREQ can
>>    be unicast to the next hop address of the LocalRoute toward TargAddr,
>>    if known.
>
>
> This can be a problem if the TargAddr is known to the routing set, but the
> link is actually broken. Because there is no related RERR messages to
> notify that, the following route discovery retries might fail also. A more
> careful design is required if unicast RREQ is to be used.
>
> VM> I think we removed this statement, and will always multicast a RREQ.

> 7.2.  Route Reply (RREP) Message
>
> I don't quite understand the purpose of multicast RREP. There are,
> however, lots of disadvantages of using an optional AckReq field.
> This has been discussed in another thread
> https://www.ietf.org/mail-archive/web/manet/current/msg18864.html
>
>
>>    Before creating an RREP, the router SHOULD check if the corresponding
>>    RREQ is redundant, i.e., a Route Reply has already been generated in
>>    response to the RREQ, or if the limit for the rate of AODVv2 control
>>    message generation has been reached.  If so, the RREP SHOULD NOT be
>>    created.
>
>
> What about if the RREQ carries a better route?
>
>
> Then it’s not redundant? Or do you mean the SHOULD NOT be created part?
> The idea of these priorities is that already established routes should be
> protected before new routes are created, so even if the new RREQ is better,
> updating this route would strain the network even more, preventing routes
> from working, which in turn triggers more RREQs.. etc etc. So I’d say it’s
> a pity but we have no other option but to drop the better RREQ?
>
> (I’ve also substituted „the limit for the rate of AODVv2 control message
> generation“ for „ CONTROL_TRAFFIC_LIMIT“ in the Draft now since that seemed
> more concise ;) )
>
> VM> The redundancy check is already done in RREQ reception. If the RREQ
was redundant (ie stale, or cost was not better) then it would have been
discarded. If it offered a better route, then we would send a new RREP.
This text should just say about the limit for control message generation.

>
> 7.3.2 RREP_Ack Reception
>
> it seems that some action is missing after the step 2 Neighbour set
> update. At least, we need to update the Local Route Set?
>
> VM> That's triggered by the Neighbor Set update (ie it's detailed in that
section in the draft: When the Neighbor.State is set to Confirmed, any
unconfirmed routes using that neighbor as a next hop can be marked as Idle,
and therefore are valid for forwarding).  I think it has also been added
here though.

> 7.4.1 RERR generation
>
> Is it necessary to generate RERR for RREP failure?
>
> VM> Yes, because routers back in the direction the RREP came from may have
installed the route to OrigAddr which is not available if the RREP failed
to be forwarded, so they should be told, so that they delete it, and so
that they dont use it to forward data, which would then get dropped (and a
RERR would get sent then anyway).

>
>
>> If there is no route to PktSource, the RERR SHOULD be
>>    multicast to LL-MANET-Routers.  If the RERR is sent in response to a
>>    broken link, i.e., PktSource is not included, the RERR is, by
>>    default, multicast to LL-MANET-Routers.
>
>
> Although I know the purpose of using RERR message is to inform as many
> routers as possible about the route breakage information, it seems to be
> huge overhead, and more importantly, an appealing attack vector for
> hackers.
>
> VM> Should RERR only be sent if data is received to be forwarded along
that route, in which case RERR can be an end-to-end message from the
reporting router to PktSource? However, it means data packets will
definitely be dropped before a route is reported as broken, whereas the
one-hop RERR would help prevent dropped packets by deleting a route as soon
as possible. I think the one-hop RERR is still very important. We do need
to make sure the attack vector is discussed in Security Considerations.

> 7.4.3 RERR regeneration
>
>    2.  For each LocalRoute that needs to be reported as identified in
>>        Section 7.4.1:
>>        *  Insert LocalRoute.Address into the AddressList.
>
>
> The AddressList can changed on every RERR regeneration. On the other hand,
> in section 6.9 (suppressing redundant RERR), the uniqueness of RERR is
> determined by AddressList and PktSource (optional). It seems to me that the
> suppressing won't be able to work correctly?
>
> VM> The RERR Set might need to store the routes that have been advertised
in a multicast RERR, rather than store the RERR itself? Anyway, suppression
of _received_ RERR has nothing to do with this table. If we receive a RERR
for a route we already deleted, we dont re-send that address in a
regenerated RERR. It's suppressed by that. The table is only so that we
don't send multiple RERRs if we are still receiving data packets which we
no longer have a route for.

VM> In {#suppressrerr} instead of "To determine if a received RERR is
redundant:" it should say "To determine if a RERR should be created:"
Also RERR.AddressList might be misleading: because its sent in response to
an undeliverable packet, there's only one address in there. Maybe we should
use RERR.UnreachableAddress in this whole section?

> One more note:
>
> Jitter is not mentioned in the draft, which is important because the
> multicast messages used.  I can see RFC5148 in the reference section, but
> didn't find where it is used.
>
> VM> We could say "The use of jitter in an implementation is RECOMMENDED by
RFC5148" in the Message Transmission section?

Kind regards, and thanks once again for your review,
Victoria.

> best
> Jiazi
> Best regards,
> Lotte

---

From: Jiazi YI <[email-masked]>
Date: Tue, 10 May 2016 01:00:35 +0200

Hi Lotte, all:

Although the chairs have made their decision on aodvv2, I'm providing my
response to your comments below, in case you want to continue working on
the draft.

On Fri, Apr 29, 2016 at 12:50 AM, Lotte Steenbrink <
[email-masked]> wrote:

> context switches].

Yes, that's reasonable. For packet forwarding, it's preferred to simply
look at the RIB (other than route discovery is required). Checking the
LocalRoute.State seems to be unnecessary and too much overhead.

> If you don’t have a RIB and LocalRoute is consulted directly when
> forwarding a packet [this would’ve been the case in RIOT a few releases
> ago.. but there’s no kernel vs user space there either, so you wouldn’t
> have the expensive context switches], you MUST check LocalRoute.State
> before you forward to make sure the route is still valid.“ ...
>

> (unless my co-authors beat me to it ;) ) and get back to you, if that’s

I think the one in -16 is much improved. thanks.

>    to received Route Error messages, and extending and enforcing route

Yes. I think it gives a big picture of the protocol.

> negative impact on readability, so there are some exceptions to the rule…

Ok, fine.

> we did that and added the pointer… :D

hmm... for me, when I try to figure out what's a term means exactly when
reading through the document, I would jump to the terminology section, and
expect a simple and clear definition in one or two sentences. Providing a
pointer to a big chunk of text in after 40 pages won't improve the
readability of the document.

>    available, as described in Section 6.6.

Yes. But pay attention that, the point is not "frequent" or "bursty" -- if
it's between very small number of source/destination pairs, it doesn't
matter.

> devices a bit of leeway there.

I don't think it would save much by removing the single entry set --
instead, it might cause confusion.

> RouterClient. Would it make more sense it this entry was called LinkCost?

It's still a metric, no? Do we need to have a difference here?

> Isn’t the how really implementation specific?

My point is that, the sentence is content-free, and we MAY do anything we
want to the RIB, which is not good.

> Thanks for the hint, I’ve checked the draft and found more of these errors,

thanks.

> mentioned in 4.5.  Local Route Set though)

I don't have a solution by hand yet. But as mentioned in my previous
comments, having multiple entries to the same destination can be dangerous.
For example, a packet requires to be routed using metric type A. Router X
has a route supporting metric type A, so it forwards the packet. When the
packet arrives at Router Y .... woops, the route with metric type A is
invalid, but Router Y has a route with metric type B... should it be used?
A safer way is, of course, discarding the packet to avoid loops -- but is
it acceptable?

> a pity but we have no other option but to drop the better RREQ?

hmmm... what I understand is that, the intermediate routers will process
the RREQ carrying better route, but then the corresponding RREP won't be
generated?

best

Jiazi

---

From: Jiazi YI <[email-masked]>
Date: Tue, 10 May 2016 01:24:08 +0200

Hi Charlie:

On Tue, May 3, 2016 at 7:16 AM, Charlie Perkins <
[email-masked]> wrote:

> As with many documents, there is a need to establish definitions and data
> structures before specifying the details of the control protocol
> interactions.
>

I see in -16 we have already two paragraphs doing the work, which seem to
be fine for me.

> We will provide some indication for useful experiments.

thx.

> been a lot of effort to provide exactly this kind of continuity, so I don't
> know what is missing.
>

They are elaborated in the detailed inline comments .

> Multicast RERR is useful when many precursors need to receive the message
> ASAP.  The justification for multicast RREP is more detailed, but I think
> on this point there is definitely room for discussion.  Even for multicast
> RERR, serial unicast is preferable whenever the number of precursors is
> small.
>
>
>

No. But I called some of them out in the detailed comments below for those
need to be clarified,

> suggested confusion with auto-configuration.  I can't think of another term
> that denotes that the routing table is built dynamically without

I couldn't come up with a better term, because I don't get what do you mean
exactly here. For me, "self-starting" is that if I throw a non-configured
routers in the network, it should be able to run properly (getting
addresses, setting up router parameters, etc.). Apparently, this is not a
part of AODVv2.

> Experimental, we need to describe the experiments that have already been

It's not a "rule", but very useful information for the readers who are
interested in the protocol. If I have been running RFC3561 for years, and
now am thinking about moving the new version, I would like to know why it's
necessary.

> instead mandated by RFC 5498. I am surprised that you request justification
> for removing the features, since you were among the people who requested
> removing them.  This was done to satisfy that request.
>

Yes, I suggested removing the features and I know the reasons -- but not
all the readers know that :)

> These are reasonable experiments and should be mentioned in a section for
> that purpose.
>
>
thx

> mentioned?  Perhaps the longer the description gets, the less likely it is
> that people will read and digest it.
>
>
> *section 2, Terminology*
>
> It seems difficult to imagine that someone would see the term "Unreachable
> Address" and be unable to determine that the address would be used in the
> Route Error message to notify other AODVv2 nodes that the address is
> unreachable.  That is exactly what the address is used for, and seems
> intuitive to me.  Do you have any suggestion about how to remedy the lack?
>
>
>
> of source/destination pairs that do not need routes to each other.  Sending
> data to multiple destinations at the same time sounds like multicast, which
> is out of scope.  I don't know about instantaneous destinations...?
>
> An obvious case that reactive protocols won't work well is that we have
large among and instantaneous source/destination pairs.

> Well, it's been done at layer 2.  It was used for 802.11s.  It's been done
> for Bluetooth.  Of course in those situations you wouldn't use the IP
> address, but that's a pretty simple translation to make.
>

Yes, but it's more the "idea of AODV" used in those L2 technologies. For
this AODVv2 specification (message formats, information bases, message
processing etc.), it can only be used with IP.

> benefit, not cost.  I don't like those metrics, myself.  Both terms are

OK. When I'm reading the document, I couldn't really tell the difference.
For example:

                           A metric value is included within the messages
 to record the cost of the route.

If they are different, it could be helpful to explain clearly in the
terminology section.

> It's just the routing information about the client in other places of the
> network, not anything related to an external operation.  Is there something
> specific that is confusing here?
>
>
> 4.3 neighbor set
>
>>
> wrap-around cases are not exceptions, do they really need to be specified?
> To me that sounds like a long and uninformative rathole.
>

I think it's necessary, and not that complicated. For example, RFC7181
simply says:

   The sequence number S1 is said to be "greater than" the sequence
   number S2 if:

   o  S1 > S2 AND S1 - S2 < MAXVALUE/2, OR

   o  S2 > S1 AND S2 - S1 > MAXVALUE/2

   When sequence numbers S1 and S2 differ by MAXVALUE/2, their ordering
   cannot be determined.  In this case, which should not occur, either
   ordering may be assumed.

> I don't think this is possible for monotonically increasing metrics.

sorry, I meant metric type here.

 <snip>

best

Jiazi

---

From: Jiazi YI <[email-masked]>
Date: Tue, 10 May 2016 01:28:33 +0200

>> VM> We could say "The use of jitter in an implementation is RECOMMENDED
> by RFC5148" in the Message Transmission section?
>

We need more details than that, such as in which message jitter is
required, and when it should be applied. Preferably with some suggested
default values. In fact, the jitter in reactive protocol should be
considered even further -- we discussed the issue in the draft
https://tools.ietf.org/html/draft-yi-manet-reactive-jitter-04

best

Jiazi