Subject: [Gen-art] Gen-art LC review of draft-ietf-mpls-rfc4379bis-07

From: Elwyn Davies <[email-masked]>
Date: Fri, 21 Oct 2016 00:42:51 +0100

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

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

Document: draft-ietf-mpls-rfc4379bis-07.txt
Reviewer: Elwyn Davies
Review Date: 2016/10/21
IETF LC End Date: 2016/10/18
IESG Telechat date: (if known) -

Summary: Not ready.  There is one major issue (already notified to 
authors and agreed as an issue) and a considerable number of minor and 
editorial issues. I have worked through the various RFCs and errata that 
are subsumed into the new version and almost everything has been 
correctly translated AFAICS.  A couple of minor points are covered in 
the comments.

Major issues:
============
s3.4: A number of items that are used in the normative Downstream 
Detailed Mapping TLV were originally defined in s3.3 (Downstream Mapping 
TLV format) but have been shifted to Appendix A.2.  This appendix is 
marked as non-normative.  Thus there are now no normative definitions 
for the various TLVs used in s3.4 that are defined in A.2.  I fear that 
these need to be returned to the normative part of the specification.

[I think it would be simplest and least error prone to swap the text 
that was in s3.3 of RFC 4379 back out of A.2 and put backward references 
to the new s3.4 into A.2 as necessary.]

Minor issues:
============
Sender/receiver terminology: The document can be somewhat confusing to a 
naive reader.  Sender and receiver are used in multiple contexts.  Where 
the context appears to relate to LSP ping, both senders and receivers 
are involved in sending LSP ping packets.  In general, sender and 
receiver appear to imply sending and receiving of Echo Request messages 
with their roles reversed with respect to Echo Responses, with the 
receiver stimulated to send an Echo Response by receiving an Echo 
Request.  It would help if this terminology and usage was explicitly set 
out early in the document. Additionally, some instances would be made 
more comprehensible by making the function explicit in the text e.g., in 
the operation of return codes.

s1.4/s3/s6.2.3: The R (Global) flag is defined in RFC 6426. 
Unfortunately it isn't in the IANA considerations there as was spotted 
in RFC Erratum 4012.  Might be worth mentioning the erratum (probably in 
s1.4?)  Alternatively this document can be made to provide the IANA 
specification for the R flag and the erratum closed.

s2.1/s6: An update to 
http://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml 
is needed to replace RFC 4379 with RFC-to-be for special exceptions to 
usage rules.

s3.5, Clandestine Channel via Pad TLV:  As specified the value part of a 
Pad TLV can serve as a clandestine channel since the  value field 
contents are ignored.

s3.5, Usefulness of Pad TLV:  Could you explain circumstances in which a 
Pad TLV would be needed please. I can't see any at present.

Nits/editorial comments:
======================
General: s/i.e. /i.e., / (two instances  s3.2, last para; s4.5.1, para 3)

s1, para 1: s/methods of reliable reply/methods of providing reliable reply/

s1.4, bullet 4: Need to expand acronym PW on first use.

s1.4, bullet 4: need to move expansion of FEC acronym to here from s2.

s1.4, bullet 8: Acronyms DSMAP/DDMAP:  When defining Return Code 14 in 
s3.1, the text is 'See DDM TLV...'.  DDM is not expanded anywhere 
although it is clearly the same as DDMAP.  But has by now made it into 
the IANA repository and is probably better to use it for 'Downstream 
Detailed Mapping', so I suggest:
OLD:
    o  Incorporate the updates from RFC 6424, by deprecating the
       Downstream Mapping TLV (DSMAP) and adding the Downstream Detailed
       Mapping TLV (DDMAP), updating two new return codes, updating the
       procedures, IANA section, Security Considerations, and obsoleting
       RFC 6424.
NEW:
    o  Incorporate the updates from RFC 6424, by deprecating the
       Downstream Mapping TLV (DSM) and adding the Downstream Detailed
       Mapping TLV (DDM), adding two new return codes, updating the
       procedures, IANA section, Security Considerations, and obsoleting
       RFC 6424.
END
Then s/DSMAP/DSM/g, s/DDMAP/DDM/g in the rest of the document.

s1.4:  Ought to mention the addition of the motivation (LSP stitching) 
for the additions in RFC 6424.

s2.1, paras 7 and 8: This contains "the newly designated IPv4 link local 
addresses".  Given that RFC 3927 is now over 11 years old, the qualifier 
is no longer appropriate, but it might be useful to provide a ref. Thus:
OLD:
the newly designated IPv4 link local addresses
NEW:
the IPv4 link local addresses [RFC3927]
END
The text in para 8 is also no longer appropriate. Suggest
OLD:
    Furthermore, the IPv4 link local address range has only recently been
    allocated.  Many deployed routers would forward a packet with an
    address from that range toward the default route.
NEW:
    Older deployed routers may not correctly implement link local addresses
    and would forward a packet with an address from that range toward the
    default route.
END

s2.1, para 9: s/embedded in as/embedded in an/

s2.1, para 9: Useful to add a reference to RFC 4291.

s2.2, para 1:  To be clearer about the distinction between IPv4 and 
IPv6, suggest:
OLD:
    This document requires the use of the Router Alert Option (RAO) set
    in IP header in order to have the transit node process the MPLS OAM
    payload.
NEW:
    This document requires that the Router Alert Option (RAO) is carried
    in the IP header in order to have the transit node process the MPLS OAM
    payload.  For IPv4 packets the RAO [RFC2111] MUST be added to the IPv4
    header; for IPv6 packets a hop-by-hop RAO [RFC2711] must be chained to
    the IPv6 header.
END

s3, para 1:
> An MPLS echo request is a (possibly labeled) IPv4 or IPv6 UDP packet;
  This format applies to both requests and responses but the response 
case is not made explicit. Suggest:
OLD:
An MPLS echo request is a (possibly labeled) IPv4 or IPv6 UDP packet;
NEW:
An MPLS LSP ping message, is a (possibly labeled) IPv4 or IPv6 UDP packet;
END

s3, main packet format and associated text: The Sender's Handle is not 
the packet sender's handle but the Echo Request Sender's Handle - it is 
copied in to the corresponding Echo Reply.  Suggest renaming the 
Sender's Handle and Sequence Number to Echo Request Sender's Handle and 
Echo Request Sequence Number.  This would affect para 5 of s4.3, para 2 
of s4.5 and para 1 of s4.6 also.

s3, Timestamp format: RFC 5905 allows for 3 different time formats - the 
32 bit basic format is intended:
OLD:
    The TimeStamp Sent is the time-of-day (according to the sender's
    clock) in NTP format [RFC5905]
NEW:
    The TimeStamp Sent is the time-of-day (according to the sender's
    clock) in 32 bit NTP format [RFC5905]
END

s3, Global flags: Technically, this doc only defines the V flag: Also 
forcing the other bits to be zero restricts addition of new flags>
OLD:
    This document defines three flags, the R, T, and V bits; the rest
    MUST be set to zero when sending and ignored on receipt.
NEW:
    At the time of writing three flags are defined, the R, T, and V 
bits; the rest
    SHOULD be set to zero when sending and ignored on receipt.
END

