Subject: [Gen-art] Gen-ART review of draft-ietf-mpls-tp-shared-ring-protection-05.txt

From: Christer Holmberg <[email-masked]>
Date: Wed, 17 May 2017 12:49:29 +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>

Document:		draft-ietf-mpls-tp-shared-ring-protection-05.txt
Reviewer:		Christer Holmberg
Review Date:		17 May 2017
IETF LC End Date:	12 May 2017
IETF Telechat Date:	N/A
 
Summary:		The document is well written, but there are a few issues I¹d
like the authors to address.

Major Issues: None

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

Section 4.1.3:

The text says:

    "When an MPLS-TP transport path, such as an LSP, enters the ring,Š²

The ³such as an LSP² statement is confusing. Could there be something else
than LSP?

Section 4.4:

Would it be useful to say that, for a given ring, an interconnect node
acts as an egress node for that ring, meaning that all LSPs using the
interconnect node will use the same tunnel within the ring?

Section 4.4.2:

The text says:   

                  "The service LSPs that traverse the interconnected rings
use separate
                  ring tunnels on each ring, and the LSPs on different
rings are
                  stitched by the interconnection node.²

It¹s unclear to me what ³separate tunnels² mean. As there are two
different rings, there will obviously be separate tunnels. Or, do you mean
to say something else?

Section 5.1:

The first sentence says:

                 "The MSRP protection operation MUST be controlled with
the help of the
                  Ring Protection Switch protocol (RPS).²

I think it would be good to have a few introduction sentences of the RPS
protocol, before mandating the usage of it.

The text says:   

                 "The RPS protocol MUST carry the ring status information
and RPS
                  requests, either automatically initiated or externally
initiated,
                  between the ring nodes.²

This text is a little confusing. Is this a protocol requirement, or a
protocol usage requirement? Similar to my previous comments, a generic
introduction to the protocol, and the requirements it has to fulfil, would
be useful. In addition, that text should reference to section 5.3
for the justification of defining a new protocol in the first place.

Editorial Issues:
-----------------

Generic:

In the document you use both ³ring node² and ³ring-node² terminology.
Unless there is a reason for that, please use consistent terminology.

Section 1:

The text says:

    "As described in [RFC5654], MPLS-TP requirements, section 2.5.6.1"

Šand later:

    "described in section 2.5.6.1 of [RFC5654]."

Please use consistent wording.

Section 3:

I like the way the section describes how the requirements have been met.
As I assume most of the solutions are described more in detail elsewhere
in the document, I wonder whether it would be possible to add references?

Something like:

"For detailed information, see section x.x.x.x."

Section 4.1:

The text says:

   "A port can carry multiple ring tunnels, and a ring tunnel can carry
multiple LSPs."

I think it would be good to add a picture showing a port carrying multiple
ring tunnels, carrying multiple LSPs.

---

From: "Weiqiang Cheng" <[email-masked]>
Date: Thu, 18 May 2017 23:32:31 +0800

Hi Christer,
Thank you very much for reviewing the draft in detail and for your valid comments.
We, the authors of the draft, will incorporate your comments.
B.R.
Weiqiang Cheng

-----邮件原件-----
发件人: Christer Holmberg [mailto:[email-masked]] 
发送时间: 2017年5月17日 20:49
收件人: [email-masked]; [email-masked]
主题: Gen-ART review of draft-ietf-mpls-tp-shared-ring-protection-05.txt

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>

Major Issues: None

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

Section 4.1.3:

The text says:

The ³such as an LSP² statement is confusing. Could there be something else than LSP?

Section 4.4:

Would it be useful to say that, for a given ring, an interconnect node acts as an egress node for that ring, meaning that all LSPs using the interconnect node will use the same tunnel within the ring?

Section 4.4.2:

The text says:   

                  "The service LSPs that traverse the interconnected rings use separate
                  ring tunnels on each ring, and the LSPs on different rings are
                  stitched by the interconnection node.²

It¹s unclear to me what ³separate tunnels² mean. As there are two different rings, there will obviously be separate tunnels. Or, do you mean to say something else?

Section 5.1:

The first sentence says:

                 "The MSRP protection operation MUST be controlled with the help of the
                  Ring Protection Switch protocol (RPS).²

