On Mon, 2014-02-24 at 21:50 +0000, Huub Van Helvoort wrote: > Hello Elwyn, > > > > Thank you for your review. > > > > See my response in-line [Huub] > > 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. > > > [Huub] I have addressed these 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. > > > > [Huub] There were originally separate drafts for each of the > capabilities. > > However, it was very complex to define the effect on the state > > transition tables for each capability or combination of capabilities. > > > > [Huub] to resolve this issue it was deceided that for any permitation > > of the possible combinations of capabilities a separate draft should > be > > developed. RFC6378 is the one where none of the capabilities is > > used, and this draft is the one where all capabiliies are used. So far > > none of the other possible drafts is written. > > > > [Huub] This draft provides the behavior similar to APS used in ITU-T > > for other technologies. > > > > 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.) > > > > [Huub] both nodes MUST support the same subset, if they don't the > > result will be unpredictable because the state machines are different. > > 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. > > > > [Huub] as indicated above, the mode name does not exist because the > > draft that describes that mode does not (yet) exist. > > > > Does this actually mean that turning on > a subset doesn't really work? > > > > [Huub] indeed, not until there is a draft that describes it. > > > > And if it doesn't work why bother with > five flag bits? > > > > [Huub] to give others the opportunity to write the missing draft(s), > this > > was the agreed by the WG chairs. > > > > Also the phraseology used here could lead to future > problems if more capabilities are defined: suppose some future new > mode > (FOO) adds <n> 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? > > > > [Huub] it is up to the authors of additional drafts to come up with a > name > > for the mode described in their draft.
To be absolutely honest, my initial reaction to the above (i.e., right back to the beginning of Minor Issues) went something like "******** ARRRGGGGHHH! Choke!! Splutter!!!". Be that as it may, if that is what is agreed then please could there be some explanation in the document. Please! > > s8: EXER appears to test the PSC channel but nothing else. I am not > clear that this fully satisfies R84 in RFC 5654. > > > > [Huub] EXER verifies that the PSC engine at the far end is still > running > > properly and not stuck at sending the same PSC message continuously. > > The PSC channel is tested by sending PSC messages regularly. > > (see RFC6378 s4.1) > > > > 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)? > > > > [Huub] the fact that RR is received after sending EXER is proof that > the > > PSC engine is healthy. If it was not healty it would not have sent the > RR. Agreed. > > Because EXER has the lowest priority is can only be used in the IDLE > > state, so the far end could only send one state. OK.. I rather thought that the requirement was looking for a bit more information to be returned. > > > 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'. > > > > [Huub] a bit error will occur in one message, RFC6378 s4.1 addresses > > that issue. I don't think this covers the problem: The last para of s6 of RFC 6378 says: > However, a new concern for this document is the accidental corruption > of messages (through faulty implementations or random corruption). > The main concern is around the Request, FPath, and Path fields as a > change to these fields would change the behavior of the peer end > point. Although this document does not define a way to avoid a > change in network behavior upon receipt of a message indicating a > change in protection status, the transition between states will > converge on a known and stable behavior in the face of messages that > do not match reality. This appears to imply that random bit errors can percolate up to the protocol engine. If the bit error affects the capability flag bits in the Capability TLV then the behaviour specified in s9.1.1 would come into play: > When a node receives a Capabilities TLV it MUST compare it to its > most recent transmitted Capabilities TLV. If the two are equal, the > protected domain is said to be running in the mode indicated by that > set of capabilities (see Section 9.2). If the sent and received > Capabilities TLVs are not equal, this indicates a capabilities > mismatch. When this happens, the node MUST alert the operator and > MUST NOT perform any protection switching until the operator resolves > the mismatch in the Capabilities TLV. The result of such a corrupted bit would a.s. be a mismatch that should be followed by a turn off of protection switching and operator alerting. > > Nits/editorial comments: > s4.4, title: s/Capability 1/Priority Modifications/ (Ok, its accurate > but bald) > > > > [Huub] OK. > > 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. > > > > [Huub] RFC6378 uses "selector", "selector bridge" is the equivalent > ITU term. Can we tie them together then please? > > s7.4, para 1: > > > the rules to resolve the equal priority requests are > > required. > Should this be s/the rules/some rules/? > > > > [Huub] or s/the rules/rules/ Either would help. > > > 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. Any thoughts here?... > I guess what is probably appropriate is to say that > it is built in the same way as an NR message would be built. > > > [Huub] the "replacing" means that in case an operator wants to verify > > the operation of PSC, the excersise commend is issued and the > > transmission of RR (or NR, or DNR) is stopped and EXER is sent > instead. > I think I see what is going on noew Perhaps adding something like the phrase "that would normally be sent periodically in the IDLE state" > > > 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. > > > > [Huub] IANA asigned the TLV type value. By not giving exact values > > we leave the possibility open that there are more than 32 flags. That's not the issue. The problem is that the protocol does not specify how the type and length numbers should be represented on the wire. The standard answer is unsigned integer in network bit order. > > 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. > > > > [Huub] this can be treated the same as a bit error in a PSC message. This should be stated. > > > s10.3, para 2: s/they SHALL be disappeared./they SHALL be discarded./ > > > > [Huub] OK. > > > > Cheers, Huub. > > > > -- > Huub van Helvoort, 海高明 > Senior Consultant, 高级顾问 > Huawei Technologies Ltd, 华为技术有限公司 - European Research Center, > 欧洲研究所 > Karspeldreef 4, 1101CJ Amsterdam, The Netherlands > Tel: +31 20 4300 936 Mob: +31 6 49248936 > > _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
