[Bier] Rtgdir last call review of draft-ietf-bier-ospfv3-extensions-07

[Adrian Farrel via Datatracker <[e-mail]>]

Reviewer: Adrian Farrel
Review result: Has Issues

This review was conducted in response to a request from the responsible
AD during AD evaluation. 

The document describes extensions to OSPFv3 to support the BIER in MPLS
encapsulation.

Please let me know if you hve any follow-up questions.

Cheers,
Adrian


== WIDER THOUGHTS ==

I did not read RFC 8444 (perhaps I should have), but I did wonder why
this should be an entirely separate document rather than re-using some
of the material. For example, the format of the BIER Sub-TLV appears to
be identical so why specify it twice?

But that said, the way you have things written, RFC 8444 is not a 
normative reference.

---

Was this document copied to the LSR working group at any point in its
life? I looked at the WGLC email and it didn't seem to get shared at 
that time. It might be a very sensible thing to do, while the document
is being polished and advancing, to send a note to LSR to ask them for
review - they are supposed to be the OSPF subject experts.


== MAJOR ==

Section 2.1 has

      BFR-id: A 2 octet field encoding the BFR-id, as documented in
      section 2 of [RFC8279].  If the BFR is not locally configured with
      a valid BFR-id, the value of this field is set to 0, which is
      defined as illegal in [RFC8279].

This is not clear and possibly a problem. You are saying that a BFR that
does not have a locally configured (valid) BFR-id MUST continue to
advertise the BIER MPLS information, but with a BFR-id that is not valid
(illegal) and which will (presumably) result in the information being
discarded by all receivers.

Surely it is better to say that a BFR that does not have a valid BFR-id
MUST NOT advertise its BIER MPLS information?

---

Section 4 needs to tell the IANA which sub-range to allocate the 
codepoints from.


== MODERATE ==

Section 2.1 shows

      Length: Variable, dependent on sub-TLVs.

That's not helpful enough. Length of what? Including or not including
the Type and Length? What units?

Or maybe, "as defined in <xref>"

---

Section 2.1

      defined by the BAR value.  Values are defined in the "IGP
      Algorithm Types" registry.

Can you be more specific about this registry giving a reference to the
RFC that defined it?

---

2.2

      Reserved: SHOULD be set to 0 on transmission and MUST be ignored
      on reception.

Really only "SHOULD". You mean, my implementation MAY fill this field 
with 1s? Doesn't that mean that forward compatibility is broken?

This is a frequent niggle. Your use of SHOULD/MUST/MAY describes 
implementations of *this* document. If a future document defines a 
meaning for one of these bits, then it says "MUST be set to 1 to
indicate" and that is not in conflict with this document because it is a
separate specification.




== MINOR ==

The document title is misleading. The document only describes extensions
to support the MPLS encapsulation of BIER. I suggest you change it to:

   OSPFv3 Extensions to Support BIER Encapsulation in MPLS

Similarly, the title to Section 2 might become

2.  Flooding of BIER Information for MPLS in OSPFv3

---

Abstract

   Support for other encapsulation types
   is outside the scope of this document.  The use of multiple
   encapsulation types is outside the scope of this document.

Isn't the second sentence superfluous? You couldn't have multiple
encapsulations without support for another encapsulation.

---

Introduction

   The BIER header
   contains a bit-string in which each bit represents exactly one BFER
   to forward the packet to.  The set of BFERs to which the multicast
   packet needs to be forwarded is expressed by setting the bits that
   correspond to those routers in the BIER header.

I think the first sentence is not true. Probably...
s/to forward the packet to/to which the packet could be forwarded/

---

Introduction

   This document describes
   extensions to OSPFv3 necessary to advertise BIER specific information
   in the case where BIER uses MPLS encapsulation as described in
   [RFC8296].

This is misleading. These extensions are only necessary when OSPFv3 is
used as the way to advertise the information. Perhaps...

   This document describes
   extensions to OSPFv3 to enable it to advertise BIER specific 
   information in the case where BIER uses MPLS encapsulation as
   described in [RFC8296].

---

BCP 14 boilerplate

1. You should move this to its own subsection: probably 1.1
2. idnits points out that you are not using the correct boilerplate. It
   should read...

   The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
   "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and
   "OPTIONAL" in this document are to be interpreted as described in
   BCP 14 [RFC2119] [RFC8174] when, and only when, they appear in all
   capitals, as shown here.

---

Section 2

   The Sub-TLV defined in this section MAY be carried
   in the below OSPFv3 Extended LSA TLVs [RFC8362]:

I think this is an incorrect use of "MAY". Perhaps...

   The Sub-TLV defined in this section can be carried
   in the  OSPFv3 Extended LSA TLVs [RFC8362] listed below:

