BGP Link-State Shortest Path First (SPF) Routing
draft-ietf-lsvr-bgp-spf-51

Thanks to David Mandelberg for his secdir review.

The tiniest of nits, and only because I happened to see it:
Section 4, para2, sentence 1:  nit:  'is up the operator' should be 'is up to the operator'.

# Éric Vyncke, INT AD, comments for draft-ietf-lsvr-bgp-spf-43
CC @evyncke

Thank you for the work put into this document, even if it seems "weird" to my non-RTG mind to use BGP to convey link-state and to use these state for a SPF ;-)

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

Special thanks to Ketan Talaulikar for the shepherd's detailed write-up including the WG consensus *but* and the justification of the intended status is rather weak.

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS (non-blocking)

### Use of BGP transport for link-state routing

Take the following with a grain of salt, as I am not a routing expert... So, there is a 3rd link-state routing protocol in addition to OSPF and IS-IS... not a problem, but how is a link failure detected ? Is BFD a requirement for such a deployment (there is no quick hello messages in BGP AFAIK), BFD is only mentioned in section 4.2 but not in section 4.1.

How is TCP congestion (and the associated delay in sending link-state information) handled? I.e., all routers may not have the same view for a longer period of time than in traditional link-state IGP.

### MSDC acronym

I have often seen MSDC as "Massively Scalable Data Center" and not as `Massively Scaled Data Centers`. Perhaps a double check ?

### Abstract & section 1

`L3 routing` should rather be "IP routing" or "layer-3 routing".

### Section 1

s/high degree of Equal Cost Multi-Path (ECMPs)/high degree of Equal Cost Multi-Path (ECMPs) load-balancing/ ?

Should `LFA` be expanded before first use ?

### Section 4

Suggest adding one sentence about "The next sub-sections will describe several deployment models" or something similar.

### Section 4.2

The expansion of "EoR" appears after its first use.

### Section 4.3

This looks very similar to PCEP deployment model. Should there be a comparison ?

### Section 5.2.2

Are the local/remote IPv6 address global or link local ?

### Section 6.2

FYI (no need to reply), draft-chroboczek-intarea-v4-via-v6 is about having a next hop in a different address family.

### Section 8

There are two different `TBD` code points defined in this I-D, please use "TBD1" and "TBD2" to avoid any ambiguity.

### Section

Should this rather be a "MUST" in `it is RECOMMENDED that the TCP Authentication Option (TCP-AO)` ? Especially, since nothing is written in the I-D about using the Generalised TTL Security Mechanism making easier for a remote attacker to inject in a TCP session (assuming sequence number are known).

# Internet AD comments for draft-ietf-lsvr-bgp-spf-41
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

### S6.2

* "is an implementation matter" ->
  "is an implementation and/or policy matter"  (just a suggestion)

## Nits

### S1.2

* "100s or 1000s prefixes" -> "100s or 1000s of prefixes"

* "verification of the whether or not to advertise" ->
  "verification of whether or not to advertise"

### S3

* "to insure backward compatibility" ->
  "to ensure backward compatibility"

### S4.2, S4.3

* "liveliness" -> "liveness" (RFC 5880 only speaks of "liveness")

### S5.2.2

* "When 0 is advertised and there parallel unnumbered links" ->
  "When 0 is advertised and there are parallel unnumbered links"

### S5.2.4

* s/insure/ensure/

  (in general throughout the doc, as well)

Section 5.1.2, paragraph 3
>    The BGP-LS attribute may potentially be quite large depending on the
>    amount of link-state information associated with a single Link-State
>    NLRI.  The BGP specification [RFC4271] mandates a maximum BGP message
>    size of 4096 octets.  It is RECOMMENDED that an implementation
>    support [RFC8654] in order to accommodate a greater amount of
>    information within the BGP-LS Attribute.  BGP SPF speakers MUST
>    ensure that they limit the TLVs included in the BGP-LS Attribute to
>    ensure that a BGP update message for a single Link-State NLRI does
>    not cross the maximum limit for a BGP message.  The determination of
>    the types of TLVs to be included by the BGP SPF speaker originating
>    the attribute is outside the scope of this document.  When a BGP SPF
>    speaker finds that it is exceeding the maximum BGP message size due
>    to addition or update of some other BGP Attribute (e.g., AS_PATH), it
>    MUST consider the BGP-LS Attribute to be malformed and the attribute
>    discard handling of [RFC7606] applies.

I support John and his DISCUSS here. It is clear that what RFC 7606 describes as handling of malformed attributes applies to the reception of a BGP UPDATE message, not to the construction of one. Therefore, using "attribute discard" as described in RFC 7606 doesn't directly translate to the transmission process.

One way to handle this would be to say:

"When a BGP SPF speaker determines that adding the BGP-LS Attribute would cause the BGP message to exceed the maximum permissible size, it MUST omit the BGP-LS Attribute from the message."

The IANA review of this document seems to not have concluded yet.

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

Document references draft-ietf-lsvr-applicability-17, but -20 is the latest
available revision.

Section 5.2, paragraph 2
> ker receiving a BGP Update containing a SPF Status TLV in the BGP-LS attribut
>                                       ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Section 5.2.2.1, paragraph 2
> ker receiving a BGP Update containing a SPF Status TLV in the BGP-LS attribut
>                                       ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Section 5.2.2.2, paragraph 5
> ker receiving a BGP Update containing a SPF Status TLV in the BGP-LS attribut
>                                       ^
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

Section 6, paragraph 1
> t recent Sequence Number TLV, i.e., highest sequence number is selected. 3. T
>                                     ^^^^^^^
A determiner may be missing.

Section 6.1.1, paragraph 3
>  have been used. The algorithm is comprised of the steps below: 1. The curre
>                                ^^^^^^^^^^^^^^^
Did you mean "comprises" or "consists of" or "is composed of"?

Section 6.3, paragraph 15
> ised so that an unnumbered link can used in the SPF computation for multiple
>                                     ^^^^
Did you mean "be used"?

Section 6.3, paragraph 25
> 6.5.2. Node Failure Convergence By default [RFC4271], all the NLRI advertised
>                                 ^^^^^^^^^^
Did you mean: "By default,"?

Section 9, paragraph 1
> cting the BGP-LS-SPF AFI/SAFI or vice-versa, isolation mechanisms such as se
>                                  ^^^^^^^^^^
The expression "vice versa" is spelled without hyphens.

Thank you to Joel Halpern for the GENART review.

** Section 8.2
   Deprecated TLV types include the SPF
   Capability TLV type, IPv4 Prefix Length TLV type, and IPv6 Prefix
   Length TLV type.
...
   The early allocation assignments for the TLV types SPF Capability
   (1180), IPv4 Prefix Length (1182), and IPv6 Prefix Length (1183) are
   no longer required and are to be deprecated.

The names “IPv4 Prefix Length” and “IPv6 Prefix Length” noted twice are inconsistent with the names in the IANA registry:

1182	IPv4 Link Prefix Length	[draft-ietf-lsvr-bgp-spf-20]

1183	IPv6 Link Prefix Length	[draft-ietf-lsvr-bgp-spf-20]

It appears that “Link” is missing in both names.

Thanks for all your work on the document and the productive discussion.