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. To that end, I would rather prefer, trimming by removing conflicting/confusing text. 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). 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. 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. 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. 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. 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. 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. 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>] OK. [Himanshu>] How about this paragraph - The lack of reliable transport protocol for the in-band OAM necessitates a presence of sequencing scheme so that the receiver can recognize newer message from retransmitted older messages. The [RFC4385] describes details of sequence number handling which include overflow detection. This document leverages the same scheme, with the exception of overflow detection which is simplified here such that sequence number exceed 2,147,483,647 (0x7fffffff) is considered overflow and reset to 1. Section 4.1: Each PW is associated with a counter to keep track of the sequence number of the transmitted MAC withdrawal messages. Whenever a node sends a new set of MAC TLVs, it increments the transmitted sequence number counter, and include the new sequence number in the message. The transmit sequence number is initialized to 1 at the onset. I'm pretty sure this is supposed to mean that the sequence number in the first message is 1. The text could be interpreted to mean that the counter is initialized to 1 and then incremented to 2 when sending the first message [Himanshu>] [Himanshu>] No. The text is correct. Each endpoint keeps the record of send and receive sequence number. These values are initialized to 1. If the sender sends the first MAC withdraw with sequence number 1, then receiver will ignore because it does not think that received MAC withdraw is new. So starting MAC withdraw send seq# is in fact 2. Do note that the purpose of the seq# logic is ONLY to detect the received message as NEWER than what was ack'ed before or re-transmits due to in-transits. _______________________________________________ Gen-art mailing list Gen-art@ietf.org https://www.ietf.org/mailman/listinfo/gen-art
