Re: [mpls] FW: I-D Action: draft-ietf-mpls-lsp-ping-mpls-tp-oam-conf-07.txt

[Gregory Mirsky <[e-mail]>]

Hi Nobo,
many thanks for your thoughtful comments and am terribly sorry it took me so long to get back with resolution. Please find my notes in-lined and tagged by GIM>>.

        Regards,
                Greg

PS. Attached is work-in progress with updates based on your review.


-----Original Message-----
From: Nobo Akiya (nobo) [mailto:[e-mail]]
Sent: Sunday, October 12, 2014 2:53 PM
To: Gregory Mirsky; [e-mail]<mailto:[e-mail]>
Subject: RE: [mpls] FW: I-D Action: draft-ietf-mpls-lsp-ping-mpls-tp-oam-conf-07.txt

Hi Greg,

I have only read through half of the document, but please find below my comments.

== Comments ==

(1) In Section 2.1

   Typically intermediate nodes should not process or modify any of the
   OAM configuration TLVs but simply forward them to the end-node.

Should above "should not" be upper case "SHOULD NOT"?
GIM>> I think that "SHOULD NOT" is too restrictive particularly because the next sentence lists the exclusion.
I've re-phrased the sentence to:
      Typically intermediate nodes simply forward OAM configuration TLVs to the end-node without any processing or modification. At least one exception to this is if the FMS sub-TLV is present.
Would you agree?

(2) In Section 2.1.1, slight rephrase.

[OLD]
   For this specification, BFD MUST be run in either one of the two
   modes:

[NEW]
   For this specification, BFD MUST run in either one of the two
   modes:
GIM≫ done

(3) In Section 2.2, naming convention

   The MPLS OAM Functions TLV presented in Figure 1 is carried as a TLV
   of the LSP Echo request/response messages [RFC4379].

Proper name is MPLS echo request/reply as per described in Section 3 of RFC4379.
GIM≫ done (MPLS Echo Request/Reply)

(4) In Section 2.2, there's an inconsistency with flags

Figure 1 describes 5 flags (CVLDF) while Table 1 describes 6 flags (CVFLDT).
GIM>> Great catch, thank you. Fixed the Figure 1.

(5) In Section 2.2, BFD Configuration sub-TLV ...

      If the I flag is set, the "BFD
      Authentication sub-TLV" may be included.

The "may" should probably be "MAY" above.
GIM>> true, done

(6) In Section 2.2.1

   Length: indicates the length of the TLV including sub-TLVs but
   excluding the Type and Length field, in octets.

This description seems wrong. I think what you want to say is:

   Length: indicates the sub-TLV length in octets, excluding the sub-
   type and Length fields (4).
GIM>> thank you, great catch. Similar wording is several other places throughout the document. Fixed all to the following:
      Length - indicates the length of the Value field in octets

(7) In Section 2.2.1

   PHB: Identifies the Per-Hop Behavior (PHB) to be used for periodic
   continuity monitoring messages.

Perhaps I missed this, but I couldn't find what goes into the PHB bits (3 bits) and what behaviors are expected. Can you elaborate?
GIM>> PHB, in this case and in 2.2.1.  BFD Configuration sub-TLV, meant as QoS setting. At the time the document was started, if I’m not mistaken, we still referred to the three bit-long field as EXP, not as TC. Would changing PHB to TC help?

(8) In Section 2.2.1

   Integrity (I): If set BFD Authentication MUST be enabled.  If the BFD
   Configuration sub-TLV does not include a BFD Authentication sub-TLV
   the authentication MUST use Keyed SHA1 with an empty pre-shared key
   (all 0s).

What happens when receiver cannot support the requested authentication (including SHA1 w/ an empty key)?
GIM>> More thanks :)
Here’s new text I’ve added:
If the egress LSR does not support BFD Authentication an error MUST be generated: "OAM Problem/BFD Authentication Unsupported". (re-named to match error code name in RSVP-TE MPLS-TP OAM configuration doc.)
(Error codes listed in IANA Considerations sub-section 3.2.  OAM configuration errors)

(9) In Section 2.2.1

   Encapsulation Capability (G): if set, it shows the capability of
   encapsulating BFD messages into G-ACh channel.  If both the G bit and
   U bit are set, configuration gives precedence to the G bit.

   Encapsulation Capability (U): if set, it shows the capability of
   encapsulating BFD messages into IP/UDP packets.  If both the G bit
   and U bit are set, configuration gives precedence to the G bit.

If neither G nor U are set, then is that considered an error case?

GIM>> Strike! Here’s additional text:
If the egress LSR does not support any of the ingress LSR Encapsulation Capabilities an error MUST be generated: "OAM Problem/Unsupported BFD Encapsulation format".
And the new Error code to be added to the table for IANA to assign.

(10) In Section 2.2.1

   The BFD Configuration sub-TLV MUST include the following sub-TLVs in
   the LSP Echo reply message:

      - Local Discriminator sub-TLV;

Are we missing some error cases? For example, if Bidirectional(B) bit is not set, then there is no need for MPLS echo reply to contain "Local Discriminator sub-TLV", no?
GIM>> Changed to “MPLS Echo Reply” and “MPLS Echo Request” were appropriate.
Updated text now:
   The BFD Configuration sub-TLV MUST include the following sub-TLVs in
   the LSP Echo reply message:

      - Local Discriminator sub-TLV, if B flag is set in the MPLS Echo Request;

