Subject: [mpls] Benjamin Kaduk's Discuss on draft-ietf-mpls-lsp-ping-lag-multipath-06: (with DISCUSS and COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 12 Mar 2019 13:30:30 -0700

Benjamin Kaduk has entered the following ballot position for
draft-ietf-mpls-lsp-ping-lag-multipath-06: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-mpls-lsp-ping-lag-multipath/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thanks for this document; it's clearly going to provide value, and it
gives a pretty well-readable description of how things are expected to
work.  That said, there's a number of details that need to be worked out
before publication...

This document has six listed authors; per RFC 7322, documents listing
more than five authors are unusual, and six is greater than five.  Let's
talk about the author count.

"optional TLV" seems to be used in MPLS contexts as a technical term for
"comprehension-optional", not in the "optional to send" sense; it's the
counterpart of "mandatory TLV", and this terminology is even documented
in the MPLS TLVs registry.  It's my understanding that the LSP
Capabilities TLV and the Detailed Interface and Label Stack TLV are
intended to be comprehension-required (i.e., "mandatory TLVs"), and thus
that we must not use the phrase "optional TLV" in their description.
This would be made clear if we listed which range of values we intended
to allocate a TLV type from, in the IANA considerations, but that
remains unspecified at this time.

In a similar vein (the "comprehension-required" behavior), there are a
few places (Section 3 (twice), and Section 6; also Section 3 for the LAG
Description Indicator flag; see COMMENT) where we state new normative
language ("MUST") that is unenforceable, since it would need to apply to
MPLS implementations that do not implement this specification.
Fortunately, the comprehension-required TLV ranges provide this
functionality for us without the need to use normative language.

Section 4.2 has some conflicting "MUST"s about the ordering/presence of
sub-TLVs in the DDMAP TLV (see COMMENT, and also the "MANDATORY"
langauge in Figure 2).

If I'm reading Section 5.1.2 correctly, the described procedure is only
intended to apply when the "G" flag is present (as well as the "I" flag,
the requirement for which is explicitly stated), but this is not
explicitly stated.  In particular, the text as written says it applies
to all responders that "understand the LAG Description Indicator flag"
with no mention of runtime check for the presence of that flag.

I also worry that Sections 8, 9, and 10 are insufficiently clear about the
encoding of the interface index value -- is it an integer in NBO, an
opaque bitstring, or something else?

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Am I reading this correctly that the "LSR Capability TLV" created here
is basically only applicable to properties of MPLS echo request/reply
exchanges?  If so, then perhaps the moniker "LSR Capability" is overly
generic.

Section 1.2

   o  Label switching over some member links of the LAG is successful,
      but will be failed over other member links of the LAG.

nit: there's some verb tense inconsistency here; maybe "but fails over
other member links" would help.

Section 2

   This document defines an optional TLV to discover the capabilities of
   a responder LSR and extensions for use with the MPLS LSP Ping and

nit: is it only the *responder* LSR we care about, or are there
potentially intermediaries in play?

   The solution consists of the MPLS echo request containing a DDMAP TLV
   and the optional LSR capability TLV to indicate that separate load

nit: Is this "optional capability TLV" the same "optional TLV" that we are
defining in this document?  If so, calling it "the new optional LSR
capability" would aid clarity.  (Also, we seem to use both the
"capability" and "Capability" capitzliations for describing this TLV.)

Section 3

   Capability TLV in the MPLS echo request message.  When the responder
   LSR receives an MPLS echo request message with the LSR Capability TLV
   included, if it knows the LSR Capability TLV, then it MUST include
   the LSR Capability TLV in the MPLS echo reply message with the LSR
   Capability TLV describing features and extensions supported by the
   local LSR.  Otherwise, an MPLS echo reply MUST be sent back to the
   initiator LSR with the return code set to "One or more of the TLVs
   was not understood".  [...]

This last MUST seems like something that this document cannot mandate
(as it attempts to apply to non-implementations of this document); it
could only work if it was an existing requirement from the core MPLS
spec, in which case lower-case "must" is appropriate (possibly with
section reference).

   o  If the responder LSR does not understand the "LAG Description
      Indicator flag":

      *  Clear both the "Downstream LAG Info Accommodation flag" and the
         "Upstream LAG Info Accommodation flag".

Similarly, if an LSR does not understand a flag, it cannot give special
treatment to packets containing that flag.  (Why an LSR would not
understand a flag defined in the same document that defines the
Capabilities TLV is beyond me, but you seem to want to allow for that
case.)

   If the responder does not know the LSR Capability TLV, an MPLS echo
   reply with the return code set to "One or more of the TLVs was not
   understood" MUST be sent back to the initiator LSR.

This duplicates the content I quoted at the top of this section's
comments.

Section 4.1

   Once the initiator LSR knows the capabilities that a responder
   supports, then it sends an MPLS echo request carrying a DDMAP with
   the "LAG Description Indicator flag" (G) set to the responder LSR.

nit: I think the key point is not that the initiator knows the
capabilities of the peer, but rather that the initiator knows that the
peer supports this specific mechanism.
Also, is this "a DDMAP TLV"?

Section 4.2

Reading this makes it sound like the "LAG Description Indicator flag" is
completely orthogonal to regular DDMAP functionality, so that if I set
the LAG description indicator flag but do not otherwise request detailed
descriptions, only the LAG interfaces are returned.  However, the flag
in question has to be inside a DDMAP TLV (and we should really say so,
perhaps as "[flag] set *in the DDMAP TLV*"!), so it seems like in practice
the LAG information can only be obtained in conjunction with full
detailed description information.  (The specific suggestion here is to
note clearly that the flag is se in the DDMAP TLV.)

      *  The responder LSR MUST include a DDMAP TLV when sending the
         MPLS echo reply.

