[mpls] Re: Shepherd review of draft-ietf-mpls-1stnibble

[Adrian Farrel <[e-mail]>]

Hi Greg,

 

Thanks for your careful work to make sure that no point is missed.

 

*	Author count. I have enough info to complete the shepherd write-up. Thanks.
*	Detailed points. I am comfortable with all of your proposed changes.

 

Please post the new version when you are ready, and I will complete the shepherd write-up.

 

Cheers,

Adrian

 

From: Greg Mirsky <[e-mail]> 
Sent: 25 July 2024 17:50
To: [e-mail]
Cc: [e-mail]; [e-mail]; [e-mail]
Subject: Re: Shepherd review of draft-ietf-mpls-1stnibble

 

Hi Adrian,

thank you for the review and your thoughtful suggestions. Please find my notes in-lined below under the GIM>> tag. Also, attached are the new working version and the diff highlighting the updates.

 

Regards,

Greg

 

On Tue, Jul 16, 2024 at 4:52 AM Adrian Farrel <[e-mail] <mailto:[e-mail]> > wrote:

Hi,

I'm your document shepherd for this journey.

I have done my shepherd review of revision -08. I have quite a lot of
nits, but also some more substantial points. I don't want to go against
WG consensus, but I'd like you to answer my comments and we can see 
where to go from there.

There may be more questions as I work on the shepherd write-up.

Thanks,
Adrian

===

Number of authors

While having more than five front-page authors is something we can
achieve, I am required (as shepherd) to provide a credible reason. Can
you supply me with this, or move someone to be a Contributor? 

Since you have one of your number designated as Editor, one solution 
would be to move everyone else to be Contributors.

GIM>> As Loa expressed in the separate thread, and I completely support him as a co-author and the Editor of this document, all authors provided essential contributions in the course of this work. 


---

Loa's affiliation.

Are you sure that Loa's affiliation is still Huawei?

---

id-nits

Line 474 at the top of section 3.1 uses smart quotes. Please squash
them.

There is a nit in your BCP 14 boilerplate. s/BCP14/BCP 14/

GIM>> Fixed both 


---

Abstract

Say what you are doing, not what you hope to do.

s/The goal of this memo is to create/This memo creates/

GIM>> Accepted, thank you. 


---

Abstract

The update reference for 4928 in the Abstract is too conditional. You
already have "if approved" in the header. What you want in the rest of
the text is to write material that can be included in the RFC without
further modification. Thus, "This document updates RFC 4928."

However, you should give a clue as to the nature of the update. You
don't have to write an essay (that comes in the main body), but a simple
"This document updates RFC 4928 by changing the colour of the curtains."

GIM>> Would the following update address this issue:

OLD TEXT:

    This memo, if published, would update RFC 4928.

NEW TEXT:

   This document updates RFC 4928 by deprecating the heuristic method

   for identifying the type of packet encapsulated in MPLS.


---

Introduction

The first paragraph makes a number of (true) statements that would
benefit from citations.

GIM>> I've spurced the paragraph with references (PW CW and BIER header). What would you suggest adding to further clarify the text? 


---

Introduction

Pedantically, and with regard to the note that the information about the
PSH and the embedded data packet is communicated via means such as the
routing protocols that signal the labels in the stack, isn't it wrong to
say that
   the
   correct interpretation of the Post-stack First Nibble (PFN) in a PSH
   can be made only in the context of the Label Stack Element (LSE) or
   group of LSEs in the preceding label stack that characterize the type
   of the PSH
This is nearly right, but I think that the LSEs indicate the context:
they are not the context.

GIM>> Thank you for bringing this to the discussion. Indeed, it is the context that is associated with label that is communicated via control or management plane. I propose the following update:

OLD TEXT:

   the

   correct interpretation of the Post-stack First Nibble (PFN) in a PSH

   can be made only in the context of the Label Stack Element (LSE) or

   group of LSEs in the preceding label stack that characterize the type

   of the PSH 

