Subject: Re: [mpls] Benjamin Kaduk's Discuss on draft-ietf-mpls-rfc6374-sfl-09: (with DISCUSS and COMMENT)

From: Stewart Bryant <[email-masked]>
Date: Thu, 4 Mar 2021 12:52:16 +0000

> On 25 Feb 2021, at 09:14, Benjamin Kaduk via Datatracker <[email-masked]> wrote:
> 
> Benjamin Kaduk has entered the following ballot position for
> draft-ietf-mpls-rfc6374-sfl-09: 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-rfc6374-sfl/
> 
> 
> 
> ----------------------------------------------------------------------
> DISCUSS:
> ----------------------------------------------------------------------
> 
> There may be a couple of internal (or external?) inconsistencies
> lingering, and while my confidence level of that is not as high as it
> often is, it's probably still worth checking.

> Specifics in the COMMENT, but at a high level, do we use ACH value
> 0x000A (what's shown in Section 9) or the TBD IANA-allocated values

When we originally proposed this we were only going to use one measurement type (type 0xA). We should have updated this text to point to both the existing additional methods and the new methods that we created during the course of this work.

I toyed with generalising the section, but in the I decided that the best way to great a simple explanation of the protocol would be as follows:

At the start of the section I changed the text to:

"We illustrate the packet format of an RFC6374 Query message using SFLs
for the case of an MPLS direct loss measurement in
{{Figure1}}.”

And then at the end of the section I added the following text:

"Where a measurement other than an MPLS direct loss measurement is to be
made, the appropriate RFC6374 measuremnet message is used (for example, one of the
new types defined in this document) and this is indicated to the receiver
by the use of the corresponding ACH type.

> ; we
> seem to refer to RFC 6374 as providing some things that I can't find in
> it; and the IANA considerations registration request includes a "TLV
> follows" column that does not appear to be present in the registry (and
> does not appear to match the described behavior in the rest of the
> document).  

We deprecated ACH TLV with RFC7026 as we could find no use for them and they complicated the handover from the forwarder to the OAM handler, This was not originally a problem as we were only doing connectivity measurement, but with the new fast OAMs such as the packet delay measurement methods the this was an issue. As we had never defined an ACH TLV deprecating them was easy.

This is of course distinct from RFC 6374 TLVs which we deal with in the RFC 6374 handler.

> Perhaps this (the current registry state) is the result of
> RFC 7026's actions, but I'm not entirely sure.
> 

I will deal with the comments in a separate email.

If this addresses your content, can I request that you clear your discuss on the basis of the version of the draft I am about to upload?

The comments will of course be addressed to the satisfaction of the new responsible AD, though I will of course try to get them fixed before Deborah steps down on Wednesday.

Many thanks for your review and for catching this issue.

Best regards

Stewart

---

From: Stewart Bryant <[email-masked]>
Date: Thu, 4 Mar 2021 13:09:22 +0000

Hi Ben,

Actually doing manual uploads is always a problem.

Here is the text change I proposed as an editor's note that Deborah could put in the IESG writeup if that satisfactorily resolves the issue. It will get picked up when I fix all the comments and issue a new version of the fully text.

OLD

9.  Carrying RFC6374 Packets over an LSP using an SFL

   The packet format of an RFC6374 Query message using SFLs is shown in
   Figure 5.

NEW

9.  Carrying RFC6374 Packets over an LSP using an SFL

   We illustrate the packet format of an RFC6374 Query message using
   SFLs for the case of an MPLS direct loss measurement in Figure 5.

END

=======

OLD

   RFC6378 and the optional UDP Return Object is defined in [RFC7876].

9.1.  RFC6374 SFL TLV

NEW

   RFC6378 and the optional UDP Return Object is defined in [RFC7876].

   Where a measurement other than an MPLS direct loss measurement is to
   be made, the appropriate RFC6374 measurement message is used (for
   example, one of the new types defined in this document) and this is
   indicated to the receiver by the use of the corresponding ACH type.

9.1.  RFC6374 SFL TLV

=========

