Last Call Review of draft-ietf-pwe3-redundancy-
review-ietf-pwe3-redundancy-genart-lc-thomson-2012-03-16-00

Request Review of draft-ietf-pwe3-redundancy
Requested rev. no specific revision (document currently at 09)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2012-03-21
Requested 2012-03-09
Draft last updated 2012-03-16
Completed reviews Genart Last Call review of -?? by Martin Thomson
Secdir Last Call review of -?? by Radia Perlman
Secdir Telechat review of -?? by Radia Perlman
Assignment Reviewer Martin Thomson
State Completed
Review review-ietf-pwe3-redundancy-genart-lc-thomson-2012-03-16
Review completed: 2012-03-16

Review
review-ietf-pwe3-redundancy-genart-lc-thomson-2012-03-16

I am the assigned Gen-ART reviewer for this draft. For background on
Gen-ART, please see the FAQ at
<

http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Please resolve these comments along with any other Last Call comments
you may receive.

Document: draft-ietf-pwe3-redundancy-06
Reviewer: Martin Thomson
Review Date: 2012-03-15
IETF LC End Date: 2012-03-21
IESG Telechat date: (if known)

Summary: The draft has some minor issues.

Major issues: none

Minor issues:

3.2.2 states: "The mechanisms for achieving this selection are outside
the scope of this document."

The example then describes the _conditions_ under which the selection
is made.  So the statement doesn't quite make sense.  It's reasonable
to presume that a standard doesn't prescribe the internal behaviour of
a PE where interoperability is no concern.  Even though this is just
an example, it makes a very specific presumption about the behaviour
of the PE in reaction to an event.  I can't imagine any other reaction
in this case, so I'm left wondering: what exactly is out of scope for
this?  Communication of the event between PE1 and PE2?

Section 4.1: "Non-revertive behavior MUST be supported, while
revertive behavior is OPTIONAL."

The reason for this requirement is non-obvious (at least to me).  Some
justification for it seems appropriate.

Section 4.1:    "Protection switchover can be triggered by the operator [...]"

Again, justification would be nice.  This actually smells more like a
product specification that requirement for interoperability.  If the
requirement is, as I suspect, that switchovers triggered by manual
intervention can be marked as such in the protocol _so that they can
be treated with lower priority_, then that is definitely
understandable.

Section 4.2:    "[...] MUST support the configuration of revertive or
non-revertive protection switching modes."

If revertive switching is optional, then this requirement makes not
sense for (T-)PEs that don't implement it.

Nits:
The figures are somewhat difficult to read overall.  It's unclear what
significance is attached to the dots in many of the diagrams, since
they aren't always consistent (see Figure 3).
Figure 5 is especially difficult to read.  Another instance is the
choice of '.' or '|' in Figure 4, which could have some significance,
but probably doesn't because the usage is quite inconsistent.  Having
labels for lines/tunnels impinge on boxes is difficult to interpret.
Reducing the noise in the diagrams would help.  The text is adequate
to make up the shortfall.  (Figure 7 is perfectly clear.)

Labeling of PEs in 3.2.2 makes it hard to follow because PE2 is
attached to CE2.  I'd suggest renames such that you have {CE1, PE1a,
PE1b} and likewise.

The first requirement in 4.1 is missing a couple of spaces (after the
comma, in "besupported").

4.2 has an empty bullet.

Not expanded on first use: PSN, LDP