I got confused on my first pass through this section; adding "There is a
single DDMAP TLV for the LAG interface, with member links described
using sub-TLVs" here would have kept me from veering onto the wrong path.
(I thought there was going to be one DDMAP TLV per member link, plus one
for the LAG aggregate, with lots of duplication.)

   Based on the procedures described above, every LAG member link will
   have a Local Interface Index Sub-TLV and a Multipath Data Sub-TLV
   entries in the DDMAP TLV.  The order of the Sub-TLVs in the DDMAP TLV
   for a LAG member link MUST be Local Interface Index Sub-TLV
   immediately followed by Multipath Data Sub-TLV.  A LAG member link
   MAY also have a corresponding Remote Interface Index Sub-TLV.  When a
   Local Interface Index Sub-TLV, a Remote Interface Index-Sub-TLV and a
   Multipath Data Sub-TLV are placed in the DDMAP TLV to describe a LAG
   member link, they MUST be placed in the order of Local Interface
   Index Sub-TLV, Remote Interface Index-Sub-TLV and Multipath Data Sub-
   TLV.

This last MUST contradicts the previous MUST.  I suggest some more text
to clarify under which conditions each requirement applies.

I'd also suggest not using the figure for conveying a (apparent?) mandatory
requirement, and instead adding some text at the end: "The block of local
interface index, (optional remote interface index) and multipath data
sub-TLVs for each member link MUST appear adjacent to each other in
order of increasing local interface index."

It's confusing to label the multiplath data sub-TLV as "MANDATORY" when
the body text says "MUST add an [sic] Multipath data Sub-TLV [...] if
the received DDMAP TLV requested multipath information", i.e., it is not
always present, based on the contents of the echo request.

   When none of the received multipath information maps to a particular
   LAG member link, then the responder LSR MUST still place the Local
   Interface Index Sub-TLV and the Multipath Data Sub-TLV for that LAG
   member link in the DDMAP TLV, the value of Multipath Length field of
   the Multipath Data Sub-TLV is set to zero.

nit: the last comma is a comma splice.

Section 5.1.2

It seems like this places a slightly stronger burden on the responder
than stock 8029 does, in that there is a MUST-level requirement to act
on the 'I' flag in combination with the 'G' flag, whereas for the 'I'
flag alone 8029 only has a SHOULD-level requirement to respond
accordingly.  We may want to call this out as a change in behavior.

Section 5.1.3

   Expectation is that there's a relationship between the interface
   index of the outgoing LAG member link at TTL=n and the interface
   index of the incoming LAG member link at TTL=n+1 for all discovered
   entropies.  In other words, set of entropies that load balances to

nit: "The expectation"
nit: "discovered entropies" is a strange wording given how the entropy
label works; is this more like "all entropies examined"?

   The initiator LSR sends two MPLS echo request messages to traverse
   the two LAG member links at TTL=n+1:

How does the initiator know (which entropy label values?) to get the echo
requests to traverse different member links?

   o  Error case:

      *  One MPLS echo request message reaches TTL=n+1 on an LAG member
         link 1.

      *  The other MPLS echo request message also reaches TTL=n+1 on an
         LAG member link 1.

      One or two MPLS echo request messages sent by the initiator LSR
      cannot reach the immediate downstream LSR, or the two MPLS echo
      request messages reach at the immediate downstream LSR from the
      same LAG member link.

The description paragraph doesn't seem to match up the scenario
described in the sub-bullets, which leaves me confused as to what
scenario(s) are intended to be considered as the "error case".

Section 6

   This document defines a new optional TLV which is referred to as the
   "LSR Capability TLV.  [...]

Is this optional to use or comprehension-optional?  (We elsewhere rely
on comprehension-mandatory behavior, so in the RFC 8029 terminology this
seems to be a "mandatory TLV" even if it is not mandatory to use.)

   included in the MPLS echo reply message.  Otherwise, if the responder
   does not know the LSR Capability TLV, an MPLS echo reply with the
   return code set to "One or more of the TLVs was not understood" MUST
   be sent back to the initiator LSR.

This duplicates a requirement stated elsewhere (that really ought to
just be using existing required behavior from 8029 anyway).

   LSR Capability TLV Type is TBD1.  Length is 4.  The value field of

In a hypothetical future where we need to allocate a 33rd capability
flag, would we rather create a new "extended capabilities" TLV (burning
another 32 bits for type+length) or leave flexibility now for 'length'
to increase in multiples of 4 with receivers ignoring bits past what
they know about?

      The "LSR Capability Flags" field is 4 octets in length, this
      document defines the following flags:

      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
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |                  Must Be Zero (Reserved)                  |U|D|
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