Best regards

Stewart

---

From: Benjamin Kaduk <[email-masked]>
Date: Thu, 4 Mar 2021 13:24:34 -0800

Hi Stewart,

That proposal looks good, and should cover the 0x000A vs IANA value topic.

I trust you to do something useful with my other comments (including the
request to incoprorate by reference the security considerations from RFCs
6374 and 8372), so I'm willing to clear my discuss if that shows up as the
RFC Editor note.

I do still have some concern about things like where we say in Section 7.1
"The packet format of an RFC6374 Time Bucket Jitter Measurement Message is
shown", but I don't understand what the correct text should be well enough
to be able to hold a discuss for it.  I suspect that the intent is
something like "an (RFC6374 Query Message) that specifically is a (Time
Bucket Jitter Measurement Message) is shown", but that's way too awkward a
phrasing to put in an RFC, and I'm not even sure I'm correct about the
relationship between the RFC 6374 Query and what we're defining here.
(There are other message formats that have similar text in later sections
but I just mention the one.)

Thanks for thinking up a less-intrusive fix for this one; I would have gone
with something messier and harder to get right.

-Ben

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 5 Mar 2021 19:29:04 +0000

Ben

Many thanks for a thorough review.

The actions I have taken on your comments are shown inline.

> 0x000A (what's shown in Section 9) or the TBD IANA-allocated values; we
> document).  Perhaps this (the current registry state) is the result of

SB> This is fixed as agreed.

> COMMENT:
> A bit more introduction about how the term "delay" is used as
> effectively synonymous with "inter-packet gap" (for purposes of the
> mechanisms presented in this document) would be appreciated.

SB> I am not sure how to address this. We really are measuring delay.
SB> The availability of good time references makes this easier than it used to be.
SB> There is some average reporting to compress the statistics by delay really is delay.

> 
> Abstract, Introduction
> 
> (editorial) the phrases "general MPLS LSPs" and "general MPLS case",
> while intended to contrast against RFC 6374's restriction to just
> MPLS-TP, looks rather similar to "generalized MPLS" and could be a bit
> confusing.  I'd suggest a phrasing like "the generic case of MPLS LSPs,
> not limited to MPLS-TP".

SB> I changed this to:
SB> This document describes a method of extending RFC 6374 performance
SB> measurements from flows carried over MPLS-TP to flows carried over generic MPLS LSPs.
> 
> Section 5
> 
> I'd suggest a legend that A/B are colors for loss measurement and C/D
> are delay groups, and what is depicted is a time series of (the marking
> of) packets transmitted.

SB> I included:
     A & B are packets where loss is being measured
     C & D are pacekts where loss and delay is being measured

> 
>   measurement types would be identical.  Thus, it is proposed that the
>   two measurements are made relatively independent from each other.
> 
> How does this "proposed" relate to the simplifying rules in Section 6
> that put some fairly strong dependencies on how delay measurements
> relate to loss measurements?

SB> changed to 
Thus, we use a technique that
permits the two measurements are made concurrently and yet relatively
independent from each other.

SB> I will look at this working again during Auth48.

SB> So long as you are making loss measurements (which you can ignore) you
Can make delay measurements whenever you wish

> 
> Section 7.1
> 
>   The packet format of an RFC6374 Time Bucket Jitter Measurement
>   Message is shown below:
> 
> It seems a little misleading to have "RFC6374" in the name here without
> some classifier such as "framework" or "style", since the actual
> structure does not appear in RFC 6374.
> (Similarly for the other message formats.)

SB> I have removed the RFC6374 from the names and called the messages by their full
Names which should address the concern.

> 
> Section 7.2.1
> 
>   The Version, Flags, Control Code, Message Length, QTF, RTF, RPTF,
>   Session Identifier, and DS Fields are as defined in section 3.7 of
>   RFC6374.  The remaining fields are as follows:
> 
> I don't see a Section 3.7 in RFC 6374.
> This does, however, look like a lot like Section 3.3 thereof.

SB> Thanks for catching that, 3.2 is what was intended.

