Thanks Ralph..

Himanshu


-----Original Message-----
From: Ralph Droms (rdroms) [mailto:rdr...@cisco.com] 
Sent: Monday, October 26, 2015 1:11 PM
To: Shah, Himanshu
Cc: A. Jean Mahoney; General Area Review Team; 
draft-ietf-pals-mpls-tp-mac-wd....@ietf.org; The IESG
Subject: Re: [Gen-art] Review: draft-ietf-pals-mpls-tp-mac-wd

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 <hs...@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....@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....@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:rdr...@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....@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 <hs...@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:rdr...@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....@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>

_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

Reply via email to