MPLS Label Switched Path (LSP) Pseudowire (PW) Status Refresh Reduction for Static PWs
draft-ietf-pals-status-reduction-05

I agree with Benoit re: Jürgen's review.

I was also quite confused by:
" In order to get a locally unique session ID, the recommended choice is to perform a CRC-16 giving as input the following data
       |Y|Y|M|M|D|D|H|H|M|M|S|S|L|L|L|

       Where:  YY: are the decimal two last digit of the current year
       MM: are the decimal two digit of the current month DD: are the
       decimal two digit of the current day HHMMSSLLL: are the decimal
       digits of the current time expressed in (hour, minutes, seconds,
       milliseconds) ... Any other method to
       generate a locally unique session ID is also acceptable."

Is the pipe character intended to mean concatenation? Or is it just for formatting?
If the former, why isn't this "YY|MM|DD|HHMMSSLLL" (or "YY|MM|DD|HH|MM|SS|LLL")?
Is this just CRC-16 (170410124223001)? An example would help...
Actually, this says that it is a "locally unique session ID" and that any other method is also acceptable, so why is any algorithm specified?

8.3. PW Status Refresh Reduction Notification Codes

   IANA needs to set up a registry of "PW status refresh reduction
   Notification Codes". These are 32-bit values. Type value 0 through 7
   are defined in this document. Type values 8 through 65536, and
   134,217,729 through 4,294,967,294 are to be assigned by IANA using
   the "Expert Review" policy defined in RFC5226. Type values 65536
   through 134,217,728, 0 and 4,294,967,295 are to be allocated using
   the IETF review policy defined in [RFC5226].

   For each value assigned IANA should also track whether the value
   constitutes an error as described in Section 5.1. When values are
   assigned by IETF Review, the setting of this column must be
   documented in the RFC that requests the allocation. For Expert Review
   and FCFS assignments, the setting of this column must be made clear
   by the requester at the time of assignment.

FCFS policy is not used in this document, so it shouldn't be mentioned. Or possibly you meant "IETF Review" here?

Thanks for addressing my concerns.

From Dan's Gen-ART review:

Nits/editorial comments: 

1. In section 8, first paragraph: s/rages/ranges/
2. In sections 8.3, 8.4 the construct "IETF Review" is worded in three
different ways (IETF review, IETF Review, "IETF Review") - better
align

As mentioned by Jürgen in his OPS-DIR review:

Section 1.2 is not really about terminology but instead it basically
expands acronyms. The section does not define any terms or does it
make it clear where terms are defined. A reader who does not know
T-PE
will not be pointed to a document that defines 'Terminating Provider
Edge Node of MS-PW'. I usally find terminology sections more useful
if
they say where definitions of terms get be found.

Section 3: s/the the/the/

Section 4: What is the 'Version' field in the message format?

Section 4: There is an 8-bit field marked U C Flags and I _assume_
the
U and C bits are the 'first' two bits and the 'remaining bits are the
flags managed by IANA. Perhaps make this clearer, e.g.:

   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |  Last Received Seq Number     | Message Type  |U|C|   Flags   |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

Or alternatively simply name the entire 8-bit flags field like you do
in the text where you refer to Message Flags and then explain in the
text under the 'Message Flags' that the first two bits have a fixed
meaning.

   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
   |  Last Received Seq Number     | Message Type  | Message Flags |
   +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

This option carries less information but then you use 'Message Flags'
in several places and you also request an IANA registry for Message
Flags where the U and C bit are allocated. Looking at the IANA text,
it says 'bit position' 0 and 1. Not sure this is clear enough, you
seem to number bits in the order 0, 1, 2, ...

It turns out we have several slightly different labels further down
in
the document for this flags field throughout the document - this
makes
searching in the text difficult, please use a single consistent name
for this message field.

Section 8.2 says values 1 and 2 are defined in this document but then
it seems value 3 is also allocated, no?

Subsections of section 8 switches several times between decimal and
hexadecimal numbers. Perhaps things get clearer if a single number
system (e.g., hexadecimal) is used when talking about a specific
registry. Numbers like 134,217,728 look somewhat confusing, 0x8000000
seems simpler.

S 1.

   Periodic retransmission
   of non-zero status messages, and a simple acknowledge of PW status

"acknowledgement", perhaps?

S 2.
I found the state machine here a bit hard to follow. Some sort of
diagram might help.


S 4.
This is kind of an odd recommendation for how to generate the session
ID. Why not just Hash(timer) rather than hash of an ASCII formatted
date?


S 5.
       The C Bit is used to signal the end of PW configuration
       transmission. If it is set, the sending PE has finished sending
       all it’s current configuration information.

"its"

Is last received sequence number a cumulative ack or the temporarally
last received packet?


S 5.2
Is Length the remaining length or the length of the entire TLV?

I agree with Yaron's comment from the SecDir review.  We shouldn't refer back to old security considerations sections, but rather revisit the considerations with current threats.

(important) nit in section 8.5:
s/PW Sytatus Refresh Reduction/PW Status Refresh Reduction/

In this text,

    -iii. If the new value is smaller then the original one, the PE
            will operate according to the original timer value for a
            period 3.5 times the original timer value, or until the
            first valid PW status refresh reduction message is received.

Perhaps it would be helpful to explain the choice of 3.5, so that if this mechanism is deployed for a network where that number is the wrong number, people will know how to adjust it? 

There are several occurrences where s/then/than/ is needed, I think. I spotted at least 3.

Other nits …
S/octetc/octets/
S/RECOMENDED/RECOMMENDED/
S/vaules/values/

In section 7. Security Considerations

   The security considerations of [RFC6478] are adequate for the
   proposed mechanism since the operating environment is almost
   identical to the one where this protocol would be deployed. It should
   also be noted that since this protocol is designed to be deployed
   between two adjacent PEs connected by a physical link, it is not
   possible to misdirect or inject traffic without compromising the PW
   transport link itself. All these situations are covered in the
   security considerations of [RFC6478].

There's an appeal to physical adjacency as a defensive mechanism. If this protocol is deployed in a tunnel over UDP, would “physical adjacency” still be true?