Skip to main content

Telechat Review of draft-ietf-bier-tether-05
review-ietf-bier-tether-05-rtgdir-telechat-farrel-2024-04-16-00

Request Review of draft-ietf-bier-tether-05
Requested revision 05 (document currently at 05)
Type Telechat Review
Team Routing Area Directorate (rtgdir)
Deadline 2024-04-22
Requested 2024-04-08
Requested by Gunter Van de Velde
Authors Zhaohui (Jeffrey) Zhang , Nils Warnke , IJsbrand Wijnands , Daniel O. Awduche
I-D last updated 2024-04-16
Completed reviews Rtgdir Telechat review of -05 by Adrian Farrel
Secdir Last Call review of -05 by Wes Hardaker
Opsdir Last Call review of -05 by Dan Romascanu
Genart Last Call review of -04 by Joel M. Halpern (diff)
Comments
Preparing this document for IESG Telechat review. RTGDIR feedback appreciated.
Assignment Reviewer Adrian Farrel
State Completed
Request Telechat review on draft-ietf-bier-tether by Routing Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/rtg-dir/JjXzMtOgyLUBJo42C7PHWD2t0UA
Reviewed revision 05
Result Has issues
Completed 2024-04-16
review-ietf-bier-tether-05-rtgdir-telechat-farrel-2024-04-16-00
Hello,

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 https://wiki.ietf.org/en/group/rtg/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-tether-05
Reviewer: Adrian Farrel
Review Date: 2024-04-14
IETF LC End Date: 2024-02-29
Intended Status: Standards Track

Summary:

I have some minor concerns about this document that I think should be
resolved before publication.

Comments:

This is a short draft that builds on existing work to offer a small 
optimisation in the case of a non-BIER router being the optimal branch
point for a BIER tree. It uses simple mechanisms to allow a subtended
router to provide the BIER forwarding plane function and so reduce the
traffic on the link to the non-BIER router.

The draft is passably well written, but would benefit from some more
polish. Technically there are no major issues, but some small points of
clarity need to be resolved.


Side note: Daniel needs to update his information in the Datatracker. It
still uses his Movaz email address!

Three points for the AD and WG chairs:

1 I note that the IPR disclosure https://datatracker.ietf.org/ipr/3331/
  was disclosed against revision -00 of the individual draft
  https://datatracker.ietf.org/doc/draft-zzhang-bier-tether/
  It has not been disclosed against this draft and I can't find any
  discussion on the mailing list about whether the IPR still applies and
  the disclosure is assumed to be "promoted" to this draft. The last
  call IPR poll in the working group only reveals "unaware of
  undisclosed IPR on the draft." Actually I can't find the announcement
  of the original IPR disclosure on the WG list, and I see no
  mention of the IPR disclosure throughout the process: something that
  the responsible WG chair should be careful about given his shared
  affiliation with the owner of the IPR.

2 I'm a bit of a fan of implementations. It might be helpful to the
  review and approval process to know whether this specification has
  been implemented, and if so, what was learnt.

3 Have the IGP extensions in this document been shown to the WG that 
  owns the relevant protocols?

== Medium ==

3.2

   To make tethering work well with BGP signaling, the following can be
   done:

   *  Configure BGP sessions between X and BFR1..N and BFRx.

   *  When X re-advertises BIER prefixes to BFRx, it does not change
      BIER Nexthop [I-D.ietf-bier-idr-extensions] in the BPA.  When X
      re-advertises BIER prefixes to BFR1..N, it does change the BIER
      Nexthop to BFRx.

   *  BFRx advertises its own BIER prefix with BPA to X, and sets the
      BIER Nexthop to itself.  X then re-advertises BFRx's BIER prefix
      to BFR1..N.

I think X is not BIER capable. There seem to be two cases:
1. X is not capable of BIER forwarding actions, but is BIER-aware in the
   control plane.
2. X is not capable of BIER and not aware of BIER.

In 3.1, you effectively handle both cases together, because X does not
need to be changed.

But here in 3.2 you seem to only address the first case, because X needs
to support draft-ietf-bier-idr-extensions.

Perhaps some clarity on this?

== Minor ==

The Abstract is too concise even for an Abstract. It leaves the reader
wondering "the handling of BIER incapable routers" by whom?

---

Introduction launches in too rapidly? Although the document is one of a
set and you clearly need to understand BIER to read it, the Introduction
should start off by setting the scene a bit: What is BIER?

It should also state what this document does. That's usually the final
paragraph of the introduction.

---

1.

   For BFR1 to forward BIER traffic towards BFR2...BFRn, it needs to
   tunnel individual copies through X.  This degrades to "ingress"
   replication to those BFRs.  If X's connections to BFRs are long
   distance or bandwidth limited, and n is large, it becomes very
   inefficient.