I'm wary of describing these as "Must Be Zero", since they're just
unallocated but not required to be zero other than by the current state
of bit allocations.  (I guess this is conventional for MPLS, though?  So
maybe it's not a problem.)  Just "Reserved" would be fine.

      This document defines two flags.  The remaining flags MUST be set
      to zero when sending and ignored on receipt.  Both the U and the D
      flag MUST be cleared in the MPLS echo request message when
      sending, and ignored on receipt.  Neither, either or both the U
      and the D flag MAY be set in the MPLS echo reply message.

Similarly, I think we want to say "unallocated flags" rather than "the
remaining flags".

It also seems like the semantics being described here are "the TLV value
is ignored in the echo request and only has meaning in the response"
(i.e., this is a "tell me your capabilities" exchange, not a "here are
mine; what are yours?" exchange).  As written, that applies only to the
U and D bits, and is not a general property.  That may well be a fine
choice if we think we might want the two-way exchange for some future
allocated bit, but if we do think it's a general property of the
mechanism, I'd suggest rewording the text.

Section 7

(Per IANA, bits 4 and 5 are already assigned but are not reflected in the
diagram.)

Section 10

   The Detailed Interface and Label Stack TLV format is derived from the
   Interface and Label Stack TLV format (from [RFC8029]).  Two changes
   are introduced.  The first is that the label stack is converted into
   a sub-TLV.  The second is that a new sub-TLV is added to describe an
   interface index.  These fields of Detailed Interface and Label Stack
   TLV have the same use and meaning as in [RFC8029].  A summary of
   these fields is as below:

nit: Are "These fields" just "The other fields"?

         The Address Type indicates if the interface is numbered or
         unnumbered.  It also determines the length of the IP Address
         and Interface fields.  The resulting total length of the
         initial part of the TLV is listed as "K Octets".  The Address
         Type is set to one of the following values:

nit: is it better to list the currently allocated values or just refer
to the IANA registry?

I seee that this "index assigned to the interface" language for
unnumbered address types originates from RFC 8029, but both this
document and 8029 leave me confused as to the encoding used for this
index (for either/both IPv4 or IPv6 address types).

[My comment about "Must Be Zero" and allocatable bits also applies here]

Section 12

This document also adds a capability "negotiation" mechanism for MPLS
echo request/reply exchanges.  As MPLS does not offer integrity
protection for its messages, this negotiation is only as trustworthy as
the network from which messages are accepted as valid, and initiators
have no recourse but to accept faithfully the echo replies received.

   To prevent leakage of vital information to untrusted users, a
   responder LSR MUST only accept MPLS echo request messages from
   designated trusted sources via filtering source IP address field of
   received MPLS echo request messages.  [...]

I'd prefer to see a note added here about how "source IP address
filtering provides only a weak form of access control and is not, in
general, a reliable security mechanism.  Nonetheless, it is required
here in the absence of any more robust mechanism that might be used."

Section 13

Should we ask IANA to update the "Interface and Label Stack Address
Types" registry to also refer to this document, since we are sharing the
registry between the Interface and Label Stack TLV and the Detailed
version?

Section 13.2

The range 4-31743 spans two different allocation procedures (Standards
Action and Specification Required - Experimental RFC needed); I assume
that we want the 4-16383 range for Standards Action - mandatory TLVs.

Section 13.4

We don't say what range we want this to be allocated from -- I assume
4-16383 since this is a negotiated feature and sending it outside the
negotiated scenario should be an error.

Section 13.4.1

(I note that some existing sub-TLV registries have "Experimental RFC
required" for the experimental ranges, but that is arguably more
stringent than necessary; Specification Required seems more appropriate
for this case.)

Appendix A

I was kind of hoping for some more guidance on what an initiator could
do in these scenarios to try to get better visibility into the switch
behavior (and, hopefully, a workaround to get reliable echo responses
that cover all LAG member links).  AFAICT there's not a better option
than "try a bunch of entropy labels and see what responses you can get
back" and that's the same remedy in all the described topologies, but it
would be nice to have this stated explicitly for the reader.

Section A.1

                    In the worst case, MPLS echo request messages with
   specific entropies to exercise every LAG members from R1 to S1 can
   all reach R2 via same LAG member.  [...]

I'm confused by the phrase "specific entropies to exercise every LAG
members [sic] from R1 to S1" -- is there some deterministic algorithm by
which a specific entropy label value will force a specific LAG member
link?  My understanding was that the entropy was used as input to an
opaque hash function and that trial-and-error was the only way to
explore the mapping from entropy value to LAG member link taken.

---

From: Loa Andersson <[email-masked]>
Date: Wed, 13 Mar 2019 09:59:29 +0800

Benjamin,

The number of authors were discussed and explained in the Shepherds
Write-up. All the 6 authors listed on the front-page has made
text contributions to the document.

/Loa

> https://datatracker.ietf.org/doc/draft-ietf-mpls-lsp-ping-lag-multipath/
<snip>

-- 

Loa Andersson                        email: [email-masked]
Senior MPLS Expert
Bronze Dragon Consulting             phone: +46 739 81 21 64

---

From: Benjamin Kaduk <[email-masked]>
Date: Tue, 12 Mar 2019 21:14:43 -0500

Hi Loa,

> text contributions to the document.

My apologies, I seem to have missed that line in my first read of the
shepherd report; I can drop that point.

Thanks,

Benjamin

---

From: Mach Chen <[email-masked]>
Date: Wed, 13 Mar 2019 07:06:42 +0000

Hi Benjamin,

Thanks for your comments!

Please see some replies inline...

> -----Original Message-----
> From: Benjamin Kaduk via Datatracker [mailto:[email-masked]]
> Sent: Wednesday, March 13, 2019 4:31 AM
> To: The IESG <[email-masked]>
> Cc: [email-masked]; [email-masked];
> [email-masked]; [email-masked]
> Subject: Benjamin Kaduk's Discuss on draft-ietf-mpls-lsp-ping-lag-multipath-
> 06: (with DISCUSS and COMMENT)
> 
> When responding, please keep the subject line intact and reply to all email
> addresses included in the To and CC lines. (Feel free to cut this introductory
> paragraph, however.)
> 
> 
> Thanks for this document; it's clearly going to provide value, and it gives a
> pretty well-readable description of how things are expected to work.  That

Thanks.

> said, there's a number of details that need to be worked out before
> publication...

OK.

> 
> This document has six listed authors; per RFC 7322, documents listing more
> than five authors are unusual, and six is greater than five.  Let's talk about the
> author count.

Loa has answered the question. 

Please refer to the Shepherd writeup where it is noted support of the six authors

> counterpart of "mandatory TLV", and this terminology is even documented in
> the MPLS TLVs registry.  It's my understanding that the LSP Capabilities TLV
> and the Detailed Interface and Label Stack TLV are intended to be
> comprehension-required (i.e., "mandatory TLVs"), and thus that we must
> not use the phrase "optional TLV" in their description.
> This would be made clear if we listed which range of values we intended to
> allocate a TLV type from, in the IANA considerations, but that remains
> unspecified at this time.

Good catch on the LSP Capabilities TLV, it should be specified as assigned from the range 0-16383 as a response is required. The TLV is optional, as noted this range also covers optional TLVs for which a response is required. Per RFC8029, the Downstream Detailed Mapping TLV which is used in this RFC is also optional.

> 
> In a similar vein (the "comprehension-required" behavior), there are a few
> places (Section 3 (twice), and Section 6; also Section 3 for the LAG Description
> Indicator flag; see COMMENT) where we state new normative language
> ("MUST") that is unenforceable, since it would need to apply to MPLS
> implementations that do not implement this specification.

OK, this is addressed with the below CMMENT.

> sub-TLVs in the DDMAP TLV (see COMMENT, and also the "MANDATORY"

OK, this is addressed with the COMMENT. 

> intended to apply when the "G" flag is present (as well as the "I" flag, the
> requirement for which is explicitly stated), but this is not explicitly stated.  In
> particular, the text as written says it applies to all responders that
> "understand the LAG Description Indicator flag"

How about add a condition at the beginning:

> encoding of the interface index value -- is it an integer in NBO, an opaque
> bitstring, or something else?
> 
> 
> Am I reading this correctly that the "LSR Capability TLV" created here is
> basically only applicable to properties of MPLS echo request/reply exchanges?
> If so, then perhaps the moniker "LSR Capability" is overly generic.

It's now defined for LSP Ping echo request/reply exchanges, but may be used to return general LSR capabilities in the future. I incline to keep it as is.  Are you suggesting  to change to another name?
> nit: there's some verb tense inconsistency here; maybe "but fails over other
> member links" would help.

OK.

> nit: is it only the *responder* LSR we care about, or are there potentially
> intermediaries in play?

Only the responder LSR cares. 

> defining in this document?  If so, calling it "the new optional LSR capability"
> would aid clarity.  (Also, we seem to use both the "capability" and
> "Capability" capitzliations for describing this TLV.)

OK.

And as suggested above, maybe we could remove the "optional" prefix. Will change it to "the new LSP capability". Is it OK for you?

> This last MUST seems like something that this document cannot mandate (as
> it attempts to apply to non-implementations of this document); it could only
> work if it was an existing requirement from the core MPLS spec, in which
> case lower-case "must" is appropriate (possibly with section reference).

OK.

> understand a flag defined in the same document that defines the Capabilities
> TLV is beyond me, but you seem to want to allow for that
> case.)

> This duplicates the content I quoted at the top of this section's comments.

OK. 

> nit: I think the key point is not that the initiator knows the capabilities of the
> peer, but rather that the initiator knows that the peer supports this specific
> mechanism.

OK, how about :

"Once the initiator LSR knows that a responder can support this mechanism, then it sends an MPLS echo request carrying a DDMAP TLV with the "LAG Description Indicator flag" (G) set to the responder LSR.

> Also, is this "a DDMAP TLV"?

Yes.

> completely orthogonal to regular DDMAP functionality, so that if I set the
> LAG description indicator flag but do not otherwise request detailed
> descriptions, only the LAG interfaces are returned.  However, the flag in
> question has to be inside a DDMAP TLV (and we should really say so, perhaps
> as "[flag] set *in the DDMAP TLV*"!), so it seems like in practice the LAG
> information can only be obtained in conjunction with full detailed description
> information.  (The specific suggestion here is to note clearly that the flag is se
> in the DDMAP TLV.)

OK, then the first sentence will like below:

"When a responder LSR received an MPLS echo request message with the "LAG Description Indicator flag" set in the DDMAP TLV."

> I got confused on my first pass through this section; adding "There is a single
> DDMAP TLV for the LAG interface, with member links described using sub-
> TLVs" here would have kept me from veering onto the wrong path.

OK.

> This last MUST contradicts the previous MUST.  I suggest some more text to
> clarify under which conditions each requirement applies.

How about adding the following sentence before "The order of ....", the it will look like:

"When a Local Interface Index Sub-TLV and a Multipath Data Sub-TLV are placed in the DDMAP TLV to describe a LAG member link, the order of the Sub-TLVs in the DDMAP TLV..."

> interface index, (optional remote interface index) and multipath data sub-
> TLVs for each member link MUST appear adjacent to each other in order of
> increasing local interface index."

OK.

> the body text says "MUST add an [sic] Multipath data Sub-TLV [...] if the
> received DDMAP TLV requested multipath information", i.e., it is not always
> present, based on the contents of the echo request.

Since it's an example, will  remove the "MANDATORY" and "OPTIONAL" prefix. 

> It seems like this places a slightly stronger burden on the responder than
> stock 8029 does, in that there is a MUST-level requirement to act on the 'I'
> flag in combination with the 'G' flag, whereas for the 'I'
> flag alone 8029 only has a SHOULD-level requirement to respond accordingly.
> We may want to call this out as a change in behavior.

I am fine to change to "SHOULD", then it can align with 8029. 

>    entropies.  In other words, set of entropies that load balances to
OK.

> nit: "discovered entropies" is a strange wording given how the entropy label
> works; is this more like "all entropies examined"?

How about  "all entropies discovered"?

> The description paragraph doesn't seem to match up the scenario described
> in the sub-bullets, which leaves me confused as to what
> Is this optional to use or comprehension-optional?  (We elsewhere rely on
> comprehension-mandatory behavior, so in the RFC 8029 terminology this

Yes, it's a "mandatory TLV" according to RFC8029.

> This duplicates a requirement stated elsewhere (that really ought to just be
> using existing required behavior from 8029 anyway).

OK, will try to remove some duplicate text.

> In a hypothetical future where we need to allocate a 33rd capability flag,
> would we rather create a new "extended capabilities" TLV (burning another
> 32 bits for type+length) or leave flexibility now for 'length'
> to increase in multiples of 4 with receivers ignoring bits past what they know
> about?

32 seems reasonable enough for future use. If there will be a 33rd capability in a hypothetical future, a sub-TLV may be created then. 

> I'm wary of describing these as "Must Be Zero", since they're just unallocated
> but not required to be zero other than by the current state of bit allocations.
> (I guess this is conventional for MPLS, though?  So maybe it's not a problem.)
> Just "Reserved" would be fine.

Yes, it's conventional for MPLS. I incline to keep it as is.

> Similarly, I think we want to say "unallocated flags" rather than "the

OK.

> 
> It also seems like the semantics being described here are "the TLV value is
> ignored in the echo request and only has meaning in the response"
> (i.e., this is a "tell me your capabilities" exchange, not a "here are mine; what
> are yours?" exchange).  As written, that applies only to the U and D bits, and
> is not a general property.  That may well be a fine choice if we think we might
> want the two-way exchange for some future allocated bit, but if we do think
> it's a general property of the mechanism, I'd suggest rewording the text.

For now, it's designed as unidirectional capabilities advertisement.  Define it as a more general property may require more considerations. I incline to keep it as-is. We can make an update if we have some specific use case in the future.

> (Per IANA, bits 4 and 5 are already assigned but are not reflected in the

OK, will fix it.

> nit: Are "These fields" just "The other fields"?

OK.

> nit: is it better to list the currently allocated values or just refer to the IANA
> registry?

Here, it does not define new registry, it just lists how the the purpose is the 

> 
> I seee that this "index assigned to the interface" language for unnumbered
> address types originates from RFC 8029, but both this document and 8029
> leave me confused as to the encoding used for this index (for either/both
> IPv4 or IPv6 address types).

> [My comment about "Must Be Zero" and allocatable bits also applies here]

OK. Will change to "Reserved".

> 
> Section 12
> 
> This document also adds a capability "negotiation" mechanism for MPLS echo
> request/reply exchanges.  As MPLS does not offer integrity protection for its
> messages, this negotiation is only as trustworthy as the network from which
> messages are accepted as valid, and initiators have no recourse but to accept
> faithfully the echo replies received.
> 
> I'd prefer to see a note added here about how "source IP address filtering
> provides only a weak form of access control and is not, in general, a reliable
> security mechanism.  Nonetheless, it is required here in the absence of any
> more robust mechanism that might be used."

OK, will add.

> 
> Section 13
> 
> Should we ask IANA to update the "Interface and Label Stack Address Types"
> registry to also refer to this document, since we are sharing the registry
> between the Interface and Label Stack TLV and the Detailed version?

Yes.

> Action and Specification Required - Experimental RFC needed); I assume that
> we want the 4-16383 range for Standards Action - mandatory TLVs.

Yes.

> negotiated scenario should be an error.

Yes.

> required" for the experimental ranges, but that is arguably more stringent
> than necessary; Specification Required seems more appropriate for this case.)

OK. Change to "Specification Required.
"
> 
> Appendix A
> 
> I was kind of hoping for some more guidance on what an initiator could do in
> these scenarios to try to get better visibility into the switch behavior (and,
> hopefully, a workaround to get reliable echo responses that cover all LAG
> member links).  AFAICT there's not a better option than "try a bunch of
> entropy labels and see what responses you can get back" and that's the
> same remedy in all the described topologies, but it would be nice to have this
> stated explicitly for the reader.

I am fine to add such a note in this section as you suggested, how about:

"Note: AFAICT there's not a better option than "try a bunch of entropy labels and see what responses you can get back" and that's the same remedy in all the described topologies below."

> which a specific entropy label value will force a specific LAG member link?

No, at least for my understanding.

> My understanding was that the entropy was used as input to an opaque hash
> function and that trial-and-error was the only way to explore the mapping
> from entropy value to LAG member link taken.

Yes, we have the same understanding.

Thanks again for your detailed and valuable comments!

Best regards,
Mach

---

From: Mach Chen <[email-masked]>
Date: Wed, 13 Mar 2019 08:33:59 +0000

Hi Benjamin,

Thanks for your thorough review and detailed comments!

Please see my replies inline...

Thanks.

OK.

> than five authors are unusual, and six is greater than five.  Let's talk about the

Please refer to Loa's answer.

> functionality for us without the need to use normative language.

Addressed with the COMMENT below.

> sub-TLVs in the DDMAP TLV (see COMMENT, and also the "MANDATORY"

Addressed with the COMMENT below.

> with no mention of runtime check for the presence of that flag.

How about add a sentence at the beginning of the section:

"When received an echo request with the "LAG Description Indicator flag" set, a responder LSR that ..."

It's currently defined for LSP Ping, but may be used in a more generic use. I incline to keep it as is. 

OK.

> nit: is it only the *responder* LSR we care about, or are there potentially

Only the responder LSR we care about.

> "Capability" capitzliations for describing this TLV.)

OK, fixed.
 
> case lower-case "must" is appropriate (possibly with section reference).

OK, how about below:

"Otherwise, an MPLS echo reply must be sent back to the initiator LSR with the return code set to "One or more of the TLVs was not understood", according to the rules as defined Section 3 of RFC8029."

> TLV is beyond me, but you seem to want to allow for that

OK, I think it's OK to remove them.  

> This duplicates the content I quoted at the top of this section's comments.

OK, remove this duplicate text.

Yes.

How about:
"Once the initiator LSR knows that a responder can support this meachanism, then it sends an MPLS echo request carrying a DDMAP TLV with  the "LAG Description Indicator flag" (G) set to the responder LSR."

Yes.

> information.  (The specific suggestion here is to note clearly that the flag is se

Fixed.

> for the LAG aggregate, with lots of duplication.)

Sure, added.

> clarify under which conditions each requirement applies.

OK, added.

> increasing local interface index."

OK, added.

> present, based on the contents of the echo request.

Since it's just an example here, will remove the MANDATORY and OPTIONAL. 

OK.

> We may want to call this out as a change in behavior.

Will change to "SHOULD", then it can align with RFC8029.

>    entropies.  In other words, set of entropies that load balances to

Fixed.

> works; is this more like "all entropies examined"?

OK, change to "all entropies examined".

> requests to traverse different member links?

The initiator cannot exactly know which entropy label values are workable. Presumably, the initiator can set different values according to the number of member links, then the responder can use entropy values for hash to traverse different member link. This is left to implementation. 

> scenario(s) are intended to be considered as the "error case".

The sub-bullets are not complete, will add a third sub-bullet as below:

"One or both MPLS echo request messages cannot reach the immediate downstream LSR on whichever link."

> seems to be a "mandatory TLV" even if it is not mandatory to use.)

It's "mandatory TLV" according to RFC8029.

> using existing required behavior from 8029 anyway).

Remove it.

> to increase in multiples of 4 with receivers ignoring bits past what they know

IMHO, 32 bit seems reasonable enough for me. I incline to keep the flag field as a fixed length.

If we want it be extensible, maybe we could add optional sub-TLVs to this TLV.

>       document defines the following flags:

OK. Change to "Reserved".

> Similarly, I think we want to say "unallocated flags" rather than "the

Fixed.

> it's a general property of the mechanism, I'd suggest rewording the text.

It's designed as an unidirectional tool, make it a two-way exchange mechanism will require more considerations and substantial changes to draft.  So, I inline to keep it as-is. If we have such use case in the future, we update this mechanism then.

> (Per IANA, bits 4 and 5 are already assigned but are not reflected in the

Fixed.

> nit: Are "These fields" just "The other fields"?

Fixed.

> nit: is it better to list the currently allocated values or just refer to the IANA

Since here it also include the length field. I'd like to keep it both here and the IANA registry.

> IPv4 or IPv6 address types).

It's normally an unsigned integer. 

> [My comment about "Must Be Zero" and allocatable bits also applies here]

Fixed as above.

> more robust mechanism that might be used."

OK, added.

> between the Interface and Label Stack TLV and the Detailed version?

Added.

Yes.

> negotiated scenario should be an error.

Yes, suggested range added.

> than necessary; Specification Required seems more appropriate for this case.)

OK, change to " Specification Required".

> stated explicitly for the reader.

A note will be added.

> which a specific entropy label value will force a specific LAG member link?

No.

Yes.

Best regards,
Mach

---

From: Benjamin Kaduk <[email-masked]>
Date: Wed, 13 Mar 2019 22:46:52 -0500

> Good catch on the LSP Capabilities TLV, it should be specified as assigned from the range 0-16383 as a response is required. The TLV is optional, as noted this range also covers optional TLVs for w…

That works for me, though I'm also watching Alvaro's ballot thread.

> "When received an echo request with the "LAG Description Indicator flag" set, a responder LSR that ..."

Just what I was looking for, thanks.

> > bitstring, or something else?

Do I need to clarify this question more?

> It's currently defined for LSP Ping, but may be used in a more generic use. I incline to keep it as is. 

Ok.

> "Otherwise, an MPLS echo reply must be sent back to the initiator LSR with the return code set to "One or more of the TLVs was not understood", according to the rules as defined Section 3 of RFC802…

Works for me.

> "Once the initiator LSR knows that a responder can support this meachanism, then it sends an MPLS echo request carrying a DDMAP TLV with  the "LAG Description Indicator flag" (G) set to the respond…

Sounds good.

> Since it's just an example here, will remove the MANDATORY and OPTIONAL. 

Ah, good point.

> Will change to "SHOULD", then it can align with RFC8029.

I think that will be okay (it wasn't the change I was expecting, but I will
definitely defer to the authors and WG for this sort of thing!); I wasn't
sure if the divergence was a conscious decision or not.

> The initiator cannot exactly know which entropy label values are workable. Presumably, the initiator can set different values according to the number of member links, then the responder can use ent…

Maybe add "(Depending on the LAG traffic division algorithm, the messages
may or may not traverse different member algorithms.)" or similar?

