[aodvv2-discuss] Re: Fwd: QA Review - Routing directorate review of draft-ietf-manet-aodvv2

  • From: Victoria Mercieca <vmercieca0@xxxxxxxxx>
  • To: "aodvv2-discuss@xxxxxxxxxxxxx" <aodvv2-discuss@xxxxxxxxxxxxx>
  • Date: Tue, 23 Jun 2015 09:53:43 +0100

Hi Stan, all,

This looks great! I can go ahead and make the changes for most of the nits
if no-one objects? Moving bits out into a separate applicability draft -
what do we think of this idea? I've put in some further comments below:


On Tue, Jun 23, 2015 at 3:56 AM, Stan Ratliff <ratliffstan@xxxxxxxxx> wrote:

Ladies & Gents -

The review from Keyur. All in all, I think it's pretty good news... ;-)

Regards,
Stan


---------- Forwarded message ----------
From: Keyur Patel (keyupate) <keyupate@xxxxxxxxx>
Date: Mon, Jun 22, 2015 at 10:05 PM
Subject: QA Review - Routing directorate review of draft-ietf-manet-aodvv2
To: Jonathan Hardwick <Jonathan.Hardwick@xxxxxxxxxxxxxx>, "
ratliffstan@xxxxxxxxx" <ratliffstan@xxxxxxxxx>, "jon.hudson@xxxxxxxxx" <
jon.hudson@xxxxxxxxx>
Cc: "bebemaster@xxxxxxxxx" <bebemaster@xxxxxxxxx>


Hi Jonathan,

I couldn’t get hold of an official format for a QA review. I have tried
to reuse the standard Rtg-dir template.

IMHO, There are two drafts here. There is a draft for AODVv2 and there
is a protocol applicability draft. The authors should consider breaking
the draft into two separate drafts. Furthermore, this draft can definitely
benefit from some state-machine diagrams to explain protocol operations.

Otherwise the draft is written well considering its size. :)

Regards,

Keyur



Document: draft-ietf-manet-aodvv2-09
Reviewer: Keyur Patel
Review Date: 22-June-2015
Intended Status: Standards Track

Summary:
No major issues found. 2 Minor issues identified. Minor nits are listed
below.

Comments:
IMHO, the document is well written and easily understood.

Major Issues:
None.

Minor Issues:

1 Section 6.5.1 comparison 3:

What happens if the old and new route has a different metric type (default
versus other metric type). What is the tie breaking mechanism? Can we add
some text to clarify it?



If its a different metric type then we conclude there's no matching route,
and we use the incoming route information to add a new route for that
metric type. We could elaborate on that description in step 1 of section
6.5.1.


2 Section 6.5.1 comparison 3: point 2, if AdvRte is better, shouldnt it
be checked to

validate if its safe against routing loops?



I agree, I think we should switch steps 3 and 4 in 6.5.1.


3 Security Section: Security section is a bit weak. Since the packets are
rate-limited, the DDOS can become a significant factor. If Encrpytion is
suggested, then authors should clearly explain 1) How would it be used, 2)
How does one go about avoiding DDOS of bogus encryption packets.



Nits:

1) Section 1, 2nd para:

<snip>

AODVv2 routers use Route Reply and Route Request messages to carry route
information between the originator of the route discovery and the target...

<snip>


Replace with : AODVv2 routers use Route Request (RREQ) and Route Reply
(RREP) messages to carry route information between the originator of the
route discovery and the target, …


2) Section 1. What exactly is a target? Can we have Terminology section
cover definition of a target?


3) Section 1, 3rd paragraph


Replace with: Metric is included in RREQ and RREP message to represent
the cost of the route….


4) Applicability Statement – IMHO, This should be a separate draft
(applicability draft for AODV2).


5) Data Structures - section a bit too implementation specifc. The
conceptual Route Table entry stucture is an implementation specific DS.


How should we address this? The way we have described each field is useful
because we refer to these later.

6) Section 5.2


<snip>

advertised route to update the route table will not cause a routing loop
to form.