(11) In Section 2.2.1

   Version: identifies the BFD protocol version.  If a node does not
   support a specific BFD version an error must be generated: "OAM
   Problem/Unsupported OAM Version".

I'm assuming that this error code is TBD2. However, in Section 3.2 (IANA Section), the name is slightly different.

| TBD2  | MPLS OAM Unsupported Functionality        | This document |

Additionally, I didn't see any description on where/how other error codes (TBD3-TBD6) are to be used. I can guess, but perhaps it would be good idea to spell them out in relevant sections.
GIM>> I’ve changed the explanation of TBD2 error to “OAM Problem/Unsupported BFD Version”.
Error codes been mapped to appropriate sections and the table in IANA Considerations sections updated accordingly.

(12) In Section 2.2.4

    0                   1                   2                   3
    0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |    BFD Auth. sub-type (103)   |             Length            |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |   Auth Type   |  Auth Key ID  |         Reserved (0s)         |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

The value of this TLV mandates latter 16 bits to be all zero(0)'s. However, all authentication TLVs defined in RFC5880 and others (ex: draft-ietf-bfd-generic-crypto-auth) use the latter 16 bits (ex: Auth Key ID) but it's dependant on the Auth Type. Perhaps this sub-TLV can better be aligned. Something like:

0                   1                   2                   3
 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|    BFD Auth. sub-type (103)   |             Length            |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
|   Auth Type   |  Auth Key ID  |      Depends on Auth Type     |
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

GIM>> Configuration of BFD Authentication is currently out of scope of this document. The BFD Authentication sub-TLV can be used to coordinate Auth. Type and Auth. Key ID selection by BFD peers, assuming that their Authentication configured. Section 2.1.1:
      The BFD Authentication sub-TLV is used to specify which authentication method that should be used and which pre-shared key/ password that should be used for this particular session.  How the key exchange is performed is out of scope of this document.

== Typos ==

(1) Typo in Table of Contents (and Section 2.2.8 title)

s/2.2.8. Fault Managemet Signal sub-TLV/2.2.8. Fault Management Signal sub-TLV/
GIM>> done

(2) Typo in Section 1.1

s/MEP - Maintanence Entity Group End Point/MEP - Maintenance Entity Group End Point/
GIM>> done

(3) Typo in Section 2.2.4

s/exluding the Type/excluding the Type/
GIM>> done

(4) Typo in Section 2.2.5

s/exluding the Type/excluding the Type/
GIM>> done

(5) Typo in Section 3.1

s/allocation specied in that/allocation specified in that/
GIM>> done
(6) Typo in Section 3.1

s/IANA maintians a registry/IANA maintains a registry/
GIM>> done
(7) Typo in Section 3.1

s/IANA maintians a registry/IANA maintains a registry/
GIM>> done
Thanks!

- Nobo

> -----Original Message-----
> From: mpls [mailto:[e-mail]] On Behalf Of Gregory Mirsky
> Sent: Tuesday, September 30, 2014 2:54 PM
> To: [e-mail]<mailto:[e-mail]>
> Subject: [mpls] FW: I-D Action:
> draft-ietf-mpls-lsp-ping-mpls-tp-oam-conf-
> 07.txt
>
> Dear All,
> minor updates and bringing this work back on track.
> Your comments, questions and suggestions always welcome and greatly
> appreciated.
>
>       Regards,
>               Greg
>
> PS. RSVP-based companion doc been in the IESG queue been prepared for
> AD to review.
>
> -----Original Message-----
> From: mpls [mailto:[e-mail]] On Behalf Of internet-
> [e-mail]<mailto:[e-mail]>
> Sent: Tuesday, September 30, 2014 11:49 AM
> To: [e-mail]<mailto:[e-mail]>
> Cc: [e-mail]<mailto:[e-mail]>
> Subject: [mpls] I-D Action:
> draft-ietf-mpls-lsp-ping-mpls-tp-oam-conf-07.txt
>
>
> A New Internet-Draft is available from the on-line Internet-Drafts
> directories.
>  This draft is a work item of the Multiprotocol Label Switching
> Working Group of the IETF.
>
>         Title           : Configuration of Pro-Active Operations, Administration, and
> Maintenance (OAM) Functions for MPLS-based Transport Networks using
> LSP Ping
>         Authors         : Elisa Bellagamba
>                           Gregory Mirsky
>                           Loa Andersson
>                           Pontus Skoldstrom
>                           Dave Ward
>                           John Drake
>       Filename        : draft-ietf-mpls-lsp-ping-mpls-tp-oam-conf-07.txt
>       Pages           : 21
>       Date            : 2014-09-30
>
> Abstract:
>    This specification describes the configuration of pro-active MPLS-TP
>    Operations, Administration, and Maintenance (OAM) Functions for a
>    given LSP using a set of TLVs that are carried by the LSP-Ping
>    protocol.
>
>
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-mpls-lsp-ping-mpls-tp-oam-
> conf/
>
> There's also a htmlized version available at:
> http://tools.ietf.org/html/draft-ietf-mpls-lsp-ping-mpls-tp-oam-conf-0
> 7
>
> A diff from the previous version is available at:
> http://www.ietf.org/rfcdiff?url2=draft-ietf-mpls-lsp-ping-mpls-tp-oam-
> conf-07
>
>
> Please note that it may take a couple of minutes from the time of
> submission until the htmlized version and diff are available at tools.ietf.org.
>
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
>
> _______________________________________________
> mpls mailing list
> [e-mail]<mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/mpls
>
> _______________________________________________
> mpls mailing list
> [e-mail]<mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/mpls