> "One or both MPLS echo request messages cannot reach the immediate downstream LSR on whichever link."

That makes it sound like the sub-bullets are distinct options, and the top
level bullet is an "error cases" plural.  My reading with just the two
sub-bullets present in the -06 was that both sub-bullets had to happen in
order to be in the error case.

> It's "mandatory TLV" according to RFC8029.

I think Alvaro is more on top of the subtleties here, so maybe we should
divert this topic to that ballot thread.

> If we want it be extensible, maybe we could add optional sub-TLVs to this TLV.

I agree that 32 bits seem likely to last for a very long time, so it's
hardly an important point.  I just wondered if anyone was going to enforce
that the length is exactly 4, which would prevent any further changes to
this TLV (other than allocating new bits).

> It's designed as an unidirectional tool, make it a two-way exchange mechanism will require more considerations and substantial changes to draft.  So, I inline to keep it as-is. If we have such use…

Sure, thanks for thinking about it.

> It's normally an unsigned integer. 

We probably should specify network byte order for it, then.

Thanks.

> Thanks again for your detailed and valuable comments!

You're selcome!

-Benjamin

---

From: Mach Chen <[email-masked]>
Date: Thu, 14 Mar 2019 04:04:18 +0000

Hi Benjamin,

Thanks for your prompt response!

