Gen-ART Review of draft-ietf-mpls-ldp-capabilities-02

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-mpls-ldp-capabilities-02
Reviewer: Spencer Dawkins
Review Date: 2008-04-30
IETF LC End Date: 2008-04-25 (yeah, I'm late)
IESG Telechat date: (if known)

Summary:

This draft is almost ready for publication as a Proposed Standard.

Comments:

Almost all my "review comments" below are about 2119 language - this is a 
very SHOULD-y draft. I'd especially call Russ's attention to my comment 
about multiple occurrences of the same capability in one message, in Section 
7.

idnits 2.08.08 reports "no issues", FWIW.

Abstract

   A number of enhancements to the Label Distribution Protocol (LDP)
   have been proposed.  Some have been implemented, and some are
   advancing toward standardization.  It is likely that additional
   enhancements will be proposed in the future.  At present LDP has no
   guidelines for advertising such enhancements at LDP session
   initialization time.  There is also no mechanism to enable and
   disable enhancements after the session is established.  This document
   provides guidelines for advertising LDP enhancements at session
   initialization time.  It also defines a mechanism to enable and
   disable enhancements after LDP session establishment.

Spencer (clarity): here and in the Introduction, the draft says "guidelines 
at session initialization time, mechanism to enable/disable after session 
establishment". Aren't both of these mechanisms? s/provides 
guidelines/defines a mechanism/?

1. Introduction

   A number of enhancements to LDP as specified in [RFC5036] have been
   proposed.  These include LDP Graceful Restart [RFC3478], Fault
   Tolerant LDP [RFC3479], multicast extensions [MLDP], signaling for
   layer 2 circuits [RFC4447], a method for learning labels advertised
   by next next hop routers in support of fast reroute node protection
   [NNHOP], upstream label allocation [UPSTREAM_LDP], and extensions for
   signaling inter-area LSPs [IALDP].  Some have been implemented, and
   some are advancing toward standardization.  It is likely that
   additional enhancements will be proposed in the future.

Spencer (clarity): I think I understand what all the words mean, but should 
this be "additional enhancements will be implemented", or even "deployed"?

   At present LDP has no guidelines for advertising such enhancements at
   LDP session initialization time.  There is also no mechanism to
   enable and disable enhancements after the session is established.

   This document provides guidelines for advertising LDP enhancements at
   session initialization time.  It also defines a mechanism to enable
   and disable enhancements after LDP session establishment.

   LDP capability advertisement provides means for an LDP speaker to
   announce what it can receive and process.  It also provides means for
   a speaker to inform peers of deviations from behavior specified by

Spencer (clarity): are "a speaker" and "an LDP speaker" interchangeable in 
this specification? I'm assuming so...

   [RFC5036].  An example of such a deviation is LDP graceful restart
   where a speaker retains MPLS forwarding state for LDP-signaled LSPs
   when its LDP control plane goes down.  It is important to point out
   that not all LDP enhancements require capability advertisement.  For
   example, upstream label allocation does but inbound label filtering,
   where a speaker installs forwarding state for only certain FECs, does
   not.

3. The LDP Capability Mechanism

   The LDP capability advertisement mechanism operates as follows:

     - Each LDP speaker is assumed to implement a set of enhancements
       each of which has an associated capability.  At any time a
       speaker may have none, one or more of those enhancements
       "enabled".  When an enhancement is enabled the speaker advertises
       the associated capability to its peers.  By advertising the
       capability to a peer the speaker asserts that it shall perform
       the protocol actions specified for the associated enhancement.
       For example, the actions may involve receiving and processing
       messages from a peer that the enhancement requires.  Unless the

Spencer (clarity): I got a bit lost here. Suggest "For example, the 
enhancement may require the LDP speaker to receive and process 
enhancement-specific messages from its peer" - is this still correct?

       capability has been advertised the speaker will not perform
       protocol actions specified for the corresponding enhancement.

     - Includes an IANA considerations section that notes that an IANA-
       assigned code point for the optional parameter corresponding to
       the enhancement is required.

Spencer (review comment): Is this saying "Includes an IANA Considerations 
section that requests assignment of a code point for the optional parameter 
corresponding to the enhancement"?

4. Specifying Capabilities in LDP Messages

   The format of a TLV that is a Capability Parameter is:

Spencer (clarity): "The format of a Capability Parameter TLV is:"?

       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
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |U|F| TLV Code Point            |            Length             |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
      |S| Reserved    |                                               |
      +-+-+-+-+-+-+-+-+       Capability Data                         |
      |                                               +-+-+-+-+-+-+-+-+
      |                                               |
      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

   where:
     U-bit
       SHOULD be 1 (ignore if not understood).

Spencer (review comment): I'm fairly comfortable with this being a SHOULD, 
but would be more comfortable with text that explains why the U-bit would be 
set to zero ("unless the LDP speaker is unwilling to continue with a peer 
that does not understand the enhancement"?)

     F-bit:
       SHOULD be 0 (don't forward if not understood).

Spencer (review comment): I'm confused by why this is a SHOULD... If the 
point is that

- I can ask you to do something you don't understand, so you ignore it, but

- you wouldn't forward the capability to a third party who might think you 
DO understand it, and start using the enhancement you don't understand

... I'm having a hard time understanding why this isn't a MUST. If I missed 
the point, my apologies.

   To ensure backward compatibility with existing implementations the
   following TLVs play the role of a Capability Parameter when included

Spencer (clarity): was this section written when more than one type of TLV 
played the role of a Capability Parameter? the phrasing is oddly plural.

   in Initialization messages:

       - FT Session TLV [RFC3479]

   This document refers to such TLVs as Backward Compatibility TLVs.

7. Procedures for Capability Parameters in Initialization Messages

   An LDP speaker SHOULD NOT include more than one instance of a
   Capability Parameter with the same type and value in an
   Initialization message.  Note, however, that processing multiple
   instances of such a parameter does not require special handling, as
   additional instances do not change the meaning of an announced
   capability.

Spencer (review comment): I'm concerned that this is SHOULD NOT, because (in 
my understanding) an implementation must still be prepared to accept 
messages that violate a SHOULD, but the document doesn't provide enough 
detail to process such messages in an interoperable way.

"does not require special handling" seems to handwave some difficult 
problems - if I advertise a capability and withdraw it in the same message, 
the order of evaluation isn't specified, for instance - so is the result 
that the capability is advertised, or withdrawn? If you really don't want to 
specify this level of detail - and please don't - just make this MUST NOT 
(and, for extra credit, specify the error handling for a message that 
violates the MUST NOT).

   These procedures enable an LDP speaker A that advertises a specific
   LDP capability C to establish an LDP session with speaker B that does
   not advertise C.  In this situation whether or not capability C may
   be used for the session depends on the semantics of the enhancement
   associated with C.  If the semantics do not require both A and B
   advertise C to one another then B could use it; that is, A's
   advertisement of C permits B to send messages to A used by the
   enhancement.

Spencer (clarity): call me thick. This text would be clearer if you s/A/X/ 
and s/B/Y/, because using A and B for speakers and C for a capability misled 
me into thinking that this paragraph was about transitive support, when A 
understands a capability, B does not, and SPEAKER C also understands it. The 
paragraph is correct as written - it's just easy to misread.

   It is the responsibility of the capability designer to specify the
   behavior of an LDP speaker that has enabled a certain enhancement,
   advertised its capability and determines that its peer has not
   advertised the corresponding capability.  The document specifying
   procedures for the capability MUST describe the behavior in this
   situation.  If the specified procedure is to terminate the session
   the LDP speaker SHOULD send a Notification message to the peer before
   terminating the session.  The Status Code in the Status TLV of the
   Notification message SHOULD be Unsupported Capability, and the

Spencer (review comment): why SHOULD and not MUST? I'm not asking for MUST, 
just a better understanding of why this wouldn't happen.

   message SHOULD contain the unsupported capabilities (see Section 9
   for more details).  In this case the session SHOULD NOT be re-
   established automatically.  How the session is re-established is
   beyond the scope of this document.

Spencer (review comment): I'm not sure why the session SHOULD NOT be 
re-established, but given the following sentence, I'd suggest striking the 
whole point - as out of scope.

   It depends on the LDP capability
   and MUST be specified along with the procedures specifying the
   capability.

   An LDP speaker that supports capability advertisement and includes a
   Capability Parameter in its Initialization message SHOULD set the TLV
   U bit to 1.  This ensures that an [RFC5036] compliant peer that does
   not support the capability mechanism will ignore the Capability
   Parameter and allow the session to be established.

Spencer (review comment): this may be the same comment as previously (on 
SHOULD for setting the U-bit to 1), but it seems like you expect that LDP 
speakers would want to allow the session to be established whether or not 
LDP Capability extension is supported or not - can you think of a scenario 
where this wouldn't be true? If not, I'd suggest MUST, or at least 
explaining the SHOULD a bit.

9. Extensions to Error Handling

   This document defines a new LDP status code named Unsupported
   Capability.  The E bit of the Status TLV carried in a Notification
   message that includes this status code SHOULD be set to 0.

Spencer (review comment): why not MUST? Or alternatively, what does the 
receiver do differently depending on whether the E-bit is 0, or 1?

   When the Status Code in a Notification Message is Unknown TLV the
   message SHOULD specify the TLV that was unknown.  When the

Spencer (review comment): so, if the message does not specify the TLV that 
was unknown, what does the LDP speaker do then? I'm not sure why this isn't 
a MUST.

   Notification message specifies the TLV that was unknown it MUST
   include the unknown TLV in a Returned TLVs TLV.

11. Backward Compatibility

   Section 3 identifies a set of Backward Compatibility TLVs that may

Spencer (clarity): again, it's a "set" of one TLV...

   appear in Initialization messages in the role of a Capability
   Parameter.  This permits existing LDP enhancements that use an ad hoc
   mechanism for enabling capabilities at sesssion initialization time
   to continue to do so. 

_______________________________________________
IETF mailing list
IETF@ietf.org
https://www.ietf.org/mailman/listinfo/ietf