Subject: [Gen-art] Genart LC (and likely telechat) review : draft-ietf-trill-tree-selection-04

From: Robert Sparks <[email-masked]>
Date: Tue, 28 Jun 2016 14:33:03 -0500

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

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

Document: draft-ietf-trill-tree-selection-04
Reviewer: Robert Sparks
Review Date: 28 Jun 2016
IETF LC End Date: 1 Jul 2016
IESG Telechat date: 7 Jul 2016

Summary: Ready (with nits) for publication as Proposed Standard

This document is easy to read, even for someone not deeply steeped in trill.

I have a few questions and suggestions to consider

1) The essence of the idea this document provides support for is that an 
operator will create and install a configuration that meets the one tree 
per identifiable thing (such as VLAN) constraint. The protocol proposed 
here does not try to enforce that the operator supplies a configuration 
meeting that constraint. Should the things that generate messages with 
the TLVs defined in this document be restricted from sending messages 
that would map the same VLAN to two trees? I understand things will 
still work (suboptimally, as pointed out in the backwards-compatibility 
section), but it seems this configuration error should be mitigated. 
Section 3.3 also pulls the punch a little with it's discussion at the 
end of the second paragraph. If you're going to leave it up to the 
unspecified way the operator installs this configuration, you might at 
least point out that this is something to look for and complain about. 
If you think the optimal configuration isn't a likely thing to reach, 
then consider a pass through the document that sets that expectation 
consistently.

2) There are a couple of places where you use 2119 where you appear to 
be restating requirements from other documents. That's dangerous, from a 
document set maintenance point of view. Please consider changing these 
to simple prose, leaving the 2119 requirements to the protocol you're 
defining in this document. Please look at the SHOULD in the Background 
Description, and the SHOULD NOT in the first paragraph of the Overview. 
(2119 in sections like backgrounds and overviews is usually a sign that 
somethings in the wrong place.)

3) In the 3rd paragraph of 3.3, why is the requirement SHOULD strength? 
What else would the RBridge do, and when would it be reasonable for it 
to do that something else?

Nits/editorial comments:

* You use a lot of domain-specific acronyms in section 1 before saying 
what they mean in section 2.

* The first sentence in the 8th paragraph of 1.2 is very complex. (It's 
the one that starts "In cases where blocks of"). Please consider 
simplifying it.

* Section 2: (I'm no fun) Do you want this alternate expansion of FGL to 
stand?

* Figure 2: the left table has a VLAN of 4095, which is inconsistent 
with the prose.

* In section 3.4 you use 2119 RECOMMENDED (which is equivalent to 
SHOULD) when describing how the operator configures things. This isn't a 
constraint on the protocol defined in this document. Please consider 
rewriting the sentence without the 2119 keyword.

* Micronits: there's spurious space at the beginning of the 3rd line on 
page 6. There's an occurrence of BRridge that probably should have been 
RBridge in section 3.4, and "assigne" appears in the IANA Considerations.

---

From: Donald Eastlake <[email-masked]>
Date: Wed, 29 Jun 2016 16:09:51 -0400

Hi Robert,

Thanks for your thorough comments. See below.

On Tue, Jun 28, 2016 at 3:33 PM, Robert Sparks <[email-masked]>
wrote:
> operator will create and install a configuration that meets the one tree per
> identifiable thing (such as VLAN) constraint.

Well, it helps as long as multi-destination traffic identified by VLAN
or whatever is carried by fewer than all trees. It need not be one.

> The protocol proposed here
> does not try to enforce that the operator supplies a configuration meeting
> that constraint. Should the things that generate messages with the TLVs
> defined in this document be restricted from sending messages that would map
> the same VLAN to two trees? I understand things will still work
> (suboptimally, as pointed out in the backwards-compatibility section), but
> it seems this configuration error should be mitigated. Section 3.3 also
> pulls the punch a little with it's discussion at the end of the second
> paragraph. If you're going to leave it up to the unspecified way the
> operator installs this configuration, you might at least point out that this
> is something to look for and complain about. If you think the optimal
> configuration isn't a likely thing to reach, then consider a pass through
> the document that sets that expectation consistently.

While restricting, for example, VLAN-x to one tree is optimal from the
point of view of using up the least amount of fast path FIB
(Forwarding Information Base) resources in some hardware
implementations, it is not optimal from the point of view of load
spreading. To get optimal load spreading, you would want to spread
different multi-destination flows onto different distribution trees.

> 2) There are a couple of places where you use 2119 where you appear to be
> restating requirements from other documents. That's dangerous, from a
> document set maintenance point of view. Please consider changing these to
> simple prose, leaving the 2119 requirements to the protocol you're defining
> in this document. Please look at the SHOULD in the Background Description,
> and the SHOULD NOT in the first paragraph of the Overview. (2119 in sections
> like backgrounds and overviews is usually a sign that somethings in the
> wrong place.)

The SHOULD in the Background Description is indeed just echoing the
same provision from [RFC6325] and can be changed to not use a 2119 keyword.

The SHOULD NOT in the first paragraph of the Overview (Section 3.1) is
entirely due to theis draft and not inhereted from any other document.

> 3) In the 3rd paragraph of 3.3, why is the requirement SHOULD strength? What
> else would the RBridge do, and when would it be reasonable for it to do that
> something else?

The "SHOULD" requirement is to use a tree that the choosing RBridge
has advertised it will use; however, it is not actually required to
advertise which tree(s) it will use. Furthermore, even if it has, that
tree(s) might just have become unavailable due to one or more
failures.  We can probably add some words to clarify that.

> Nits/editorial comments:
>
> * You use a lot of domain-specific acronyms in section 1 before saying what
> they mean in section 2.

Looks to me like the terminology section could be moved up.

> * The first sentence in the 8th paragraph of 1.2 is very
> complex. (It's the one that starts "In cases where blocks
> of"). Please consider simplifying it.

I think it can be re-worded.

> * Section 2: (I'm no fun) Do you want this alternate expansion of
> FGL to stand?

Nope... Looks like a global replace run amok or something, that should
be fixed :-)

> * Figure 2: the left table has a VLAN of 4095, which is inconsistent

Shold be fixed. 4095 (0xFFF) is not a valid VLAN ID.

> SHOULD) when describing how the operator configures things. This
> isn't a constraint on the protocol defined in this document. Please
> consider rewriting the sentence without the 2119 keyword.

Humm. I think those are good operational recommendations. We can try
changing "RECOMMEND" to "suggest" and see if we get push back to
change it back :-)

> * Micronits: there's spurious space at the beginning of the 3rd line
> on page 6. There's an occurrence of BRridge that probably should
> have been RBridge in section 3.4, and "assigne" appears in the IANA
> Considerations.

OK.

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 155 Beaver Street, Milford, MA 01757 USA
 [email-masked]