Subject: [Gen-art] Gen-ART telechat review of draft-ietf-pwe3-iccp-stp-04

From: Brian E Carpenter <[email-masked]>
Date: Fri, 25 Sep 2015 12:56:30 +1200

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 wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

Document: draft-ietf-pwe3-iccp-stp-04.txt
Reviewer: Brian Carpenter
Review Date: 2015-09-25
IETF LC End Date: 2015-09-23
IESG Telechat date: 2015-10-01

Summary: Ready with issues
--------

Comments:
---------

The author responded helpfully to the following Last Call comments
but a new version is needed to fix them.

It's impossible for a reviewer who is not expert in the details
of 802.1Q to check many details in this draft, so I didn't.

Major Issues:
-------------

The draft does not properly explain the theory of operation.
The messages are defined but it is not explained when a spanning
tree is formed. Section 4 does not help with this. I think it
should be explained at the end of the Use Case section.

The main normative reference appears to be IEEE 802.1Q-2005. The current
standard is IEEE 802.1Q-2014, which appears to be very different.
I think this should be discussed in the text to avoid confusion.

> 3.6. STP Synchronization Data TLV
...
> When the total size of the TLVs to be transmitted
> exceeds the maximal size of a fragment, these TLVs SHOULD be divided
> into multiple sets, delimited by multiple pairs of STP
> Synchronization Data TLVs, and filled into multiple fragments.

There needs to be discussion of what happens if a fragment
is lost.

Minor Issues:
-------------

> 3.2.1. STP Disconnect Cause sub-TLV
...
>       - Disconnect Cause String
>
>        Variable length string specifying the reason for the disconnect,
>        to be used for operational purposes.

Should it be specified whether this is ASCII, UTF-8,...?

Nits:
-----

Please expand Spanning Tree Protocol in the main title.

Abbreviation PE used but not defined. Also, "provider edge" means an edge,
which is an abstract concept, not a device. If the draft is discussing
specific devices, it should say "PE device" or "PE router" or "PE switch".

Abbreviation AC used but not defined.

Abbreviation CE used but not defined.

---

From: "Andrew G. Malis" <[email-masked]>
Date: Wed, 30 Sep 2015 18:27:10 -0400

Brian,

On behalf of the authors, I would like to thank you for your review. Most
of your points should be addressed as you recommend.

I've just got a comment/question regarding your first major issue.

Note that STP operationally is not impacted with the use of a RG. Section 2
says: “The link between the peering PEs is not visible to the bridge
domains of the STP network. In this way, the STP will always break a
possible loop within the multi-homed STP network by breaking the whole
network into separate islands so that each is attached to one PE.” Other
than that, operation is identical to the way VPLS works now, including the
use of split-horizon to perform loop-breaking through the core. and that's
also discussed in section 2.

Does this satisfy your first comment?

Thanks,
Andy

---

From: Brian E Carpenter <[email-masked]>
Date: Thu, 1 Oct 2015 13:12:57 +1300

Andy,

I think most people think of a spanning tree as being constructed over a
set of bridges, so I guess that in addition to the sentences you quote,
I would be happy to see a more conceptual description in the Introduction.

"The spanning tree will be formed over [some set of nodes] without
the multi-homing links leading to loop formation. This is possible
because each redundancy group is treated as a single node for
spanning tree purposes."

I'm sure this is blindingly obvious to y'all but not so much to someone
new to the topic.

Regards
   Brian

---

From: "Andrew G. Malis" <[email-masked]>
Date: Wed, 30 Sep 2015 20:34:59 -0400

Brian,

Thanks, I'll ask the authors to add that.

Cheers,
Andy