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

From: Brian E Carpenter <[email-masked]>
Date: Tue, 15 Sep 2015 13:17:46 +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 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-pwe3-iccp-stp-04.txt
Reviewer: Brian Carpenter
Review Date: 2015-09-15
IETF LC End Date: 2015-09-23
IESG Telechat date:

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

Comment:
--------

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,...?

> 3.3.1. STP System Config
...
>       - MAC Address

Excuse my ignorance, but are there any scenarios where this would need
to be EUI-64?

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: Mingui Zhang <[email-masked]>
Date: Thu, 17 Sep 2015 02:18:44 +0000

Hi Brian, 

Thanks for your careful review! Please see my responses in-line below.

> -----Original Message-----
> From: Brian E Carpenter [mailto:[email-masked]]
> Sent: Tuesday, September 15, 2015 9:18 AM
> To: [email-masked]; General Area Review Team
> Subject: Gen-ART Last Call review of draft-ietf-pwe3-iccp-stp-04
> 
> 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.
> 
> 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 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.

Sure. This will be added.

> I think this should be discussed in the text to avoid confusion.

The text related to the reference will updated. 

> > 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.

Since there is "Request Number", the lost fragment can be identified and be re-requested. This will be clarified.

> Minor Issues:

Sure. According to RFC 7275, UTF-8 will be used.. 

> 
> > 3.3.1. STP System Config
> ...
> >       - MAC Address
> 
> Excuse my ignorance, but are there any scenarios where this would need to be
> EUI-64?

The document defines it to be the 48-bit MAC. It is in consistency with RFC 7275. I think EUI-64 could be supported in the future with a protocol version update.

> 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".
> 

These nits will be corrected.

Thanks,
Mingui

---

From: Brian E Carpenter <[email-masked]>
Date: Sun, 20 Sep 2015 07:53:05 +1200

Hi Mingui,

Thanks for your reply. Your proposed changes look good to me.

Regards
   Brian