---

2.1

   Each BFR sub-domain MUST be associated

Is that the same as a BIER sub-domain that was mentioned earlier?

---

2.1

   If the association between
   BIER sub-domain and OSPF topology advertised in the BIER sub-TLV by
   other BFRs is in conflict with the association locally configured on
   the receiving router, the BIER Sub-TLV MUST be ignored.

To be totally clear, "the received BIER Sub-TLV MUST be ignored."

Does "ignored" mean "not propagated" or "not processed into the BIER
system"?

Does "OSPF topology" mean "value of the MT-ID field"?

But I am not quite sure. I think this means that multiple sub-domains 
with the same sub-domain-ID can exist in the same OSPF network with 
different MT-IDs, but this will just fragment the sub-domain into 
multiple unassociated sub-topologies. It also seems to mean that one
MT-IDs can be used for different sub-domains (each with its own sub-
domain-ID) such that multiple sub-domains exist. In other words, each
BFR has a {sub-domain-ID, MT-ID} pair configured, but multiple 
overlapping pairs are allowed in the network.

Thus "in conflict with" may be a bit confusing!

---

2.1

   If the MT-ID value is outside of the values specified in [RFC4915],
   the BIER Sub-TLV MUST be ignored.

Again, need clarity on "ignored"

And...

   If a BFR advertises the same Sub-domain-ID in multiple BIER sub-TLVs,
   the BFR MUST be treated as if it did not advertise a BIER sub-TLV for
   such sub-domain.

This is a variation of "ignored" and is double trouble:
1. "Treated as if it did not" means "don't propagate"?
2. If the first advertisement has been processed when the second 
   arrives, is the first dug out and discarded? Or do you mean 
   "multiple BIER sub-TLVs within the same OSPFv3 Extended LSA TLV"?
   What if it happens in different OSPFv3 Extended LSA TLVs?

---

2.1

   All BFRs MUST detect advertisement of duplicate valid BFR-IDs for a
   given MT-ID and Sub-domain-ID.  When such duplication is detected by
   the BFR, it MUST behave as described in section 5 of [RFC8279].

But you have just defined some behavior for this situation. What takes
precedence: this document or 8279?

---

2.1

   The supported BAR and IPA algorithms MUST be consistent for all
   routers supporting a given BFR sub-domain.  A router receiving BIER
   Sub-TLV advertisement with a value in BAR or IPA fields which does
   not match the locally configured value for a given BFR sub-domain,
   MUST report a misconfiguration for such BIER sub-domain and MUST
   ignore such BIER sub-TLV.

Assuming that "ignore" still allows propagation, doesn't this cause a
reporting storm in the network that is particularly bad when half of the
network is out of synch? Should you mandate that the reporting must be 
subject to a configurable threshold? I do see the final paragraph of the
security section.

---

2.2

   It
   MAY appear multiple times in the BIER Sub-TLV.

There are two things hiding here.
- Is zero appearances allowed? If so, why would you choose to do that?
- Why would you choose to include more than one BIER MPLS Encapsulation
  Sub-TLV

---

2,2

      Label: A 3 octet field, where the 20 rightmost bits represent the
      first label in the label range.  The 4 leftmost bits MUST be
      ignored.

Did you not consider offering up 4 Reserved bits for future use? I
suppose this is just me saying "I wouldn't have done it like that", so 
you can choose to ignore this comment.

---

2.2

   If the label associated with the Maximum Set Identifier exceeds the
   20 bit range, the BIER MPLS Encapsulation Sub-TLV MUST be ignored.

Again with "ignored"

---

2.2

      Bit String Length: A 4 bits field encoding the supported BitString
      length associated with this BFR-prefix.  The values allowed in
      this field are specified in section 2 of [RFC8296].

and

   If the BS length is set to a value that does not match any of the
   allowed values specified in [RFC8296], the BIER MPLS Encapsulation
   Sub-TLV MUST be ignored.

This made me go and read 8296 section 2 where I found the 4-bit BSL
field in the BIER Header described as:

   The default BitStringLength value for the encapsulations defined in
   this document is 256.  See Section 3 of [RFC8279] for a discussion of
   the default BitStringLength value.

I was confused at how you would encode 256 in a 4 bit field :-) The 
point is that the initial paragraph would be so much clearer if it was 
replaced as:

      Bit String Length: A 4 bits field indicating the supported
      BitString length associated with this BFR-prefix using the 
      exponential encoding defined in section 2.1.2 od [RFC8296]. The
      set of values allowed in this field are specified in that section.

---

