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

[Charlie Perkins <[e-mail]>]

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.