IGP Unreachable Prefix Announcement
draft-ietf-lsr-igp-ureach-prefix-announce-11

Thanks to Valery Smyslov for their secdir review.

# Éric Vyncke, INT AD, comments for draft-ietf-lsr-igp-ureach-prefix-announce-09
CC @evyncke

Thank you for the work put into this document.

Please find below some non-blocking COMMENT points/nits (replies would be appreciated even if only for my own education).

Special thanks to Yingzhen Qu for the shepherd's detailed write-up including the WG consensus (important to note for this I-D) _but it lacks_ the justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS (non-blocking)

### Title

s/IGP Unreachable Prefix Announcement/Unreachable Prefix *IGP* Announcement/ sounds clearer to me but English is not my primary language.

### Abstract

s/This document describes/This document *specifies*/ as this is proposed standard.

### Section 1

Who is the "we" in `we want to signal unreachability` ? The authors ? The LSR WG ? The IETF ? Please avoid using ambiguous "we" by rephrasing the sentence.

s/ISIS/IS-IS/ ? and in other places.

### Section 2

s/for a prefix that is summarized by the summary *address*/for a prefix that is summarized by the summary *prefix*/ ?

There are two "SHOULD" in this section but nothing is written the cases when these "SHOULD" can be bypassed and/or what are the consequences.

Add "and OSPFv3" at the end of `so the above optimisation does not apply to OSPF` ? It seems that the I-D sometimes uses OSPF for OSPFv2 alone and sometimes for OSPFv2 and OSPFv3.

### Section 3.1

Add "to support this specification" after `without the need to upgrade all nodes in a network.` ?

`Those ABRs or ASBRs, which are responsible for propagating UPA advertisements into other areas or domains MUST also recognize UPA advertisements.` what are the consequences if this is not the case (e.g., not all ABR/ASBR have been upgraded). Should this be mentioned in an operational considerations section ?

### Section 5.1

Was the use of a 2-bit field considered rather than using 2 flags ? It would have allowed 4 values as opposed as the current 3 values. Out of curiosity because it is probably too late to change it...

### Section 8

It is often preferable to add an informational reference to the URI of the registries.

# Internet AD comments for draft-ietf-lsr-igp-ureach-prefix-announce-09
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

### S5.1,5.2

* It seems to me that using the letters "UP" to mean "no, actually down"
  could bring no small amount of confusion.

  But I assume the working group discussed options and this was the best
  that could achieve consensus.

## Nits

### S1

* "reachability of the summary address" ->
  "reachability of the summary prefix"

Thanks for the work is described in this document. I do not see any transport-related concerns for this I-D.

Thanks to the authors and the WG for their work on this document.

I believe this is a useful feature in specific deployment use cases where 
summarization is used for scaling purposes.

Thanks to Peter for addressing my comments in the DISCUSS and COMMENTS sections as the editor of the document.

Section 1, paragraph 0
>    Link-state IGP protocols like IS-IS [ISO10589], OSPF [RFC2328], and
>    OSPFv3 [RFC5340] are primarily used to distribute routing information
>    between routers belonging to a single Autonomous System (AS) and to
>    calculate the reachability for IPv4 or IPv6 prefixes advertised by
>    the individual nodes inside the AS.  Each node advertises the state
>    of its local adjacencies, connected prefixes, capabilities, etc.  The
>    collection of these states from all the routers inside the area form
>    a link-state database (LSDB) that describes the topology of the area
>    and holds additional state information about the prefixes, router
>    capabilities, etc.

Use of OSPF, OSPFv2 and OSPFv3 is confusing in this draft. The title of RFC2328 is "OSPF Version 2", not "OSPF", which is often used to refer to both OSPFv2 and OSPFv3. Could the authors correct and clarify when the statement is applicable to OSPFv2, or OSPFv3 or both (by using OSPF).

Section 2, paragraph 9
>    It is also RECOMMENDED that implementations limit the number of UPA
>    advertisements which can be originated at a given time.