s3, TLV types: The values 4, 6 and 8 for TLV type and the value 5 for 
Tthe sub-type of TLV type 1 are specified as 'Not assigned':  To be 
clear for the future, should these really be marked as 'Reserved' or 
could they be assigned in future (and hence s/b marked as 'Available for 
assignment')?

s3: For clarity it would be useful to add a sentence to the end of the 
section stating:
      In Sections 3.2 - 3.4 and their various sub-sections, only the 
value section of the TLV is specified.

s3, TLV length calculation:  This is shown by example only.  I think it 
ought to be explained explicitly in text.  I suggest:
     The length of a sub-TLV or a TLV whose value is not a list of sub-TLVs
     is the number of significant octets in the value part of the (sub-)TLV
     excluding any final padding.  If the value of a TLV is a list of 
sub-TLVs,
     the length of the TLV is the sum of the overall lengths of the 
sub-TLVs
     including the sub-TLV header and the length of the padding, i.e.
     4 + ((sub-TLV.length + 4) mod 4)

s3.1, para 1: I think this should be interpreted as saying that the 
Return Code MUST always be zero in an Echo Request and the Return Code 
is set to an appropriate one of the possible values in an Echo Reply.  
To be clear: I take it that it would not be normal for an Echo Reply to 
carry a zero Return Code.  Assuming this is right...
OLD:
    The Return Code is set to zero by the sender.  The receiver can set
    it to one of the values listed below.
NEW:
    The Return Code MUST be set to zero in an Echo Request message.
    The responder sets the Return Code in the Echo Reply message to
    an appropriate value other then zero from the list below.
END

s3.1, Return code 14:  Some of the extra text from Section 3.2.1 of RFC 
6424 ought to be essential as it contains 'MUSTS'.  Suggest adding this 
as an extra note against Return Code 14:
    Note 2:
       Return Code 14 is used to indicate that an Echo Reply contains 
one or more
       DDM TLVs (see Section 3.4).  In this case there will be one 
Return Code and
       corresponding <RSC> for each path described and these are passed 
in the
       DDM TLV(s).  This Return Code MUST only be used in the Echo Reply 
message
       header and MUST NOT be used in the Echo Request message even if 
the message
       contains a DDM TLV.

s3.1:  The term IS_EGRESS is used later in the document to indicate an 
Echo Reply message with a Return  Code of 3.  It should defined here.  
The meaning is fairly obvious at its first use in s3.4(e) but there is 
not a formal definition.  (AFAICS textual acronyms are not used for any 
of the other codes.)

s3.2, last but one para: s/previx/prefix/

s3.2.8/s3.2.9/s3.2.11: It would be useful to use the name of the FEC 
type from RFC 4447 (PWid FEC) rather than just its number. (Also in A.1.1).

s3.2.9: s/sender's PE IPv4 address/Sender's PE IPv4 Address/; s/remote 
PE IPv4 address/Remote PE IPv4 Address/

s3.2.9, para 3: Need to expand PE acronym on first use.

s3.2.10, para 1: The text uses source PE IPv4 address whereas the 
diagram uses Sender's PE IPv4 Address.  Consistency is needed.  See also 
the previous comment regarding consistency and capitalization.

s3.2.10/s3.2.12: : It would be useful to use the name of the FEC type 
from RFC 4447 (Generalized PWid FEC) rather than just its number.

s3.2.12: The text uses source whereas the diagram and field name use 
Sender's... consistency again?

s3.4, DS Flags:
>        I  Interface and Label Stack Object Request
>
>           When this flag is set, it indicates that the replying
>           router SHOULD include an Interface and Label Stack
>           Object in the echo reply message.
>
What circumstances would cause the replaying router not to do this? What 
should it do otherwise?

s3.4, Return Code:
>        The Return Code is set to zero by the sender.  The receiver can
>        set it to one of the values specified in the "Multi-Protocol Label
>        Switching (MPLS) Label Switched Paths (LSPs) Ping Parameters"
>        registry, "Return Codes" sub-registry.
a) I suspect that in the basic LSP ping described in this document, the 
return codes that ought to be available are only those specified in s3.1 
of this document except for 14 (which is specifically only allowed in 
the header).  The registry now contains a number of other return code 
values but a basic implementation wouldn't understand them in general.
b)  See the previous comments on meaning of sender and receiver. Suggest:
OLD:
       The Return Subcode is set to zero by the sender.  The receiver can
       set it to one of the values specified in the "Multi-Protocol Label
       Switching (MPLS) Label Switched Paths (LSPs) Ping Parameters"
       registry, "Return Codes" sub-registry.
NEW:
       The Return Code in the (one) DMM TLV in an Echo Request message
       MUST be set to zero. The responder sets the Return Code in any
       DMM TLV in the Echo Reply message to an appropriate value other
       then zero or 14  ("See DDM TLV for Return Code and Return Subcode")
       taken from the list in Section 3.1.
END

s3.4, Sub-tlv Length:  I think that the components of the DSM are all 
multiples of 4 octets long so there is no padding to consider (apart 
from possibly in FECs ).
OLD:
       Total length in bytes of the sub-TLVs associated with this TLV.
NEW:
       Total length in octets   of the sub-TLVs associated with this TLV 
including the TLV headers and any padding.
END

s3.4.1.3, FEC TLV length: Does this include any trailing padding and the 
TLV header?

s3.4.1.3, Operation Rules: Shouldn't these be in s4?

s3.6: Should contain a reference to the IANA registry URL.

s4.1, last para: s/some information how each/some information as to how 
each/

s4.2: s/to differentiate whether/to ascertain whether/

s4.3, para 1: s/MUST be set in IP header/MUST be set in appropriate IP 
options/

s4.4, item 1: It would be helpful to remind implementers how TLVs are 
marked to be ignored:
OLD:
If there are any TLVs not marked as "Ignore"
NEW:
If there are any TLVs not marked as "Ignore" (i.e., if the TLV type is 
less than 32768, see Section 3)
END

s4.4: s/subsection/Section/g

