Re: [mpls] Rtgdir early review of draft-ietf-mpls-1stnibble-02

[Greg Mirsky <[e-mail]>]

Hi Joel,
thank you for your thoughtful comments. Please find my notes below tagged
by GIM>>.

Regards,
Greg

On Wed, Dec 27, 2023 at 7:37 AM Joel Halpern via Datatracker <
[e-mail]> wrote:

> Reviewer: Joel Halpern
> Review result: Not Ready
>
> Hello,
>
> I have been selected as the Routing Directorate reviewer for this draft.
> The
> Routing Directorate seeks to review all routing or routing-related drafts
> as
> they pass through IETF last call and IESG review, and sometimes on special
> request. The purpose of the review is to provide assistance to the Routing
> ADs.
> For more information about the Routing Directorate, please see
> https://wiki.ietf.org/en/group/rtg/RtgDir
>
> This is a requested early Routing Directorate review, and as such is
> intended
> to help the Working Group and Document Editors with the subject document.
>
> Document: draft-ietf-mpls-1stnibble-02
> Reviewer: Joel Halpern
> Review Date: 27-Dec-2023
> IETF LC End Date: N/A
> Intended Status: Proposed Standard
>
> Summary:
>     I have some minor concerns about this document that I think should be
>     resolved.
>
> Major:
>
> Minor:
>    The parantheticl about IP packets in the "embedded packet" definition is
>    worded to imply that one would only put IP into MPLS for traffic
> engineering
>    or VPN purposes.  This seems misleading to me, and I strongly suggest
>    removing the parenthetical.
>
GIM>> Our intention was to use two as examples, not to imply that the text
in parentheses presents an exhaustive list:
   Embedded Packet:  All octets beyond the PSH (if any).  That could be
      an IPv4 or IPv6 packet (e.g., for traffic engineering of IP
      packets, or for a Layer 3 VPN [RFC4364]), an Ethernet packet (for
      VPLS ([RFC4761], [RFC4762]) or EVPN [RFC7432]), or some other type
      of Layer 2 frame [RFC4446].
Would s/e.g./for example/ make that clearer?

>
>     Bullet 3 in section 2.1.1 on Load Balancing asserts that guessing the
>     content type is always better than not doing so for load balancing
>     purposes.  If one guesses wrong, that may well not be true.  I would
>     suggest adding to the bullet a forward reference to the text below to
>     caveat "even better".
>
GIM>>  Thank you for the suggestion. Extended it as follows:
   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.1.3.

>
>      Section 2.1.3 is titled "recommendation" and starts with a "SHOULD",
> but
>      then has a "MUST NOT" which does not seem to be qualified by the
> "SHOULD"
>      It is unclear whether this is a flat requirement (belonging in the
>      previous section) or is intended for when the "SHOULD" is being
> obeyed.
>
GIM>> To address your concern, the following update is proposed:
OLD TEXT:
    It is RECOMMENDED that where load-balancing of MPLS packets is
   desired, either an Entropy Label or a FAT Pseudowire Label SHOULD be
   used; furthermore, the heuristic in Section 2.1.1.1 MUST NOT be used.
NEW TEXT:
   It is RECOMMENDED that where load-balancing of MPLS packets is
   desired, the load-balancing mechanism uses the value of a dedicated
   label, for example, either an Entropy Label [RFC6790] or a FAT
   Pseudowire Label [RFC6391].  Furthermore, the heuristic of guessing
   the type of the embedded packet (Section 2.1.1.1) SHOULD NOT be used.
I hope that the update is acceptable.

>
> Nits:
>     The reference in the introduction to the MPLS Open Design team should
> be
>     edited to refer to the MPLS Working group, since there is no longer an
> MPLS
>     Open Design Team.
>
GIM>> Thank you. Done.

>
>     Should "LSE" be expanded on first use? (And included in the list of
>     abbreviations?)
>
GIM>> Added Abbreviations as a new sub-section with LSE in it.

>
>     The paragraph at the end of the introduction needs to be resolved.  I
> would
>     suggest removing it.  As far as I can tell, the WG has evinced little
>     desire to make the change described there.
>
GIM>> Agreed and removed the last paragraph.

>
>     Paragraph 2 of section 2.1.3 on "recommendation" referes to
> "recommendation
>     2".  But the recommendations (and requirements) are not numbered.  So
> what
>     is the referent?
>
GIM>> Ineed confusing. Would the following rewording make it clearer?
OLD TEXT:
    A consequence of Recommendation 2 is that, while legacy routers may
NEW TEXT:
    A consequence of the latter recommendation is that, while legacy

>
>     From I-D Nits
>
>   ** The document seems to lack a Security Considerations section.
>
GIM>>  Added Security Consideration section as follows:
NEW TEXT:
4.  Security Considerations

   This document proposes a new IANA registry and does not raise any
   security concerns or issues in addition to ones common to networking
   and those specific to MPLS networks.

>
>   ** The abstract seems to contain references ([RFC4928]), which it
>      shouldn't.  Please replace those with straight textual mentions of the
>      documents in question.
>
GIM>> Done

>
>   == The 'Updates: ' line in the draft header should list only the
> _numbers_
>      of the RFCs which will be updated by this document (if approved); it
>      should not include the word 'RFC' in the list.
>
GIM>> Thank you for pointing that out. Done.