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

[Greg Mirsky <[e-mail]>]

Sincere thanks, Adrian. Uploaded -09 version.

Regards,
Greg

On Thu, Jul 25, 2024 at 10:10 AM Adrian Farrel <[e-mail]> wrote:

> 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]> 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/ 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.
>