Skip to main content

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 revision 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
I-D 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
Request Last Call review on draft-ietf-mpls-spring-entropy-label by Ops Directorate Assigned
Reviewed revision 11 (document currently at 12)
Result Has nits
Completed 2018-06-25
review-ietf-mpls-spring-entropy-label-11-opsdir-lc-clarke-2018-06-25-00
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/