</snip>


Replace with: advertised route does not create any routing loops.


7) Section 5.3 Default Metric type 3rd paragraph.


<snip>

The LoopFree function is derived from the fact that route cost...

</snip>


This paragraph should be moved to secion 5.2 where the loop free
function is described.


Not sure if I agree. That statement directly relates to the LoopFree
function for the default metric. It wouldnt necessarily apply to all metric
types. We could clarify with "This LoopFree function", or "The LoopFree
function for the hopcount metric"?

8) Section 5.4 4th paragraph


<snip>


Further, any strictly increasing metric can be supported using the
LoopFree

</snip>


replace with: Furthermore, any strictly increasing metric can be
supported using the LoopFree



9) It is worth considering moving section 6: Protocol operations ahead of
section 5 Metric section (Swap them).


What do you all think of this? I thought it made sense to explain lots of
supporting info up front then get into how it worked.


10) Section 6.3


<snip>

o avoid congestion, each AODVv2 router's rate of message generation

(CONTROL_TRAFFIC_LIMIT) SHOULD be limited and administratively

configurable. The implementation is free to choose the algorithm for

limiting messages, including prioritizing messages when approaching

the limit. AODVv2 messages SHOULD be discarded in the following

order: RERR for invalidated routes, RREQ, RREP, RERR for

undeliverable packet, RREP_Ack.


</snip>


It would be nice to move the definition of an invalidated route before
referencing it in this section.


Could add this in to the route table entry data structure section, would
make sense there.


11) Section 6.4: 4th para:


<snip>

Determining which packets

to discard first when the buffer is full is a matter of policy at

each AODVv2 router; in the absence of policy constraints, older data

packets SHOULD be discarded first.

</snip>


Possible to take the suggestion out and leave it for implementations to
decide?

12) Section 6.5.2: It would be nice for Section 6.5.2 to cover under
what circumstances Route.State moves to confirm/Active. Note that the first
paragraph does make a passing reference of the state change and section
6.7.1 explains the states. Section 6.5.2 seems to be missing an event
(description) in a state machine (explained in section 6.5.2)

Hmm. 6.5.2 is about applying incoming information. But we do talk about
states here before we've explained them. I suppose where we have step 7 in
6.5.2, where we say what happens if this results in a valid route, we could
also say what happens if it results in an invalid route... ie how it could
become valid in future.

13) Section 6.7.2: Can we add a case that describes if the looked up route
itself is invalid one (aside from an active route becoming invalid)?

This is the beginning of the second paragraph, isnt it?


14) Section 7.1, 7.2, and 7.4 should cover size for all data elements
described along with their descriptions.


So... something like "8 bit prefix length" or "16 bit sequence number",
address lengths? size of hop limit field, etc? 8 bit validity time. metric
field size depends on metric type. Because we have this separation of data
elements and RFC5444 TLVs, it doesnt seem appropriate in all cases to write
a size. For the actual data, we could do this. For metric would we assume
hopcount and write size = 8 bits? and note that different metric types
would be of different sizes?

15) Section 7.4.1: Isnt it harmful to rate limit RERR messages (route
withdrawals)?


Should we disallow this? We have 2 relevant parts here: we say we should
check if we already sent a similar RERR recently, and not re-send if we
have. Then we also say about not exceeding the rate of AODVv2 control
message generation. Should we remove the bit about control message
generation limit? We would then have the threat that if someone could
maliciously bombard us with packets with many different source/destination
addresses, we would be responding to each one with a RERR with no limit
imposed.

16) Section 9 should be moved out to a separate applicability draft. Some
of the sections in Appendix could also be moved out to an applicability
draft.



Move the gateway stuff out to a separate draft? How do we like this idea?
:)

Keyur also mentions some state machine diagrams, specifically for route
state changes. Any others that would be valuable? Neighbor state changes?

Kind regards,
Vicky.

Other related posts:

  1. Home
  2. aodvv2-discuss
  3. Archive
  4. 06-2015