Bit Index Explicit Replication (BIER) Support via IS-IS
draft-ietf-bier-isis-extensions-11

The document is StdTrack, but contains 2 normative references to experimental RFCs - was this called out?

Nit Section 2:
" BAR  BIER Algorithm.  Algorithm used to calculate nexthops." -- the other definitions have a colon (:) after the term.

Section 4.1:
" Additionally, per supported bitstring length in the sub-domain, each router will advertise the necessary label ranges to support it."
I found this sentence really difficult to parse - can it be reworded? I'm actually not sure what it is trying to say, the "per supported bitstring length" threw me. "Additionally, each router will advertise the label ranges to support the bitstring length in the sub-domain"? Sorry, really don't get it.

Section 4.2:
"   o  When the Prefix Attributes Flags sub-TLV is present N flag MUST" - I think that this is missing "the"; sub-TLV is present the N flag..."

Section 5.1.  Multi Topology and Sub-Domain
"A router receiving an <MT,SD> advertisement which does not match the locally configured pair MUST report a misconfiguration of the received <MT,SD> pair."
When I first read this I thought "Yikes, for every non-matching advertisement it must report? Won't that DOS the logging?", and so I made a note. Then, further on I found Section 5.6, which talks about this sort of thing. Please insert a "See Section 5.6" (or similar) to prevent others from panic :-)

Section 5.7.  Flooding Reduction
   "BIER domain information SHOULD change infrequently."
What does "infrequently" mean here? Brushing my teeth frequently has very different meaning to washing my car frequently - is this "around one a minute" or "on the order of many months"? 

Section 7:
"Implementations must assure that malformed TLV and Sub-TLV permutations do not result in errors which cause hard protocol failures." - I'm not a security AD, but this feels inadequate. Is "malformed TLV and Sub-TLV " the only *only* security issues? And, does this really *mean* anything (I could equally write "Don't write bad code. K?")

I support EKR's discuss position.

§6.1:

>   BAR  BIER Algorithm. 0 is the only supported value defined in this
>      document.  Other values may be defined in the future. 8 bits

I expected to find a registry for this in the IANA section. Does some other document establish this registry? If so, is 0 already registered? If "no" and "no", then this document needs to do both. If "yes" and "no", then this document need to register "0". If "yes" and "yes", then the phrase "in this document" needs to be replaced by "in [RFCxxxx]".

>   subdomain-id:  Unique value identifying the BIER sub-domain. 1 octet

You need to scope "Unique" (e.g. "Value identifying the BIER sub-domain, unique within the domain.")

>   Local BitString Length (BS Len):  Encoded bitstring length as per
>      [RFC8296]. 4 bits.
>
>   Max SI  Maximum Set Identifier (section 1 of [RFC8279]) used in the
>      encapsulation for this BIER sub-domain for this bitstring length,
>      1 octet.  Each SI maps to a single label in the label range.  The
>      first label is for SI=0, the second label is for SI=1, etc.

This is a bit confusing, since the fields appear in the opposite order from this in the diagram. Would recommend re-ordering to match.

I find excessive use of unexpanded acronyms to be difficult for somebody not versed in the technology.

I support Ekr's DISCUSS.

Substantive Comments:

I support Ekr's DISCUSS comment.

Abstract: It would be nice to see a bit more context in the abstract. Also, the entire abstract is a sentence fragment. (IMO the last paragraph in §1 would make a good abstract.)

§5.2: The "MUST" seems like a statement of fact.

Editorial Comments and Nits:

§3: The draft uses an odd organization. IANA considerations (along with security considerations) should be near the end of the draft body.

Coming from Joe Clarke in his OPS-DIR review:
The only substantive comment I have is in section 5.6 regarding reporting
misconfigurations.  What is not stated is _how_ this is to be done.  There is
no reference to a standard BIER or IS-IS mechanism with how this reporting
should be carried out.  For operational consistency, I feel some guidance
should be offered.

The one nit I noticed is in section 6.2:

OLD:

Label ranges in multiple sub-sub-TLV MUST NOT overlap.

NEW:

Label ranges in multiple sub-sub-TLVs MUST NOT overlap.

Thank you for addressing my DISCUSS

I support EKRs discuss and think it's just s reference that needs to be added as this is an extension.

I also had a question on 5.6, what does dampen the logging mean?  Is this something that is programmed and can't be changed or configurable in case debugging is not adequate?

1) Minor comment: The shepherd write-up says wrongly that this docoment is experimental. Assume that was changed at some point during the process.

2) There further seem to be unaddressed comments in the write-up... 

3) Please use rfc8174 boilerplate.

4) The IANA section should contain the exact name and URL of the registry that the new sub-TLV value should be addd to.

5) The document should maybe provide further guidance on how the stated goal in the security  considerations section could be achieved.

Thanks for addressing my DISCUSS point.

* Section 4.2.

"When the Prefix Attributes Flags sub-TLV is present N flag MUST be set."

Given that RFC7794, which defines this sub-TLV, clearly mentions that setting this flag is purely optional, is there a reason this is made mandatory? I do not see any checking for this flag in the behavior.