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

[Greg Mirsky <[e-mail]>]

Hi Joel,
thank you for your expedient response. Would the following text address
your concern:
   Embedded Packet:  All octets beyond the PSH (if any).  That could be
      an IPv4 or IPv6 packet , an Ethernet packet (for VPLS ([RFC4761],
      [RFC4762]) or EVPN [RFC7432]), or some other type of Layer 2 frame
      [RFC4446].

Regards,
Greg

On Sun, Jan 7, 2024 at 7:35 PM Joel Halpern <[e-mail]> wrote:

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