s4.4, item 3: s/If there is no entry for L {/If there is no entry for 
Label-L {/

s4.4, item 4:
OLD:
                Set Best-return-code to Return Code 9, "Label switched
                but no MPLS forwarding at stack-depth" and set Best-rtn-
                subcode to Label-stack-depth and goto Send_Reply_Packet.
NEW:
                Set Best-return-code to Return Code 9, "Label switched
                but no MPLS forwarding at stack-depth" and set Best-rtn-
                subcode to Label-stack-depth and goto step 7 (Send Reply 
Packet).
END

s4.4.1, item 5: s/advertise FEC/advertise the FEC/

s4.5:
>     If the replying router is the destination of the FEC, then Downstream
>     Detailed Mapping TLVs SHOULD NOT be included in the echo reply.
Under what circumstances  might one be included?  I think this is a MUST 
NOT.

s4.5.2:  This section is derived from s4.1.2 of RFC 6424.  Whilst the 
new version appears to contain sufficient to define the proper normative 
behaviour, RFC 6424 contains additional examples of usage.  These look 
useful to me. I wonder if it might be useful either to copy the 
illustrative material to an appendix or maybe point back to RFC 6424. I 
am not sure how the powers-that-be would consider back pointers to 
obsoleted documents!  Maybe something like:
    [RFC6242] which originally specified the techniques needed to 
support tunnel transition contains some
    examples, in Section 4.1.2, of situations where the technique would 
be applied.

s4.6:
    If the echo reply contains Downstream Detailed Mappings, and X wishes
    to traceroute further, it SHOULD copy the Downstream Detailed
    Mapping(s) into its next echo request(s) (with TTL incremented by
    one).
Presumably this means one DMM per Echo Request... might be worth being 
more explicit.

s5: Security risks of Router Alert.  Mention RFC 6398 and maybe copy 2nd 
para of s6 of RFC 7506.

s5, Security risks of DoS using Errored TLV?

s6: Given the responses from IANA, a note is needed to say that entries 
originated other than from RFC 4379 should remain unaltered in the 
registry.  The only exception might be the R flag in Global Flags where 
it might be sensible to use this document to fix erratum 4012.

s6.2.5, last line: Remove ']]' which appears to be spurious.

s8: Several new references are mentioned in these comments and would 
need to be added if the suggestions are actioned.

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Fri, 28 Oct 2016 01:29:52 +0000

Deal Elwyn,

Many thanks for a great review!

I just finished addressing all your comments: the major issue (easy to address, editorial fix, but with important implications), the minors, and all the nits. Surprisingly, I found a few small additional editorials, which I fixed as well.

Rev -08 would address all outstanding issues, from this review, Mirja, and a couple others.

Please see inline for a line-by-line set of responses.

On Oct 20, 2016, at 4:42 PM, Elwyn Davies <[email-masked]<mailto:[email-masked]>> wrote:

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

Summary: Not ready.  There is one major issue (already notified to authors and agreed as an issue) and a considerable number of minor and editorial issues. I have worked through the various RFCs and errata that are subsumed into the new version and almost everything has been correctly translated AFAICS.  A couple of minor points are covered in the comments.

Major issues:
============
s3.4: A number of items that are used in the normative Downstream Detailed Mapping TLV were originally defined in s3.3 (Downstream Mapping TLV format) but have been shifted to Appendix A.2.  This appendix is marked as non-normative.  Thus there are now no normative definitions for the various TLVs used in s3.4 that are defined in A.2.  I fear that these need to be returned to the normative part of the specification.

This is an excellent catch. Thank you. The fix is simple and purely editorial but the implication is clear.

I finished addressing this, which you will see posted as the new revision. I am super happy with the outcome.

[I think it would be simplest and least error prone to swap the text that was in s3.3 of RFC 4379 back out of A.2 and put backward references to the new s3.4 into A.2 as necessary.]

Minor issues:
============
Sender/receiver terminology: The document can be somewhat confusing to a naive reader.  Sender and receiver are used in multiple contexts.  Where the context appears to relate to LSP ping, both senders and receivers are involved in sending LSP ping packets.  In general, sender and receiver appear to imply sending and receiving of Echo Request messages with their roles reversed with respect to Echo Responses, with the receiver stimulated to send an Echo Response by receiving an Echo Request.  It would help if this terminology and usage was explicitly set out early in the document.  Additionally, some instances would be made more comprehensible by making the function explicit in the text e.g., in the operation of return codes.

Re-reading after fixing all the nits below, which include some sender clarifications, looks good.

s1.4/s3/s6.2.3: The R (Global) flag is defined in RFC 6426.  Unfortunately it isn't in the IANA considerations there as was spotted in RFC Erratum 4012.  Might be worth mentioning the erratum (probably in s1.4?)  Alternatively this document can be made to provide the IANA specification for the R flag and the erratum closed.

The WG decided to keep the definition of the R Flag in RFC 6426 and not here — consequently, there’s little that can really be done as the erratum (which really is symbolic since the IANA registry is fixed) applies to RFC 6426 and not to RFC 4379.

s2.1/s6: An update to http://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml is needed to replace RFC 4379 with RFC-to-be for special exceptions to usage rules.

Done.

s3.5, Clandestine Channel via Pad TLV:  As specified the value part of a Pad TLV can serve as a clandestine channel since the  value field contents are ignored.

Added the following to S5:
   The value part of the Pad TLV contains a variable number of octets.
   With the exception of the first octet, these contents, if any, are
   ignored on receipt, and can therefore serve as a clandestine channel.

s3.5, Usefulness of Pad TLV:  Could you explain circumstances in which a Pad TLV would be needed please. I can't see any at present.

Sure — when you want to send pings of various sizes for troubleshooting. I’ve used it in productions :-)

Nits/editorial comments:
======================

Thank for for these. Unless I make a specific follow-up inline, the nit is fixed.

s1.4, bullet 8: Acronyms DSMAP/DDMAP:  When defining Return Code 14 in s3.1, the text is 'See DDM TLV...'.  DDM is not expanded anywhere although it is clearly the same as DDMAP.  But has by now made it into the IANA repository and is probably better to use it for 'Downstream Detailed Mapping', so I suggest:
OLD:

This is a good point, where DDM came from RFC 6424. However, these fields are known as DSMAP and DDMAP.

Consequently, global replacing DDM -> DDMAP

s1.4:  Ought to mention the addition of the motivation (LSP stitching) for the additions in RFC 6424.

s2.1, paras 7 and 8: This contains "the newly designated IPv4 link local addresses".  Given that RFC 3927 is now over 11 years old, the qualifier is no longer appropriate, but it might be useful to provide a ref. Thus:
OLD:

Yes, many thanks. Updated with a slight change “Older deployed routers may not (correctly) implement IPv4 link local addresses …"

s2.2, para 1:  To be clearer about the distinction between IPv4 and IPv6, suggest:
OLD:

I wanted to keep that paragraph IP version agnostic, since the specifics for IPv4 and IPv6 come in the next two paragraphs.

s3, para 1:

 This format applies to both requests and responses but the response case is not made explicit. Suggest:
OLD:
An MPLS echo request is a (possibly labeled) IPv4 or IPv6 UDP packet;
NEW:
An MPLS LSP ping message, is a (possibly labeled) IPv4 or IPv6 UDP packet;
END

That would leave out “traceroute” mode. I’ll add “request/reply"

s3, main packet format and associated text: The Sender's Handle is not the packet sender's handle but the Echo Request Sender's Handle - it is copied in to the corresponding Echo Reply.  Suggest renaming the Sender's Handle and Sequence Number to Echo Request Sender's Handle and Echo Request Sequence Number.  This would affect para 5 of s4.3, para 2 of s4.5 and para 1 of s4.6 also.

That would be too big of a departure for very well-known fields.

s3, Timestamp format: RFC 5905 allows for 3 different time formats - the 32 bit basic format is intended:
OLD:
   The TimeStamp Sent is the time-of-day (according to the sender's
   clock) in NTP format [RFC5905]
NEW:
   The TimeStamp Sent is the time-of-day (according to the sender's
   clock) in 32 bit NTP format [RFC5905]
END

64-bit.

I changed to “64-bit NTP Timestamp format”.

s3, Global flags: Technically, this doc only defines the V flag:  Also forcing the other bits to be zero restricts addition of new flags>
OLD:
   This document defines three flags, the R, T, and V bits; the rest
   MUST be set to zero when sending and ignored on receipt.
NEW:
   At the time of writing three flags are defined, the R, T, and V bits; the rest
   SHOULD be set to zero when sending and ignored on receipt.
END

I changed the first part but leave in the MBZ.

s3, TLV types: The values 4, 6 and 8 for TLV type and the value 5 for Tthe sub-type of TLV type 1 are specified as 'Not assigned':  To be clear for the future, should these really be marked as 'Reserved' or could they be assigned in future (and hence s/b marked as 'Available for assignment')?

They are not assigned. IANA now calls these as Not Assigned as “Unassigned” — updated..

s3: For clarity it would be useful to add a sentence to the end of the section stating:
     In Sections 3.2 - 3.4 and their various sub-sections, only the value section of the TLV is specified.

Sure. But it’s really from 3.2 through 3.9.

As part of this, I also cleaned up all the “[sub][-]section” citations.

s3, TLV length calculation:  This is shown by example only.  I think it ought to be explained explicitly in text.  I suggest:
    The length of a sub-TLV or a TLV whose value is not a list of sub-TLVs
    is the number of significant octets in the value part of the (sub-)TLV
    excluding any final padding.  If the value of a TLV is a list of sub-TLVs,
    the length of the TLV is the sum of the overall lengths of the sub-TLVs
    including the sub-TLV header and the length of the padding, i.e.
    4 + ((sub-TLV.length + 4) mod 4)

The examples are clear enough and have been clear throughout many implementations.

s3.1, para 1: I think this should be interpreted as saying that the Return Code MUST always be zero in an Echo Request and the Return Code is set to an appropriate one of the possible values in an Echo Reply.  To be clear: I take it that it would not be normal for an Echo Reply to carry a zero Return Code.  Assuming this is right...
OLD:

Current text is OK.

s3.1, Return code 14:  Some of the extra text from Section 3.2.1 of RFC 6424 ought to be essential as it contains 'MUSTS'.  Suggest adding this as an extra note against Return Code 14:
   Note 2:
      Return Code 14 is used to indicate that an Echo Reply contains one or more
      DDM TLVs (see Section 3.4).  In this case there will be one Return Code and
      corresponding <RSC> for each path described and these are passed in the
      DDM TLV(s).  This Return Code MUST only be used in the Echo Reply message
      header and MUST NOT be used in the Echo Request message even if the message
      contains a DDM TLV.

Sure, added different text (above is incorrect), but this is a really good point. Updated also a section citation to point to this note.

s3.1:  The term IS_EGRESS is used later in the document to indicate an Echo Reply message with a Return  Code of 3.  It should defined here.  The meaning is fairly obvious at its first use in s3.4(e) but there is not a formal definition.  (AFAICS textual acronyms are not used for any of the other codes.)

s3.2.8/s3.2.9/s3.2.11: It would be useful to use the name of the FEC type from RFC 4447 (PWid FEC) rather than just its number. (Also in A.1.1).

The names are wildly used, and citations to 4447 exist. I’ll leave it as is.

s3.2.9: s/sender's PE IPv4 address/Sender's PE IPv4 Address/; s/remote PE IPv4 address/Remote PE IPv4 Address/

OK, same for the Appendix and IPv6 PE addresses.

s3.2.10, para 1: The text uses source PE IPv4 address whereas the diagram uses Sender's PE IPv4 Address.  Consistency is needed.  See also the previous comment regarding consistency and capitalization.

This is explained:
        Sender's Provider Edge (PE) IPv4 Address (the
          source address of the targeted LDP session),

