Re: [Gen-art] Last Call Review: draft-ietf-mpls-proxy-lsp-ping-03

Thanks for your review, Tom, and for the updates, Adrian & the team. 

FWIW, I also could had trouble understanding how the text for items 2-4 should
be read.

Jari

On 05 Mar 2015, at 02:02, Tom Taylor <tom.taylor.stds@gmail.com> wrote:

> Thank you for your update (version -04).
> 
> I cannot see any changes in the -04 version responding to my minor issues 2-4. I gather the ambiguity is considered to be in my eyes only.
> 
> Tom Taylor
> 
> On 13/02/2015 1:19 AM, Tom Taylor 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 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".
>> 
> 
> _______________________________________________
> Gen-art mailing list
> Gen-art@ietf.org
> https://www.ietf.org/mailman/listinfo/gen-art

Re: [Gen-art] Last Call Review: draft-ietf-mpls-proxy-lsp-ping-03

Attachment: signature.asc