[Roll] AD review of draft-ietf-roll-of0

"Adrian Farrel" <adrian@olddog.co.uk> Sat, 11 June 2011 21:37 UTC

Return-Path: <adrian@olddog.co.uk>
X-Original-To: roll@ietfa.amsl.com
Delivered-To: roll@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 21F4711E80AF for <roll@ietfa.amsl.com>; Sat, 11 Jun 2011 14:37:56 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.602
X-Spam-Level:
X-Spam-Status: No, score=-2.602 tagged_above=-999 required=5 tests=[AWL=-0.003, BAYES_00=-2.599]
Received: from mail.ietf.org ([64.170.98.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id d+NjLVsq4ee1 for <roll@ietfa.amsl.com>; Sat, 11 Jun 2011 14:37:55 -0700 (PDT)
Received: from asmtp4.iomartmail.com (asmtp4.iomartmail.com [62.128.201.175]) by ietfa.amsl.com (Postfix) with ESMTP id 6A0E311E80B6 for <roll@ietf.org>; Sat, 11 Jun 2011 14:37:54 -0700 (PDT)
Received: from asmtp4.iomartmail.com (localhost.localdomain [127.0.0.1]) by asmtp4.iomartmail.com (8.13.8/8.13.8) with ESMTP id p5BLbgEP025861; Sat, 11 Jun 2011 22:37:42 +0100
Received: from 950129200 (dsl-sp-81-140-15-32.in-addr.broadbandscope.com [81.140.15.32]) (authenticated bits=0) by asmtp4.iomartmail.com (8.13.8/8.13.8) with ESMTP id p5BLbe6m025852 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NO); Sat, 11 Jun 2011 22:37:41 +0100
From: Adrian Farrel <adrian@olddog.co.uk>
To: draft-ietf-roll-of0@tools.ietf.org
Date: Sat, 11 Jun 2011 22:37:44 +0100
Message-ID: <0bd201cc287f$cd278a60$67769f20$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 14.0
Thread-Index: Acwof6imW3R71k0UTtWgnG7jsTjMYg==
Content-Language: en-gb
Cc: roll@ietf.org, roll-chairs@tools.ietf.org
Subject: [Roll] AD review of draft-ietf-roll-of0
X-BeenThere: roll@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
Reply-To: adrian@olddog.co.uk
List-Id: Routing Over Low power and Lossy networks <roll.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/roll>, <mailto:roll-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/roll>
List-Post: <mailto:roll@ietf.org>
List-Help: <mailto:roll-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/roll>, <mailto:roll-request@ietf.org?subject=subscribe>
X-List-Received-Date: Sat, 11 Jun 2011 21:37:56 -0000

Hi,

Don't panic!

I have done my usual AD review of your draft to ensure that it has a
smooth passage through IETF last call and IESG review.

The good news is that I don't have any technical issues with your I-D,
but I am afraid I have a very large number of suggested editorial nits
to make the document read more clearly. This clarity is important both
to allow successful implementation and interoperability, and to ensure
that the reviewers can quickly reach the correct understanding.

I think that a new version of the I-D is needed before I can issue the
IETF last call. However, since the changes are purely editorial (and,
for the avoidance of doubt, these are the only changes you should make
at this stage) we will be able to go ahead quickly as soon as the new 
revision is posted.

Thanks,
Adrian

---

Could you please capitalise all section headers.

---

Abstract

OLD
   of a specific Objective Function
NEW
   of specific Objective Functions.
END

---

Abstract

I would like the Abstract to say what an objective function is used for.

How about adding a second sentence...

   An Objective Function defines how a RPL node selects and optimizes
   routes within a RPL Instance based on the information objects
   available.

---                                  

Introduction (line 1)

OLD
   The Routing Protocol for Low Power and Lossy Networks
NEW
   The Routing Protocol for Low Power and Lossy Networks (RPL)
END                          

---

Introduction

The Abstract says it better than paragraph 1, here. The word "problem"
is quite ambiguous. Can you...
                                                
s/problem/deployment and application is a specific design of network"

---

Introduction

Can you add the same sentence as added to the Abstract as sentence 2 in
para 1.

---

Introduction para 1

s/RPL to adapt/RPL to be adapted/

---

Introduction para 2

s/in a new Version/as a new DODAG Version/

---

Introduction para 3

OLD
   An Objective Function selects the DODAG Version that a device joins
   within an instance, and a number of neighbor routers within that
   DODAG Version as parents or feasible successors. The OF generates
   the Rank of the device, that represents an abstract distance to the
   root within the DODAG.  In turn, the Rank is used by the generic RPL
   core to avoid loops and verify forward progression towards a
   destination, as specified in [I-D.ietf-roll-rpl].
NEW
   An instance of RPL running on a device uses an Objective Function to
   help it determine which DODAG Version it should join. The OF is also
   used by the RPL instance to select a number of routers within the 
   DODAG Version to serve as parents or as feasible successors.

   The RPL instance uses the OF to compute a Rank for the device. This
   value represents an abstract distance to the root of the DODAG within
   the DODAG Version. The Rank is exchanged between nodes using RPL and
   allows other RPL nodes to avoid loops and verify forward progression
   toward the destination, as specified in [I-D.ietf-roll-rpl].
END

---
                                                                                
Introduction 

s/( excellent)/(excellent)/

---

Introduction

   As a result, OF0 with
   default settings allows to encode a minimum of 28 (worst acceptable)
   hops and a maximum of 255 (excellent) hops.

This is true (and 9*29 > 255), but is more than a little opaque!

How about:

   Because RPL allows a leaf-to-root path in a DODAG to have a distance 
   of any value up to 255, OF0 with default settings allows to encode a
   minimum of 28 (worst acceptable) hops and a maximum of 255
   (excellent) hops.

---

Introduction

OLD
   Since there is no default OF or metric container in the RPL main
   specification, it might happen that, unless two given implementations
   follow the same guidance for a specific problem or environment, those
   implementations will not support a common OF with which they could
   interoperate.
NEW
   It is important that devices deployed in a particular network or
   environment use the same OF to build and operate DODAGs. If they do
   not, it is likely that sub-optimal paths will be selected, and it is
   possible that parts of the network will become disconnected (perhaps
   because devices select paths that would cause loops to be formed). In
   practice, without a common definition of an OF, RPL implementations
   cannot guarantee to interoperate correctly.

   The RPL specification [I-D.ietf-roll-rpl] does not include any OF
   definitions. This is left for other documents specific to different
   deployments and application environments.
END

---

Section 2

s/The term feasible successor/The term 'feasible successor'/

---

Section 3

OLD
   For the purpose of
   OF0, Grounded thus means that the root provides such connectivity.
NEW
   Thus, for the purpose of
   OF0, the term Grounded [I-D.ietf-roll-rpl] means that the DODAG root
   provides such connectivity.
END

---

Section 3

OLD
   This can be achieved if the Rank of a node represents closely its
   distance to the root. 
NEW
   This can be achieved if the Rank of a node is very close to an
   abstract function of its distance to the root. 
END

---

The terms LLN and DAG are used without expansion.

---

Section 3

   OF0 assigns a rank_increase to each link to another node that it
   monitors.

I think it is the link that is monitored not the node. Furthermore,
OF0 doesn't monitor anything. So...

   A RPL node monitors links to neighbor nodes, and can use OF0 to
   assign a rank_increase to each link.

---

Section 4.1

A couple of terms are introduced as being computed and implying
something, but the use of the terms is not explained. For example,
at the head of 4.1, step_of_rank is defined as something that is
computed, but we don't find out what it is for until the end of the
section (where it turns out just to be a multiplier used to help
determine the increase in rank to apply).

I suggest

OLD
   An OF0 implementation first computes a step_of_rank associated with a
   given parent from relevant link properties and metrics.
NEW
   An OF0 implementation first computes a step_of_rank associated with a
   given parent from relevant link properties and metrics. The 
   step_of_rank is used to compute the amount by which to increase the
   rank along a particular link, as explained later in this section.
END

---

Section 4.1

s/recommended to base/RECOMMENDED to base/

---

Section 4.1

s/less but longer/fewer but longer/

---

Section 4.1

I think that draft-ietf-roll-routing-metrics would be a better reference
than [DeCouto03] for ETX.

---

Section 4.1

   An implementation MAY allow to stretch the step_of_rank with a
   stretch_of_rank up to no more than MAXIMUM_RANK_STRETCH in order to
   enable the selection of at least one feasible successor and thus
   maintain path diversity.

Not sure what this means!

Possibly...

   An implementation MAY stretch the step_of_rank in order to enable the
   selection of at least one feasible successor and thus maintain path 
   diversity. It uses a stretch_of_rank to do this and may set it to any
   value between 0 (no stretch) and the fixed constant 
   MAXIMUM_RANK_STRETCH (see Section 6.3).

---

Section 4.1

s/not recommended/NOT RECOMMENDED/

But I don't like the order of progression in the paragraph! First you
say that an implementation may do something. Then you say it should not.

Can you reorder the paragraph. Also put it into the positive, not the
negative...

The implementation SHOULD do something (or is RECOMMENDED to do that
thing). Although it has bad side effects, an implementation MAY do 
something else.

---

Section 4.1

OLD
   MUST maintain the stretched step_of_rank between
   MINIMUM_STEP_OF_RANK and MAXIMUM_STEP_OF_RANK
NEW
   MUST maintain the stretched step_of_rank between
   the fixed constants MINIMUM_STEP_OF_RANK and MAXIMUM_STEP_OF_RANK 
   (see Section 6.3).
END

---

Section 4.1

   An implementation SHOULD allow a configurable factor called
   rank_factor and to apply the factor on all links and peers.

You need to say how this is applied!

---

Section 4.1

   The rank_factor
   SHOULD be set between MINIMUM_RANK_FACTOR and MAXIMUM_RANK_FACTOR.

Surely this is s/SHOULD/MUST/

Please say "between the fixed constants...   ... (see Section 6.3)."

---

Section 4.1

   The rank_increase is expressed in units of MinHopRankIncrease, which
   defaults to DEFAULT_MIN_HOP_RANK_INCREASE; with that setting, the
   least significant octet in the RPL Rank is not used.

becomes...

   The rank_increase is expressed in units expressed by the variable
   MinHopRankIncrease which defaults to the fixed constant 
   DEFAULT_MIN_HOP_RANK_INCREASE (see Section 6.3); with that setting,
   the least significant octet in the RPL Rank field in the DIO Base
   Object is not used.

---

Section 4.2.1 step 2

s/should/SHOULD/
s/out of scope/out of scope of this document/
s/succeeded that/passes that/

---

Section 4.2.1 step 7

s/2/two/

---

Section 4.2.1 step 9

s/optional/OPTIONAL/

---

Section 4.2.1 is full of "SHOULD" rules. This is fine, but it means that
you need to describe the variance. This might be a catch-all such as...

   These rules MAY be varied by an implementation according to 
   configured policy.

---

Section 4.2.2

Ditto the issue about variance of "SHOULD" rules.

---

Section 5

The term "RPL core" is not introduced or discussed. It doesn't show in
the RPL spec or in the terminology draft.

Reading the section, I think you are referring to some implementation-
specific element of the code. If that is so, I wish you wouldn't! 
Whether I implement RPL as a core protocol component, or a number of
flimby wing-nuts is entirely my business.

Can you please re-write the section in terms of what OF0 is used for and
what information it has to access. What code component calls what other
code component really is out of scope.

---

Section 5

s/OCP)/(OCP)/
s/DIO ./DIO./
                                                                  