I think it would be good to have a few introduction sentences of the RPS protocol, before mandating the usage of it.

The text says:   

                 "The RPS protocol MUST carry the ring status information and RPS
                  requests, either automatically initiated or externally initiated,
                  between the ring nodes.²

This text is a little confusing. Is this a protocol requirement, or a protocol usage requirement? Similar to my previous comments, a generic introduction to the protocol, and the requirements it has to fulfil, would be useful. In addition, that text should reference to section 5.3 for the justification of defining a new protocol in the first place.

Editorial Issues:
-----------------

Generic:

Section 1:

The text says:

Šand later:

Section 3:

I like the way the section describes how the requirements have been met.
As I assume most of the solutions are described more in detail elsewhere in the document, I wonder whether it would be possible to add references?

Something like:

Section 4.1:

The text says:

   "A port can carry multiple ring tunnels, and a ring tunnel can carry multiple LSPs."

I think it would be good to add a picture showing a port carrying multiple ring tunnels, carrying multiple LSPs.

---

From: Huub van Helvoort <[email-masked]>
Date: Mon, 22 May 2017 00:32:25 +0200

Hej Christer,

Thank you very much for reviewing this draft.
The minor issues and editorial issues you identified will be addressed
by the authors.

Best regards, Huub (on behalf of the co-authors).

-------

-- 
================================================================
Always remember that you are unique...just like everyone else...

---

From: Eric Gray <[email-masked]>
Date: Mon, 22 May 2017 08:11:30 +0000

Huub,

Many (if not most) of Christer's comments are in the form of questions.  Hence I suggest that - in addressing the comments - you and your co-authors should first answer the questions and then determine what changes are actually needed.

I only suggest this as - for at least some of the questions - it might not be a good idea to post a new version and ask if Christer's issues are addressed, at least not as a first step.

I do not assume you did not know this, but your ACK message below does not seem to include this step in your plan to address Christer's comments...

--
Eric
(Responding as Document Shepherd)

-----Original Message-----
From: Gen-art [mailto:[email-masked]] On Behalf Of Huub van Helvoort
Sent: den 22 maj 2017 00:32
To: Christer Holmberg <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [Gen-art] Gen-ART review of draft-ietf-mpls-tp-shared-ring-protection-05.txt

Hej Christer,

Thank you very much for reviewing this draft.
The minor issues and editorial issues you identified will be addressed by the authors.

-------
> different rings, there will obviously be separate tunnels. Or, do you 
> mean to say something else?
>
>
>
> Section 5.1:
>
> The first sentence says:
>
> I think it would be good to have a few introduction sentences of the 
> RPS protocol, before mandating the usage of it.
>
>
> The text says:
>
> introduction to the protocol, and the requirements it has to fulfil, 
> would be useful. In addition, that text should reference to section 
> 5.3 for the justification of defining a new protocol in the first place.
>
>
>
> Editorial Issues:
> -----------------
>
> Generic:
>
> As I assume most of the solutions are described more in detail 
> elsewhere in the document, I wonder whether it would be possible to add references?
>
> Something like:
>
> I think it would be good to add a picture showing a port carrying 
> multiple ring tunnels, carrying multiple LSPs.
>
>
>
>

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

---

From: Huub van Helvoort <[email-masked]>
Date: Mon, 22 May 2017 12:14:30 +0200

Hello Eric,

Thanks for the advise.
My reply was to acknowledge that we have seen the review. The "address" was indeed to answer questions or update the draft.

The draft is on the IESG telechat list of May 25. I think we cannot provide a new draft before that date and/or answer all of Christers questions.

Best regards, Huub.

⁣Sent from my Nexus 6P ​

