IS-IS Traffic Engineering (TE) Metric Extensions
draft-ietf-lsr-isis-rfc7810bis-05

I do have a question and a suggestion:

1: From the shepherd writeup:
(7) Has each author confirmed that any and all appropriate IPR
disclosures required for full conformance with the provisions of BCP 78
and BCP 79 have already been filed. If not, explain why.

    Pending Response at WG adoption:
    Authors: S Giacolone, D Ward 
    Contributors: A Atlas, C Filsfils

    There was no IPR poll done during/after WGLC. 
---
I'm not sure I really understand what happened, and if they ever replied - I'll trust that the AD did the right thing.

2: The abstract says: This document obsoletes RFC 7810. 
Once this is published it won't be clear that this is a -bis. It might be useful to include something like: This document obsoletes RFC 7810 (see the Appendix for details). 
(if the RFC editor will allow it that is :-))

I also agree with Mirja's comments.

This is well written document, thank you. One small thing:

4.5.  Unidirectional Residual Bandwidth Sub-TLV


   Residual Bandwidth: This field carries the residual bandwidth on a
   link, forwarding adjacency [RFC4206], or bundled link in IEEE
   floating-point format with units of bytes per second.

Please add a Normative reference to the IEEE document.

In section 13 it seems a little awkward to reference the "first version" and "second version" of the document since they will be published with different RFC numbers. Might be clearer to say RFC 7810 in the first instance and "this document" in the second instance.

Why does this need to least more than the usual 5 authors, especially since there is already a contributors section that says the entries should be treated as co-authors?

The nice tidy diff against RFC 7810 and description in Appendix A made this
document very easy to review; thank you!

That said, I think that there are some lingering factual errors that will
require some text changes; in particular, relating to the IANA
registrations.  E.g,. Section 2 shouldn't say "this document registers new
IS-IS TE sub-TLVs" and "this document registers several sub-TLVs", since
the sub-TLVs are not new.  Similarly, one could make a case that the IANA
considerations should request for IANA to update the registrations for the
indicated sub-TLVs to point to this document instead of RFC 7810.

(I'm still balloting No Objection and not Discuss, because I trust the
responsible AD to make the right thing happen.)

Rich version of this review at:
https://mozphab-ietf.devsvcdev.mozaws.net/D5671


This seems like a straightforward document.

Looking at S 14, I see the following "The following people contributed
substantially to the content of this
document and should be considered co-authors". Is this just an
artifact of the 5 author limit? Perhaps we should make an exception.







COMMENTS
S 4.1.
>   
>      A bit: The A bit represents the Anomalous (A) bit.  The A bit is set
>      when the measured value of this parameter exceeds its configured
>      maximum threshold.  The A bit is cleared when the measured value
>      falls below its configured reuse threshold.  If the A bit is clear,
>      the sub-TLV represents steady-state link performance.

Just to be clear, I have no way of knowing remotely what the threshold
is, right?


S 4.2.
>      value (in microseconds) over a configurable interval, encoded as an
>      integer value.
>   
>      Implementations MAY also permit the configuration of an offset value
>      (in microseconds) to be added to the measured delay value, to
>      facilitate the communication of operator-specific delay constraints.

I'm probably missing something, but I don't think I understand the
purpose of this. Would you mind adding a sentence or two about how you
would use this offset?


S 4.3.
>   
>      RESERVED: This field is reserved for future use.  It MUST be set to 0
>      when sent and MUST be ignored when received.
>   
>      Delay Variation: This 24-bit field carries the average link delay
>      variation over a configurable interval in microseconds, encoded as an

So the peer has no idea of what that interval is, right?

Based on the TSV-ART review (Thanks Yoshi!) and the information provided in the shepherd write-up, I understand that this document intends to "fix" a specific errata. However, there are  IPPM RFCs for each of these metrics that provide further insights on how the calculate them correctly, e.g. rfc3393 IP Packet Delay Variation. I will not block publication on this doc but I would still welcome and recommend if the respective references could be added to provide at least some guidance instead of just declaring calculation out of scope.

* Backward compatibility

There is text in Appendix A that provides guidance to implementers on how to handle TLVs with length 5 from RFC7810. I think this would be much more helpful inside the main body of the document. I was wondering how the backward compatibility was handled until I read through the Appendix that lists the *diffs*.