Re: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd

"Ralph Droms (rdroms)" <rdroms@cisco.com> Mon, 26 October 2015 17:11 UTC

Return-Path: <rdroms@cisco.com>
X-Original-To: gen-art@ietfa.amsl.com
Delivered-To: gen-art@ietfa.amsl.com
Received: from localhost (ietfa.amsl.com [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 5D31F1B4FAA; Mon, 26 Oct 2015 10:11:05 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -14.511
X-Spam-Level:
X-Spam-Status: No, score=-14.511 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, RCVD_IN_DNSWL_HI=-5, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, USER_IN_DEF_DKIM_WL=-7.5] autolearn=ham
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id SPoseLnHLlN0; Mon, 26 Oct 2015 10:11:01 -0700 (PDT)
Received: from rcdn-iport-4.cisco.com (rcdn-iport-4.cisco.com [173.37.86.75]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 306BA1B4414; Mon, 26 Oct 2015 10:11:01 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=cisco.com; i=@cisco.com; l=13726; q=dns/txt; s=iport; t=1445879461; x=1447089061; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-id:content-transfer-encoding: mime-version; bh=FfMAPjEbMGTAzTYaFaVkLfI+1DsX8wbxraaeagGZvsA=; b=K0QyazxLRYrbTrAqa1inGnW7K8Jv9iZji9CugWsy8hYoFBWQUj5ubM4p DsTK+ijIWCldWMqqk0DdPG+d9rE8Ydvr9at4T9bRhL8C51KF8gfjvZO6O ra6ig2/4HG94UemndCqSunWnVBMOdgen00bbIrj/MBIzYLKeKUgYp8L0W c=;
X-IronPort-Anti-Spam-Filtered: true
X-IronPort-Anti-Spam-Result: A0ADAgA8Xi5W/4wNJK1egzZUbwa+PgENgVohhXwCgTQ4FAEBAQEBAQGBCoQyAQEBAwEOLCsUBQcEAgEIEQECAQEBAR4JBzIUAwYIAgQOBYgoCA3EegEBAQEBAQEBAQEBAQEBAQEBAQEBARiGd4IQgm6EKhEBHjMHBoMUgRQFljYBhRuCK0WFFoFZSIN3jUeEYINvAR8BAUKCDiCBVXIBhVc6gQYBAQE
X-IronPort-AV: E=Sophos;i="5.20,201,1444694400"; d="scan'208";a="41044236"
Received: from alln-core-7.cisco.com ([173.36.13.140]) by rcdn-iport-4.cisco.com with ESMTP; 26 Oct 2015 17:11:00 +0000
Received: from XCH-ALN-020.cisco.com (xch-aln-020.cisco.com [173.36.7.30]) by alln-core-7.cisco.com (8.14.5/8.14.5) with ESMTP id t9QHAx1w011458 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Mon, 26 Oct 2015 17:10:59 GMT
Received: from xch-aln-016.cisco.com (173.36.7.26) by XCH-ALN-020.cisco.com (173.36.7.30) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Mon, 26 Oct 2015 12:10:35 -0500
Received: from xch-aln-016.cisco.com ([173.36.7.26]) by XCH-ALN-016.cisco.com ([173.36.7.26]) with mapi id 15.00.1104.000; Mon, 26 Oct 2015 12:10:35 -0500
From: "Ralph Droms (rdroms)" <rdroms@cisco.com>
To: "Shah, Himanshu" <hshah@ciena.com>
Thread-Topic: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd
Thread-Index: AQHRBrtB0K0DtUmA2kyW/x0Nq3/Sjp57F8ewgAAfkhCAAAiqAIADKOeA
Date: Mon, 26 Oct 2015 17:10:35 +0000
Message-ID: <8F469545-72FE-4C45-ACA6-20C33A0CDCD7@cisco.com>
References: <095AB7C0-C1B7-4963-852C-A5A613AD261A@cisco.com> <40746B2300A8FC4AB04EE722A593182B9ACC4AF3@ONWVEXCHMB04.ciena.com> <D98030BB-32EB-4859-A71B-8D54BDBF031C@cisco.com> <40746B2300A8FC4AB04EE722A593182B9C37860E@ONWVEXCHMB04.ciena.com>
In-Reply-To: <40746B2300A8FC4AB04EE722A593182B9C37860E@ONWVEXCHMB04.ciena.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
x-ms-exchange-messagesentrepresentingtype: 1
x-ms-exchange-transport-fromentityheader: Hosted
x-originating-ip: [10.131.118.75]
Content-Type: text/plain; charset="us-ascii"
Content-ID: <88A4BEAFEE637A42903844692A5543D4@emea.cisco.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
Archived-At: <http://mailarchive.ietf.org/arch/msg/gen-art/gW5jlbb7sC9KF4WnLkTRtwxibvE>
Cc: General Area Review Team <gen-art@ietf.org>, "draft-ietf-pals-mpls-tp-mac-wd.all@ietf.org" <draft-ietf-pals-mpls-tp-mac-wd.all@ietf.org>, The IESG <iesg@ietf.org>
Subject: Re: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd
X-BeenThere: gen-art@ietf.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: "GEN-ART: General Area Review Team" <gen-art.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/gen-art>, <mailto:gen-art-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/gen-art/>
List-Post: <mailto:gen-art@ietf.org>
List-Help: <mailto:gen-art-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/gen-art>, <mailto:gen-art-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 26 Oct 2015 17:11:05 -0000

Himanshu - I've received your revised draft.  I've been stuck in a variety of meetings Monday and haven't had time to review it.  I should be able to look at it before the end of the day.

- Ralph

> On Oct 24, 2015, at 5:56 PM 10/24/15, Shah, Himanshu <hshah@ciena.com> wrote:
> 
> Now with the draft..
> 
> Thanks,
> Himanshu
> 
> 
> -----Original Message-----
> From: Shah, Himanshu 
> Sent: Saturday, October 24, 2015 5:52 PM
> To: 'Ralph Droms (rdroms)'
> Cc: 'A. Jean Mahoney'; 'General Area Review Team'; 'draft-ietf-pals-mpls-tp-mac-wd.all@ietf.org'; 'The IESG'
> Subject: RE: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd
> 
> One more update that was discussed in previous emails but not included in your last email -
> 
> Original text: 
>  Only half of the sequence number space is used.  Modular arithmetic
>  is used to detect wrapping of sequence number.  When sequence number
>  wraps, all MAC addresses are flushed and the sequence number is
>  reset.  The 16-bit sequence number handling is described in
>  [RFC4385]. This document uses 32-bits sequence numbers and hence
>  sequence number in half the number space (i.e. 31-bits or ~2billion)
>  is considered in the valid receive range.
> 
> [Ralph]
> This paragraph is not at all clear to me. Reading section 4 of RFC 4385 helped but left me unsure about how my understanding of how to extend the sequence number mechanism to 32 bits corresponds to the expectations of this document.
> 
> 
> [Himanshu>] 
> 
> New text:
> 
>   The lack of reliable transport protocol for the in-band OAM
>   necessitates a presence of sequencing and acknowledgement
>   scheme so that the receiver can recognize newer message from
>   retransmitted older messages. The [RFC4385] describes the details
>   of sequence number handling which includes overflow detection for
>   sequence number field of size 16-bits. This document leverages
>   the same scheme with the two exemptions
>       - sequence number field is of size 32-bits
>       - overflow detection is simplified such that sequence
>         number exceed 2,147,483,647 (0x7FFFFFFF) is considered
> 	   overflow and reset to 1.
> 
> Thanks,
> Himanshu
> 
> 
> -----Original Message-----
> From: Shah, Himanshu
> Sent: Saturday, October 24, 2015 4:00 PM
> To: 'Ralph Droms (rdroms)'
> Cc: A. Jean Mahoney; General Area Review Team; draft-ietf-pals-mpls-tp-mac-wd.all@ietf.org; The IESG
> Subject: RE: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd
> 
> I am updating the drafts to address your comments where relevant.
> 
> Since there is too many indentations below, I am bringing you latest comments here, and respond.
> 
> ---
> [ralph] What I think would improve this specification is clarification that trims redundant specification details from draft-ietf-pals-mpls-tp-mac-wd and cites, concisely and exactly, the other documents from which the specifications are copied.
> 
> [Himanshu] Trimming where relevant.
> ---
> 
> [ralph] It wasn't obvious to me what is intended as a protocol specification and what is intended as a description of the protocol.  I see that RFC 4762 includes text that describes how to process an empty MAC List TLV, so perhaps removal of the text altogether would be best.
> 
> [Himanshu] removed the reference to empty MAC List TLV
> 
> ----
> [ralph]
>> The PW OAM message header is exactly the same as what is defined in 
>> [RFC6478].
>> 
> Is this statement really true?  The MAC Address Withdraw header seems to replace the "Refresh Timer" field with a "Reserved" field, and adds a new "R" flag.  The statement might be misleading to an implementor.
> 
> [Himanshu] replaced with following text:
> "The MAC withdraw PW OAM message follows the same guidelines used in  [RFC6478], whereby first 4-bytes of OAM message header is followed by  message specific field and a set of TLVs relevant for the message"
> 
> -----
> [ralph]
> I think it would be helpful to state explicitly that the Sequence Number TLV MUST be the first TLV in the message.
> 
> [Himanshu] added.
> ----
> [Ralph] I apologize if I appear to be finicky, again, but the sentence I quoted simply wasn't clear to me.  Common sense interpretation of specifications is, of course, expected, but in my experience a simple sentence like:
> 
> A MAC Address Withdraw OAM message with the A-bit set is sent by a receiver to acknowledge receipt and processing of a MAC Address Withdraw OAM message
> 
> [Himanshu]  Modified description of A-bit as follows -
> 
>   A single bit (called A-bit) is set by a receiver to acknowledge
>   receipt and processing of a MAC Address Withdraw OAM message.
>   In the acknowledge message, with A-bit set, MAC TLV(s) is/are
>   excluded.
> 
> ---
> 
> 
> Will send out the updated draft shortly..
> 
> 
> Thanks,
> Himanshu
> 
> -----Original Message-----
> From: Ralph Droms (rdroms) [mailto:rdroms@cisco.com]
> Sent: Thursday, October 22, 2015 11:29 AM
> To: Shah, Himanshu
> Cc: A. Jean Mahoney; General Area Review Team; draft-ietf-pals-mpls-tp-mac-wd.all@ietf.org; The IESG
> Subject: Re: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd
> 
> (originally sent 10/16; second try)
> 
> Hi, Himanshu - responses in line...
> 
>> On Oct 15, 2015, at 7:44 PM 10/15/15, Shah, Himanshu <hshah@ciena.com> wrote:
>> 
>> Hi Ralph -
>> Thanks for your thorough review.
>> 
>> Let me first address your major concern.
>> 
>> As you point out, this draft builds on existing standards.
>> We (authors and WG) had to carefully balance between the right amount 
>> of information and wanting to describe details of methods described in other RFCs.
>> 
>> This is frustrating to implementer (including myself) having to go 
>> back and forth between the documents. So I share that concern.
>> 
>> But we would like to refrain from indulging in beefing up the doc and 
>> risk deviating from other base standards. However, for certain, if 
>> there is any conflict or lack of clarity, we would prefer to rectify.
> 
> I agree that, in general, duplicating specifications from other documents increases the possibility that the respective documents unintentionally conflict with each other or are not updated in parallel.
>> 
>> To that end, I would rather prefer, trimming by removing conflicting/confusing text.
> 
> I wasn't clear in my review - I think making the references to other documents concise and clear, along with trimming unnecessary text from draft-ietf-pals-mpls-tp-mac-wd, will result in the best document.
> 
>> For example, sequence number processing - I rather would ask reader to 
>> get all details from relevant RFC, and point to only delta (which is 
>> to apply same logic that is used for 16-bit sequence number field to 
>> 32-bit field sequence number field that is used in this document).
> 
> This example is sort of an interesting one to consider, as I was thinking more of the examples in which the format and semantics of the MAC TLVs are exactly the same as in the cited defining documents.
> 
>> 
>> More comments in line.. and I look forward to your further guidance so 
>> we can wrap this up in time.
>> 
>> As a data point, there are two implementations of this draft deployed 
>> in a Telco network in Asia.
> 
> Noted.
> 
>> 
>> 
>> Thanks,
>> Himanshu
>> 
>> 
>> -----Original Message-----
>> From: Ralph Droms (rdroms) [mailto:rdroms@cisco.com]
>> Sent: Wednesday, October 14, 2015 4:02 PM
>> To: A. Jean Mahoney; General Area Review Team; 
>> draft-ietf-pals-mpls-tp-mac-wd.all@ietf.org
>> Subject: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd
>> 
>> 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-pals-mpls-tp-mac-wd-02, "MAC Address Withdrawal over Static Pseudowire"
>> Reviewer: Ralph Droms
>> Review Date: 2015-10-14
>> IETF LC End Date: 2015-10-19
>> IESG Telechat date: (if known) N/A
>> 
>> Summary:
>> 
>> This draft is on the right track but has open issues, described in the review.
>> 
>> 
>> Major issues:
>> 
>> While this document is describing a straightforward adaptation of previously defined standards to statically provisioned PWs, in my opinion an implementor would not necessarily be able to construct a fully interoperable implementation from this document.  There are several sections of the document that are not clear in their description of how to use mechanisms from referenced documents in this standard.
>> 
>> If it appears that some of my comments are overly finicky, I'll agree that the correct implementation could probably be deduced from the text in most cases.  That is, I didn't find many outright errors or inconsistencies.  Many of my comments took a lot of paging back and forth, reading of referenced documents and head-scratching, which, in my experience, can lead to implementations that don't interoperate.
>> 
>> [Himanshu>] Please see above for the justification of this approach.
> 
> Again, I wasn't clear in my review - my paging back and forth was both within draft-ietf-pals-mpls-tp-mac-wd and between draft-ietf-pals-mpls-tp-mac-wd and cited documents.  What I think would improve this specification is clarification that trims redundant specification details from draft-ietf-pals-mpls-tp-mac-wd and cites, concisely and exactly, the other documents from which the specifications are copied.
> 
>> 
>> Section 1:
>> 
>> When the number of MAC addresses to be removed is large, the empty MAC 
>> List TLV may be used.  The empty MAC List TLV signifies wildcard MAC 
>> Address withdrawl.
>> 
>> This text seems to be the only reference to the processing of an empty MAC List TLV. Specification of how the protocol works doesn't belong in the Introduction, and "wildcard MAC Address withdrawal" could certainly use some more explanation.
>> 
>> [Himanshu>] I would prefer taking the text out if its mention in "Introduction" section is not desirable.
>> Wildcard MAC withdraw is a very well-known concept in VPLS architecture and needs no more description to L2VPNers, IMO.
>> So absence of its reference in subsequent sections does not dilute the purpose of this document.
> 
> It wasn't obvious to me what is intended as a protocol specification and what is intended as a description of the protocol.  I see that RFC 4762 includes text that describes how to process an empty MAC List TLV, so perhaps removal of the text altogether would be best.
> 
>> 
>> Section 3:
>> 
>> The PW OAM message header is exactly the same as what is defined in 
>> [RFC6478].
>> 
>> Is this statement really true?  The MAC Address Withdraw header seems to replace the "Refresh Timer" field with a "Reserved" field, and adds a new "R" flag.  The statement might be misleading to an implementor.
>> 
>> [Himanshu>] I agree with you. This is statement is used loosely.
>> The PW OAM message header is meant to apply only to first 4 bytes.
>> 
>> Perhaps -
>> "The MAC withdraw PW OAM message follows the same guidelines used in 
>> [RFC6478], whereby first 4-bytes of OAM message header is followed by 
>> message specific field and a set of TLVs relevant for the message"
>> 
>> 
>> 
>> a MAC address withdraw OAM message MUST contain a "Sequence Number 
>> TLV" otherwise the entire message is dropped.
>> 
>> Is the "Sequence Number TLV" required to be the first TLV in the message?  Are the TLVs required to appear in any particular order?
>> 
>> [Himanshu>] Yes. It is important to determine the "newness" of the 
>> message first for processing eligibility and should not require hunting through entire message to find that TLV. My hope is that this is obvious to the reader who 'follows' the concepts in the draft.
>> 
>> If you feel, that such an explicit mention is necessary, I do not mind.
> 
> I think it would be helpful to state explicitly that the Sequence Number TLV MUST be the first TLV in the message.
> 
>> A single bit (called A-bit) is set to indicate if a MAC withdraw 
>> message is for ACK.  Also, ACK does not include MAC TLV(s).
>> 
>> Does this mean that the message is an ACK if the A-bit is set?  Can an ACK contain a "Sequence Number" TLV?
>> 
>> [Himanshu>] I do not quite follow. ACK HAS TO INCLUDE THE SEQ NO TLV, 
>> how else receiver know what is ACK of what seq# message is of? I think 
>> this falls into commonsense category BUT, the text explicitly says that ONLY MAC TLVs are not required.
> 
> I apologize if I appear to be finicky, again, but the sentence I quoted simply wasn't clear to me.  Common sense interpretation of specifications is, of course, expected, but in my experience a simple sentence like:
> 
> A MAC Address Withdraw OAM message with the A-bit set is sent by a receiver to acknowledge receipt and processing of a MAC Address Withdraw OAM message
> <draft-ietf-pals-mpls-tp-mac-wd-03.txt>