Distribution of Link-State and Traffic Engineering Information Using BGP
draft-ietf-idr-rfc7752bis-17

# Internet AD comments for draft-ietf-idr-rfc7752bis-14
CC @ekline

Thanks to Brian for the INT DIR review.

"""
Reviewer: Brian Haberman
Review result: Ready with Nits

I reviewed the diff of this draft with RFC 7752 and did not find any issues
related to the normal concerns that come from Internet Area reviews. I do agree
with John Scudder's DISCUSS on the subtle changes to BGP processing rules.
"""

Thanks for addressing my DISCUSS and comments. I have a few residual comments, which I sent in a followup email to the DISCUSS thread.

Note that my knowledge here is severely limited but I did compare the bis to the rfc and for as far as I could understand, the changes look reasonable.

This is feedback on text that remains unchanged from RFC7752.  The underlying process of BGP-LS seems to be sharing “link-state and TE information ... collected from networks ... with external components using the BGP routing protocol” (per Section 1).  The Security Considerations (Section 10) reiterate that “[p]rocedures and protocol extensions defined in this document do not affect the BGP security model”.   Additionally, the following caution is noted:

   Additionally, it may be considered that the export of link-state and
   TE information as described in this document constitutes a risk to
   confidentiality of mission-critical or commercially sensitive
   information about the network.  BGP peerings are not automatic and
   require configuration; thus, it is the responsibility of the network
   operator to ensure that only trusted BGP Speakers are configured to
   receive such information.

Not fully appreciating the deployment scenarios of BGP-LS, but also not finding any written constraints or a definition of what “external components” means, it seems like this “mission-critical or commercially sensitive information about the network” could be shared via BGP without transport-provided confidentiality (e.g., only TCP-AO) subjecting it to on-path inspection.  Why would that be acceptable?  Is there some expectation that these BGP peers are directly connected, or that these “external components” are in the same administrative domain?  How should it be handled if they are not-directly attached and in separate administrative domains?

Would it make sense to add a recommendation to use transport mode protection (e.g., IPsec) for BGP-LS communication in some prescribed circumstances?

Thanks for working on this -bis specification.

A comparison of this document to RFC9029 and RFC7752 didn't raise TSV related issues in my review.

# Éric Vyncke, INT AD, comments for draft-ietf-idr-rfc7752bis-14
CC @evyncke

Thank you for the work put into this document, it appears to add welcomed clarification to RFC 7752. Due to lack of time, I have only reviewed the diff with RFC 7752 and I trust Brian Haberman's intdir review at:
https://datatracker.ietf.org/doc/review-ietf-idr-rfc7752bis-14-intdir-telechat-haberman-2022-12-14/ (thank you Brian).

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

Special thanks to Jeffrey Haas for the *very detailed* shepherd's detailed write-up including the WG consensus and the justification of the intended status. 

I have also appreciated that you list the RFC 7752 authors as contributors to this document.

I hope that this review helps to improve the document,

Regards,

-éric
## COMMENTS

### Section 5.2.2

The IPv4 address family has also link-local addresses (RFC 3927), to the text about link-local should be address family agnostic, i.e., either use "IPv6/IPv4 link-local address" or simply "link-local address". As this is mostly academic (IPv4 LLA are usually never used by routers), I am not raising a DISCUSS though.

### Section 5.3.2.3

What is the expected behaviour of the receiver/consumer when the trailing be are not 0 ?

### Section 5.3.2.7

Unsure what a `subset of the FQDN` is ... Do you mean a substring ?

### Section 5.4

Rather than `The Enterprise Codes are listed at <http://www.iana.org/assignments/enterprise-numbers>.`, suggest to use a normative reference.

## NITS

### ISIS vs. IS-IS

Isn't the right writing "IS-IS" ? (I saw at least one such occurence)

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

# GEN AD review of draft-ietf-idr-rfc7752bis-14

CC @larseggert

## Comments

I have restricted my review to the parts that have changed
relative to RFC7752.

### Section 5.1, paragraph 5
```
     To compare NLRIs with unknown TLVs, all TLVs within the NLRI MUST be
     ordered in ascending order by TLV Type.  If there are multiple TLVs
     of the same type within a single NLRI, then the TLVs sharing the same
     type MUST be in ascending order based on the value field.  Comparison
     of the value fields is performed by treating the entire field as
     opaque binary data and ordered lexicographically.
```
I think you need to say a bit more about what variant of lexicographic
ordering you have in mind, since the values can be of different
lengths. (I assume value bytes are compared left-to-right?)

