Telechat Review of draft-ietf-lisp-
review-ietf-lisp-genart-telechat-davies-2012-01-20-00

Request Review of draft-ietf-lisp
Requested rev. no specific revision (document currently at 24)
Type Telechat Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2012-01-17
Requested 2012-01-12
Draft last updated 2012-01-20
Completed reviews Genart Last Call review of -?? by Elwyn Davies
Genart Telechat review of -?? by Elwyn Davies
Assignment Reviewer Elwyn Davies
State Completed
Review review-ietf-lisp-genart-telechat-davies-2012-01-20
Review completed: 2012-01-20

Review
review-ietf-lisp-genart-telechat-davies-2012-01-20


  
  
    Hi, Dino.





    I have had a look at the attached version -20  docs.  





    Several of the fixes seem to have gone awry, and there are various
    cases where I think there is still discussion needed.





    I also spotted a couple more issues:


    - I don't know how an ETR can do a return routability check - see
    discussion below


    - How does an ITR know a locator is anycast - see right at the end.





    Regards,


    Elwyn





    On 16/01/12 19:50, Dino Farinacci wrote:
    







I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at


<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>

.

Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document: draft-ietf-lisp-19.txt
Reviewer: Elwyn Davies
Review Date: 20120115
IETF LC End Date: 20111130
IESG Telechat date: 20120119

Summary: This is an updated version of the review I did for Version 16.
The status is STILL 'almost ready'. I noticed one additional minor issue
(s3, ITRs). I did not receive any explicit reponses to the comments in
the previous review although some of the comments have been addressed
and I have removed those comments. So...

Given that this document is experimental, there don't appear to be any
showstopping issues.  However, I found that having the functionality
spread over several documents without a really clear overview did not
make it easy to follow. There are a number of areas where topics appear
unannounced in ways that are not immediately meaningful, and may or may
not be clarified later in the document (especially the topics of
multicast and anycast).

There are areas of the detailed format description that are very dense 
and difficult to follow, and the split of functionality between this 
document and ALT does not help.  I got the distinct feeling that extra 
bits of functionality had been shoe horned into this section over time 
without really thinking through the system aspects.  However given that 
we are considering an experimental protocol this is not a major issue, 
provided that it is cleaned up before becoming a real standard.

Major issues: None.









This is good to know. Thanks for your comments Elwyn. See responses below. I have a new -20 diff file enclosed to make sure I reflected your comments. Please have a look and comment as soon as you can. Thanks.









Minor issues:
s3, Ingress Tunnel Router (ITR): "An ITR is a router which accepts an IP
packet with a single IP header": Technically this rules out a site
attached to an ITR using any form of IP tunneling (such as VPNs or IP
mobility or 6-in-4 tunnels) through the ITR because a tunneled packet
would already have more than one IP header. In practice, I think the
qualifying statement in brackets immediately after the quoted statement
clarifies the situation but this should be rewrittten to avoid
ambiguity.









Fixed.






    Hmm.  You have substituted '(more precisely, an IP packet that does
    not contain a LISP header)' by '(typically, the IP packet that does
    not contain an IP header)'.  These are definitely not the same.  Are
    there circumstances where an IP packet coming out of a LISP site
    would already contain a LISP header.  That was not my
    understanding.  If this is not correct then what you have now is
    even more confusing.  Also in the next sentence, the phrase 'this IP
    destination address' no longer makes sense. 
















s3, EID: "EIDs MUST NOT be used as RLOCs":  But the remainder of the
definition goes on to talk about what must/must not happen when they are
numerically equivalent. This is still somewhat confusing.









When they are numerically equivaldent does not mean they are the same definition. Remember there are two namespaces so 10.1.1.1 in one namespace serves a different purpose than 10.1.1.1 in the other namespace.

I made no change here because we revised this definition dozens of times to make the working group happy. So this definition is rough concensus.






    Doubtless it is rough concensus, but the piece we are discussing is
    not part of the definition as such.  The point is that seen from
    point of view of the bits on the wire, RLOCs and EIDs are
    indistinguishable, and the later text implies that the same bit
    pattern can be either depending on context.  I think what you need
    to say is 'It is important that a value specified as an EID is only
    used in contexts where an EID is required and never in contexts
    where an RLOC is required (and vice versa).  Since the same bit
    pattern could be valid for either as explained later, it is vital
    that this logical distinction is maintained in implementations.'  I
    note that there is a context (called out below) where the EID is
    used in a context where an RLOC would normally be expected.