s3.2.10/s3.2.12: : It would be useful to use the name of the FEC type from RFC 4447 (Generalized PWid FEC) rather than just its number.

s3.2.12: The text uses source whereas the diagram and field name use Sender's... consistency again?

s3.4, DS Flags:

What circumstances would cause the replaying router not to do this?  What should it do otherwise?

s3.4, Return Code:

a) I suspect that in the basic LSP ping described in this document, the return codes that ought to be available are only those specified in s3.1 of this document except for 14 (which is specifically only allowed in the header).  The registry now contains a number of other return code values but a basic implementation wouldn't understand them in general.

Similar issue with the Subcode (you are mixing RC with RSC in the OLD/NEW).

s3.4, Sub-tlv Length:  I think that the components of the DSM are all multiples of 4 octets long so there is no padding to consider (apart from possibly in FECs ).
OLD:
      Total length in bytes of the sub-TLVs associated with this TLV.
NEW:
      Total length in octets   of the sub-TLVs associated with this TLV including the TLV headers and any padding.
END

Leaving this does not hurt — however, fixed the bytes -> octets throughout.

s3.4.1.3, FEC TLV length: Does this include any trailing padding and the TLV header?

Sure, why not :-)

s4.1, last para: s/some information how each/some information as to how each/

s4.3, para 1: s/MUST be set in IP header/MUST be set in appropriate IP options/

s4.4, item 1: It would be helpful to remind implementers how TLVs are marked to be ignored:
OLD:
If there are any TLVs not marked as "Ignore"
NEW:
If there are any TLVs not marked as "Ignore" (i.e., if the TLV type is less than 32768, see Section 3)
END

s4.4, item 3: s/If there is no entry for L {/If there is no entry for Label-L {/

s4.4, item 4:
OLD:
               Set Best-return-code to Return Code 9, "Label switched
               but no MPLS forwarding at stack-depth" and set Best-rtn-
               subcode to Label-stack-depth and goto Send_Reply_Packet.
NEW:
               Set Best-return-code to Return Code 9, "Label switched
               but no MPLS forwarding at stack-depth" and set Best-rtn-
               subcode to Label-stack-depth and goto step 7 (Send Reply Packet).
END

s4.5:

Under what circumstances  might one be included?  I think this is a MUST NOT.

s4.5.2:  This section is derived from s4.1.2 of RFC 6424.  Whilst the new version appears to contain sufficient to define the proper normative behaviour, RFC 6424 contains additional examples of usage.  These look useful to me. I wonder if it might be useful either to copy the illustrative material to an appendix or maybe point back to RFC 6424. I am not sure how the powers-that-be would consider back pointers to obsoleted documents!  Maybe something like:
   [RFC6242] which originally specified the techniques needed to support tunnel transition contains some
   examples, in Section 4.1.2, of situations where the technique would be applied.

This was discussed and decided did not want to over copy when the current text is enough.

s4.6:
   If the echo reply contains Downstream Detailed Mappings, and X wishes
   to traceroute further, it SHOULD copy the Downstream Detailed
   Mapping(s) into its next echo request(s) (with TTL incremented by
   one).
Presumably this means one DMM per Echo Request... might be worth being more explicit.

s5: Security risks of Router Alert.  Mention RFC 6398 and maybe copy 2nd para of s6 of RFC 7506.

I believe the RA usage (which is specific and not generic) is adequately covered.

s6: Given the responses from IANA, a note is needed to say that entries originated other than from RFC 4379 should remain unaltered in the registry.  The only exception might be the R flag in Global Flags where it might be sensible to use this document to fix erratum 4012.

s8: Several new references are mentioned in these comments and would need to be added if the suggestions are actioned.

Very many thanks again for the review!

— Carlos.

---

From: Elwyn Davies <[email-masked]>
Date: Fri, 28 Oct 2016 23:29:30 +0100

Hi Carlos,

:-)
Thanks for addressing the comments.  I have looked through -08 and there 
are a couple of extra points that I noticed - the s3.4 issue was 
effectively mentioned wrt s4.5 in my previous notes.

Generally things are in good shape but there are some items that haven't 
been addressed or there is a quibble.

If there is another version over the weekend I'll do my very best to 
check it before Monday.

Regards,
Elwyn

Extra Points:
===========

I Forgot to mention that there is lack of consistency in capitalisation 
of the message names: Personally I would go with Echo Request and Echo 
Reply throughout to make it clear that these are message names.

s3,4, para 1:
> If the
>     replying router is the destination (Label Edge Router) of the FEC,
>     then a Downstream Detailed Mapping TLV SHOULD NOT be included in the
>     MPLS echo reply.  Otherwise, the replying router SHOULD include a
>     Downstream Detailed Mapping object for each interface over which this
>     FEC could be forwarded.
I suspect that the SHOULD NOT ought to be MUST NOT.  Otherwise it needs 
an explanation of the circumstances in which the DDMAP TLV could be 
included.  Similarly, the SHOULD needs to explain in what circumstances 
you wouldn't include one or more DDMAP TLVs.

s6.2.3: The Unassigned row should have a blank reference.

On 28/10/2016 02:29, Carlos Pignataro (cpignata) wrote:
> I just finished addressing all your comments: the major issue (easy to 
> address, editorial fix, but with important implications), the minors, 
> and all the nits. Surprisingly, I found a few small additional 
> editorials, which I fixed as well.
>
>> authors and agreed as an issue) and a considerable number of minor 
>> and editorial issues. I have worked through the various RFCs and 
>> errata that are subsumed into the new version and almost everything 
>> has been correctly translated AFAICS.  A couple of minor points are 
>> covered in the comments.
>>
>> Major issues:
>> ============
>> Detailed Mapping TLV were originally defined in s3.3 (Downstream 
>> Mapping TLV format) but have been shifted to Appendix A.2.  This 
>> appendix is marked as non-normative.  Thus there are now no normative 
>> definitions for the various TLVs used in s3.4 that are defined in 
>> A.2.  I fear that these need to be returned to the normative part of 
>> the specification.
>>
>
> I finished addressing this, which you will see posted as the new 
> revision. I am super happy with the outcome.
Looks good to me!
>> that was in s3.3 of RFC 4379 back out of A.2 and put backward 
>> references to the new s3.4 into A.2 as necessary.]
>>
>> Minor issues:
>> ============
>> Sender/receiver terminology: The document can be somewhat confusing 
>> to a naive reader.  Sender and receiver are used in multiple 
>> contexts.  Where the context appears to relate to LSP ping, both 
>> senders and receivers are involved in sending LSP ping packets.  In 
>> general, sender and receiver appear to imply sending and receiving of 
>> Echo Request messages with their roles reversed with respect to Echo 
>> Responses, with the receiver stimulated to send an Echo Response by 
>> receiving an Echo Request.  It would help if this terminology and 
>> usage was explicitly set out early in the document. Additionally, 
>> some instances would be made more comprehensible by making the 
>> function explicit in the text e.g., in the operation of return codes.
>
There is one place (s3.1, para 1) where I think it could be made 
clearer.  Adding a few words to that section will help overall as well 
as just in that section.
>> Unfortunately it isn't in the IANA considerations there as was 
>> spotted in RFC Erratum 4012.  Might be worth mentioning the erratum 
>> (probably in s1.4?)  Alternatively this document can be made to 
>> provide the IANA specification for the R flag and the erratum closed.
>
> The WG decided to keep the definition of the R Flag in RFC 6426 and 
> not here — consequently, there’s little that can really be done as the 
> erratum (which really is symbolic since the IANA registry is fixed) 
> applies to RFC 6426 and not to RFC 4379.
>
OK
>> is needed to replace RFC 4379 with RFC-to-be for special exceptions 
>> to usage rules.
>>
>
> Done.
>
>> s3.5, Clandestine Channel via Pad TLV:  As specified the value part 
>> of a Pad TLV can serve as a clandestine channel since the  value 
>> field contents are ignored.
>>
>
Fine.
>
>
>> s3.5, Usefulness of Pad TLV:  Could you explain circumstances in 
>> which a Pad TLV would be needed please. I can't see any at present.
>>
>
> Sure — when you want to send pings of various sizes for 
> troubleshooting. I’ve used it in productions :-)
I think a short note in s3.5 about why it might be present would be 
useful. e.g.,
        The Pad TLV can be added to an Echo Request to create a message 