### Section 7.2, paragraph 0
```
  7.2.  Guidance for Designated Experts
```
Much of this seems to duplicate a variant RFC7120 early allocation
procedures, which is a bit disappointing. If RFC7120 isn't suitable,
it'd be better to fix it rather than to define one-offs like this
one.

### IANA

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

## Nits

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.

### Grammar/style

#### Section 2.1, paragraph 2
```
r of the path. This may lead to sub-optimal paths, makes alternate/back-up p
                                ^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 2.1, paragraph 3
```
 of the network, but this may be sub-optimal when the network is subject to a
                                 ^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 4, paragraph 2
```
 or auxiliary Router-IDs of nodes, etc.. It is desirable to keep the dependen
                                      ^^
```
Two consecutive dots.

#### Section 5.1, paragraph 5
```
], by using capability code 1 (multi-protocol BGP), with AFI 16388 / SAFI 71
                               ^^^^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 5.2, paragraph 18
```
omains. The Identifier field carries a 8-octet BGP-LS Instance Identifier (I
                                     ^
```
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

#### Section 5.2, paragraph 21
```
port multiple IGP instances, an implementations needs to support the configur
                             ^^^^^^^^^^^^^^^^^^
```
The plural noun "implementations" cannot be used with the article "an". Did you
mean "an implementation" or "implementations"?

#### Section 5.2.1.2, paragraph 1
```
 provides a similar functionality. Hence the inclusion of the BGP-LS Identif
                                   ^^^^^
```
A comma may be missing after the conjunctive/linking adverb "Hence".

#### Section 5.2.3, paragraph 1
```
ny trailing bits MUST be set to 0. Thus the IP Prefix field contains the most
                                   ^^^^
```
A comma may be missing after the conjunctive/linking adverb "Thus".

#### Section 5.3.1.5, paragraph 3
```
k TLV The following bits are defined and the reserved bits MUST be set to zer
                                    ^^^^
```
Use a comma before "and" if it connects two independent clauses (unless they
are closely connected and short).

#### Section 5.3.3.6, paragraph 1
```
realize that area 1 is partitioned. Also if R2 continues to report Node and P
                                    ^^^^
```
A comma may be missing after the conjunctive/linking adverb "Also".

#### Section 5.10, paragraph 2
```
istries listed in the following sub-sections are BGP-LS specific and are acc
                                ^^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 7.1.1, paragraph 2
```
ted in the form of an Internet-Draft but MAY arrive in any form that can be
                                    ^^^^
```
Use a comma before "but" if it connects two independent clauses (unless they
are closely connected and short).

#### Section 7.1.1, paragraph 2
```
 that can be reviewed and exchanged amongst reviewers. 2. The designated expe
                                    ^^^^^^^
```
Do not mix variants of the same word ("amongst" and "among") within a single
text.

#### Section 7.2, paragraph 10
```
 MUST allow the operator to configure a 8-octet BGP-LS Instance-ID. Refer to
                                      ^
```
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

#### Section 8.2.2, paragraph 23
```
een BGP Speaker and BGP-LS Consumers but their discussion is outside the sco
                                    ^^^^
```
Use a comma before "but" if it connects two independent clauses (unless they
are closely connected and short).

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Hi,

Thanks for this update to RFC 7752.  I've been short of time this week, and hence I'm somewhat relying on the responsible AD and OPSDIR review (thanks Gyan!).

I did have a couple of minor comments:

On:
   To compare NLRIs with unknown TLVs, all TLVs within the NLRI MUST be
   ordered in ascending order by TLV Type.  If there are multiple TLVs
   of the same type within a single NLRI, then the TLVs sharing the same
   type MUST be in ascending order based on the value field.  Comparison
   of the value fields is performed by treating the entire field as
   opaque binary data and ordered lexicographically.

I'm not convinced that lexicographical comparison of binary data is that well-defined, since it is generally applied to strings, so I this could potentially be stated more clearly.

It also wasn't obvious to me why BGP-LS Attribute TLVs SHOULD be ordered, but are allowed to be unordered.  I.e., if sending them unordered doesn't  break interop then is it really a 'should' rather than a 'SHOULD'?

Regards,
Rob