s3, Data Probe: Some
        clarification/explanation is needed regarding the 


        fact that a Data Probe uses an EID as its destination address in
        the 


        core, but EIDs are specifically described as not routable in the
        core 


        earlier in the document. I understand that ALT puts caveats on
        the 


        usability of Data Probes, but still potentially offers
        routability of 


        the EID over the alternative topology. I think some reflection
        of this 


        discussion would be helpful here.








      I added to the definition that when Data Probes are used, the
      underlying routing system must advertise EIDs. This is not
      desirable but there are rare cases, it may be used.





    I think your new text compounds the problem.  It explicitly
    contradicts earlier statements that EIDs are not globally routable. 
    ALT moves the EID routing problem to a separated overlay network
    which sidesteps the problem, but the new words effectively change a
    MUST NOT into a MAY.  As I said previously, there are words in ALT
    that probably need to be reproduced here.  







s4.1, item 4: ETR "processes as a control
        message"... does this imply 


        anything about prioritization in the rest of the network?








      No different than anything else and doesn't deserve a mention in
      my opinion.





    Maybe not, but why is that the only traditional use of
    prioritisation for IP packets was for router control messages.  It
    strikes me that Map-Requests and Map-Replies need to get quickly to
    the front of the queues in the routers they arrive at like BGP
    messages when travelling on the underlying routing topology.







s5.3, LISP Nonce: What are the
        consequences of using the same nonce for 


        'too long'.. and how can an implementor decide what is too long?








      A possible replay attack if a man-in-the-middle can guess a 24-bit
      or 128-bit nonce.





    Guidance to implementers is required.  OK, maybe you don't know yet
    'cos its an experiment, but the draft ought to state that guidance
    will be given after experiments, if only as a reminder to somebody
    turning this into a standard.







s6.1: "Implementations MUST be prepared to
        accept packets when either the


        source port or destination UDP port is set to 4342 due to NATs


        changing port number values." I (still) don't understand this.
        My 


        understanding was that these UDP packets are generated in the
        ITR with 


        globally routable RLOCs in the headers. Why are they going
        through NATs?


        Does this mean when only one of the ports is 4342? (but the
        previous


        comments still stands).








      An ITR could be behind a NAT. And when it is its "local RLOC" is a
      private address. When the packet traverses a NAT, the source
      address will be changed to a global address. The outside world
      believes the source of the packet is the source RLOC which is
      globally reachable and advertised in the underlying routing
      system.





    Is the problem a missing 'not'? 




when either the


      source port or destination UDP port is ***not*** (?) set to 4342
      due to NATs


      changing port number values.








      NATs also translate/change the source port of the outer header. So
      the if a LISP control packet is returning a reply to a request
      that had 4342 in the destination port, then the source port would
      be translated and the requester would not know the packet is a
      LISP control packet. So if the source port of the request is 4342,
      the replier can return that in the destination port of the reply
      without a NAT touching the port (while it is touching the other
      port).







s5.3: Copying of TTL/Hop Count fields and
        Type of service fields: This 


        discussion is somewhat confusing. Initially these are described
        as 


        SHOULD mostly without any description of why one would not. A
        couple of 


        paragraphs later they become MUSTs with some minor and explicit
        


        exceptions. This should be clarified.








      We said should because we were being practical. Some
      implementations cannot copy the TTL and set it statically. We want
      those implementations to not violate the spec.





      We use MUST later for the Type of Service field. When the TOS is
      copied, then the sub-parts MUST follow what the text says.








    OK.  That is correct.   Its possible there is a better wording than
    'caveat' but I can't think of a short form.  Caveat tends to make me
    think of get outs, whereas what you are doing is making it more
    strict for the ECN case. Leave it be. 







s6.1, last para: "This main LISP
        specification is the authoritative 


        source for message


        format definitions for the Map-Request and Map-Reply messages."
        So 


        what spec is authoritative for Map-Register and Map-Notify? It
        is later 


        stated that this spec defines the message format of these
        messages 


        (first paras of s6.1.6/6.1.7). This seems pretty authoritative.
        I 


        suspect that this para could be dropped wlog.








      I will fix this text. Thanks for finding this.