of a specific length
        in cases where pings of various sizes are needed for 
troubleshooting.
> Thank for for these. Unless I make a specific follow-up inline, the 
There are a couple of places where this doesn't seem to be the case, 
especially in a number of places where there are SHOULDs in the text but 
the reasons for/consequences of not following the SHOULD are not spelt 
out.  This can be important for interoperability.
>> s1.4, bullet 8: Acronyms DSMAP/DDMAP:  When defining Return Code 14 
>> in s3.1, the text is 'See DDM TLV...'.  DDM is not expanded anywhere 
>> although it is clearly the same as DDMAP.  But has by now made it 
>> into the IANA repository and is probably better to use it for 
>> 'Downstream Detailed Mapping', so I suggest:
>> OLD:
That's OK by me.  I wondered whether the DDM in the return code had been 
too firmly ensconced to change.
>
>>
>> s1.4:  Ought to mention the addition of the motivation (LSP 
>> stitching) for the additions in RFC 6424.
>>
>> s2.1, paras 7 and 8: This contains "the newly designated IPv4 link 
>> local addresses".  Given that RFC 3927 is now over 11 years old, the 
>> qualifier is no longer appropriate, but it might be useful to provide 
>> a ref. Thus:
>> OLD:
> Yes, many thanks. Updated with a slight change “Older deployed routers 
> may not (correctly) implement IPv4 link local addresses …"
>
This didn't happen.  Not essential but useful.
> I wanted to keep that paragraph IP version agnostic, since the 
> specifics for IPv4 and IPv6 come in the next two paragraphs.
Fair enough.
> That would leave out “traceroute” mode. I’ll add “request/reply"
OK
>
>> s3, main packet format and associated text: The Sender's Handle is 
>> not the packet sender's handle but the Echo Request Sender's Handle - 
>> it is copied in to the corresponding Echo Reply.  Suggest renaming 
>> the Sender's Handle and Sequence Number to Echo Request Sender's 
>> Handle and Echo Request Sequence Number. This would affect para 5 of 
>> s4.3, para 2 of s4.5 and para 1 of s4.6 also.
>>
>
I just realized that the draft doesn't say that the Sequence Number is 
also (I assume) returned unchanged in the Echo Reply.  To emphasize this 
copying, it would probably be good to use the MUST word about both 
request -> reply copies.
> I changed to “64-bit NTP Timestamp format”.
Yes.. oops.
>
>> s3, Global flags: Technically, this doc only defines the V flag:  
>> Also forcing the other bits to be zero restricts addition of new flags>
>> OLD:
OK
>> could they be assigned in future (and hence s/b marked as 'Available 
>> for assignment')?
>>
>
Fine.  I was just wondering.
> Sure. But it’s really from 3.2 through 3.9.
True!
> As part of this, I also cleaned up all the “[sub][-]section” citations.
Good.
>
>>
>> s3, TLV length calculation:  This is shown by example only.  I think 
>> it ought to be explained explicitly in text.  I suggest:
The market may go down as well as up!  I'll have to live with this.
>> Return Code MUST always be zero in an Echo Request and the Return 
>> Code is set to an appropriate one of the possible values in an Echo 
>> Reply.  To be clear: I take it that it would not be normal for an 
>> Echo Reply to carry a zero Return Code.  Assuming this is right...
>> OLD:
Hmm.  I found this potentially confusing.  This was the main point at 
which I thought the use of sender and receiver needed clarifying.  Am I 
right in thinking that the 'sender' is always the node sending an Echo 
Request and the 'receiver' is a node that is triggered by the Echo 
Request to send an 'Echo Reply'.  The issue for me is that the receiver 
also sends messages.  How about:
OLD:
    The Return Code is set to zero by the sender.  The receiver can set
    it to one of the values listed below.
NEW:
    The Return Code is set to zero in the Echo Request message by the 
(Echo Request) sender.
    The ((Echo Request) receiver can set it to one of the values listed 
below in the corresponding
     Echo Reply that it generates.
END
It is possible that I am misinterpreting sender or receiver.. but if so 
I would say that some additional words would help.
>
>> s3.1, Return code 14:  Some of the extra text from Section 3.2.1 of 
>> RFC 6424 ought to be essential as it contains 'MUSTS'.  Suggest 
>> adding this as an extra note against Return Code 14:
>>    Note 2:
> Sure, added different text (above is incorrect), but this is a really 
> good point. Updated also a section citation to point to this note.
Thew new text is fine.
>
>>
>> s3.1:  The term IS_EGRESS is used later in the document to indicate 
>> an Echo Reply message with a Return  Code of 3.  It should defined 
>> here.  The meaning is fairly obvious at its first use in s3.4(e) but 
>> there is not a formal definition. (AFAICS textual acronyms are not 
>> used for any of the other codes.)
This didn't happen.

> The names are wildly used, and citations to 4447 exist. I’ll leave it 
Wild!
>
>> s3.2.9: s/sender's PE IPv4 address/Sender's PE IPv4 Address/; 
>> s/remote PE IPv4 address/Remote PE IPv4 Address/
>>
>
>> diagram uses Sender's PE IPv4 Address.  Consistency is needed.  See 
>> also the previous comment regarding consistency and capitalization.
>>
>
> This is explained:
OK.  I think I read the text incorrectly here.  On reflection it looks 
fine as is.
>> What should it do otherwise?
This hasn't been addressed AFAICS.
>> a) I suspect that in the basic LSP ping described in this document, 
>> the return codes that ought to be available are only those specified 
>> in s3.1 of this document except for 14 (which is specifically only 
>> allowed in the header).  The registry now contains a number of other 
>> return code values but a basic implementation wouldn't understand 
>> them in general.
Ah!  Actually the original text and what I wrote are both wrong! The 
text in -08 is arguably not wrong, but is confusing because it looks 
like the instructions for Return Code.
VERSION 08 TEXT:
       The Return Subcode is set to zero by the sender.  The receiver can
       set it to an appropriate value as specified in Section 3.1. This
       field is filled in with the stack-depth for those codes that
       specify the stack-depth.  For all other codes, the Return Subcode
       MUST be set to zero.
MORE NEWER:
       The Return Subcode is set to zero by the sender.  The receiver 
can [MUST?]
       fill this field with the stack-depth for those codes that
       specify the stack-depth as indicated in Section 3.1  For all 
other codes, the Return Subcode
       MUST be set to zero.

>>       Total length in octets   of the sub-TLVs associated with this 
>> TLV including the TLV headers and any padding.
>> END
>>
>
Octets: good.  I'll live with the rest.
>> s3.4.1.3, Operation Rules: Shouldn't these be in s4?
Thinking about this some more, I see these aren't operation rules in the 
same sense as s4 uses operation.  They are actually ordering rules:
OLD:
    FEC stack change sub-TLV operation rules are as follows:
NEW:
    When a DDMAP TLV needs several FEC stack change sub-TLVs to record 
the changes
    that the LSR makes to the label stack, the following number and 
ordering rules MUST
    be respected:
END

>> Under what circumstances  might one be included?  I think this is a 
See comment on s3.4 at head of message.
>> new version appears to contain sufficient to define the proper 
>> normative behaviour, RFC 6424 contains additional examples of usage. 
>> These look useful to me. I wonder if it might be useful either to 
>> copy the illustrative material to an appendix or maybe point back to 
>> RFC 6424. I am not sure how the powers-that-be would consider back 
>> pointers to obsoleted documents!  Maybe something like:
OK
> I believe the RA usage (which is specific and not generic) is 
I was thinking of whether (e.g.) having a RA option set in a packet 
other than a MPLS Echo Request/Response could be a way of doing a DoS 
attack on an MPLS network since it could overload the data->control 
plane link... might wish to filter any packets with RA set at ingress to 
MPLS network.  Not sure if the UDP port rate limit would help with this.
>> s5, Security risks of DoS using Errored TLV?
Injecting MPLS Ping packets with bad TLVs could be a way of creating a 
DoS perhaps as it would produce Errored TLV messages.
>>
>> s6: Given the responses from IANA, a note is needed to say that 
>> entries originated other than from RFC 4379 should remain unaltered 
>> in the registry.  The only exception might be the R flag in Global 
>> Flags where it might be sensible to use this document to fix erratum 
>> 4012.
>>

---

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Sat, 29 Oct 2016 03:26:50 +0000

Hi Elwyn,

Thanks! Please see inline.

Hi Jari, Deborah,

All major, minor, and nits/editorials (including these extra additional points) are now addressed.

On Oct 28, 2016, at 6:29 PM, Elwyn Davies <[email-masked]<mailto:[email-masked]>> wrote:

Hi Carlos,

:-)
Thanks for addressing the comments.  I have looked through -08 and there are a couple of extra points that I noticed - the s3.4 issue was effectively mentioned wrt s4.5 in my previous notes.

