Last Call Review of draft-ietf-mpls-proxy-lsp-ping-03
review-ietf-mpls-proxy-lsp-ping-03-genart-lc-taylor-2015-02-13-00

Request Review of draft-ietf-mpls-proxy-lsp-ping
Requested rev. no specific revision (document currently at 05)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2015-02-12
Requested 2015-02-04
Draft last updated 2015-02-13
Completed reviews Genart Last Call review of -03 by Tom Taylor (diff)
Secdir Last Call review of -03 by Taylor Yu (diff)
Opsdir Last Call review of -03 by Warren Kumari (diff)
Assignment Reviewer Tom Taylor
State Completed
Review review-ietf-mpls-proxy-lsp-ping-03-genart-lc-taylor-2015-02-13
Reviewed rev. 03 (document currently at 05)
Review result Ready with Issues
Review completed: 2015-02-13

Review
review-ietf-mpls-proxy-lsp-ping-03-genart-lc-taylor-2015-02-13

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 resolve these comments along with any other Last Call comments
you may receive.

Sorry to be so last-minute. Life happens.

Tom Taylor

Document:  draft-ietf-mpls-proxy-lsp-ping-03
Reviewer:  Tom Taylor
Review Date: 2015-02-12
IETF LC End Date: 2015-02-12
IESG Telechat date: 2015-02-19



Summary: Generally well written, but a few minor issues and a number of 


nits and editorial suggestions.




Major issues:

Minor issues:

1. Section 3.1, "Proxy Echo Parameter TLV MPLS payload size field".



This section describes initiator actions, but the paragraph contains the 


text:




" When the payload size is non
   zero, if sending the MPLS Echo Request involves using an IP header,
   the Do not Fragment (DF) bit MUST be set to 1."



This sentence logically belongs in Section 3.2 (3.2.4.1), which has no 


instructions at all regarding the payload size field.






Noted that the description of the MPLS Payload Size field in Section 5.1 


has a full description, but says the DF bit SHOULD be set. Is it MUST or 


SHOULD, and if the latter, what is the exception?




2. Section 3.2.1, third paragraph, second sentence currently reads:

  "If the Proxy LSR is a budnode, a MPLS
   proxy ping reply SHOULD be sent to the initiator with the return code
   set to 3 (Reply router is Egress for FEC) with return Subcode set to
   0 and DS/DDMAPs only if the Proxy initiator requested information to
   be returned in a MPLS proxy ping reply."



Do you mean that the DS/DDMAP TLV is included only if the initiator 


asked for it (which seems like redundant advice) or do you mean that the 


reply is sent only if the initiator asked for the DS/DDMAP information?






3. Same paragraph, next sentence: why SHOULD rather than MUST? What is 


the exception?






4. Section 3.2.1, last paragraph (dealing with MP2MP case): the second 


to fourth sentences read:




"LSP pings sent from a leaf of a MP2MP has
   different behavior in this case. MPLS Echo Request are sent to all
   upstream/downstream neighbors. The Proxy LSRs need to be consistent
   with this variation in behavior."
s/has/have/ to get that out of the way.



Do you mean that the initiator is a leaf of the MP2MP? How does the 


Proxy LSR know this? Does it matter? If it is the Proxy LSR that is the 


leaf, the statement makes more sense. The different behavior is that the 


Proxy LSR always sends MPLS Echo Requests in at least one direction if 


it is not sending an MPLS proxy ping reply. You could refer to the 


previous paragraph for the remaining behavioural description.






The next sentence in this paragraph has the same problem as Minor Issue 


2. above, except that I see a stronger hint that the second 


interpretation suggested above is your intention.






5. You are creating a new IANA registry for sub-types of the Proxy Echo 


Parameters TLV. You need to document registration procedures as per RFC 


5226.





Nits/editorial comments:



General: Is there a reason for capitalizing "MPLS Echo Request" but not 


"MPLS proxy ping request[reply]"?






General: Sometimes return code text is enclosed in quotes, sometimes in 


parentheses. Choose one.




1. ABSTRACT third line: s/Routers/Router/

2. Section 2.1.1, first line: s/If/if/

3. Same section, second paragraph has both typos and editorial glitches.

OLD

   In the case the targeted proxy LSR does not understand LSP ping Echo
   Request at all, like any other LSR which do not understand the
   messages, they MUST be dropped and no messages is set back to the
   initiator.

NEW

   In the case where the targeted proxy LSR does not understand
   the LSP ping Echo Request at all, like any other LSR which does not
   understand the messages, it MUST drop them and MUST NOT(?) send any
   message back to the initiator. [Was the MUST NOT intended? -- PTT]

4. Section 3.1, last sentence: s/is//