On 22 May 2017, 10:11, at 10:11, Eric Gray <[email-masked]> wrote:
>Huub,
>
>Many (if not most) of Christer's comments are in the form of questions.
>Hence I suggest that - in addressing the comments - you and your
>co-authors should first answer the questions and then determine what
>changes are actually needed.
>
>I only suggest this as - for at least some of the questions - it might
>not be a good idea to post a new version and ask if Christer's issues
>are addressed, at least not as a first step.
>
>I do not assume you did not know this, but your ACK message below does
>not seem to include this step in your plan to address Christer's
>comments...
>
>--
>Eric
>To: Christer Holmberg <[email-masked]>;
>[email-masked];
>[email-masked]
>Subject: Re: [Gen-art] Gen-ART review of
>draft-ietf-mpls-tp-shared-ring-protection-05.txt
>
>Hej Christer,
>
>> The ³such as an LSP² statement is confusing. Could there be something
>
>> else than LSP?
>>
>>
>>
>> Section 4.4:
>>
>>                    "The service LSPs that traverse the interconnected
>
>> rings use separate

---

From: Loa Andersson <[email-masked]>
Date: Mon, 22 May 2017 14:14:31 +0200

Huub,

On 2017-05-22 12:14, Huub van Helvoort wrote:
> Hello Eric,
>
> Thanks for the advise.
> My reply was to acknowledge that we have seen the review. The "address"
> was indeed to answer questions or update the draft.
>
> The draft is on the IESG telechat list of May 25. I think we cannot
> provide a new draft before that date and/or answer all of Christers
> questions.

Even if you can't do the update of the document, it would be good to 
tell Deborah if you think there are any issues relating to Christer's
comments or if they are possible to address in a straightforward manner.

/Loa
>
> Best regards, Huub.
>
>
> Sent from my Nexus 6P
> On 22 May 2017, at 10:11, Eric Gray <[email-masked]
> <mailto:[email-masked]>> wrote:
>
>     Huub,
>
>         "As described in [RFC5654], MPLS-TP requirements, section
>         2.5.6.1 <http://2.5.6.1>"
>
>         Šand later:
>
>         "described in section 2.5.6.1 <http://2.5.6.1> of [RFC5654]."
>
>     ------------------------------------------------------------------------
>
>     ------------------------------------------------------------------------
>
>     Gen-art mailing list
>     [email-masked]

-- 

Loa Andersson                        email: [email-masked]
Senior MPLS Expert                          [email-masked]
Huawei Technologies (consultant)     phone: +46 739 81 21 64

---

From: Huub van Helvoort <[email-masked]>
Date: Wed, 24 May 2017 00:36:09 +0200

Hej Christer,

As I mentioned in my previous email I have discussed your

questions and comments with the co-authors of this draft.

Please find our response in line [Huub].

We hope that we have provided the required clarification.

Best regards, Huub.

---

Between ourselves:

here is my suggested reply to Christer in line [Huub]

On 17/05/2017 14:49, Christer Holmberg wrote:

I am the assigned
Gen-ART reviewer for this draft. For background on

Document:

Review Date:        17 May 2017

IETF Telechat Date:    N/A

Summary:        The document is well written, but there are a
few issues I¹d

Major Issues: None

Minor Issues:

-------------

Section 4.1.3:

The text says:

"When an MPLS-TP transport path, such as an LSP, enters the
ring,Š²

The ³such as an LSP² statement is confusing. Could there be
something else

than LSP?

[Huub] the MPLS-TP transport path is an LSP.

Change the text to: "when an MPLS-TP transport path, i.e.
an LSP, enters..."

Section 4.4:

Would it be useful to say that, for a given ring, an
interconnect node

acts as an egress node for that ring, meaning that all LSPs
using the

[Huub] indeed useful.

Change text to: "For a given ring, an interconnect node
acts as an egress

node for that ring, meaning that all LSPs using the
interconnect node will

use the same tunnel within the ring.

Section 4.4.2:

The text says:

"The service LSPs that traverse the interconnected rings use
separate

It¹s unclear to me what ³separate tunnels² mean. As there are
two

different rings, there will obviously be separate tunnels. Or,
do you mean

to say something else?

[Huub] indeed separate tunnels are used.

Change the text to:  "The service LSPs that traverse the
interconnected rings

use a ring tunnels in each ring, and the traversing LSPs
are stitched in the

interconnection node."

Section 5.1:

The first sentence says:

"The MSRP protection operation MUST be controlled with the
help of the

I think it would be good to have a few introduction sentences of
the RPS

[Huub] Understood. A short introduction of the purpose of
the RPS protocol

can be added.

(we will try to have text ready before the IESG telechat)

The text says:

"The RPS protocol MUST carry the ring status information and
RPS

between the ring nodes.²

This text is a little confusing. Is this a protocol requirement,
or a

protocol usage requirement? Similar to my previous comments, a
generic

introduction to the protocol, and the requirements it has to
fulfil, would

be useful. In addition, that text should reference to section
5.3

for the justification of defining a new protocol in the first
place.

[Huub] text can be added for clarification.

Change the text as follows:  "The RPS protocol MUST
distribute the ring

status information and RPS requests to all the ring
nodes.

Changes in the ring status information and RPS requests can
be

initiated automatically based on link status or caused by
external commands."

Editorial Issues:

-----------------

Generic:

In the document you use both ³ring node² and ³ring-node²
terminology.

Unless there is a reason for that, please use consistent
terminology.

[Huub] can be fixed as suggested.

Section 1:

The text says:

2.5.6.1"

Šand later:

[Huub] can be fixed as suggested.

Section 3:

I like the way the section describes how the requirements have
been met.

elsewhere

in the document, I wonder whether it would be possible to add
references?

Something like:

[Huub] can be fixed as suggested.

Section 4.1:

The text says:

"A port can carry multiple ring tunnels, and a ring tunnel
can carry

multiple LSPs."

I think it would be good to add a picture showing a port
carrying multiple

[Huub] we will add a figure showing how a port carries LSPs and

ring tunnels.

(we will try to have the figure ready before the IESG
telechat)

The figure will be based on the following:

Because the LSPs on an ingress port can egress at different
egress

ports/nodes a port can carry multiple ring tunnels.

Because separate LPSs on a ingress port can egress on the same

egress port/node they are using the same ring tunnel.

Because an egress port can terminate ring tunnels from different

ingress ports/nodes a port can carry multiple ring tunnels.

Because a ring tunnel can carry several LSPs, an egress port can

carry several LSPs in the same ring tunnel.

---------

Best regards, Huub.

--

---

From: Eric Gray <[email-masked]>
Date: Wed, 24 May 2017 07:54:53 +0000

Huub,

I have minor comments on some of your proposed changes below.

From: Gen-art [mailto:[email-masked]] On Behalf Of Huub van Helvoort
Sent: den 24 maj 2017 00:36
To: Christer Holmberg <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [Gen-art] Gen-ART review of draft-ietf-mpls-tp-shared-ring-protection-05.txt

Hej Christer,

Best regards, Huub.
---

Between ourselves:

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><http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>

Major Issues: None

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

Section 4.1.3:

The text says:

[Huub] the MPLS-TP transport path is an LSP.
Change the text to: "when an MPLS-TP transport path, i.e. an LSP, enters..."

Section 4.4:

[Huub] indeed useful.
Change text to: "For a given ring, an interconnect node acts as an egress
node for that ring, meaning that all LSPs using the interconnect node will
use the same tunnel within the ring.

I am not sure what text this changes.  Are you proposing to add this text?  If so, it is misleading without context.  As I understand rings, all traffic going through the interconnect node is not necessarily using it as an egress to the interconnected ring.  In this statement, you imply either that all traffic going through a ring node that is also the “interconnected node” is traffic destined to cross from one ring to another, or that an “interconnected node” comprises that part of a ring node for which this is true.

I suspect you need to add “as an egress from one specific ring to another” after “using the interconnect node.”

Some care needs to be used in wording this as the fact that a ring node provides an interconnect between two rings does not mean the same node cannot also provide at least one separate interconnect involving an additional ring.  That is the same node might provide an interconnect between Ring A and Ring B, and between Ring A and Ring C.  If this (or anything much like it) is the case, then I assume that the traffic for the first interconnect would be on one tunnel while the traffic for the second would be on another.

Section 4.4.2:

The text says:

[Huub] indeed separate tunnels are used.
Change the text to:  "The service LSPs that traverse the interconnected rings
use a ring tunnels in each ring, and the traversing LSPs are stitched in the
interconnection node."

I had previously read this text to have more meaning than remains in your suggested change.  While it is obvious that the tunnels used in each ring are separate, I had read this to also mean that these service LSPs use a separate tunnel in a ring to allow for them to traverse from ring to ring at the interconnection node.