NEW TEXT:

   the correct interpretation of the Post-stack First Nibble (PFN) in a

   PSH can be made only in the context associated using the control or

   management plane with the Label Stack Element (LSE) or group of LSEs

   in the preceding label stack that characterize the type of the PSH

 

WDYT?


---

1.

   *  To document the assignments already made.

Technically, there being no registry up to now, no assignments have
been made. Perhaps...

   *  To document the values already in use.

GIM>> It makes text more accurate, thank you. 


---

1.

   *  To provide straightforward documentation of future assignments
      through the creation of a "Post-stack First Nibble registry".

This document doesn't provide that documentation. It provides a way to
achieve that documentation. So...

   *  To provide a mechanism to document future assignments through the
      creation of a "Post-stack First Nibble" IANA registry.

GIM>> Thank you for your help in improving the quality of this document. That is excellent suggestion. 


---

1.

   *  Provide a method for tracking usage by requiring more consistent
      documentation.

Consistent with what? Perhaps "more detailed" or "more rigorous"?

GIM>> I think that "detailed" is the right one. Thank you. 


---

1.

Getting ahead of the details of the document, but...

   *  To stress the importance that any MPLS packet not carrying plain
      IPv4 or IPv6 packets contains a PSH.

So, when IPv13 comes along, this will or will not require the use of a 
PSH?

I found the answer to this in 2.4, but I think you should call out that
all other versions of IP MUST be encoded using a PSH.

GIM>> Would the forward reference to Sect.2.4 be acceptable and sufficient:

NEW TEXT:

   *  To stress the importance that any MPLS packet not carrying plain

      IPv4 or IPv6 packets contains a PSH, including any new version of

      IP (Section 2.4).


---

1.

Again, avoid the conditional and provide RFC-ready text

s/, if published, would update [RFC4928]/updates [RFC4928]/

GIM>> Right, it's about time ;) 


---

1.1 

Expand BoS on first use.

GIM>> It turns out that it is the only place we use BoS. Replaced with the expanded form. 


---

1.1

      Examples include a control
      word or an associated channel.

Needs a reference.

GIM>> Are these references what you envisioned:

NEW TEXT:

     Examples include a control

      word [RFC4385] or an associated channel [RFC4385], [RFC5586].


---

1.1

Very odd to have the figures in this section especially as figure 2 is 
not mentioned until 2.1.1.1.

I suggest breaking out to a new section "Reference Figures", adding a
mention of Figure 2 within that section, and giving Figure 2 a caption.

GIM>> Was I able to address your recommendation with the updates? 


---

1.1 Embedded packet

Perhaps,
OLD
   Embedded Packet:  all octets beyond the PSH (if any).
NEW
   Embedded Packet:  all octets beyond the MPLS label stack excluding 
   the PSH (if present). 
END

s/packet ,/packet,/

GIM>> Would the follwoing be acceptable:

NEW TEXT:

   Embedded Packet:  an embedded packet follows immediately after the

      MPLS Label Stack and an optional PSH.

END

WDYT?


---

1.1

   Deprecation:  regardless of how the deprecation is understood in
      other IETF documents, the interpretation in this document is that
      if a practice has been deprecated, that practice should not be
      included in the new implementations or deployed in deployments.

s/the new/new/

When you say "should not be ... deployed in deployments" what does that
say about pre-existing implementations that are already deployed?

GIM>> We extensively discussed two actions - deprecate and obsolete. As I understand them, your question about the latter. To that the document, in update to Section 3 RFC 4928, states:

   |  At the time RFC XXXX was written, it was planned to obsolete MPLS

   |  encapsulations without PSH of non-IP payload when sufficient

   |  evidence exists that there are no marketed or deployed

   |  implementations using the heuristic practice.


---

1.2

LSR is not used in the document

GIM>> I found two occurences in the definition of a Post-Stack Header:

      Post-stack Header (PSH):  optional field of interest to the egress

      Label Switching Router (LSR) (and possibly to transit LSRs).

SPL could use a reference

