Re: Gen-ART Last Call Review of draft-ietf-pwe3-static-pw-status-08

Luca Martini <lmartini@cisco.com> Fri, 07 October 2011 23:18 UTC

Return-Path: <lmartini@cisco.com>
X-Original-To: ietf@ietfa.amsl.com
Delivered-To: ietf@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id B454521F8A56; Fri, 7 Oct 2011 16:18:06 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.357
X-Spam-Level:
X-Spam-Status: No, score=-1.357 tagged_above=-999 required=5 tests=[AWL=-1.058, BAYES_00=-2.599, MANGLED_LIST=2.3]
Received: from mail.ietf.org ([12.22.58.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id pRFe3iXMIqJX; Fri, 7 Oct 2011 16:18:06 -0700 (PDT)
Received: from napoleon.monoski.com (napoleon.monoski.com [70.90.113.113]) by ietfa.amsl.com (Postfix) with ESMTP id DD50421F899D; Fri, 7 Oct 2011 16:18:05 -0700 (PDT)
Received: from confusion.monoski.com (confusion.monoski.com [209.245.27.2]) (authenticated bits=0) by napoleon.monoski.com (8.13.8/8.13.8) with ESMTP id p97NLF1v026634 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 7 Oct 2011 17:21:18 -0600 (MDT)
Message-ID: <4E8F896A.4040305@cisco.com>
Date: Fri, 07 Oct 2011 17:21:14 -0600
From: Luca Martini <lmartini@cisco.com>
User-Agent: Mozilla/5.0 (X11; Linux i686 on x86_64; rv:7.0.1) Gecko/20110929 Thunderbird/7.0.1
MIME-Version: 1.0
To: Ben Campbell <ben@nostrum.com>
Subject: Re: Gen-ART Last Call Review of draft-ietf-pwe3-static-pw-status-08
References: <65E76746-02D2-4116-8306-7A41B425D50F@nostrum.com>
In-Reply-To: <65E76746-02D2-4116-8306-7A41B425D50F@nostrum.com>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 8bit
Cc: "gen-art@ietf.org Review Team" <gen-art@ietf.org>, "PWE3 WG (E-mail)" <pwe3@ietf.org>, draft-ietf-pwe3-static-pw-status.all@tools.ietf.org, The IETF <ietf@ietf.org>
X-BeenThere: ietf@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: IETF-Discussion <ietf.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/ietf>, <mailto:ietf-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/ietf>
List-Post: <mailto:ietf@ietf.org>
List-Help: <mailto:ietf-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/ietf>, <mailto:ietf-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 07 Oct 2011 23:18:06 -0000

Ben,
Thank you for the review .
Some comments below.
Luca


On 10/04/11 16:13, Ben Campbell 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>.
>
> Please resolve these comments along with any other Last Call comments you may receive.
>
> Document: draft-ietf-pwe3-static-pw-status-08
> Reviewer: Ben Campbell
> Review Date: 2011-10-04
> IETF LC End Date: 2011-10-05
>
> Summary: The draft is almost ready for publication as a proposed standard, but there are a few minor issues and editorial issues that need addressing first.
>
> Major issues:
>
> None
>
> Minor issues:
>
> -- 5.3:
>
> Has the work group considered how the retransmit scheme and 30 second refresh default will scale to very large deployments? Seems like this could result in a lot of retransmissions.
Yes. that is correct. This will most likely not scale for large deployments.
We have another document draft-ietf-pwe3-status-reduction-00.txt that
addresses this issue.
That extension is not necessary for most common small deployments in the
order of 100 PWs per access PE.

> -- 5.3, last paragraph: " This will cause the PE sending the all clear message to stop sending, and to continue normal operation."
>
> Where is that specified? Can you offer a reference?
An "all clear message" is just a pw status message with status value =0
. I'll make this clear in the text.

> -- 5.3.1, 1st paragraph: "The receiving PE will then set its timeout timer according to the timer value that is in the packet received, regardless of what timer value it sent."
>
> It's probably worth making a normative statement here, since it seems like this could easily result in an argument if the PEs disagree on the timer value.
An argument between who ?
I see a problem is the fact that we need to state a good practice
implementation policy.
Basically the PE should not insist on the value that was just refused.
I'll add some text to clarify this.
What that what you intended ?

> -- 5.3.1, 2nd paragraph:
>
> Please elaborate on "match exactly". What uniquely matches a message to an ack? How do you compare them?
>
> "… the sender PE MUST use the new timer value received."
>
> Doesn't this contradict the previous paragraph that lets the sender disagree? Is the subsequent treated different than the first attempt? (If so, is there a state machine that could be elaborated on?)
no, this text is redundant, I'll remove it.

> -- 5.3.2:
>
> I don't understand the guidance here. Are you saying a PE should send status even when it can't?
yes, this section is redundant, and adds no value. I'll remove it.
> -- 5.5, 1st paragraph: "… MUST be supported by both T-PEs to be enabled."
>
> How does one T-PE know another supports this?
Via a bit in the interface parameters as explained in sec 5.5.1.
> -- 5.5, last paragraph:
>
> This could use some elaboration. What is the purpose of having to send both ways?
>
>
Keep the state of S-PEs in sync between LDP and the in-band bypass mode.
That is what is stated here.
S-PEs are PE that are in the middle of the path. They can also originate
PW status messages , but only using LDP.
There is fairly complicated state machine described in rfc6073 that
would break if the messages are not sent in LDP as well.


> Nits/editorial comments:
>
> -- general: 
>
> IDNits returns some comments. I think something about the header format is confusing it.
Yes, I think that there is a bug in IDNits. I cannot figure out what is
causing this.
> -- abstract:
>
> Please expand BFD on first mention
fixed.
> -- Section 2:
>
> Please expand LDP and PE on first mention. (even though they are in the terminology section, since they are mentioned here before that section.)
fixed.
> -- section 2: "… without control plane."
>
> Missing article ("a" or "the")
fixed
> -- section 4:
>
> Please expand PSN, MPLS-TP, and BFD on first mention.
fixed.
> -- section 5.1, 2nd to last paragraph: "...PW OAM Message code"
>
> Missing "the"
fixed.
> -- section 5.1, last paragraph:
>
> This seems redundant with the IANA considerations section.
yes - removed.
> -- 5.2, 1st paragraph: "PW Status messages are indicated…"
>
> Indicated by what? (passive voice obscures responsible actor).
reworded this text.
> -- same paragraph: "PW Status TLV defined in [RFC4447].  The PW Status TLV format is as follows:"
>
> I'm confused is defined in 4447 and just repeated here, or defined here? If the first, please be very clear about that, so there's no question about where the normative definition lives. For example: "PW Status TLV defined in [RFC4447], and is repeated here for the reader's convenience:"
yes- fixed.
> -- 5.2, last paragraph: "PW OAM Status Messages MUST NOT be used as a connectivity verification method."
>
> That sounds like it belongs in the "applicability" section.
I think that it's missing an "also" here.

"PW OAM Status Messages MUST NOT be also used as a connectivity verification method."


> -- 5.3, 2nd paragraph: "...will be transmitted immediately."
>
> Transmitted by what? (passive voice obscures responsible actor).
fixed.
> -- 5.3.1, 1st paragraph: "The timer value set in the reply packet SHOULD then be used as the new transmit interval."
>
> By what? (passive voice obscures responsible actor).
fixed.
> -- 5.3.1, last paragraph: "standard procedures"
>
> Reference?
yes rfc4447.
> -- 5.5.1, last paragraph: "If Bit "B" is set , then the T-PE can receive S-PE bypass status messages in the G-ACH. If Bit "B" is not set the T-PE MUST NOT send S-PE bypass status messages in the G-ACH."
>
> I think the sender and receiver are mixed up here. The text seems to say if the bit is set you may receive but if the bit is not set you must not send?
re-wrote the paragraph:
If the T-PE receives an LDP label mapping message containing a generic
protocol flags interface parameter TLV with the bit "B" set, then the
T-PE receiving the label mapping message MAY send S-PE bypass status
messages in the G-ACH. If Bit "B" of said TLV, is not set, or the TLV is
not present, then the T-PE receiving the label mapping message MUST NOT
send S-PE bypass status messages in the G-ACH.

> -- 8 : IANA Considerations
>
> It would help to include the formal definition tables here, or reference them here. Also, can you include the URLs for each registry?
Tables have always been called out by name. What are "formal definition
tables" ?
I do not understand this comment. Iana section is quite clear.

> 0x0022 appears to be already assigned to MPLS-TP CC message.
Iana will have to sort that out then.
> "Pseudowire Switching Point PE TLV Type""
>
> I don't find that registry.  Did you mean sub-type?
It was missing the "sub-" in the TLV , fixed.
> 0x16 appears to be already assigned to "Stack capability"
>
>
>
I fixed the rest of the IANA
The PWE3 chairs and IANA will have to resolve the conflicts.



>