Draft: draft-ietf-pwe3-satop-02.txt Reviewer: Elwyn Davies [elwynd@dial.pipex.com] Review Date: Tuesday 7/26/2005 11:12 AM CST LC Date: 27 July 2005 Summary: --------- This document is on the right track but there are a significant number of issues as well as a few editorial nits to sort out. The important things IMO are: - The idea of a 'fixed RTP header' embedded in the SAToP header - does this mean anything as regards, for instance, the use of SRTP? - Whether the packets should contain a PWE3 payload identifier: IANA has assigned payload types for the various SAToP payloads but there isn't anywhere in the packet to carry them. - How the payload identifier and RTP dynamic type is negotiated (PWE3 control protocol?) The document needs another revision before proceeding to the IESG. Review: -------- There are both substantive and editorial points to sort out but the document is reasonably on track. Also the overall format as regards organisation of sections is slightly odd. Substantive: What is a fixed RTP header? Starting in s4.3, there are large numbers of references to a 'fixed RTP header'. Whilst RFC3550 refers to some fixed header fields the implication is that these are merely the common part of a variety of possibilities. I think that it is unlikely that the RTP header used would be restricted to just these fields - in particular, the security considerations appear to leave open the possibility of using SRTP which clearly has more fields. This needs sorting out. s4.3: s5.4.4 of RFC3985 does not exist. Archaeology suggests that it was removed between the -06 (final) version of the draft and the RFC publication. I take it the ref should either be to draft-bryant-mcpherson-pwe3-cw-00.txt or draft-ietf-pwe3-cw-05.txt, but this latter explicitly applies to MPLS control words. s4.3: SAToP uses a PWE3 control word but should it actually be (also?) using a PWE3 payload type identifier to distinguish between the E1, T1 etc values.. IANA has assigned these values and there is sort o some indication about them in the IANA Considerations but there doesn't seem to be any place to carry them in the packets. Maybe they are just used in setup negotiations but this needs to be clarified. s4.3: The wording on the reserved bits in the control word is overly restrictive in case they are ever used in a future release. Try SHOULD BE 0 when generated and ignored on reception. On the other hand the FRG bits MUST be 0 because fragmentation is explicitly not allowed and should be tested as such on reception. s4.3.2: The value for the RTP PT field needs to be negotiated. There should be a ref to RC3551 and some indication of how the negotiation is carried out. s5.1: I note that the E3/T3 packet size requirement for 1024 byte payload might be problematic on some IPv4 links. I presume that adding the headers will not break the 1280 byte guaranteed MTU on IPv6 but there is quite a lot of overhead specified. s6.2.2: para 4: s2.1 says that 'all 1's' is the AIS pattern or E1, T1 and E3. Is it actually also this for T3. If so, s2.1 should say so, if not s6.2.2 need to say whatever is right for T3 if the circuit is a T3. s6.2.2: paras 6/7: When exactly is the intermediate state timer started? Some suggested default value for the period would be helpul (e.g., time taken to play out the jitter buffer?). s6.2.2: para 8: 'once a preconigured number of consecutive SAToP packets'.. should this be 'valid (or not malformed) packets'? Also what about misordering? consecutive seems to imply that misordered packets might not count for this purpose? s6.3: I have no idea what a stray packet is or how I would detect one! The previous section talks about lost packets, so presumably we are not talking about these, but I thought astray usually might mean misrouted in which case the packet would never arrive and can only be detected as lost. Does it mean 'excessively misordered'? I am not sure what role the sync source plays in this. s6.3: There are lots of other malformations that might occur (bad RTP headers come to mind). s7: para 2: In line with the 'can beneit' in para 1, I would have thought the 'SHOULD' should be either 'is RECOMMENDED to' or 'MAY'. s7: para 3: 'GS... *shall* be used'... is this supposed to be a SHALL? I would have thought this should be either a RECCOMMENDED or a MAY as or para 2. s7: para 3: measuring the delay is unlikely to produce a meaningul result for this purpose. SAToP is interested in the service guarantees of the network. Also this presumbaly not specific to Intserv but applies to any sort of network. s8: It strikes me that there are some DoS attacks that are not mentioned. Setting the L or R bit in a SAToP control word could severely disrupt the stream and waste bandwidth - is there any protection against this in the MPLS case? Also the ability to use SRTP doesn't seem to be addressed anywhere (see comment on 'fixed RTP headers'). s11: As mentined above, there doesn't seem to be any place to carry the PW type so the comment here may be moot. On the other hand, 4 values have been defined and are attributed to SAToP in the registry, so this document should reference the document that set up the registry and document the 4 values for E1, T1, E3 and T3. Editorial: Global: s/bytes/octets/ Abstract: Should not contain any references. There are large numbers of unexpanded acronyms in the document: Abstract, s1: TDM s3: SONET/SDH, ATM, ATM-CES, PE, CE, NSP Most of these need expansion and a reference to the PWE3 Architecture which contains many of the definitions of these terms. s4.3: Unexpanded acronym: IWF s5.1: REQUIRES is not a RFC2119 word. possibly rephrase as 'all SAToP packets are REQUIRED to carry a fixed...' s5.1: Unexpanded acronym CEP/TDM s6.2.2: para 1: s/payload/the payload/