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 <[email protected]> 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 > [email protected] > https://www.ietf.org/mailman/listinfo/gen-art
signature.asc
Description: Message signed with OpenPGP using GPGMail
_______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