Please see some responses inline...

> From: Benjamin Kaduk [mailto:[email-masked]]
> Sent: Thursday, March 14, 2019 11:47 AM
> To: Mach Chen <[email-masked]>
> Cc: Benjamin Kaduk via Datatracker <[email-masked]>; The IESG
> <[email-masked]>; [email-masked]; draft-ietf-mpls-lsp-ping-lag-
> [email-masked]; [email-masked]; [email-masked]
> Subject: Re: Benjamin Kaduk's Discuss on draft-ietf-mpls-lsp-ping-lag-
> multipath-06: (with DISCUSS and COMMENT)
> 
> > > Cc: [email-masked];
> > > [email-masked]; [email-masked]; [email-masked]
> > > When responding, please keep the subject line intact and reply to
> > > all email addresses included in the To and CC lines. (Feel free to
> > > cut this introductory paragraph, however.)
> > >
> > >
> > > Please refer to
> > > https://www.ietf.org/iesg/statement/discuss-criteria.html
> > > Thanks for this document; it's clearly going to provide value, and
> > > it gives a pretty well-readable description of how things are
> > > expected to work.  That
> >
> > Thanks.
> >
> > > This document has six listed authors; per RFC 7322, documents
> > > listing more than five authors are unusual, and six is greater than
> > > five.  Let's talk about the author count.
> >
> > > "optional TLV" seems to be used in MPLS contexts as a technical term
> > > for "comprehension-optional", not in the "optional to send" sense;
> > > it's the counterpart of "mandatory TLV", and this terminology is
> > > even documented in the MPLS TLVs registry.  It's my understanding
> > > that the LSP Capabilities TLV and the Detailed Interface and Label
> > > Stack TLV are intended to be comprehension-required (i.e.,
> > > "mandatory TLVs"), and thus that we must not use the phrase "optional
> TLV" in their description.
> > > This would be made clear if we listed which range of values we
> > > intended to allocate a TLV type from, in the IANA considerations,
> > > but that remains unspecified at this time.
> >
> > Good catch on the LSP Capabilities TLV, it should be specified as assigned
> from the range 0-16383 as a response is required. The TLV is optional, as
> noted this range also covers optional TLVs for which a response is required.
> Per RFC8029, the Downstream Detailed Mapping TLV which is used in this RFC
> is also optional.
> 

