Subject: Draft reviews

From: Sébastien Barré <[email-masked]>
Date: Tue, 12 Feb 2008 14:24:26 +0100

Hi,

While writing the shim6-impl draft, I have noted some minor things with 
the drafts. Most of them are editorial comments, I have found also some 
may/must/should conflicts.
Comments about shim6-proto-09,failure-detection-10, 
locator-pair-selection-02 and multihome-shim-api are detailed below.

regards,

Sébastien.

draft-ietf-shim6-proto-09.txt
-----------------------------------------------

Section 5.6  : ULID pair : ... addresses in the IPv6 header does not 
match ->
    do not match
           CGA PDS : ...so the receiver...-> so that the receiver
Section 5.15 : C : critical, One if the parameter is critical, and MUST 
be recognized by the recipient -> I don't think this is a MUST in the 
sense of RFC2119, since currently no option is defined as critical. The 
draft thus specifies as an "absolute requirement of the specification" 
something that does not exist yet. I would say that this may be rather a 
"must" of a given implementation, and as such I just would prefer to see 
it in lowercase.
Section 5.15.3 (at the end) : a of a 1 octet flag field followed... -> a 
sequence of a 1 octet ...
Section 7.1 : ...cycle through the unstrucutred -> unstructured
Section 7.2 : different than the ULID -> different from the ULID
Section 7.6 : I would add a pointer to section 7.15 : "A more detailed 
description of what should be done to detect confusions is given in 
section 7.15"
Section 7.15 : Forked Intance Identifier different than the ones... -> 
are different from the ones...
Section 7.15 : Conflicting MAY and MUST with section 7.6 : 7.6 says "The 
requirement is that the old context which used the context tag MUST be 
removed" while 7.15 says "the host MUST NOT use the old context, it MAY 
just discard the old context."
Section 8 : with an payload extension header -> with a payload...
Section 11 : Then it there is also no effect -> Then there is also...
Section 12.1 : for reachability detection porpuses -> purposes
Section 12.2 : for packets that does not have -> that do not have
Section 12.3 : it checks that the neither the ->it check that neither the...
Section 12.3 : I think that the checksum should be verified _after_ the 
length
    field, since this field is used by the checksum verification procedure.
    Failing to do that could result in reading memory areas past the end
    of the IPv6 packet.
Section 12.3 : Conflicting SHOULD and MUST with section 5.15 : 5.15 says 
"If a host receives an option that it does not understand and If C=1 
then the host SHOULD send back a Shim6 Error Message with Error 
Code=1..." while 12.3 says "If there is any option in the message with 
C=1 that isn't known to the host, the host MUST send a Shim6 Error 
Message with Error Code=1...".
Section 15.1 : it recommended -> it is recommended
Section 15.2 : §1 : the presudoheader -> pseudoheader
Section 15.2 : §2 : ...the next header header -> the next header
Section 15.2 : §3 : unless they implement are able -> unless they are able

draft-ietf-shim6-failure-detection-10.txt
-------------------------------------------------------------------

Section 3.3 : §5 : explicit rechability tests -> reachability
Section 4.1 : "Nodes SHOULD employ techniques listed in Section 3.1 and
   Section 3.2 to track the local situation." : These two sections are 
already
   completely covered by MUST/MAY statements. IMHO this creates a
   SHOULD/MUST/MAY conflict
   between section 3.1/3.2 and 4.1.
Section 4.1, item 5 : "REAP keepalive
       packets SHOULD continue to be sent at the Keepalive Interval
       until either a data packet in the SHIM6 context has been received
       from the peer or the Keepalive Timeout expires" : I would replace
       "has been received from" with "has been sent to"
Section 4.2 : §4 : so that the ULP SHOULD be informed -> thus the ULP 
SHOULD be informed.
Section 5.1 and 5.2 : The same field has name Reserved1 for keepalive 
and   
    Reserved for Probe. Probably it should also be Reserved1 for probe
    (since the probe has a Reserved2 field as well).
Section 5.1 and 5.2 : Strange use of the MUST statement regarding the 
Reserved*
    fields : All those fields MUST be ignored on receipt, but only
    the Reserved2 field of the probe message MUST be set to 0 upon
    transmission. I would replace this MUST with the usual 'It is set to 0
    ...'

draft-ietf-shim6-locator-pair-selection-02.txt
---------------------------------------------------------------------------

Section 2.2 : a locator pair is know to be...-> is known to be

draft-ietf-shim6-multihome-shim-api-03.txt
------------------------------------------------------------------------

Section 1 : §5 : a singed integer of... -> a signed integer

-- 
Sébastien Barré
Researcher,
CSE department, UCLouvain, Belgium
http://inl.info.ucl.ac.be/sbarre

---

From: marcelo bagnulo braun <[email-masked]>
Date: Tue, 12 Feb 2008 15:33:09 +0100

thanks i will address those of the proto, locator in the next reviews

El 12/02/2008, a las 14:24, Sébastien Barré escribió:

> Hi,
>
> While writing the shim6-impl draft, I have noted some minor things  
> with the drafts. Most of them are editorial comments, I have found  
> also some may/must/should conflicts.
> Comments about shim6-proto-09,failure-detection-10, locator-pair- 
> selection-02 and multihome-shim-api are detailed below.
>
> regards,
>
> Sébastien.
>
>
> Section 5.15 : C : critical, One if the parameter is critical, and  
> MUST be recognized by the recipient -> I don't think this is a MUST  
> in the sense of RFC2119, since currently no option is defined as  
> critical. The draft thus specifies as an "absolute requirement of  
> the specification" something that does not exist yet. I would say  
> that this may be rather a "must" of a given implementation, and as  
> such I just would prefer to see it in lowercase.
> Section 5.15.3 (at the end) : a of a 1 octet flag field followed... - 
> > a sequence of a 1 octet ...
> Section 7.6 : I would add a pointer to section 7.15 : "A more  
> detailed description of what should be done to detect confusions is  
> given in section 7.15"
> Section 7.15 : Forked Intance Identifier different than the ones... - 
> Section 7.15 : Conflicting MAY and MUST with section 7.6 : 7.6 says  
> "The requirement is that the old context which used the context tag  
> MUST be removed" while 7.15 says "the host MUST NOT use the old  
> context, it MAY just discard the old context."
> Section 12.3 : I think that the checksum should be verified _after_  
> the length
> Section 12.3 : Conflicting SHOULD and MUST with section 5.15 : 5.15  
> says "If a host receives an option that it does not understand and  
> If C=1 then the host SHOULD send back a Shim6 Error Message with  
> Error Code=1..." while 12.3 says "If there is any option in the  
> message with C=1 that isn't known to the host, the host MUST send a  
> Shim6 Error Message with Error Code=1...".
> Section 4.2 : §4 : so that the ULP SHOULD be informed -> thus the  
> ULP SHOULD be informed.
> Section 5.1 and 5.2 : The same field has name Reserved1 for  
> keepalive and      Reserved for Probe. Probably it should also be  
> Reserved1 for probe
> Section 5.1 and 5.2 : Strange use of the MUST statement regarding  
> the Reserved*