GIM>> RFC 7274? 


Add
BoS

GIM>> I've found it is used only once and I've replaced it with the expanded form. 

G-ACh

GIM>> It is used only in the table for IANA new registry. I've replaced it with the expanded form (and it fits) 

NSH

GIM>> Used several times but, as G-ACh, only in the table for IANA new registry. Added the expanded form. WDYT? 

OUI

GIM>> I found it is used once and replaced it with the expanded form "Organizationally Unique Identifier". Would you agree? 


---

2.1

Should the section title have a question mark?

GIM>> I defer that to people more proficient in English than I am. AFAICS, "why" could be an introduction to an explanation, e.g., "Why we fight".


---

2.1

   An MPLS packet can contain many types of embedded packets.

I think an MPLS packet can only contain one type of embedded packet. So,

   MPLS packets can contain many types of embedded packet.

Or,

   There are many types packet that may be carried by MPLS packets.

GIM>> I borrowed from your note:

NEW TEXT:

   An MPLS packet can contain one of many types of embedded packets.

WDYT?


---

2.1

   The most common types are

I have no doubt what you say is true, but it is something of an
unsubstantiated assertion. It is also not clear that it will stand the
test of time.

Perhaps:
   Three common types are

GIM>> Indeed. Thank you. 


---

2.1

   In addition, there may be a PSH ahead of the embedded packet, and it
   needs to be parsed.  The PFN is currently used for both of these
   purposes.

a) "it needs to be parsed"  By whom? Why?
b) "The PFN is currently used for both of these purposes." This is not
   very clear as the first purpose (identifying the payload packet type) 
   has not be introduced as something that needs to be done, and the 
   second purpose (indicting that there is a PSH present) has not been
   mentioned.

GIM>> To address a) and b) I propose the following update:

OLD TEXT:

   In addition, there may be a PSH ahead of the embedded packet, and it

   needs to be parsed.  The PFN is currently used for both of these

   purposes.

NEW TEXT:

   In addition, there may be a PSH ahead of the embedded packet.  The

   value of PFN is considered to ensure that the PSH can be correctly

   parsed.  If no PSH follows the label stack, then the value of PFN

   indicates the version number of the IP packet header. 

 

Is it more clear now?


---

2.1.1

   3.  One can do even better by "divining" the type of embedded packet,
       and using fields from the guessed header.  The ramifications of
       using this load-balancing technique are discussed in detail in
       Section 2.2.

   4.  One can do best by using either an Entropy Label [RFC6790] or a
       Flow-Aware Transport (FAT) Pseudowire Label [RFC6391]; see
       Section 2.2.)

I think that is s/2.2/2.1.1.1/ <http://2.1.1.1/>  in both cases.

GIM>> Great catch, thank you. 


---

2.1.1

"dangerous" may be over doing it. Perhaps "risky"?

GIM>> Agreed. 


---

2.1.1.1

   For example, if payload B is an
   Ethernet frame, then the PFN is the first nibble of the OUI of the
   destination MAC address, which can be 0x4 or 0x6, and if so would
   lead to very bad load-balancing.

"Very bad load-balancing" is an interesting phrase. Maybe expand a 
little, such as...

   would lead to the packet being treated as an IPv4 or IPv6 packet such
   that data at the offsets of specific relevant fields would be used as
   input to the load-balancing heuristic resulting in unpredictable load
   balancing.

BTW "load-balancing" as an adjective, but "load balancing" as a noun.

GIM>> Thank you for the suggested text. 


---

2.1.1.1

   A consequence of that heuristic approach is that while legacy routers
   may look for a PFN of 0x4 [RFC0791] or 0x6 [RFC8200], no router will
   look for any other PFN, regardless of what future IP version numbers
   will be, for load-balancing purposes.

When you say "no router will look for", I there are two things:
- No *legacy* router will look for any other future IP version numbers

GIM>> s/no router/no legacy router/ 

- New routers that follow the recommendations in this document will not
  look for other IP versions except if they are included in the registry

