Subject: [Gen-art] Gen-ART telechat review of draft-ietf-pwe3-iccp-13.txt

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Wed, 19 Feb 2014 08:23:00 +0000

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

Document: draft-ietf-pwe3-iccp-13.txt

Reviewer: Dan Romascanu

Review Date: 2/19/14

IETF LC End Date: 2/11/14

IESG Telechat date: 2/20/14

Summary:

Ready with issues. I did not see any answer from the editors or shepherd to the issues raised in the IETFLC Gen-ART review and there was no revision of the document since then. Although none of the issues raised seems to be blocking, I believed that they should be considered and answered as part of the IETF Last Call Comments.

This is a complex but well written document. It is ready, a number of minor issues need clarification and possibly editing. Some nits also may be considered to fix

Major issues:

None

Minor issues:

1.       The document (and the name of the protocol defined here) uses the notion of 'chassis'. However there is no definition or reference to a definition that would clarify what a 'chassis' is.

2.       In section 7.2.2.1 - I am not a fan of transferring information in text format like in the Disconnect Cause String - no interoperability results if there no agreement on a finite set of causes. Anyway - should this not be UTF-8 format?

3.       Similar question in Section 7.2.4 - why is Aggregator Name a text? Why not using AggregatorID as per IEEE 802.1AX?

4.       In Section 7.2.5 - if Port Speed corresponds to the ifHighSpeed object in the IF-MIB, should not also Port (interface) name correspond to ifName truncated to 20 characters whenever possible?

Nits/editorial comments:

1.       Some acronyms need expansion at the first occurrence - e.g. POP, CO

2.       In section 3.3/i: PE nodes MAY be collocated or remote - this MAY needs not be capitalized.

3.       The first two lines in the diagram in page 16 are mis-aligned

4.       The diagrams in 7.1.1., 7.1.5   end at 16-bit boundary with the last field defined for optional sub-TLVs. Is this intentional? Do they suggest that the number of octets is always 4*n + 2? What happens else?

5.       In section 7.3.1:' -ii. PW ID TLV or generalized PW ID TLV' I think what is meant is actually ' -ii. One of PW ID TLV or generalized PW ID TLV'

---

From: "Andrew G. Malis" <[email-masked]>
Date: Wed, 19 Feb 2014 09:31:05 -0500

Dan,

Thanks for the review (twice!).

Authors,

Could you please respond to Dan's review and comments?

Thanks,
Andy

On Wed, Feb 19, 2014 at 3:23 AM, Romascanu, Dan (Dan)
<[email-masked]> wrote:
> 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>.
>
>
>
> Ready with issues. I did not see any answer from the editors or shepherd to
> the issues raised in the IETFLC Gen-ART review and there was no revision of
> the document since then. Although none of the issues raised seems to be
> blocking, I believed that they should be considered and answered as part of
> the IETF Last Call Comments.
>
>
>
> This is a complex but well written document. It is ready, a number of minor
> issues need clarification and possibly editing. Some nits also may be
> considered to fix
>
>
>
> Major issues:
>
>
>
> None
>
>
>
> Minor issues:
>
>
>
> 1.       The document (and the name of the protocol defined here) uses the
> notion of 'chassis'. However there is no definition or reference to a
> definition that would clarify what a 'chassis' is.
>
> 2.       In section 7.2.2.1 - I am not a fan of transferring information in
> text format like in the Disconnect Cause String - no interoperability
> results if there no agreement on a finite set of causes. Anyway - should
> this not be UTF-8 format?
>
> 3.       Similar question in Section 7.2.4 - why is Aggregator Name a text?
> Why not using AggregatorID as per IEEE 802.1AX?
>
> 4.       In Section 7.2.5 - if Port Speed corresponds to the ifHighSpeed
> object in the IF-MIB, should not also Port (interface) name correspond to
> ifName truncated to 20 characters whenever possible?
>
>
>
>
>
> Nits/editorial comments:
>
>
>
> 4.       The diagrams in 7.1.1., 7.1.5   end at 16-bit boundary with the
> last field defined for optional sub-TLVs. Is this intentional? Do they
> suggest that the number of octets is always 4*n + 2? What happens else?
>
> 5.       In section 7.3.1:' -ii. PW ID TLV or generalized PW ID TLV' I think
> what is meant is actually ' -ii. One of PW ID TLV or generalized PW ID TLV'
>
>
>
>

