MPLS Upstream Label Assignment for LDP
draft-ietf-mpls-ldp-upstream-10

SECTION 3: s/MUST be carried only LDP initialization messages/MUST be carried only in LDP initialization messages/

A few comments from Ari Keränen:

The format of the packet format figures is not consistent: in the 
section 5's sub-TLVs "type" is given with syntax "Type  = XX" whereas in 
rest of the figures it's "Name (code)".

The second field of sub-TLVs (length?) is not defined at all in the 
figures but just the value is given. Also the length seems to include 
the type and length of the TLV -- is that intentional? Especially since 
in the other TLVs do not seem to include type and length.

Also the sentence "The TLV value in the sub-TLV acts as the tunnel 
identifier" is strange. Do you mean "the value of the sub-TLV"?


    1. RSVP-TE P2MP LSP TLV. Type = 28 (To be assigned by IANA). Value of
    the TLV is as carried in the RSVP-TE P2MP LSP SESSION Object
    [RFC4875].

The second sentence above is a bit confusing. Perhaps removing the "as 
carried in" part from the sentence and changing "value of the TLV" into 
"value of the sub-TLV" would help, if you mean that the whole object is 
in the value of the sub-TLV.

It's also somewhat confusing that with RSVP-TE P2MP LSP TLV there are 
Type and Length in the figure and with LDP P2MP LSP TLV there aren't.

The format of the 3rd and 4th sub-TLV seems underspecified. For example, 
how do you encode a "<Source Address, Multicast Group Address> tuple" in 
a sub-TLV?


6. LDP Point-to-Multipoint LSPs on a LAN

    Processing
    of the Label Request and Label Mapping messages for LDP upstream-
    assigned labels is as described in section 4.2.

Section 4.2. does not exist.


    2. The following hash is performed: H = (Sum Opaque value) modulo N,
    where N is the number of candidate upstream LSRs. Opaque value is
    defined in [MLDP] and comprises the P2MP LSP identifier.

What does the "Sum" in the hash equation mean? I would assume it's sum 
over all opaque values if there is more than one, but it's not really 
clear from the context. Also, how do you convert the opaque value(s) 
into number(s)?

  The Gen-ART Review by Avshalom Houri on 13-Oct-2010 includes some
  editorial suggestions.  Please consider them.

Section 4: Shouldn't the two "optional"s be "OPTIONAL"?  They're indicating support requirements.

This text is a little confusing

"As described in [RFC5331] an upstream LSR Ru MAY transmit a MPLS packet, the top label of which (L) is upstream-assigned, to a downstream LSR Rd"

Since it looks like Ru is a special type of LSR.

How about something like:

"As described in [RFC5331], a specific upstream LSR (Ru) MAY transmit a MPLS packet, the top label of which (L) is upstream-assigned, to it downstream neighbor LSR (Rd)"