GIM>> I think that that will happen only after a new IP version is published and the registry updated. Before that, a router that follows the recommendation in the draft will not look for any value other than 0x4 and 0x6 as an indication that the embedded packet is an IP packet.


But, possibly I am confused by "that heuristic" because the previous 
paragraph recommends a "mechanism" and recommends against using a
"heuristic".

GIM>> s/that heuristic approach/the heuristic approach/? 


---

2.2

All of the text in 2.2 is very forward referential.
I don't think it is right to update 4928 to say "if conformant to XXXX".
You should just update the text to what you want the case to be. Thus...

   NEW TEXT:

   |  Network equipment MUST use a PSH (Post-Stack Header) with a PFN
   |  (Post-stack First Nibble) value that is neither 0x4 nor 0x6 in all
   |  cases when the MPLS payload is not an IP packet.

GIM>> Thank you for suggestions. 


Except, do you want "not an IPv4 or IPv6 packet."

GIM>> Yes, that is the practical outcome of the requirement. But we wanted to arrive to it from the point of PFN values. 


...and...

   NEW TEXT:

   |  The practice of deducing the payload type based on the PFN value
   |  is deprecated to avoid inaccurate load balancing.  This means that
   |  means that older implementations and deployments can continue to
   |  use that heuristic, while it must not be part of new 
   |  implementations or deployments.  It also means that concerns about
   |  load balancing for future IP versions with a version number of 0x0
   |  or 0x1 are no longer relevant.

GIM>> Thenk you for the suggested text that further improves the document. 


I was also surprised by...

   |  At the time RFC XXXX was written, it was planned to obsolete MPLS
   |  encapsulations without PSH of non-IP payload when sufficient
   |  evidence exists that there are no marketed or deployed
   |  implementations using the heuristic practice.

... There was no discussion of this anywhere else in the draft. Anyway,
it is really odd to see what-will-be RFC XXX saying "at the time 
RFC XXXX was written".

I suggest you move this paragraph to a separate section that discusses
the issue and proposes to collect evidence and make the further change
in the future.

GIM>> Extracting that paragraph and adding a new section:

2.5.  Next Step to More Deterministic Load -balancing in an MPLS Network

 

   Network evolution is impossible to control, but it develops over a

   period of time determined by various factors.  This document prevents

   further proliferation of the implementations that could lead to

   undesired effects affecting data flow.  At some time in the future,

   it was planned to obsolete MPLS encapsulations without PSH of non-IP

   payload.  Before that it is paramount to collect sufficient evidence

   that there are no marketed or deployed implementations using the

   heuristic practice to load-balancing MPLS data flows.

What are your thoughts? Suggestions to improve the new section?


---

I think section 2.3 is referring to the MNA work when it says...

   The MPLS WG is currently engaged in updating the MPLS architecture;

The world has moved on a bit, and you can be a bit more specific.

Perhaps...

   Support for MPLS Network Actions (MNAs) is described in
   [I-D.ietf-mpls-mna-fwk] and is an enhancement to the MPLS 
   architecture.  The use of post-stack data (PSD) to encode the MNA
   indicators and ancillary data is described in section 3.6 might place
   data in the PFN that could conflict with other uses of that nibble.
   This issue is described in section 3.6.1 of [I-D.ietf-mpls-mna-fwk]
   and is further illustrated by the PFN value of 0x0 which has two
   different formats depending on whether the PSH is a pseudowire 
   control word or a DetNet control word: disambiguation requires the 
   context of the service label.  

   With a registry, PSHs become easier to parse; not needing means
   outside the data plane to interpret them correctly; and their
   semantics and usage are documented.

GIM>> Updated as you sugegsted, thank you. 


---

3.1

I wonder whether the new registry will ever be used for anything other
than MPLS.  The draft describes it as being to capture the first nibble
after an MPLS label stack.  Doesn't that make it a part of the MPLS
registry group?

 

GIM>> We had an early IANA review and, after a number of iterations, it seems like IANA has agreed with the proposed update.