This and other Sections documents several configuration parameters, including a knob to enable the advertisment of UPA and the reception and processing of them. It would be useful to summarize all the configuration (and maintenance) options this document is introducing in a separate section called "Operational Considerations".

Found terminology that should be reviewed for inclusivity; see
https://www.rfc-editor.org/part2/#inclusive_language for background and more
guidance:

 * Term "traditionally"; alternatives might be "classic", "classical",
   "common", "conventional", "customary", "fixed", "habitual", "historic",
   "long-established", "popular", "prescribed", "regular", "rooted",
   "time-honored", "universal", "widely used", "widespread"

-------------------------------------------------------------------------------
NIT
-------------------------------------------------------------------------------

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

Section 1, paragraph 3
>    IGP protocols typically only advertise the reachability of the
>    prefix.  Prefix that was previously advertised as reachable is made
>    unreachable just by withdrawing the previous advertisement of the
>    prefix.  In our use case, we want to signal unreachability for a
>    prefix for which the reachability was not explicitly signaled
>    previously, because it was covered by the reachability of the summary
>    address.

As Eric has already commented, who is "ours" or "we"? Please avoid the usage of such terms.

Section 1, paragraph 3
>    This document also defines how the UPA is propagated across ISIS
>    levels and OSPF areas.

s/ISIS/IS-IS/ here and everywhere in the document.

Section 2, paragraph 6
>    The intent of UPA is to provide an event driven signal of the
>    transition of a destination from reachable to unreachable.  It is not
>    intended to advertise a persistent state.  UPA advertisements SHOULD
>    therefore be withdrawn after some amount of time, that would provides
>    sufficient time for UPA to be flooded network-wide and acted upon by
>    receiving nodes, but limits the presence of UPA in the network.  The
>    time the UPA is kept in the network SHOULD also reflect the intended
>    use-case for which the UPA was advertised.

s/that would provides/that would provide/

Document references draft-ietf-lsr-ospf-prefix-extended-flags, but that has
been published as RFC9792.

Section 2, paragraph 5
> e UPA was advertised. s/that would provides/that would provide/ Implementatio
>                                    ^^^^^^^^
The modal verb "would" requires the verb's base form.

Section 2, paragraph 6
> nd external prefix is advertised in it's own LSA, so the above optimisation 
>                                     ^^^^
Did you mean "its" (possessive pronoun) instead of "it's" (short for "it is")?

Section 2, paragraph 9
> efix is advertised with a metric larger then MAX_PATH_METRIC (0xFE000000, see
>                                  ^^^^^^^^^^^
Did you mean "larger than"?

Section 3, paragraph 6
> s initially defined in [RFC7370], e.g.,: - SRv6 Locator [RFC9352] - Extended
>                                       ^^
Did you just mean "," or ":"?

Section 1:
- Expand terms on first use; at least IGP and possibly OSPF, IS-IS, BGP, PIC, etc.
- "multi-area/level and/or multi-domain" Is there a reason to split area/level from domain?
- "Summarization" appears to be a key concept here. Can you add a pointer to a definition?

It's unfortunate that the abbreviation "UP" is being used to indicate that something is down.

===NITS FOLLOW===

Abstract: "the two new flags" haven't been described yet. Maybe drop "the"

1: "OVERLOAD bit" => "the OVERLOAD bit", "high metric" => "a high metric", "all-links" => "all links", "such load" => "such a load", "advertise prefix" => "advertise a prefix"

4.2: Remove comma after "ASBRs"

Hi Peter, all, 

Thank you for the discussion and addressing raised points in [1]. -10 looks much more better.

I still think that we can git rid of the UP-flag and leave it to when we have more clarity on the intended application use, though. 

Cheers,
Med

[1] https://mailarchive.ietf.org/arch/msg/lsr/ZBY0n8RSx0NAOuG_00E6dW6OgEg/

Are there any operational considerations worth mentioning in the document? Would setting the UP flag cause other properly implemented systems not supporting the flag to change behaviour? I assume this would not be the case and that is why no operational considerations were added?

Thank you to Dale Worley for the GENART review.  I have no further GEN-specific feedback.