Last Call Review of draft-ietf-mpls-spring-entropy-label-11
review-ietf-mpls-spring-entropy-label-11-opsdir-lc-clarke-2018-06-25-00

Request Review of draft-ietf-mpls-spring-entropy-label
Requested rev. no specific revision (document currently at 12)
Type Last Call Review
Team Ops Directorate (opsdir)
Deadline 2018-06-19
Requested 2018-06-05
Authors Sriganesh Kini, Kireeti Kompella, Siva Sivabalan, Stephane Litkowski, Rob Shakir, Jeff Tantsura
Draft last updated 2018-06-25
Completed reviews Rtgdir Last Call review of -08 by Daniele Ceccarelli (diff)
Genart Last Call review of -11 by Francis Dupont (diff)
Opsdir Last Call review of -11 by Joe Clarke (diff)
Assignment Reviewer Joe Clarke
State Completed
Review review-ietf-mpls-spring-entropy-label-11-opsdir-lc-clarke-2018-06-25
Reviewed rev. 11 (document currently at 12)
Review result Has Nits
Review completed: 2018-06-25

Review
review-ietf-mpls-spring-entropy-label-11-opsdir-lc-clarke-2018-06-25

I have been asked to review this document on behalf of the ops directorate.  I know my review is a few days late, but I hope it will still be of some use.

This document describes how to use/place entropy labels in a SPRING path over an MPLS dataplane for purposes of optimizing load balancing.  Overall, I think the document is ready, but I did have a couple of operational questions/comments and found a number of nits.

I found a couple of MAY requirements in here that I feel should be stronger or offer some additional recommendation (mainly to favor operators or help guide vendors).  First, section 4 states "For simplicity, an implementation MAY use the minimum ERLD between each linecard as the ERLD value for the system."  I feel there should be a stronger recommendation on what to do here given that ERLD is later described as a key piece of the algorithm for EL placement.  A vendor may opt for the simple approach, but should they consider a more robust approach to favor operators?

Second, section 5 states "this node MAY advertise its MSD value or a subset of its MSD value to the controller."  It MAY NOT do that, too.  It would be good to highlight pros and cons to this.

Finally, section 7.2 states "In case of a trade-off, an implementation MAY provide flexibility to the operator to select the criteria to be considered when placing EL/ELIs..."  I can see this being of great value to operators to have vendors allow this.  But as with any MAY, the vendor MAY choose NOT to do it.  SHOULD seems better here for that reason.

On to the nits:

Section 1

Expand LSP before using

===

Section 1

s/local node who/local node which/

===

Section 1

s/disordering/misordering/

I think the latter makes more sense, yes?

===

Section 2

Add LSP to your list of terminology

===

Section 6

s/destinated/destined/

===

Section 6

s/accomodate/accommodate/

===

Section 7.1.2

s/node has not a sufficient/node does not have a sufficient/

===

Section 7.1.2

<Adj_P1P2, Adj_set_P2P3, Adj_P3P4,
   Adj_P4P5, Adj_P5P6, Adj_set_P6P7, Adj_P7P8; Adj_set_P8PE2,VPN_label>

Note the ';' after Adj_P7P8.  This is a comma everywhere else.  I think it should be a comma here, too.

===

Section 7.1.2

s/PE1 is limited to push a maximum of 11 labels,/PE1 is limited to push a maximum of 11 labels./

Note the change of a comma to a period.

===

Section 7.2 and throughout

You use EL/ELI and ELI/EL interchangeably.  For consistency, pick one and go with it.

===

Section 7.2

s/considered as exhaustive/considered exhaustive/

===

Section 7.2.2.2

I would like to point out that you define an adjacency set as a set of adjacencies.  Perhaps you want to further refine this to add more clarity.

===

Section 9

s/In future/In the future/

===

Section 9

s/capabilities and may be remove/capabilities and may remove/