OK, let's wait Alvaro's response.

> 
> > >
> > > In a similar vein (the "comprehension-required" behavior), there are
> > > a few places (Section 3 (twice), and Section 6; also Section 3 for
> > > the LAG Description Indicator flag; see COMMENT) where we state new
> > > normative language
> > > Section 4.2 has some conflicting "MUST"s about the ordering/presence
> > > of sub-TLVs in the DDMAP TLV (see COMMENT, and also the
> "MANDATORY"
> > > langauge in Figure 2).
> >
> > > If I'm reading Section 5.1.2 correctly, the described procedure is
> > > only intended to apply when the "G" flag is present (as well as the
> > > "I" flag, the requirement for which is explicitly stated), but this
> > > is not explicitly stated.  In particular, the text as written says
> > > it applies to all responders that "understand the LAG Description Indicator
> flag"
> > > I also worry that Sections 8, 9, and 10 are insufficiently clear
> > > about the encoding of the interface index value -- is it an integer
> > > in NBO, an opaque bitstring, or something else?
> 

No, this has been answered within the below COMMENT.

The interface index value is normally an unsigned integer.

You added we should specify network byte order. Will add some clarify text.

> > > Am I reading this correctly that the "LSR Capability TLV" created
> > > here is basically only applicable to properties of MPLS echo request/reply
> exchanges?
> > > nit: there's some verb tense inconsistency here; maybe "but fails
> > > over other member links" would help.
> >
> > OK.
> >
> > >
> > > Section 2
> > >
> > > nit: Is this "optional capability TLV" the same "optional TLV" that
> > > we are defining in this document?  If so, calling it "the new optional LSR
> capability"
> > > This last MUST seems like something that this document cannot
> > > mandate (as it attempts to apply to non-implementations of this
> > > document); it could only work if it was an existing requirement from
> > > the core MPLS spec, in which case lower-case "must" is appropriate
> (possibly with section reference).
> >
> > OK, how about below:
> >
> > "Otherwise, an MPLS echo reply must be sent back to the initiator LSR with
> the return code set to "One or more of the TLVs was not understood",
> according to the rules as defined Section 3 of RFC8029."
> 
> Works for me.
> 
> > >
> > > Similarly, if an LSR does not understand a flag, it cannot give
> > > special treatment to packets containing that flag.  (Why an LSR
> > > would not understand a flag defined in the same document that
> > > defines the Capabilities TLV is beyond me, but you seem to want to
> > > allow for that
> > > case.)
> >
> > > capabilities of the peer, but rather that the initiator knows that
> > > the peer supports this specific mechanism.
> >
> > Yes.
> >
> > How about:
> > "Once the initiator LSR knows that a responder can support this
> meachanism, then it sends an MPLS echo request carrying a DDMAP TLV with
> the "LAG Description Indicator flag" (G) set to the responder LSR."
> 
> Sounds good.
> 
> >
> > > Reading this makes it sound like the "LAG Description Indicator
> > > flag" is completely orthogonal to regular DDMAP functionality, so
> > > that if I set the LAG description indicator flag but do not
> > > otherwise request detailed descriptions, only the LAG interfaces are
> > > returned.  However, the flag in question has to be inside a DDMAP
> > > TLV (and we should really say so, perhaps as "[flag] set *in the
> > > DDMAP TLV*"!), so it seems like in practice the LAG information can
> > > only be obtained in conjunction with full detailed description
> > > information.  (The specific suggestion here is to note clearly that
> > > the flag is se in the DDMAP TLV.)
> >
> > Fixed.
> >
> > >
> > > I got confused on my first pass through this section; adding "There
> > > is a single DDMAP TLV for the LAG interface, with member links
> > > described using sub- TLVs" here would have kept me from veering onto
> the wrong path.
> > > (I thought there was going to be one DDMAP TLV per member link, plus
> > > one for the LAG aggregate, with lots of duplication.)
> >
> > Sure, added.
> >
> > >
> > > This last MUST contradicts the previous MUST.  I suggest some more
> > > text to clarify under which conditions each requirement applies.
> >
> > OK, added.
> >
> > >
> > > I'd also suggest not using the figure for conveying a (apparent?)
> > > mandatory requirement, and instead adding some text at the end: "The
> > > block of local interface index, (optional remote interface index)
> > > and multipath data sub- TLVs for each member link MUST appear
> > > adjacent to each other in order of increasing local interface index."
> >
> > OK, added.
> >
> > >
> > > It's confusing to label the multiplath data sub-TLV as "MANDATORY"
> > > when the body text says "MUST add an [sic] Multipath data Sub-TLV
> > > [...] if the received DDMAP TLV requested multipath information",
> > > i.e., it is not always present, based on the contents of the echo request.
> >
> > > It seems like this places a slightly stronger burden on the
> > > responder than stock 8029 does, in that there is a MUST-level
> requirement to act on the 'I'
> accordingly.
> definitely defer to the authors and WG for this sort of thing!); I wasn't sure if
> the divergence was a conscious decision or not.
> 
> > >
> > > Section 5.1.3
> > >
> > > nit: "discovered entropies" is a strange wording given how the
> > > entropy label works; is this more like "all entropies examined"?
> >
> > > How does the initiator know (which entropy label values?) to get the
> > > echo requests to traverse different member links?
> >
> > The initiator cannot exactly know which entropy label values are workable.
> Presumably, the initiator can set different values according to the number of
> member links, then the responder can use entropy values for hash to
> traverse different member link. This is left to implementation.
> 