OLD
      OF0 corresponds to the OCP 0 (to be validated by IANA).
NEW
      OF0 is identified by OCP 0 (to be validated by IANA).
END                                                        

---

Section 6.2
           
What is a "Configurable parameter" and how is it optional.
I don't think you mean that it is a constant.
Is it a variable that is only used if the optional feature is 
implemented?
Maybe you mean that it is a configurable value.

Can you clarify the text, please.

---

Section 6.3


OLD
   OF0 fixes the following constants:
NEW
   Section 17 of [I-D.ietf-roll-rpl] defines RPL constants. OF0 fixes
   the values of the following constants:
END

---
 

We need a discussion of manageability.
This fits best between Sections 6 and 7.

You can say which values should be configurable and when (build time,
load time / run-time, dynamically).

You should also say what information it should be possible to retrieve 
from a system that implements OF0.

---

Section 7

Please identify the registry you want IANA to make an allocation from.

---

Section 8

   Security Considerations for OCP/OF are to be developed in accordance
   with recommendations laid out in, for example,
   [I-D.tsao-roll-security-framework].

I am not sure that this means anything at all!

You should have:

What RPL protocol elements are involved?
- Rank
- OCP
What are the risks if they are compromised or sniffed?
Then point to the security framework for a discussion of how to secure
RPL protocol elements.

What happens if an element says it is implementing OF0, but through an
error or malign action implements something else? Can this be detected
or mitigated?

---

Section 9

I am not sure, but I think...

s/implementer's feedback/implementers' feedback/                     

---

Section 10.2

The use in Section 4.2.1 makes [I-D.ietf-roll-rpl] a normative reference

[I-D.tsao-roll-security-framework] is surely 
[I-D.ietf-roll-security-framework]

---