Last Call Review of draft-ietf-bier-te-arch-10
review-ietf-bier-te-arch-10-rtgdir-lc-qu-2021-08-20-00

I have been selected as the Routing Directorate reviewer for this draft. The
Routing Directorate seeks to review all routing or routing-related drafts as
they pass through IETF last call and IESG review, and sometimes on special
request. The purpose of the review is to provide assistance to the Routing ADs.
For more information about the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
<http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir>

Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft.

Document: draft-ietf-bier-te-arch-10
Reviewer: Yingzhen Qu
Review Date: Aug 20th, 2021
Intended Status: Standards Track

Summary:

This document has some issues/nits that should be at least considered prior to
publication.

Comments:

Typically when BIER-TE controller calculates BitStrings, the result “overlay”
topology has to be trees, no circles. Then ring topology becomes a special
case. This should be explained/stressed.

Comments inline:

[Line numbers from idnits]

From IDNITS:
  == 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?

     RFC 8174, paragraph 11:
        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.

     ... text found in draft:
        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.

The extra “,” should be removed.

142        BIER-TE introduces a new semantic for bit positions (BP) that
143        indicate adjacencies, as opposed to BIER in which BPs indicate Bit-
144        Forwarding Egress Routers (BFER).  With BIER-TE, the BIFT of each BFR
145        is only populated with BP that are adjacent to the BFR in the BIER-TE
146        Topology.  Other BPs are empty in the BIFT.  The BFR replicate and
147        forwards BIER packets to adjacent BPs that are set in the packet.
148        BPs are normally also cleared upon forwarding to avoid duplicates and
149        loops.  This is detailed further below.

[nits]: BIFT, BFR should be expanded the first time in this document.
[nits]: detailed further below, may reference a section number.

359        BIER-TE is designed so that is forwarding plane is a simple extension

[nits]: s/so that is/so that its

376            3.  The supportable encapsulations, [RFC8296] or other (future)
377                encapsulations.

[minor]: I don’t think you can say it works with “future” encapsulations.

388            1.  In BIER, bits in the BitString of a BIER packet header
389                indicate a BFER and bits in the BIFT indicate the BIER
390                control plane calculated next-hop toward that BFER.  In BIER-
391                TE, bits in the BitString of a BIER packet header indicate an
392                adjacency in the BIER-TE topology, and only the BFRs that are
393                upstream of this adjacency have this bit populated with the
394                adjacency in their BIFT.

[nits]: The English language in this paragraph needs to be improved, especially
whether it’s singular or plural. For example, bits in the BitString of a BIER
packet header indicate BFERs.

430            4.  BIER-TE forwarding does not use the BFR-id field of the BIER
431                packet header.

[minor]: what’s BFR-id field? Do you mean BFIR-id?

443            2.  BIER-TE deployments will have to assign BFR-ids to BFR and
444                insert them into the BFR-id field of BIER packet headers as

[minor]: the BFR-id field?

512        1.  During initial provisioning of the network and/or during
513            modifications of its topology and/or services: protocols and/or
514            procedures to establish BIER-TE BIFTs:

[nits]: two “:” in a row.

523                BIER-TE headers on BFIR.  Alternatively, bfir-id in BIER

[nits]: s/bfir-id/BFIR-id as in RFC8296.

527            4.  Install/update the BIFTs into the BFRs and optionally BFR-id
528                into BFIR.

[minor]: “optionally BFR-id into BFIR” duplicates with point 3 above.

530        2.  During operations of the network: Protocols and/or procedures to
531            support creation/change/removal of overlay flows on BFIR:

[nits]: two “:” in a row.

553           BFR in step 1, such YANG/Netconf/RestConf.

[nits]: s/ Netconf/Restconf /NETCONF/RESTCONF, also you may consider adding
informative references YANG (RFC7950), NETCONF [RFC6241] and RESTCONF [RFC8040]

578        extending a Link-State-Protocol (LSP) based IGP into the BIER-TE

[minor]: I’d suggest remove “LSP” abbreviation here. LSP means Link-State PDU
in IS-IS.

600        models such as Netconf/RestConf/Yang/PCEP.  Vendor-specific CLI on

[nits]: s /Netconf/RestConf/Yang/ NETCONF/RESTCONF/YANG. They are used in
multiple times in the draft, please fix all of them.

1016            AdjacentBits = Packet->BitString &= ~AdjacentBits[SI];
1017            Packet->BitString &= AdjacentBits[SI];

[major]: I’m confused about these two lines of code. Can you please explain what
it is trying to achieve? Packet->BitString got masked twice?

1047    4.5.  Basic BIER-TE Forwarding Example

1049       [RFC Editor: remove this section.]

[minor]: an example that matches the pseudocode would be helpful. Whether this
example needs to be removed is up to the author.

1127       is in the BitString and this is an adjacency towards BFR3.  BFIR2
1128       therefore clears p2 in the BitString and sends a copy towards BFR2.

[major]: line 1128 should be “a copy towards BFR3” in case the example is kept.

1142       Further processing of the packet in BFR4, BFR5 and BFER2 accordingly.

[major]: BFR4 sees BitStrings of p5, p10, p11 and p12, should it send the packet
To BFER2 directly because of p10? I’m not sure whether p10 is adjacent to BFR4
in the figure 7.

1180       topologies with fewer bit positions (4.1, 4.3, 4.4, 4.5, 4.6, 4.7,
1181       4.8).

[major]: the section numbers in “()” are not correct.

1209       A leaf BFERs is one where incoming BIER-TE packets never need to be

[nits]: s/BFERs/BFER

1241                BFR1
1242                 |p1
1243          LAN1-+-+---+-----+
1244              p3|  p4|   p2|
1245              BFR3 BFR4  BFR7

1247                              Figure 11: LAN Example

[nits]: Please center this figure

1266       bit position on the hub's BIFT is set up with a list of
1267       forward_connected() adjacencies, one for each Spoke.

[comments]: my understanding is that this optimization only works when the hub
needs to forward a received packet to all spokes except the spoke sending the
packet. Correct?

1326    5.1.7.  Equal Cost MultiPath (ECMP)

[minor]: How a hash function is chosen is up implementations. This section talks
about “polarization” and how it can be avoided, personally I think it’s not
relevant.

1475       segments: (1) BFR2 via link L1, (2) BFR2 via link L2, or (3) via
1476       BFR3.

[nits]: I think it should be: (3) BFR3 L0.

1494       bit positions can be re-used across multiple BFR to minimize the

[nits] s/bit/Bit, s/BFR/BFRs

1492    5.1.9.  Reuse of bit positions (without DNC)

[comments]: About the reuse of bit positions in this section, my understand is
that this needs to be calculated carefully to meet condition (A) or (B). In
case of new Multicast overflows added or topology changes, there are risks
neither of these two condition holds any more, so the BP has to be reassigned
hence BitStrings at BFIR and BIFTs.