I think this is slightly wrong. The problem is the length of the
connection from BFR1 to X combined with the size of n. Because, if X
were BIER capable, the packets would still be sent out on all the links
from X to BFR2-n.

---

1.

   There
   could be fat and local pipes between the tethered BFRx and X, so
   ingress replication from BFRx is acceptable.

I think that "there could be fat and local pipes" is actually noting
that there is a problem here depending on the size n and the volume of 
traffic. Rather than skip to the potential ("there could be") solution,
I think you should describe the situation and call out that the links
between BFRx and X must be adequate for the needs.

---

2.

You are missing a textual description of what is contained in Figure 3.
Additionally...

OLD
  Figure 3 below is a simple case
NEW
  Figure 3 shows a simple case:
END

---

2.

   This can easily be prevented if BFR1 does an SPF calculation with the
   helper BFRx as the root.  For any BFERn reached via X from BFR1, if
   BFRx's SPF path to BFERn includes BFR1 then BFR1 must not use the
   helper.  Instead, BFR1 must directly tunnel packets for BFERn to X's
   BFR (grand-)child on BFR1's SPF path to BFERn, per section 6.9 of
   [RFC8279].

This seems to say that the solution to avoid loops is to not use the
tethered "helper". Which is fine except it doesn't address the problem
this document is trying to solve - in this example it basically returns
us to Figure 1.

---

3.1

   At step 2, the removed node is added to an ordered list maintained
   with each child that replaces the node.  If the removed node already
   has a non-empty list maintained with itself, add the removed node to
   the tail of the list and copy the list to each child.

A bit confusing. You have:
- "added to an ordered list" which makes me wonder: added to the head,
  tail or middle?
- "If the removed node..." "...add ... to the tail of the list" which is
  good, but leaves us asking: what if not?

---

3.1

   If the above procedure finishes without finding any helper, then the
   original BFR next hop via a tunnel is used to reach the BFER.

Probably add...

  The problem posited in Section 1 is not solved, but nothing is lost 
  and forwarding continues as if there were no helpers available.

---

3.2

  BFR1..N advertises BIER prefixes

s/advertises/advertise/

But why does 3.1 use "MUST" where 3.2 uses a statement of fact?

---

3.2

   There are three situations regarding X's
   involvement:

But I only see two listed and the text that follows says "In either 
case". Is this just a typo, of is there a third case missing?

== Nits ==

The "Requirements Language" section should move from the document header
into a section (such as 1.1) in the body of the document.

---

I think the title of Figure 1 should be in the singular.

---

1.

   A solution to the inefficient tunneling from BFRs is to attach
   (tether) a BFRx to X as depicted in Figure 2:

Is that s/BFRs/BFR1/

---

1.

   Instead of BFR1 tunneling to BFR2, ..., BFRn directly, BFR1 will get
   BIER packets to BFRx, who will then tunnel to BFR2, ..., BFRn.

s/directly/direct/
s/who/which/

What does "will get BIER packets to BFRx" mean? I suspect it means...
   BFR1 will tunnel packets to BFRx

---

1.

   For BFR1 to tunnel BIER packets to BFRx, the BFR1-BFRx tunnel need to

s/need/needs/

---

1.

   Section 6.9 of BIER architecture specification [RFC8279] describes a

s/of/of the/

---

2.

OLD
   While the example shows a local connection between BFRx and X, it
   does not have to be like that.
NEW
   While the example in Section 1 shows BFRx subtended to X, other
   network configurations are possible.
END

Note: In fact, in all your examples in Section 2, BFRx is connected to X

---

2.

   As long as packets can arrive at BFRx
   without requiring X to do BIER forwarding, it should work.

"It should work"? I think we are looking for a little more certainty!

---

2.

s/triangle, loop/triangle, a loop/

---

2.

   helper is not different from sending packets to a LFA backup.

s/a LFA/an LFA/

---

3.1

   At the end, the calculating node BFR-B would use a unicast tunnel to

This sent me looking for "BFR-B" on figures, but I see you are deep in 
the context of RFC8279. Perhaps...


   At the end, the calculating node (called BFR-B in [RFC8279]) uses a
   unicast tunnel to

---

3.1

   *  Order all the helper nodes of N1 based descending (priority, BIER
      prefix).

Possibly...

   *  Order all the helper nodes of N1 in descending order based on the
      numeric values of priority and BIER prefix.

---

3.1

s/H1 as LFA/H1 as an LFA/

---

3.2 etc.

While it is certainly convenient to use "BPA", I note that 
draft-ietf-bier-idr-extensions doesn't.