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