Anytime!

We addressed the major and the two minor issues, as that was the priority and what Jari recorded in the datatracker. We also did address the very vast majority of nits and editorials (a couple escaped), including some additional ones I found in editing.

Generally things are in good shape but there are some items that haven't been addressed or there is a quibble.

All addressed now — though “addressing” resulted in a no-change disposition in a couple instances.

Quibble fixed! :-)

If there is another version over the weekend I'll do my very best to check it before Monday.

I submitted a new revision -09, but *please* do not waste your precious weekend time on this. All issues are addressed, but if there is something we missed, it is within the mini-nit category. I don’t believe there’s any (naturally) but if there were, it can be addressed by the RFC Editor.

Enjoy your weekend!

More inline.

Regards,
Elwyn

Extra Points:
===========

I Forgot to mention that there is lack of consistency in capitalisation of the message names: Personally I would go with Echo Request and Echo Reply throughout to make it clear that these are message names.

We’ll leave this capitalization one for the RFC Editor.

s3,4, para 1:

If the

I suspect that the SHOULD NOT ought to be MUST NOT.  Otherwise it needs an explanation of the circumstances in which the DDMAP TLV could be included.  Similarly, the SHOULD needs to explain in what circumstances you wouldn't include one or more DDMAP TLVs.

If you ask me, personally, I’d say: use MUST / MUST NOT as sparingly as possible, only when interop is affected. Additionally, not every SHOULD / SHOULD NOT needs to explain the exceptional case.

It is perfectly OK to say that the SHOULD (NOT) is to cover for unforeseen circumstances, for future ideas, etc.

Consequently, no change regarding this.

No. Because “Unassigned” comes with an Assignment criteria which is given in this document. Left as-is, as with all other Unassigned.

Major issues:
============

Minor issues:
============

Re-reading after fixing all the nits below, which include some sender clarifications, looks good.
There is one place (s3.1, para 1) where I think it could be made clearer.  Adding a few words to that section will help overall as well as just in that section.

I’ll add a very small change in S3.1.

OK

Done.

Sure — when you want to send pings of various sizes for troubleshooting. I’ve used it in productions :-)
I think a short note in s3.5 about why it might be present would be useful. e.g.,
       The Pad TLV can be added to an Echo Request to create a message of a specific length
       in cases where pings of various sizes are needed for troubleshooting.

Sure, it does not hurt. We are rapidly approaching diminishing returns though.

This is what I added:
“  The Pad TLV can be added to an Echo Request to create a message of a specific length
   in cases where messages of various sizes are needed for troubleshooting. The first octet
   allows for controlling the inclusion of this additional padding in the respective Echo Reply.
"

Nits/editorial comments:
======================

Thank for for these. Unless I make a specific follow-up inline, the nit is fixed.
There are a couple of places where this doesn't seem to be the case, especially in a number of places where there are SHOULDs in the text but the reasons for/consequences of not following the SHOULD are not spelt out.  This can be important for interoperability.

See above. There is no need to explain every reason for every SHOULD. We did look at each on of these and edited the ones where we saw value in doing so. Most of these SHOULDs are because a MUST is too terminal of a requirement, and we want to future-proof the protocol.

This nit has a closed disposition as “no further change", with thanks.

Consequently, global replacing DDM -> DDMAP
That's OK by me.  I wondered whether the DDM in the return code had been too firmly ensconced to change.

Sure — the industry well-known used terms are DDMAP and DSMAP.

Chose not to. Sorry I missed saying so before.

s3, para 1:

That would be too big of a departure for very well-known fields.
I just realized that the draft doesn't say that the Sequence Number is also (I assume) returned unchanged in the Echo Reply.  To emphasize this copying, it would probably be good to use the MUST word about both request -> reply copies.

The current text looks good and is proven to interoperate well. I understand you like “MUSTs” though :-)

64-bit.

Current text is OK.
Hmm.  I found this potentially confusing.  This was the main point at which I thought the use of sender and receiver needed clarifying.  Am I right in thinking that the 'sender' is always the node sending an Echo Request and the 'receiver' is a node that is triggered by the Echo Request to send an 'Echo Reply'.  The issue for me is that the receiver also sends messages.  How about:
OLD:
   The Return Code is set to zero by the sender.  The receiver can set
   it to one of the values listed below.
NEW:
   The Return Code is set to zero in the Echo Request message by the (Echo Request) sender.
   The ((Echo Request) receiver can set it to one of the values listed below in the corresponding
    Echo Reply that it generates.
END
It is possible that I am misinterpreting sender or receiver.. but if so I would say that some additional words would help.

Sure. You got it right. I’ll update as per above too (to be consistent with the way I updated the other paragraph).

Decided to leave as-is.

:-)

You can search the pwe3 / pals archives to see that “FEC 128” is even more used that “FEC PW Id”.

This is explained:
        Sender's Provider Edge (PE) IPv4 Address (the
          source address of the targeted LDP session),
OK.  I think I read the text incorrectly here.  On reflection it looks fine as is.

OK.

s3.4, DS Flags:

Explained above.

Thanks for raising this nit — disposition: text unchanged.

s3.4, Return Code:

Similar issue with the Subcode (you are mixing RC with RSC in the OLD/NEW).
Ah!  Actually the original text and what I wrote are both wrong!  The text in -08 is arguably not wrong, but is confusing because it looks like the instructions for Return Code.
VERSION 08 TEXT:
      The Return Subcode is set to zero by the sender.  The receiver can
      set it to an appropriate value as specified in Section 3.1.  This
      field is filled in with the stack-depth for those codes that
      specify the stack-depth.  For all other codes, the Return Subcode
      MUST be set to zero.