---

From: "Samer Salam (ssalam)" <[email-masked]>
Date: Wed, 19 Feb 2014 18:30:22 +0000

Hi Dan, 

Thank you for your review. Please find responses inlineŠ

Hi Andy,

Please let us know when we can go ahead an update the draft to address the
review comments.

Regards,
Samer

On 2014-02-19 6:31 AM, "Andrew G. Malis" <[email-masked]> wrote:

>> Please wait for direction from your document shepherd or AD before
>>posting a
>> new version of the draft.
>>
>>
>>
>>
>>

Good point, will add a definition to that effect.

>> this not be UTF-8 format?

This is meant to relay information to a human user, and not intended to be
used to take any automated actions. Will clarify that it is UTF-8.

>> Why not using AggregatorID as per IEEE 802.1AX?

The AggregatorID is already part of the TLV. The name is to enhance human
usability, same reason as above.

>> ifName truncated to 20 characters whenever possible?

Agreed, will update accordingly.

>> 1.       Some acronyms need expansion at the first occurrence - e.g.

Will update the draft accordingly.

>>
>> 2.       In section 3.3/i: PE nodes MAY be collocated or remote - this
>>MAY
>> needs not be capitalized.

Will fix.

>> 3.       The first two lines in the diagram in page 16 are mis-aligned

Will fix.

>> suggest that the number of octets is always 4*n + 2? What happens else?

No that was not intentional. Will add text to clarify that there are no
such assumptions.

>> what is meant is actually ' -ii. One of PW ID TLV or generalized PW ID

Will update.

---

From: "Andrew G. Malis" <[email-masked]>
Date: Wed, 19 Feb 2014 16:33:54 -0500

Samer,

Thanks for responding to Dan.

The timing for a new revision is up to Stewart - don't take any action
until you hear from him.

Cheers,
Andy

---

From: Jari Arkko <[email-masked]>
Date: Wed, 19 Feb 2014 23:57:00 +0100

Thanks your review, Dan. Thank you authors for addressing the concerns (in a new version, once it appears :-)

Jari

On Feb 19, 2014, at 10:33 PM, Andrew G. Malis <[email-masked]> wrote:

> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: "Romascanu, Dan (Dan)" <[email-masked]>
Date: Thu, 20 Feb 2014 09:57:11 +0000

Hi Samer,

Thank you for your response and for addressing my comments. I am fine with the proposed solutions. At this point in the process the change control of the document is with the shepherding AD, so please wait for his instructions about when to update the document. 

Regards,

Dan

> -----Original Message-----
> From: Samer Salam (ssalam) [mailto:[email-masked]]
> Sent: Wednesday, February 19, 2014 8:30 PM
> To: Andrew G. Malis; Romascanu, Dan (Dan)
> Cc: [email-masked]; [email-masked]; BOCCI Matthew
> Subject: Re: Gen-ART telechat review of draft-ietf-pwe3-iccp-13.txt
> 
> Hi Dan,
> 
> >>posting a  new version of the draft.
> >>
> >>
> >>
> >>
> >>
> >> Ready with issues. I did not see any answer from the editors or
> >>shepherd to  the issues raised in the IETFLC Gen-ART review and there
> >>was no revision of  the document since then. Although none of the
> >>issues raised seems to be  blocking, I believed that they should be
> >>considered and answered as part of  the IETF Last Call Comments.
> >>
> >>
> >>
> >> This is a complex but well written document. It is ready, a number of
> >>minor  issues need clarification and possibly editing. Some nits also
> >>may be  considered to fix
> >>
> >>
> >>
> >> Major issues:
> >>
> >>
> >>
> >> None
> >>
> >>
> >>
> >> Minor issues:
> >>
> >>
> >>
> >>results if there no agreement on a finite set of causes. Anyway -
> >>should  this not be UTF-8 format?
> 
> >> object in the IF-MIB, should not also Port (interface) name
> >>correspond to  ifName truncated to 20 characters whenever possible?
> >>
>