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

[Joel Halpern <[e-mail]>]

With regard to the parenthetical in the embedded backet definition, I 
would suggest removing it.  The examples do not seem to add value.  This 
document is not about why one carries things in MPLS.

The other changes seem appropriate and reasonable.  Thank you.

Yours,

Joel

On 1/7/2024 9:36 PM, Greg Mirsky wrote:
> 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.