MORE NEWER:
      The Return Subcode is set to zero by the sender.  The receiver can [MUST?]
      fill this field with the stack-depth for those codes that
      specify the stack-depth as indicated in Section 3.1  For all other codes, the Return Subcode
      MUST be set to zero.

I don’t mind addressing this — but the existing text is perfectly OK, and the distinction between NEW and NEWER a bit too subtle.

In fact, as I read NEWER, it is confusing as it can imply that S3.1 specification applies to the first and not last sentence. I think adding a “:” will clarify.

We are really in RFC Editor land and diminished returns edits realm.

EVEN NEWER:
                      <t>The Return Subcode is set to zero by the sender.  The receiver can
                      set this field to an appropriate value as specified in <xref target="rc" />:
                      The Return Subcode is filled in
                      with the stack-depth for those codes that specify the stack-depth.
                      For all other codes, the Return Subcode MUST be set to zero.
                      </t>

s3.4.1.3, Operation Rules: Shouldn't these be in s4?
Thinking about this some more, I see these aren't operation rules in the same sense as s4 uses operation.  They are actually ordering rules:
OLD:
   FEC stack change sub-TLV operation rules are as follows:
NEW:
   When a DDMAP TLV needs several FEC stack change sub-TLVs to record the changes
   that the LSR makes to the label stack, the following number and  ordering rules MUST
   be respected:
END

The current text is correct. These are more than ordering rules. I’d rather not use that additional MUST there again :-)

Sure, why not :-)

s4.4, item 4:
OLD:

s4.5:

Same response.

s4.6:

I believe the RA usage (which is specific and not generic) is adequately covered.
I was thinking of whether (e.g.) having a RA option set in a packet other than a MPLS Echo Request/Response could be a way of doing a DoS attack on an MPLS network since it could overload the data->control plane link... might wish to filter any packets with RA set at ingress to MPLS network.  Not sure if the UDP port rate limit would help with this.

This document does not specify what happens when MPLS Echo Request/Reply are not used :-)

s5, Security risks of DoS using Errored TLV?
Injecting MPLS Ping packets with bad TLVs could be a way of creating a DoS perhaps as it would produce Errored TLV messages.

The security considerations are generic enough, without describing each potential error that can be purposefully introduced.

— Carlos.

Thanks,

Carlos.

---

From: Elwyn Davies <[email-masked]>
Date: Mon, 31 Oct 2016 00:09:59 +0000

Hi Carlos.

So I think we are done here. Thanks for doing the last couple of changes.

I won't press you any further on the SHOULD front if everybody else is 
happy.

As a matter of general principle, I (and my fellow gen-arters) tend to 
be fairly heavy on SHOULDs and MAYs.  Personally, I always ask myself 
several questions when I see a SHOULD (NOT) or MAY (NOT) (thinking 
primarily as a potential implementer):
- How do I test if the code ought to take the path other then specified 
in the SHOULD/MAY?
- If I do take the decision to go the other way, is it consistent with 
other things being done in the same message?
- Will I be able to correctly process the message at the receiver?
- Can I make an unambiguous deduction about the reason that the other 
path was taken when the message is received at the other end?

Clearly the code at the receiving end has to be able to accept both 
cases for interoperability, but the implementation needs to understand 
what meaning is associated with each of the cases.  I am uneasy about 
saying that a relaxed approach to SHOULDs etc. might cover 'unforeseen 
circumstances' because the receiving end may not understand what is 
being said since the circumstances were unforeseen.. that way lie hard 
to diagnose bugs I fear.
But If all parties are prepared to go with -09, then so be it.

Best wishes
Elwyn

BTW My 'weekends' are rather less well defined these days, being mostly 
retired.  Time to do what I(we) want occurs when I make it/want it!  But 
thanks for thinking of the precious weekend.  Oh, and many years ago, I 
had a job where the weekends were Thursday and Friday - had its 
advantages.  OTOH most Saturdays finished about 3am on Sunday. ;-)

/E