2.2

   If same BS length is repeated in multiple BIER MPLS Encapsulation
   Sub-TLV inside the same BIER Sub-TLV, the BIER sub-TLV MUST be
   ignored.

   Label ranges within all BIER MPLS Encapsulation Sub-TLVs advertised
   by the same BFR MUST NOT overlap.  If the overlap is detected, the
   advertising router MUST be treated as if it did not advertise any
   BIER sub-TLVs.

There isa small point here. Suppose a BIER Sub-TLV is advertised with 
BIER MPLS Encapsulation Sub-TLVs and some future BIER Foo Encapsulation 
Sub-TLVs. If the BIER MPLS Encapsulation Sub-TLVs are in error, but the 
BIER Sub-TLV was, itself, OK, must any non-MPLS Sub-TLVs (the BIER Foo
Encapsulation Sub-TLVs) also be discarded? Possibly this is exactly
what you intend, but just want you to be sure.

---

3.

A reasonable Security Considerations section, but...

- What could happen to the network if a router advertised a very large
  number of Sub-TLVs? Should there be an option for a receiver to:
  - not process beyond a configurable number
  - not store and replicate such advertisements

---

Are there any manageability considerations for this?
- List all the new configuration parameters
- Are there any OAM implications for the protocol itself (rather than for
  BIER)


== Nits ==

Abstract

OLD
   BIER architecture uses MPLS or other encapsulation to steer
   the multicast traffic towards the receivers.
NEW
   The BIER architecture uses MPLS or another encapsulation to steer
   the multicast traffic towards the receivers.
END

---

Introduction

   Bit Index Explicit Replication (BIER) is an architecture

Please add a reference. Probably to the BIER architecture.

---

Introduction

OLD
   Neither does BIER explicitly require a tree-building
   protocol for its operation.
NEW
   BIER also does not explicitly require a tree-building
   protocol for its operation.
END

---

Introduction

s/BIER architecture requires/The BIER architecture requires/
s/BIER architecture permits/The BIER architecture permits/
s/[RFC8444] proposes the OSPFv2/[RFC8444] defines the OSPFv2/

---
   This document describes
   extensions to OSPFv3 necessary to advertise BIER specific information
   in the case where BIER uses MPLS encapsulation as described in
   [RFC8296].


  == The document seems to lack the recommended RFC 2119 boilerplate, even if
     it appears to use RFC 2119 keywords -- however, there's a paragraph with
     a matching beginning. Boilerplate error?

---

Section 2

OLD
   [RFC8362] defines the encoding of OSPFv3 LSA in TLV format that
   allows to carry additional informations.
NEW
   [RFC8362] defines the format of TLV that allows additional 
   information to be carried in OSPFv3 LSAs.
END

OLD
   This section defines the
   required Sub-TLVs to carry BIER information that is associated with
   the BFR-Prefix.  The Sub-TLV defined in this section MAY be carried
   in the below OSPFv3 Extended LSA TLVs [RFC8362]:
NEW

---

Section 2

Might be nice to give the TLV Type numbers for the two Extended LSA TLVs

---

2.1

   The use of non-zero values in either the BAR field or the IPA field
   is outside the scope of this document.

I don't think so! That is, you have defined exactly how non-zero values
are carried, and where the meanings of the values are specified. I think
you mean that:
- Implementations should set these fields to zero by default
- Other values may be carried in these fields, but the processing of the
  BIER-Sub-TLV in these cases is set out in the documents that that 
  define the BAR and IPA values.

---
                                                                                                    
2.2

      Max SI: A 1 octet field encoding the maximum Set Identifier
      (section 1 of [RFC8279]), used in the encapsulation for this BIER
      sub-domain for this bitstring length.

I think...
s/this bitstring length/the bitstring length indicated by the BS Len field/

---

2.2

      The "label range" is the set of labels beginning with the Label
      and ending with (Label + (Max SI)). 

      The size of the label range is determined by the number of Set
      Identifiers (SI) (section 1 of [RFC8279]) that are used in the
      network.  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.

To be clear, you are happy with a label range of 256 labels. If you are
happy, no need to do anything.

---

2.2

   If same BS length is repeated in multiple BIER MPLS Encapsulation
   Sub-TLV inside the same BIER Sub-TLV, the BIER sub-TLV MUST be
   ignored.

This would be clearer if you used "Sub-TLVs" when you mean the plural
and possibly clarified "the entire BIER Sub-TLV"

---

Please be consistent with "Sub-TLV" or "sub-TLV" throughout.

---

2.3

      When an OSPFv3 Area Border Router (ABR) advertises E-Inter-Area-
      Prefix-LSA from an intra-area or inter-area prefix to all its
      attached areas, it determines whether a BIER Sub-TLV should be
      included in this LSA.  When doing so, an OSPFv3 ABR will:

My response to the first sentence was, "How?" I suspect that the
second sentence is supposed to mean "To achieve this, an OSPFv3 ABR
will:"

---