That information may be considerably less obvious than that separate tunnels are used in separate rings.

Section 5.1:

The first sentence says:

[Huub] Understood. A short introduction of the purpose of the RPS protocol
can be added.
(we will try to have text ready before the IESG telechat)

The text says:

This text is a little confusing. Is this a protocol requirement, or a
protocol usage requirement? Similar to my previous comments, a generic
introduction to the protocol, and the requirements it has to fulfil, would
be useful. In addition, that text should reference to section 5.3
for the justification of defining a new protocol in the first place.
[Huub] text can be added for clarification.
Change the text as follows:  "The RPS protocol MUST distribute the ring
status information and RPS requests to all the ring nodes.
Changes in the ring status information and RPS requests can be
initiated automatically based on link status or caused by external commands."

Editorial Issues:
-----------------

Generic:

Section 1:

The text says:

Šand later:

Section 3:

Something like:

Section 4.1:

The text says:

I think it would be good to add a picture showing a port carrying multiple
ring tunnels, carrying multiple LSPs.
[Huub] we will add a figure showing how a port carries LSPs and
ring tunnels.
(we will try to have the figure ready before the IESG telechat)

Because the LSPs on an ingress port can egress at different egress

---------

Best regards, Huub.

---

From: Christer Holmberg <[email-masked]>
Date: Wed, 24 May 2017 10:53:55 +0000

Hi,

I’ll provide my input as a reply to Eric’s reply, trying to keep everything in one place :)

From: Gen-art [mailto:[email-masked]] On Behalf Of Huub van Helvoort
Sent: den 24 maj 2017 00:36
To: Christer Holmberg <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>; [email-masked]<mailto:[email-masked]>
Subject: Re: [Gen-art] Gen-ART review of draft-ietf-mpls-tp-shared-ring-protection-05.txt

Hej Christer,

Best regards, Huub.
---

Between ourselves:

Major Issues: None

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

Section 4.1.3:

The text says:

Looks good.

Section 4.4:

[Huub] indeed useful.

I think Eric raises a good question (if I understand his comment correctly): can a given egress node be used BOTH as an interconnected node and a “normal” egress node? Or, are all LSPs using that node always interconnected into another ring? And, can a given egress node be used as an interconnect node to multiple rings? Whatever the answers are, I think it would be good to clarify in the text.

Section 4.4.2:

The text says:

I don’t think the text need to say that there will be multiple tunnels, because that is obvious. I guess what Eric is asking is whether there are, within a given ring, interconnected-specific procedures how a ring is selected? Or, is the ring selected using normal procedures, based on the egress node of that ring, no matter whether the traffic is interconnected or not? If so, I think you could simply say that, within a given ring, the tunnel is selected using normal ring selection procedures.

Section 5.1:

The first sentence says:

OK.

The text says:

I don’t think that addresses my issue: is the MUST a protocol requirement, or a usage requirement?

Assuming it’s a usage requirement, couldn’t you simply say “The RPS protocol is used to distribute…”?

OK.

Editorial Issues:
-----------------

Generic:

OK.

Section 1:

The text says:

Šand later:

OK.

Section 3:

Something like:

OK.

Section 4.1:

The text says:

Looks good.

Regards,

Christer

---

From: Alissa Cooper <[email-masked]>
Date: Wed, 24 May 2017 11:37:03 -0400

Christer, thanks for your review and continued engagement, and thanks to the authors and shepherd for discussing the issues Christer raised. I’d like to see this discussion conclude and the associated changes folded into the next version of the draft. I’ve balloted No Objection.

Alissa

> On May 24, 2017, at 6:53 AM, Christer Holmberg <[email-masked]> wrote:
> 
> Hi,
> 
> From: Gen-art [mailto:[email-masked] <mailto:[email-masked]>] On Behalf Of Huub van Helvoort
> To: Christer Holmberg <[email-masked] <mailto:[email-masked]>>; [email-masked] <mailto:[email-masked]>; [email-masked] <mailto:[email-masked]>
>> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>
>> 
> [email-masked] <mailto:[email-masked]>
> https://www.ietf.org/mailman/listinfo/gen-art <https://www.ietf.org/mailman/listinfo/gen-art>