OK, will do.

> That makes it sound like the sub-bullets are distinct options, and the top level
> bullet is an "error cases" plural.  My reading with just the two sub-bullets
> present in the -06 was that both sub-bullets had to happen in order to be in
> the error case.
> 
> > >
> > > Section 6
> > >
> > > Is this optional to use or comprehension-optional?  (We elsewhere
> > > rely on comprehension-mandatory behavior, so in the RFC 8029
> > > terminology this seems to be a "mandatory TLV" even if it is not
> > > mandatory to use.)
> >

OK, let's wait Alvaro's response.

> > > flag, would we rather create a new "extended capabilities" TLV
> > > (burning another
> I agree that 32 bits seem likely to last for a very long time, so it's hardly an
> important point.  I just wondered if anyone was going to enforce that the
> length is exactly 4, which would prevent any further changes to this TLV
> (other than allocating new bits).

Maybe it can be relaxed to variable in length, and say the default is 4.

> > > unallocated but not required to be zero other than by the current state of
> bit allocations.
> > > (I guess this is conventional for MPLS, though?  So maybe it's not a
> > > problem.) Just "Reserved" would be fine.
> >
> > > Similarly, I think we want to say "unallocated flags" rather than
> > > "the remaining flags".
> >
> > Fixed.
> >
> > >
> > > It also seems like the semantics being described here are "the TLV
> > > value is ignored in the echo request and only has meaning in the
> response"
> > > (i.e., this is a "tell me your capabilities" exchange, not a "here
> > > are mine; what are yours?" exchange).  As written, that applies only
> > > to the U and D bits, and is not a general property.  That may well
> > > be a fine choice if we think we might want the two-way exchange for
> > > some future allocated bit, but if we do think it's a general property of the
> > It's designed as an unidirectional tool, make it a two-way exchange
> mechanism will require more considerations and substantial changes to draft.
> So, I inline to keep it as-is. If we have such use case in the future, we update
> this mechanism then.
> 
> > > nit: is it better to list the currently allocated values or just
> > > refer to the IANA registry?
> >

OK.

Best regards,
Mach

> > > This document also adds a capability "negotiation" mechanism for
> > > MPLS echo request/reply exchanges.  As MPLS does not offer integrity
> > > protection for its messages, this negotiation is only as trustworthy
> > > as the network from which messages are accepted as valid, and
> > > initiators have no recourse but to accept faithfully the echo replies
> received.
> > >
> Types"
> > > registry to also refer to this document, since we are sharing the
> > > The range 4-31743 spans two different allocation procedures
> > > (Standards Action and Specification Required - Experimental RFC
> > > needed); I assume that we want the 4-16383 range for Standards Action -
> mandatory TLVs.
> >
> > Yes.
> >
> > >
> > > Section 13.4
> > >
> > > 4-16383 since this is a negotiated feature and sending it outside
> > > the negotiated scenario should be an error.
> >
> > > stringent than necessary; Specification Required seems more
> > > appropriate for this case.)
> >
> > > I was kind of hoping for some more guidance on what an initiator
> > > could do in these scenarios to try to get better visibility into the
> > > switch behavior (and, hopefully, a workaround to get reliable echo
> > > responses that cover all LAG member links).  AFAICT there's not a
> > > better option than "try a bunch of entropy labels and see what
> > > responses you can get back" and that's the same remedy in all the
> > > described topologies, but it would be nice to have this stated explicitly for
> the reader.
> >
> > A note will be added.
> 
> Thanks.
> 
> > >
> > > Section A.1
> > >
> > > members [sic] from R1 to S1" -- is there some deterministic
> > > algorithm by which a specific entropy label value will force a specific LAG
> member link?
> >
> > No.
> >
> > > My understanding was that the entropy was used as input to an opaque
> > > hash function and that trial-and-error was the only way to explore
> > > the mapping from entropy value to LAG member link taken.
> >
> > Yes.
> >