s6.1.4: S bit: How do I find out how big
        the Mapping Protocol Data is in 


        order to determine where the extra security fields are located?








      This was commented on quite a bit and was used for CONS. Since the
      commenters won the argument to remove references to CONS, we will
      remove the Mapping Protocol Data part, which actually is not used
      by any implementations.





    OK.. But, should it go from s6.1.2 as well?







s6.1.4: M priority/weight: Multicast
        suddenly makes an appearance. Not 


        sure why?








      Because we have to encode multicast priorities. And to get a
      mapping for an ETR to join an ITR, we use a Map-Request/Map-Reply
      exchange. I will add a reference to LISP-multicast for each
      reference to M priority and M weight.







s10.3: How is a "route-returnability
        check" performed in LISP?








      I added another sentence to the defintion of
      "Route-returnability".





    The new sentence tells the principle of what messages to send - but
    where to send them?  The problem in my mind is that it is apparently
    the ETR performing the check.  Is the check sent to the ITR or the
    original source (EID)?  If to the original source, why should that
    source believe it ought to answer this ETR (which will have to have
    an EID that the return message is sent) when it has never heard from
    the ETR and has no idea whether the ETR is a kosher node?  If the
    source is behind a firewall or NAT there is a good chance the
    message will never arrive.  This case differs from the standard
    route returnability case where messages are always end-to-end.   







Nits/editorial comments:


        General: s/bytes/octets/








      Changed throughout.







General: Figures all need titles and
        numbers.








      There are section titles for the figures. There are no figure
      numbers just like RFC 2460. I don't see the point since there is
      no text that back references the figures. And I think we wrote the
      text to sufficiently introduce the figures that are about to come
      up.





    You can discuss this one with the RFC Editor.







s3: s/PA addresses are an an address/PA
        addresses are an address/





        s3, RLOC: Link to IPv4/v6 specifications needed.





        s3, EID-to-RLOC Database: "This has no negative implications." 


        Justification?





        s3, Negative Mapping Entry: Needs cross-reference for Negative
        Map-Reply.








      Changed all the above.







s3, Data Probe: needs cross-reference(s)








      I don't think it does because it is being defined here for the
      first time.





    See discussion above. ALT?







s3, PITR: The alternative term PTR is now
        not used at all and can be


        removed.








      Fixed.







s4.1, item 7: Needs cross references.








      Can you be more specific please. Are you asking for a reference
      for DNS names or for the Map-Reply or for something else?





    No.  To the piece that tells about the policies (Section 6.5). 







s5.1 and s5.2 Need references to base IP
        specifications and notes about 


        the items that are defined in those specifications, and the
        equivalences 


        between source/destination IP addresses and EIDs/RLOCs. (also
        applies to 


        s6.1)








      We did this in a later update. We put the references in 5.3 where
      we describe such fields.





    Fine.










s5.3: This section should also refer to
        the various abbreviations used 


        in the figures (e.g., OH for Outer Header, etc).








      It is expanded in the "Packet header descriptions:" part.








    True but hardly 'first use'.  All it needs is s/Inner header/Inner
    Header (IH)/ and similarly for OH in s5.3. Plus a definition of IHL
    (internet header length).







s5.3, LSB: First mention of anycast
        addresses here. Needs a cross 


        reference or earlier introduction.








      I have added a definition for anycast address in the Definition of
      Terms section.







s6.x: Encodings of numeric fields not
        specified.








      All fixed. New text enclosed. Please acknowledge. The -20 has not
      been posted. This is a work in progress -20 waiting Adrian's
      comments as well.





      Dino











    EXTRA INTRODUCED NIT and EXTRA ISSUE SPOTTED:


    s4, first bullet: "get to destination, which in turn, LISP routers
    deliver" has got garbled.  I think something like the following was
    intended:


        assume packets get to their intended destinations.  In a system
    where LISP is deployed, LISP routers intercept EID addressed packets
    and assist in delivering them across the network core where EIDs
    cannot be routed.





    s5.3, LISP Locator Status Bits, last sentence:    "If the LSB for an
    anycast locator.." How does the ITR know it is an anycast locator? 
    They are indistinguishable for any other IP address in most cases.