I am the assigned Gen-ART reviewer for this draft. For background on Gen-ART, please see the FAQ at < http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. Please resolve these comments along with any other Last Call comments you may receive. Document: draft-ietf-mpls-tp-psc-itu-02.txt Reviewer: Elwyn Davies Review Date: 24 Feb 2014 IETF LC End Date: 24 Feb 2014 IESG Telechat date: (if known) - Summary: Almost Ready for IESG. There is a statement in s1 that could indicate that the consequences of turning on a subset of the new capabilities has not been fully thought through. I am also not clear whether EXER fulfils the intentions of R84 in RFC 5654 as stated. The repeated transmission of the advertised capabilities adds to the scope for random bit errors to halt protection operation. A few minor nits noted below. Major issues: None Minor issues: s1, next to last para: > This document describes the behavior of the PSC protocol including > the priority logic and the state machine when all the capabilities > associated with the APS mode are enabled. The PSC protocol behavior > for the PSC mode is as defined in RFC 6378. APS mode involves five capabilities which can, apparently, be implemented and advertised indpendently, so presumably there might be reasons for either implementing or turning on just a subset. Are there any implications for the PSC protocol if only some subset of the the capabilities are available in a given node? (This may be a dumb question and I haven't tried to work out what might go wrong if you did have any of the available subsets.) s9.1.1/s9.2: Following on from the previous point, s9.1.1 implies that the system can operate happily with some subset of the five capabilities turned on provided that both ends concur. However there is no mode name that would cover such a subset. Does this actually mean that turning on a subset doesn't really work? And if it doesn't work why bother with five flag bits? Also the phraseology used here could lead to future problems if more capabilities are defined: suppose some future new mode (FOO) adds more capabilities but the two 'modes' can be turned on independently - as currently expressed you would have to define three mode names for APS only, FOO only and APS + FOO.. and so on with more capability sets. Unintended consequence? Oh, and what if a capability is used in more than one mode? s8: EXER appears to test the PSC channel but nothing else. I am not clear that this fully satisfies R84 in RFC 5654.. It may be that I don't know what information would go back in the RR response, but should the response say anything about the state in the remote node (like the remote end of the working path is not working in some way and/or what is the PSC state - RFC 6378 s3.6)? s13: The addition of the Capabilities TLV and the requirement that it is carried accurately and repeatedly in every PSC message introduces a new aspect to the DoS/random corruption problem mentioned in s6 of RFC 6378. A single bit corruption in a PSC message will lead to disablement of protection switching and requirement of operator intervention. I am not sure if this is a serious issue, but it probably merits a comment in s13 and verification that it does match with the words in RFC 6378 as regards 'converging on a stable state'. Nits/editorial comments: s4.4, title: s/Capability 1/Priority Modifications/ (Ok, its accurate but bald) s7.3 et seq: The term "selector bridge" is introduced without definition. I suspect it is a piece of jargon I am supposed to know but I think a reference would help. s7.4, para 1: > the rules to resolve the equal priority requests are > required. Should this be s/the rules/some rules/? s8, definition of Exercise (EXER): Describing EXER as 'replacing' one of NR, RR or DNR seems a bit odd. Mentioning RR is particularly problematic since it makes the definitions sort of circular as RR is the response to EXER. I guess what is probably appropriate is to say that it is built in the same way as an NR message would be built. s9.1: RFC 6378 doesn't define the encoding of the TLV type and TLV length fields, so it needs to be done here (Unsigned integers). (It also doesn't define encoding of the (unnecessary) overall TLV length field in the PSC header. s9.1: What happens if the receiver gets a TLV with some a flags length that isn't a multiple of 4 (or indeed a TLV it deosn't recognize?) It might be cleaner to define the length in terms of units of 32 bits. s10.3, para 2: s/they SHALL be disappeared./they SHALL be discarded./