[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 11:03:53 +0100

Hi John, all,

If we can separate the applicability and gateway stuff out, can we do it
after last call of whats left? Or would the draft and the applicability
draft need to be ready at the same time?

Regards,
Vicky.

On Tue, Jun 23, 2015 at 10:58 AM, John Dowdell <john.dowdell486@xxxxxxxxx>
wrote:

All

I think we should be proud of this review. Well done all.

However ... what's a protocol applicability draft? Is this the view from
10,000 feet up, overall architecture and purpose? If so, I'm open to
suggestions, but since we only have two weeks to go I would suggest we stay
as we are for the time being, otherwise we (and by that I mean Vicky with
her editor hat) could end up in editing hell.

Regards
John

On 23/06/15 09:53, Victoria Mercieca wrote:

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: