Re: [mpls] Rtgdir last call review of draft-ietf-mpls-lspping-norao-06

[Greg Mirsky <[e-mail]>]

Hi Dhruv,
thank you for your detailed review and thoughtful comments. Please find my
notes below tagged GIM>>. I've attached the new working version of the
draft that includes all the updates. Please let me know if these address
your concerns.

Regards,
Greg

On Mon, Feb 5, 2024 at 1:13 AM Dhruv Dhody via Datatracker <[e-mail]>
wrote:

> Reviewer: Dhruv Dhody
> Review result: Has Issues
>
> 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 the IETF last call and IESG review, and sometimes on
> special
> request. The purpose of the review is to assist the Routing ADs. For more
> information about the Routing Directorate, please see
> https://wiki.ietf.org/en/group/rtg/RtgDir
>
> Although these comments are primarily for the use of the Routing ADs, it
> would
> be helpful if you could consider them along with any other IETF Last Call
> comments that you receive, and strive to resolve them through discussion
> or by
> updating the draft.
>
> Document: draft-ietf-mpls-lspping-norao-06
> Reviewer: Dhruv Dhody
> Review Date: 2024-02-05
> Intended Status: Standard Tracks
>
> ## Summary
>
> This document is basically ready for publication but has issues that
> should be
> considered prior to publication.
>
> The draft retires the use of Router Alert Option (RAO) for LSP Ping. I have
> some concerns but they are easy enough to resolve and the document should
> be
> ready to go once those are handled!
>
> ## Major
>
> - It would be incorrect to put RFC 7056 (which is being made historic) in
> the
> "updates:" list at the top of the page. Note that, one needs to follow the
> "status-change" document process to mark the RFC as historic which should
> be
> triggered by the AD. The job of this I-D is to articulate the reasons why
> the
> RFC7506 should be made historic. Update section 1 and section 3
> accordingly.
> Section 3 could list or refer to existing sections on why RFC 7506 should
> be
> historic.
>
GIM>> Thank you for pointing this out. Updated the header accordingly.

>
> - RFC 8029 includes text outside of sections 2.1 and 2.2 that mentions
> router
> alert. For instance,
> https://datatracker.ietf.org/doc/html/rfc8029#section-4.5
> tells how to handle it with "MUST" when we now are asking implementation to
> ignore it - ````
>    If the Reply Mode in the echo request is "Reply via an
>    IPv4 UDP packet with Router Alert", then the IP header MUST contain
>    the Router Alert IP Option of value 0x0 [RFC2113] for IPv4 or 69
>    [RFC7506] for IPv6.  If the reply is sent over an LSP, the topmost
>    label MUST in this case be the Router Alert label (1) (see
>    [RFC3032]).
>
 GIM>> Thank you for catching another reference to the Reply mode 3.
Appended the following text to Section 4:
NEW TEXT
   Resulting from the removal of the Reply mode 3 "Reply via an IPv4/
   IPv6 UDP packet with Router Alert" (see Section 2.2), this
   specification updates Section 4.5 of [RFC8029] by removing the
   following text:

   If the Reply Mode in the echo request is "Reply via an IPv4 UDP
   packet with Router Alert", then the IP header MUST contain the Router
   Alert IP Option of value 0x0 [RFC2113] for IPv4 or 69 [RFC7506] for
   IPv6.  If the reply is sent over an LSP, the topmost label MUST in
   this case be the Router Alert label (1) (see [RFC3032]).

````
> - If you do update the text, note that there is an erratum
> https://www.rfc-editor.org/errata/eid7639. Please do a check of all other
> instances of "alert" as well in RFC 8029.
>
GIM>> That was my errata. What would be the right way to handle it?

>
> ## Minor
>
> - Section 6, instead of a normative reference to the IANA URL, it would be
> better to name the IANA registry that needs to be updated and then
> optionally
> have an informative reference to the IANA URL.
>
GIM>> Agree with you. Done.

>
> ## Nits
>
> - Expand LSP in the tile
> - Expand on first use
>     - OAM (it is not expanded on first use)
> - Some instances of RFC XXXX without the reference [RFCXXXX] in the body.
> - s/ALL/all/
>
GIM>> Done for all.

>
> Thanks!
> Dhruv
>
>
>