> 
> Section 7.4
> 
>   In this approach the packet is time-stamped at arrival and the
>   Responder returns the sum of the time-stamps and the number of times-
>   tamps.  From this the analytics engine can determine the mean delay.
>   An alternative model is that the Responder returns the time stamp of
>   the first and last packet and the number of packets.  This method has
>   the advantage of allowing the average delay to be determined at a
>   number of points along the packet path and allowing the components of
>   the delay to be characterized.
> 
> This prose suggests that an implementation or deployment might choose
> only one or the other of the "alternative model"s, but the packet layout
> and accompanying field descriptions suggest that all fields are to be
> populated, always.  Some clarity on whether it is permissible to select
> only one of the alternatives would be welcome.

SB> I added

Unless specifically configured otherwise, the
responder may return either or both types of response and
the analytics engine should process the response appropriately.

The responder will be an NPU, but I assume that the analytics engine
Is a heavyweight computer so can cope with any of the response types.

> 
>   The Version, Flags, Control Code, Message Length, QTF, RTF, RPTF,
>   Session Identifier, and DS Fields are as defined in section 3.7 of
>   RFC6374.  The remaining fields are as follows:
> 
> (There is still no Section 3.7 in RFC 6374.)

SB> Fixed as well
=======

> Section 8
> 
> (editorial) The purpose of this section is somewhat unclear.  It seems
> that the current text can (very roughly) be summarized as: "up until
> now, we assume we measure every packet in a colored interval.  RFC 8321
> did some other stuff; [details elided].  The RFC 8321 approach is not
> impacted by ECMP effects."  Nowhere does it seem to say that the RFC
> 8321 approach is directly applicable to the RFC 6374-style packets
> defined in this document, or some other reason why these facts are
> noteworthy in the context of this document.

SB> I have added

This sampled approach may be used if supported by the Responder and
configured by the opertor.

> Section 9

=============
> 
> Why do we only depict the use of ACH 0x000A (MPLS Direct Loss
> Measurement) when we also perform delay measurement (which might use ACH
> 0x000C) and also seem to be allocating three additional codepoints that
> would go in this range?

SB> I think that the text to address the discuss fixes this.

> 
> Should the figure still refer to an "RFC6374 Fixed Header" even though
> we admit variable-length headers for some measurements?  (Also, it's a
> bit surprising to use that terminology when RFC 6374 itself does not.)

SB> We are using a (pure) RFC6474 example so I think the references to 6374 are OK

SB> RFC6374 used the term “fixed-format” portion of the message, so I have used that in the figure and the corresponding text

> 
>   The RFC6374 measurement message consists of the three components, the
>   RFC6374 fixed header as specified in [RFC6374] carried over the ACH
>   channel type specified the type of measurement being made (currently:
>   loss, delay or loss and delay) as specified in RFC6374.
> 
> It doesn't quite seem that "as specified in RFC 6374" applies, since
> (IIUC) the "fixed header" is referring to the message formats from
> Sections 7.1, 7.2.1, and 7.4 of this document.

SB> We are extending RFC6374 with some additional measurement types, but the original measurements types (messages) still work. The section is an illustration, so I think it is OK to use the illustrative format to explain the protocol.

> 
>   Two optional TLVs MAY also be carried if needed.  The first is the
>   SFL TLV specified in Section 9.1.  This is used to provide the
>   implementation with a reminder of the SFL that was used to carry the
>   RFC6374 message.  This is needed because a number of MPLS
>   implementations do not provide the MPLS label stack to the MPLS OAM
>   handler.  This TLV is required if RFC6374 messages are sent over UDP
>   [RFC7876].  This TLV MUST be included unless, by some method outside
>   the scope of this document, it is known that this information is not
>   needed by the RFC6374 Responder.
> 
> Is any entity tasked with ensuring consistency between the actual label
> stack and the value in this TLV?

SB> I am not sure that we can include a receiver check as it is there for some hardware implementations that do not preserve the information, and if the transmitter get it wrong all bets are off. However if it is wrong the measurement will be wrong and this will be noticed by the operator. However I will add a note in the security section.