On 29/10/2016 04:26, Carlos Pignataro (cpignata) wrote:
>> Thanks for addressing the comments.  I have looked through -08 and 
>> there are a couple of extra points that I noticed - the s3.4 issue 
>> was effectively mentioned wrt s4.5 in my previous notes.
>>
>
> Anytime!
>
> We addressed the major and the two minor issues, as that was the 
> priority and what Jari recorded in the datatracker. We also did 
> address the very vast majority of nits and editorials (a couple 
> escaped), including some additional ones I found in editing.
>
>> Generally things are in good shape but there are some items that 
>> haven't been addressed or there is a quibble.
>>
>
> I submitted a new revision -09, but *please* do not waste your 
> precious weekend time on this. All issues are addressed, but if there 
> is something we missed, it is within the mini-nit category. I don’t 
> believe there’s any (naturally) but if there were, it can be addressed 
> by the RFC Editor.
>
> Enjoy your weekend!
>
> More inline.
>
>> Regards,
>> Elwyn
>>
>> Extra Points:
>> ===========
>>
>> I Forgot to mention that there is lack of consistency in 
>> capitalisation of the message names: Personally I would go with Echo 
>> Request and Echo Reply throughout to make it clear that these are 
>> message names.
>>
>
>> I suspect that the SHOULD NOT ought to be MUST NOT. Otherwise it 
>> needs an explanation of the circumstances in which the DDMAP TLV 
>> could be included.  Similarly, the SHOULD needs to explain in what 
>> circumstances you wouldn't include one or more DDMAP TLVs.
>
> If you ask me, personally, I’d say: use MUST / MUST NOT as sparingly 
> as possible, only when interop is affected. Additionally, not every 
> SHOULD / SHOULD NOT needs to explain the exceptional case.
>
> It is perfectly OK to say that the SHOULD (NOT) is to cover for 
> unforeseen circumstances, for future ideas, etc.
>
> No. Because “Unassigned” comes with an Assignment criteria which is 
> given in this document. Left as-is, as with all other Unassigned.
>
>>> I just finished addressing all your comments: the major issue (easy 
>>> to address, editorial fix, but with important implications), the 
>>> minors, and all the nits. Surprisingly, I found a few small 
>>> additional editorials, which I fixed as well.
>>>
>>>> appendix is marked as non-normative.  Thus there are now no 
>>>> normative definitions for the various TLVs used in s3.4 that are 
>>>> defined in A.2.  I fear that these need to be returned to the 
>>>> normative part of the specification.
>>>>
>>>
>>>> [I think it would be simplest and least error prone to swap the 
>>>> text that was in s3.3 of RFC 4379 back out of A.2 and put backward 
>>>> general, sender and receiver appear to imply sending and receiving 
>>>> of Echo Request messages with their roles reversed with respect to 
>>>> Echo Responses, with the receiver stimulated to send an Echo 
>>>> Response by receiving an Echo Request.  It would help if this 
>>>> terminology and usage was explicitly set out early in the 
>>>> document.  Additionally, some instances would be made more 
>>>> comprehensible by making the function explicit in the text e.g., in 
>> clearer.  Adding a few words to that section will help overall as 
>> well as just in that section.
>
>>> not here — consequently, there’s little that can really be done as 
>>> the erratum (which really is symbolic since the IANA registry is 
>>> fixed) applies to RFC 6426 and not to RFC 4379.
>>>
>> OK
>>>>
>>>> s2.1/s6: An update to 
>> especially in a number of places where there are SHOULDs in the text 
>> but the reasons for/consequences of not following the SHOULD are not 
>> spelt out.  This can be important for interoperability.
>
> See above. There is no need to explain every reason for every SHOULD. 
> We did look at each on of these and edited the ones where we saw value 
> in doing so. Most of these SHOULDs are because a MUST is too terminal 
> of a requirement, and we want to future-proof the protocol.
>
>>>> in s3.1, the text is 'See DDM TLV...'.  DDM is not expanded 
>>>> anywhere although it is clearly the same as DDMAP.  But has by now 
>>>> made it into the IANA repository and is probably better to use it 
>>>> for 'Downstream Detailed Mapping', so I suggest:
>>>> OLD:
>>>> local addresses".  Given that RFC 3927 is now over 11 years old, 
>>>> the qualifier is no longer appropriate, but it might be useful to 
>>>> provide a ref. Thus:
>>>> OLD:
>>> Yes, many thanks. Updated with a slight change “Older deployed 
>>> routers may not (correctly) implement IPv4 link local addresses …"
>>>
>>>>  This format applies to both requests and responses but the 
>>>> response case is not made explicit. Suggest:
>>>> OLD:
>>>> not the packet sender's handle but the Echo Request Sender's Handle 
>>>> - it is copied in to the corresponding Echo Reply.  Suggest 
>>>> renaming the Sender's Handle and Sequence Number to Echo Request 
>>>> Sender's Handle and Echo Request Sequence Number.  This would 
>>>> affect para 5 of s4.3, para 2 of s4.5 and para 1 of s4.6 also.
>>>>
>>>
>> I just realized that the draft doesn't say that the Sequence Number 
>> is also (I assume) returned unchanged in the Echo Reply.  To 
>> emphasize this copying, it would probably be good to use the MUST 
>> word about both request -> reply copies.
>
>>>> s3, TLV types: The values 4, 6 and 8 for TLV type and the value 5 
>>>> for Tthe sub-type of TLV type 1 are specified as 'Not assigned':  
>>>> To be clear for the future, should these really be marked as 
>>>> 'Reserved' or could they be assigned in future (and hence s/b 
>>>> marked as 'Available for assignment')?
>>>>
>>>
>>>> s3, TLV length calculation:  This is shown by example only.  I 
>>>> think it ought to be explained explicitly in text.  I suggest:
>> which I thought the use of sender and receiver needed clarifying.  Am 
>> I right in thinking that the 'sender' is always the node sending an 
>> Echo Request and the 'receiver' is a node that is triggered by the 
>> Echo Request to send an 'Echo Reply'.  The issue for me is that the 
>> receiver also sends messages.  How about:
>> OLD:
>> It is possible that I am misinterpreting sender or receiver.. but if 
>> so I would say that some additional words would help.
>
> Sure. You got it right. I’ll update as per above too (to be consistent 
> with the way I updated the other paragraph).
>
>>>
>>> Sure, added different text (above is incorrect), but this is a 
>>> really good point. Updated also a section citation to point to this 
>>> note.
>> Thew new text is fine.
>>>
>>>>
>>>> here.  The meaning is fairly obvious at its first use in s3.4(e) 
>>>> but there is not a formal definition.  (AFAICS textual acronyms are 
>>>> not used for any of the other codes.)
>> This didn't happen.
>
> Decided to leave as-is.
>
>>
>>>>
>>>> s3.2.8/s3.2.9/s3.2.11: It would be useful to use the name of the 
>>>> FEC type from RFC 4447 (PWid FEC) rather than just its number. 
>>>> (Also in A.1.1).
>>>>
>>>
>>>> s3.2.10/s3.2.12: : It would be useful to use the name of the FEC 
>>>> type from RFC 4447 (Generalized PWid FEC) rather than just its number.
>>>>
>>>> the return codes that ought to be available are only those 
>>>> specified in s3.1 of this document except for 14 (which is 
>>>> specifically only allowed in the header).  The registry now 
>>>> contains a number of other return code values but a basic 
>>>> implementation wouldn't understand them in general.
> I don’t mind addressing this — but the existing text is perfectly OK, 
> and the distinction between NEW and NEWER a bit too subtle.
>
> In fact, as I read NEWER, it is confusing as it can imply that S3.1 
> specification applies to the first and not last sentence. I think 
> adding a “:” will clarify.
>
>>>> s3.4, Sub-tlv Length:  I think that the components of the DSM are 
>>>> all multiples of 4 octets long so there is no padding to consider 
>>>> (apart from possibly in FECs ).
>>>> OLD:
>> Thinking about this some more, I see these aren't operation rules in 
>> the same sense as s4 uses operation.  They are actually ordering rules:
>> OLD:
> The current text is correct. These are more than ordering rules. I’d 
> rather not use that additional MUST there again :-)
>
>>>>
>>>> s4.5.2:  This section is derived from s4.1.2 of RFC 6424.  Whilst 
>>>> the new version appears to contain sufficient to define the proper 
>>>> normative behaviour, RFC 6424 contains additional examples of 
>>>> usage.  These look useful to me. I wonder if it might be useful 
>>>> either to copy the illustrative material to an appendix or maybe 
>>>> point back to RFC 6424. I am not sure how the powers-that-be would 
>>>> consider back pointers to obsoleted documents!  Maybe something like:
>> plane link... might wish to filter any packets with RA set at ingress 
>> to MPLS network. Not sure if the UDP port rate limit would help with 
>> this.
>
>> Injecting MPLS Ping packets with bad TLVs could be a way of creating 
>> a DoS perhaps as it would produce Errored TLV messages.
>
> The security considerations are generic enough, without describing 
> each potential error that can be purposefully introduced.
>
>>>>
>>>> Flags where it might be sensible to use this document to fix 
>>>> erratum 4012.
>>>>
>>>> s8: Several new references are mentioned in these comments and 
>>>> would need to be added if the suggestions are actioned.
>>>
>>>

---

From: "BRUNGARD, DEBORAH A" <[email-masked]>
Date: Mon, 31 Oct 2016 17:07:23 +0000

Hi Elwyn,

Much thanks for all you help with this document!
All the best-
Deborah

From: Elwyn Davies [mailto:[email-masked]]
Sent: Sunday, October 30, 2016 8:10 PM
To: Carlos Pignataro (cpignata) <[email-masked]>
Cc: General area reviewing team <[email-masked]>; [email-masked]; BRUNGARD, DEBORAH A <[email-masked]>
Subject: Re: Post-Telechat update (was Re: Gen-art LC review of draft-ietf-mpls-rfc4379bis-07)

Hi Carlos.

I won't press you any further on the SHOULD front if everybody else is happy.

As a matter of general principle, I (and my fellow gen-arters) tend to be fairly heavy on SHOULDs and MAYs.  Personally, I always ask myself several questions when I see a SHOULD (NOT) or MAY (NOT) (thinking primarily as a potential implementer):
- How do I test if the code ought to take the path other then specified in the SHOULD/MAY?
- If I do take the decision to go the other way, is it consistent with other things being done in the same message?
- Will I be able to correctly process the message at the receiver?
- Can I make an unambiguous deduction about the reason that the other path was taken when the message is received at the other end?

Clearly the code at the receiving end has to be able to accept both cases for interoperability, but the implementation needs to understand what meaning is associated with each of the cases.  I am uneasy about saying that a relaxed approach to SHOULDs etc. might cover 'unforeseen circumstances' because the receiving end may not understand what is being said since the circumstances were unforeseen.. that way lie hard to diagnose bugs I fear.
But If all parties are prepared to go with -09, then so be it.

Best wishes
Elwyn

BTW My 'weekends' are rather less well defined these days, being mostly retired.  Time to do what I(we) want occurs when I make it/want it!  But thanks for thinking of the precious weekend.  Oh, and many years ago, I had a job where the weekends were Thursday and Friday - had its advantages.  OTOH most Saturdays finished about 3am on Sunday. ;-)

/E

Hi Jari, Deborah,

Hi Carlos,

:-)

Anytime!

Quibble fixed! :-)

Enjoy your weekend!

More inline.

Regards,
Elwyn

Extra Points:
===========

s3,4, para 1:

If the

   FEC could be forwarded.

Deal Elwyn,

Major issues:
============

Minor issues:
============

OK

Done.

This is what I added:

Nits/editorial comments:
======================

s3, para 1:

64-bit.

Current text is OK.

Decided to leave as-is.

:-)

This is explained:

OK.

s3.4, DS Flags:

Explained above.

s3.4, Return Code:

EVEN NEWER:

Sure, why not :-)

s4.4, item 4:
OLD:

s4.5:

Same response.

s4.6:

— Carlos.

Thanks,

Carlos.