IS-IS Extensions in Support of Inter-Autonomous System (AS) MPLS and GMPLS Traffic Engineering
draft-ietf-lsr-isis-rfc5316bis-07

I support Alvaro's DISCUSS, and add my own comments related to his first point:

The first two SHOULDs in Section 3.1 would benefit from some guidance about when an implementer might opt to deviate from that advice.  This occurs again Sections 3.3.4, 3.4.1, 3.4.2, the top of Section 4 (two SHOULDs) and the bottom of Section 4 (two SHOULD NOTs).

Given Section 6.3, I think RFC7981 should be a normative reference rather than an informative one.

I think RFC4271 also needs to be normative since it's referenced by a SHOULD.

I support Alvaro Retana's DISCUSS position.

Could the example HMAC use in the Security Considerations section be updated from HMAC-MD5 to something more modern (eg HMAC-SHA2) or is there a valid operational reason to stick with HMAC-MD5 ?

Thank you to Stephen Farrell for the SECDIR.

I support Alvaro Retana's DISCUSS position.

# Éric Vyncke, INT AD, draft-ietf-lsr-isis-rfc5316bis-04
CC @evyncke

Thank you for the work put into this document especially about `This document builds on RFC 5316 by adding support for IPv6-only operation.`

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

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

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS

### Section 3.1

```
   The Router ID field of the inter-AS reachability TLV is 4 octets in
   length, which contains the IPv4 Router ID of the router who generates
   the inter-AS reachability TLV.  The Router ID SHOULD be identical to
   the value advertised in the Traffic Engineering Router ID TLV
   [RFC5305].  If no Traffic Engineering Router ID is assigned, the
   Router ID SHOULD be identical to an IP Interface Address [RFC1195]
   advertised by the originating IS.
```

AFAIK, the router ID is 'just' a 32-bit value that it is protocol version agnostic. So, s/IPv4 Router ID/Router ID/ ?

Suggest: s/IP Interface Address [RFC1195]/IPv4 Interface Address [RFC1195]/ ?

### Section 6.1 & 6.2

`This document defines the following new IS-IS TLV type` but this type is already defined in RFC 5316, so does it still qualify as "new" ? Propose to rewrite the IANA section to simply request IANA to update the registries to point to this I-D rather than to RFC 5316.

### Section 7

While Les was not an author of RFC 5316, he is an author of this I-D, so no more need to acknowledge him ;-)

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

Thanks for addressing my DISCUSS.
https://mailarchive.ietf.org/arch/msg/lsr/gj8iu7HFLFYZdOudzYHKtEY9TFI/

Thanks for the work on this document.

I also support Alvaro's DISCUSS position.

# GEN AD review of draft-ietf-lsr-isis-rfc5316bis-04

CC @larseggert

## Comments

### Section 3.1, paragraph 2
```
      0                   1                   2                   3
      0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |   Router ID                                     (4 octets)    |
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |   default metric                              | (3 octets)
     +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
     |     Flags     |                                 (1 octet)
     +-+-+-+-+-+-+-+-+
     |sub-TLVs length|                                 (1 octet)
     +-+-+-+-+-+-+-+-+-+-+-+-
     | sub-TLVs ...                                    (0-246 octets)
     +-+-+-+-+-+-+-+-+-+-+-+-
```
It's somewhat unusual to have a header diagram that doesn't fill its
rows and uses that space to instead define the field length for a
second time (it's already defined by the horizontal space used,
which is why there is bit count at the top.)

### Section 3.3.1, paragraph 4
```
     The remote AS number field has 4 octets.  When only 2 octets are used
     for the AS number, the left (high-order) 2 octets MUST be set to 0.
     The remote AS number sub-TLV MUST be included when a router
     advertises an inter-AS TE link.
```
Would the higher-order octets not be zero anyway if the value is <= 0xffff?

### Section 3.3.4, paragraph 4
```
     If the originating node does not support IPv4, the IPv6 Local ASBR ID
     sub-TLV MUST be present in the inter-AS reachability TLV.  Inter-AS
     reachability TLVs which have a Router ID of 0.0.0.0 and do NOT have
     the IPv6 Local ASBR ID sub-TLV present MUST be ignored.
```
NOT is not an RFC2119 term.

## 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 5, paragraph 5
```
 this document. No additional acknowledgments are made for the new version (
                              ^^^^^^^^^^^^^^^
```
Do not mix variants of the same word ("acknowledgment" and "acknowledgement")
within a single text.

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

I support Alvaro's discuss.

I would like to thank Menachem for the OPSDIR review.

I also have a few minor nits for the authors to consider:

(1) p 3, sec 2.  Problem Statement

   Two methods for determining inter-AS paths are currently being
   discussed.

It was unclear what is meant by this, please clarify.  I.e., Do you mean described in this document?  Or there is ongonig discussion in the WG?  Or ...


(2) p 5, sec 2.2.  Per-Domain Path Determination

   Suppose that the Path message enters AS2 from R3.  The next hop in
   the ERO shows AS3, and R5 must determine a path segment across AS2 to
   reach AS3.  It has a choice of three exit points from AS2 (R6, R7,
   and R8), and it needs to know which of these provide TE connectivity
   to AS3, and whether the TE connectivity (for example, available
   bandwidth) is adequate for the requested LSP.
   Alternatively, if the next hop in the ERO is the entry ASBR for AS3
   (say R9),

Should this be "an entry ASBR" rather than "the entry ASBR"?


(3) p 7, sec 3.  Extensions to ISIS-TE

     Also, two other new sub-TLVs are defined for
   inclusion in the IS-IS router capability TLV to carry the TE Router
   ID when the TE Router ID is needed to reach all routers within an
   entire IS-IS routing domain.

As a nit, I would put the last sentence above into its own paragraph.  "This document also defines two other new sub-TLVs ..."


(4) p 8, sec 3.1.  Inter-AS Reachability TLV

   Rsvd bits MUST be zero when originated and ignored
   when received.

Perhaps "Reserved (Rsvd) bits MUST be zero ..."