SB> I have added

An attacker that manages to corrupt the RFC6374 SFL TLV {{sfltlv}} could
disrupt the measurements in a way that the RFC6374 responder is unable to
detect. However, the network opertator is likely to notice the
anomalous network performance measurements, and in any case
normal MPLS network security proceedures make this type of attack extremely unlikley.

> 
>   otherwise MUST be carried.  The return address TLV is defined in

SB> Thanks. For some reason this was our ascii not a reference so did not get picked up by the normal compilation checks. 

> The Return Address TLV is specified in RFC 6374, not RFC 6378.
> 
SB> Thanks. For some reason this was our ascii not a reference so did not get picked up by the normal compilation checks. 

> Section 9.1
> 
>   The required RFC6374 SFL TLV is shown in Figure 6.  This contains the
> 
> (nit) per the previous section, this TLV is only mostly required.

SB> Thanks - shanked the text to 

The RFC6374 SFL TLV is shown

> 
>   SPL Index The index into the list of SFLs that were assigned
>             against the FEC that corresponds to the SFL.
> 
>             Multiple SFLs can be assigned to a FEC each
>             with different actions. This index is an optional
>             convenience for use in mapping between the TLV
>             and the associated data structures in the LSRs.
> 
> Is there any entity tasked with ensuring consistency between the index,
> the SFL, and the batch?
> 

SB> No, but I think this would be picked up by normal defensive programming.

> Section 12
> 
> While I agree that there's not much new in this document, incorporating
> by reference the considerations from (e.g.) 6374 and 8372, if not more,
> seems in order.  If I understand correctly those would also pull in the
> documentation of the generic privacy considerations with using user data
> for monitoring.
> 
> One thing that might be new in this document is the potential for data
> skew between the label stack and the SFL TLV, so that depending on one's
> implementation the reported results would be different.  (Similarly for
> batch/index/SFL relationships within the SFL TLV.)

SB> I added:

The security considerations documented  in {{RFC6374}} and {{RFC8372}}
(which in turn calls up {{RFC7258}} and {{RFC5920}}) are applicable to this
protocol.

> 
> Section 13.1
> 
>   As per the IANA considerations in [RFC5586], IANA is requested to
>   allocate the following codeponts in the "MPLS Generalized Associated
>   Channel (G-ACh) Type" registry, in the "Generic Associated Channel
>   (G-ACh) Parameters" name space:
> 
> The only thing I see at
> https://www.iana.org/assignments/g-ach-parameters/g-ach-parameters.xhtml
> that seems to match this description is written there as "MPLS
> Generalized Associated Channel (G-ACh) Types (including Pseudowire
> Associated Channel Types)".
> 
> I also don't see any "TLV follows" columns on that page, and don't
> understand why "no" is listed for these, since Section 9 is clear that
> the SFL TLV is mostly mandatory.

SB> We deprecated the TLVs. I have added the RFCs that updated RFC5586 and deleted the TLV column.
> 
> Section 16
> 
> If we are deferring to draft-bryant-mpls-sfl-control for the contents of
> some protocol message fields, I can't see how it's anything other than a
> normative reference.
> 

SB> draft-bryant-mpls-sfl-control is only one of the control protocols that some claim we need (though it is the only one that I propose to write). My concern was that if I made it normative this would stall on that text, and yet the design is valid without any control protocol as it could be controlled by an SDN controller. Indeed I suspect that SDN controllers are now so popular compared to when I started this that it will be the overwhelming deployment model.

- Stewart

---

From: Stewart Bryant <[email-masked]>
Date: Fri, 5 Mar 2021 19:51:17 +0000

> On 5 Mar 2021, at 19:29, Stewart Bryant <[email-masked]> wrote:
> 
> Ben
> 
>> 
>> Section 16
>> 

Actually I had a couple of requests to move this to normative.

On reflection you are correct so I have moved it.

The control draft is ST so it will simply mean that this stays in the editors queue for a while longer.

Best regards

Stewart