5. Section 3.2, first paragraph, fourth line: s/set to/to/

6. Same section, fifth paragraph:
  s/Request messages/Request message/
  s/one is sent/either is sent/

7. Same section, sixth paragraph, end of second-last line: s/an//



8. Same section, seventh paragraph: invocation is something that happens 


after configuration. Maybe you want to say:




  "If a configured filter has been invoked and an address ..."

10. Same section, later paragraph as shown below (editorial suggestion):

OLD

   If the "Request for FEC Neighbor Address info" flag is set, a
   Upstream Neighbor Address TLV and/or Downstream Neighbor Address
   TLV(s) is/are formatted for inclusion in the MPLS proxy ping reply.
   If the Upstream or Downstream address is unknown they are not
   included in the Proxy Reply.

NEW

   If the "Request for FEC Neighbor Address info" flag is set, the
   Upstream Neighbor Address and Downstream Neighbor Address
   TLVs are formatted for inclusion in the MPLS proxy ping reply.
   If the Upstream or Downstream address is unknown the corresponding
   TLV is omitted.

11. Same section, paragraph re detailed downstream mapping:
    s/LSR/Proxy LSR/
    s/inclusions/inclusion/


You use the abbreviation DS/DDMAP in the following section. Perhaps, 


since you spell that out in this section, you might add "(DS/DDMAP)"



before "TLV".

12. Same section, next paragraph:
    s/vary/varies/
Capitalization is inconsistent: Proxy/proxy, egress/ Egress.



The last sentence is referring to Section 3.2.1. Suggest either copying 


that section's heading exactly or using the section number.




13. Section 3.2.1: inconsistent capitalization of "egress".

14. Section 3.2.2:

    s'DSMAP/DDMAP'DS/DDMAP' (3 instances) or spell out if this is a
    new abbreviation.



    In the first paragraph, mLDP is not a well-known abbreviation 


<

http://www.rfc-editor.org/rfc-style-guide/abbrev.expansion.txt

>, hence 


needs to be spelled out. ("Multipoint LDP", RFC 6826).




    In the second paragraph, fourth from last line: s/egresses/egress/

15. Section 3.2.4.1 first paragraph, second sentence:

OLD

   If Next Hop sub-TLVs were
   included in the received Proxy Parameters TLV, the Next_Hop_List
   created from the address in those sub-TLVs as adjusted above.

NEW

   If Next Hop sub-TLVs were
   included in the received Proxy Echo Parameters TLV, the Next_Hop_List
   is created from the addresses in those sub-TLVs adjusted as
   described in Section 3.2.

16. Section 5.1, first paragraph:
    s/to either to/either to/

    Why are "Composing" and "Sending" capitalized?

    Seventh line down, end of line, add comma after "stack".

    Next line: s/ie/i.e.,/

    Next line: s/Proxy Reply/MPLS Proxy Ping Reply/

17. Section 5.1, Proxy Flags field description, 0x01, first paragraph:
     Fourth line: s/FEC types/FEC type/
     Fifth line: s/LSPs/LSP/.
     Following sentence:

OLD
            The Proxy LSR MUST
            respond as applicable with a Upstream Neighbor Address TLV
            and Downstream Neighbor Address TLV(s) in the MPLS proxy
            ping reply message.

NEW
            The Proxy LSR MUST
            respond as applicable with Upstream Neighbor Address
            and Downstream Neighbor Address TLV(s) in the MPLS proxy
            ping reply message.

Following sentence: begin with "The".

Last sentence:

OLD
    for which the LSR learned bindings from.

NEW
    from which the LSR learned bindings.

17. Section 5.1, Proxy Flags field description, 0x01, second paragraph:
    s/an Echo request/any MPLS Echo Request/

    Second sentence:

OLD
    Information learned with such proxy reply may be used by the proxy
    initiator to generate subsequent proxy requests.

NEW
    The initiator may use information learned from the MPLS proxy
    ping reply that is sent instead to generate subsequent proxy
    requests.

[Also applies to the last sentence of the 0x04 description)



18. Section 5.1, Sub-TLVs: "TLV-encoded list" means to my mind that 


there is a TLV type and length preceding the first Sub-TLV. Maybe you 


mean "List of TLV-encoded sub-TLVs"?






19. Section 5.1.1, "Next Hop Interface": why is the interface identifier 


16 octets long in the case of IPv6 numbered? Should there be text saying 


that numbered interfaces are identified by the addresses assigned to 


them, unnumbered interfaces by ??




20. Section 6, second paragrph: refers to "these points". What points?

21. IANA Considerations



It would be helpful to indicate that the registries involved are found 


under the heading "Multi-Protocol Label Switching (MPLS) Label Switched 


Paths (LSPs) Ping Parameters".