Subject: [RTG-DIR] RtgDir review: draft-ietf-pals-mpls-tp-mac-wd-01

From: Martin Vigoureux <[email-masked]>
Date: Wed, 23 Sep 2015 16:23:13 +0200

Hello,

I have been selected as the Routing Directorate reviewer for this draft. 
The Routing Directorate seeks to review all routing or routing-related 
drafts as they pass through IETF last call and IESG review, and 
sometimes on special request. The purpose of the review is to provide 
assistance to the Routing ADs. For more information about the Routing 
Directorate, please see ​ 
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it 
would be helpful if you could consider them along with any other IETF 
Last Call comments that you receive, and strive to resolve them through 
discussion or by updating the draft.

Document: draft-ietf-pals-mpls-tp-mac-wd-01
Reviewer: Martin Vigoureux
Review Date: 2015-09-23
IETF LC End Date: 2015-10-02
Intended Status: Standards Track

Summary:
This document is basically ready for publication, but has nits that 
should be considered prior to publication.

Comments:
The document is concise and (nearly) clear in the description it makes 
of the technology it defines.

Issues:
Section 3
       |0 0 0 1|Version|   Reserved    |0xZZ MAC Withdraw OAM Message  |
Shouldn't this be (TBD) instead of 0xZZ ?

It might only be me having difficulties but I feel that this paragraph 
could be rewritten to be clearer:
    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 sequence number handling is described in [RFC4385] with
    the exception that sequence number in this case is 32-bits and hence
    sequence number in half the number space (~2billion) is considered in
    the valid receive range.
Why use half? Half of what (of 32bits or of 16 bits)? ...
Since you refer to two sequence number fields we loose track of which is 
which in the sentences.

Section 4
The drafts says:
                                                        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.
As I read it, in the case of the first set of MAC TLVs to withdraw, the 
counter will move to 2 and the seq# in the message also = 2. Is that 
correct?

Now, on the receiving side, register is at 1, and the draft says:
        Whenever a MAC withdrawal message is received, and if the
    sequence number on the message is greater than the value in the
    register, the MAC address(es) contained in the MAC TLV(s) is/are
    removed, and the register is updated with the received sequence
    number plus 1.  The receiver sends an ACK whose sequence number is
    the same as that in the received message.
we are in that case, so register moves at 3 and sends an ACK with seq#=2

Now the sender needs to send a new list of MAC TLVs to withdraw.
It increments its counter to 3 and sends the message with seq#=3

But at the receiver side we are now in the situation:
    If the sequence number in the received message is smaller than or
    equal to the value in the register, the MAC TLV(s) is/are not
    processed.

So, I am sure I have missed something, but what?
Maybe the sender also increments its counter when it receives an ACK,
but I have not seen this written.
Or is it that the receiver should set its counter to the received seq# 
and not received seq# + 1

Section 6.2
I am doubtful about the need to create a sub registry for the TLVs 
behind a MAC withdrawal OAM message, but I can leave with it.
What is really puzzling me is why do you want to call this sub-registry 
the "Sequence Number" registry (in order to control "Sequence Number 
TLV" type value assignments).
Do you really foresee the need for 16 382 different Sequence Number TLVs?
By the way, I think you should explicitly give a name to that 
sub-registry. I think the title of the sub-section is not enough for that.

Also, the draft says:
    The Type space is divided into assignment ranges;
Well, in reality there is only one assignment range [0 16,383], the rest 
is reserved. Also you do not mention the rest of the range.
So I'd write:
    The Type space is divided into different ranges:
    The value 0 and the values of the range 16 384 to 65 535 are
    reserved and not available for assignments.
    The range 1 to 16 384 is available for assignments, with the
    "Standards Action" policy (RFC5226]).
    This document defines value 0x0001.
    The initial registry should look like:
    Type        Description
    --------    -----------------------------
    0           Reserved (not available for allocation)
    1           Sequence Number
    2-16383     Unassigned
    16384-65535 Reserved (not available for allocation)

Section 7
I am surprised to see RFC2119 as an Informative Reference

Nits:
Not being a native English speaker I have hesitated before making this 
comment, but it seems to me that in many places you use withdraw while 
withdrawal should have been used instead.

Section 1
s/withdrawl/withdrawal/
s/re-uses concepts of -/re-uses concepts of/
s/as is the/as it is the/ ?
s/but does/but do/

Section 4.1
s/node,it/node, it/
s/implementer/implementers/ ?
s/retrasmission/retransmission/

Section 6
s/IANA to a assign new/IANA to assign a new/

---

From: "Shah, Himanshu" <[email-masked]>
Date: Thu, 24 Sep 2015 10:42:46 -0400

Hi Martin -

Appreciate your thorough review.
Let me go through your comments and either update the draft or respond with clarification.

Thanks,
Himanshu

-----Original Message-----
From: Martin Vigoureux [mailto:[email-masked]] 
Sent: Wednesday, September 23, 2015 10:23 AM
To: <[email-masked]>
Cc: [email-masked]; [email-masked]
Subject: RtgDir review: draft-ietf-pals-mpls-tp-mac-wd-01

Hello,

I have been selected as the Routing Directorate reviewer for this draft. 
The Routing Directorate seeks to review all routing or routing-related drafts as they pass through IETF last call and IESG review, and sometimes on special request. The purpose of the review is to provide assistance to the Routing ADs. For more information about the Routing Directorate, please see ​ http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs, it would be helpful if you could consider them along with any other IETF Last Call comments that you receive, and strive to resolve them through discussion or by updating the draft.

Summary:
This document is basically ready for publication, but has nits that should be considered prior to publication.

Comments:
The document is concise and (nearly) clear in the description it makes of the technology it defines.

Issues:
Section 3

It might only be me having difficulties but I feel that this paragraph could be rewritten to be clearer:
    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 sequence number handling is described in [RFC4385] with
    the exception that sequence number in this case is 32-bits and hence
    sequence number in half the number space (~2billion) is considered in
    the valid receive range.
Why use half? Half of what (of 32bits or of 16 bits)? ...
Since you refer to two sequence number fields we loose track of which is which in the sentences.

Section 4
The drafts says:
                                                        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.
As I read it, in the case of the first set of MAC TLVs to withdraw, the counter will move to 2 and the seq# in the message also = 2. Is that correct?

So, I am sure I have missed something, but what?
Maybe the sender also increments its counter when it receives an ACK, but I have not seen this written.
Or is it that the receiver should set its counter to the received seq# and not received seq# + 1

Section 6.2
I am doubtful about the need to create a sub registry for the TLVs behind a MAC withdrawal OAM message, but I can leave with it.
What is really puzzling me is why do you want to call this sub-registry the "Sequence Number" registry (in order to control "Sequence Number TLV" type value assignments).
Do you really foresee the need for 16 382 different Sequence Number TLVs?
By the way, I think you should explicitly give a name to that sub-registry. I think the title of the sub-section is not enough for that.

Also, the draft says:
    The Type space is divided into assignment ranges; Well, in reality there is only one assignment range [0 16,383], the rest is reserved. Also you do not mention the rest of the range.
So I'd write:

Section 7

Nits:

Section 1
s/withdrawl/withdrawal/

Section 4.1
s/node,it/node, it/

Section 6