Subject: [Last-Call] Secdir last call review of draft-ietf-mpls-lsp-ping-ospfv3-codepoint-04

From: Tero Kivinen via Datatracker <[email-masked]>
Date: Thu, 10 Jun 2021 07:20:25 -0700

Reviewer: Tero Kivinen
Review result: Has Nits

I have reviewed this document as part of the security directorate's 
ongoing effort to review all IETF documents being processed by the 
IESG.  These comments were written primarily for the benefit of the 
security area directors.  Document editors and WG chairs should treat 
these comments just like any other last call comments.

This document allocates a code point for OSPFv3 for MPLS LSP Ping and 
updates previous allocation to only cover OSPFv2. It also defines
behavior when using IPv6 with OSPv3.

This document is quite short but hard to ready because of heavy use of acronyms
and just referencing code points with numbers and same with RFCs.

The security considerations section just says:

   This document updates [RFC8287] and does not introduce any additional
   security considerations.

And I am not completely sure if that is true, if this document really allows using
IPv6 when it was not possible before. Quite often having multiple address families do 
cause new security considerations too. Also RFC8287 refers to the RFC8029 for its
security considerations, so perhaps direct reference to RFC8029 would be needed here.

There are several acronyms which are not expanded on their first use (including
in title, and in abstract). Examples of such are IS, TLV, OSPF, IS+IS, IGP, SUb-TLV (is the 
spelling correct in abstract with uppercase u?),  FEC.

The use of just RFC numbers in reference format makes the document hard to read
as not everybody remembers what RFC is RFC number 8287, 8402 etc. It would be 
much nicer to at least on the first time use the format where the text refers to RFC
with title or similar and just has the reference in parenthesis, i.e.:

   RFC5340 "OSPF for IPv6" ([RFC5340]) describes OSPF version 3 (OSPFv3) to 
   support IPv6. RFC5838 "Support of Address Families in OSPFv3" ([RFC5838])
   describes the mechanism to support multiple address families (AFs) in OSPFv3.
   Accordingly, OSPFv3 may be used to advertise IPv6 and IPv4 prefixes.

is easier for reader than current format:

   [RFC5340] describes OSPF version 3 (OSPFv3) to support IPv6.
   [RFC5838] describes the mechanism to support multiple address
   families (AFs) in OSPFv3. Accordingly, OSPFv3 may be used to
   advertise IPv6 and IPv4 prefixes.

Or, as the rfc title tells what the RFC is about you do not need to explain it that much
you can simply say:

   RFC5340 "OSPF for IPv6" ([RFC5340]) describes OSPF version 3 (OSPFv3) and
   RFC5838 "Support of Address Families in OSPFv3" ([RFC5838])
   describes how OSPFv3 may be used to advertise IPv6 and IPv4 prefixes.

Also someone who is not at all familiar with this it is bit hard to know what are
Type 34, 35, and 36 in Segment Id Sub-TLV registry. 

As a personal note, I have never liked to just use the reference inside text
(for example "This document updates [RFC8287] ...") as in case the RFC 
rendering engine decides to render references in some other way than just 
text with [] around it, the text might get unreadable (For example it replaces the
text inside [] with number or footnote or similar). Thats why I myself usually
want to write those either as "This document updates RFC8287 ([RFC8287])..." or 
even "This document updates RFC8287..." as RFC8287 is referenced so many times
in the document that there is no need to make each instance a reference. But this
is just my personal view, and authors might have different views...