Draft: draft-ietf-ipdvb-ule-06.txt Reviewer: Michael A. Patton [MAP@MAP-NE.com] Date: Thursday 6/30/2005 1:55 AM CST Telechat Date: TBD (review of -05 was for 26 May 2005 Telechat) Summary: This draft is basically ready for publication, but has problems that should be fixed before publication. Most of the things I mentioned in my first review have been fixed, with the exception of the first two comments, the two most important. The bulk of this review will deal with expanding on those. These are each in a section below delimited by a line of dashes. Since I expect the real problem is just that the authors are too close, and therefore knowing what everything is don't notice when the text is slightly ambiguous. So, therefore, each section is a short text on what _I_ understand to be the correct resolution, followed by a set of proposed wording changes to make everything consistent with that. Of course, if my understanding isn't right, that means parts other than the ones I included need to be fixed. ---------------------------------------------------------------- First is ambiguities around whether PP (when it occurs) is part of the (variable length) header, or follows a fixed header but is not part of it. This leads to ambiguities as to where the count in the PP field starts from...it starts "after the header" in one place and "after the PP byte" in another. My understanding, both from our earlier discussion, and from figuring out which parts of the document are normative and which aren't, is that PP actually is part of the header and therefore the header is either 4 or 5 bytes depending on whether PP is there and that PP starts counting after the header and, since you only do that when there is a PP, the count starts both immediately after the header and immediately after the PP, which is always the last byte of the header. Section 2: ---------- Since the "TS Packet header" is not "fixed-length" replace this: Adaptation Field: An optional variable-length extension field of the fixed-length TS Packet header, intended to convey clock references and timing and synchronization information as well as stuffing over an MPEG-2 Multiplex [ISO-MPEG2]. with this: Adaptation Field: An optional variable-length extension field of the TS Packet header, intended to convey clock references and timing and synchronization information as well as stuffing over an MPEG-2 Multiplex [ISO-MPEG2]. Since the PP is part of the header, it doesn't follow the header. Replace this: PP: Payload Pointer [ISO-MPEG2]. An optional one byte pointer that directly follows the 4 byte TS Packet header. It contains the number of bytes that follow the Payload Pointer, up to the start of the first Payload Unit (counted from the first byte of the TS Packet payload field, and excluding the PP field itself). The presence of the Payload Pointer is indicated by the value of the PUSI bit in the TS Packet header. The Payload Pointer is present in DSM-CC, Table Sections, and ULE. It is not present in TS Logical Channels that use the PES-format. with this: PP: Payload Pointer [ISO-MPEG2]. An optional one byte pointer that is added to the TS Packet header directly following the 4 byte fixed part. It contains the number of bytes that follow the Payload Pointer, up to the start of the first Payload Unit (counted from the first byte of the TS Packet payload field, and excluding the PP field itself). The presence of the Payload Pointer is indicated by the value of the PUSI bit in the TS Packet header. The Payload Pointer is present in DSM-CC, Table Sections, and ULE. It is not present in TS Logical Channels that use the PES-format. Since the PP can be part of the header, it may be either 4 or 5 bytes. Replace this: TS Header: The 4 byte header of a TS Packet [ISO-MPEG2]. Each 188B TS Packet incorporates a 4B header with the following fields (those referenced within this document are marked with *): Field Length Name/Purpose (in bits) 8b Synchronisation pattern equal 0x47 *1b Transport Error Indicator *1b Payload Unit Start Indicator (PUSI) 1b Transport Priority *13b Packet IDentifier (PID) 2b Transport scrambling control *2b Adaptation Field Control (AFC) *4b Continuity Counter (CC) with this: TS Header: The 4 or 5 byte header of a TS Packet [ISO-MPEG2]. Each 188B TS Packet incorporates a header, with 4B in a fixed format (those referenced within this document are marked with *): Field Length Name/Purpose (in bits) 8b Synchronisation pattern equal 0x47 *1b Transport Error Indicator *1b Payload Unit Start Indicator (PUSI) 1b Transport Priority *13b Packet IDentifier (PID) 2b Transport scrambling control *2b Adaptation Field Control (AFC) *4b Continuity Counter (CC) if the PUSI bit is set, there is one additional field: *8b Payload Pointer (PP) Section 6.2: ------------ Again, the PP is part of the header, so it doesn't follow it. Replace this: (v) If at least two bytes are available for SNDU data in the TS Packet payload (i.e. three bytes if the PUSI was NOT previously set, and two bytes if it was previously set), the Encapsulator MAY encapsulate further queued PDUs, by starting the next SNDU in the next available byte of the current TS Packet payload. When the Encapsulator packs further SNDUs into a TS Packet where the PUSI has NOT already been set, the PUSI MUST be updated (set to 1) and an 8- bit Payload Pointer MUST be inserted in the first byte directly following the TS Packet header. The value of the Payload Pointer MUST be set to the position of the byte following the end of the first SNDU in the TS Packet payload. If no further PDUs are available, an Encapsulator MAY wait for additional PDUs to fill the incomplete TS Packet. The maximum period of time an Encapsulator can wait, known as the Packing Threshold, MUST be bounded and SHOULD be configurable in the Encapsulator. If sufficient additional PDUs are NOT received to complete the TS Packet within the Packing Threshold, the Encapsulator MUST insert an End Indicator (using rule iv). with this: (v) If at least two bytes are available for SNDU data in the TS Packet payload (i.e. three bytes if the PUSI was NOT previously set, and two bytes if it was previously set), the Encapsulator MAY encapsulate further queued PDUs, by starting the next SNDU in the next available byte of the current TS Packet payload. When the Encapsulator packs further SNDUs into a TS Packet where the PUSI has NOT already been set, the PUSI MUST be updated (set to 1) and an 8- bit Payload Pointer MUST be inserted after the fixed four bytes of the TS Packet header, extending it to be 5 bytes. The value of the Payload Pointer MUST be set to the position of the byte following the end of the first SNDU in the TS Packet payload. If no further PDUs are available, an Encapsulator MAY wait for additional PDUs to fill the incomplete TS Packet. The maximum period of time an Encapsulator can wait, known as the Packing Threshold, MUST be bounded and SHOULD be configurable in the Encapsulator. If sufficient additional PDUs are NOT received to complete the TS Packet within the Packing Threshold, the Encapsulator MUST insert an End Indicator (using rule iv). Section 7.1.1: -------------- The wording again implies that PP is not part of the header... Replace this: A Receiver in the Idle State MUST check the PUSI value in the header of all received TS Packets. A PUSI value of 1 indicates the presence of a Payload Pointer. Following a loss of synchronisation, values between 0 and 181 are permitted, in which case the Receiver MUST discard the number of bytes indicated by the Payload Pointer (counted from the first byte of the TS Packet payload field, and excluding the PP field itself), before leaving the Idle State. It then enters the Reassembly State, and starts reassembly of a new SNDU at this point. with this: A Receiver in the Idle State MUST check the PUSI value in the header of all received TS Packets. A PUSI value of 1 indicates the presence of a Payload Pointer. Following a loss of synchronisation, values between 0 and 181 are permitted, in which case the Receiver MUST discard the number of bytes indicated by the Payload Pointer (counted from the first byte of the TS Packet payload field, which immediately follows the PP field), before leaving the Idle State. It then enters the Reassembly State, and starts reassembly of a new SNDU at this point. ---------------------------------------------------------------- The second thing is what the IANA registry is and how the values are used. In this case, you did improve the text and I think it would probably not be a catastrophe if we went with it as is. However, while IANA tends to be pretty smart, I think it can be clearer. The following wording changes are to change it from one registry of numbers from 0-511 where half are interpreted differently and instead make it two registries. Your IANA Considerations also specifies the criteria as "expert review leading to prior issue of an IETF RFC" which is not one of the terms in RFC2434 (which, BTW, you should probably cite) and actually seems to conflate two (or even three) contradictory terms. It seems to me that the mandatory extension values should be "Standards Action" and that optional extension values probably only need to be "Specification Required" although I could see making it "IETF Consensus" (i.e. RFC Required). Section 4.4: ------------ Since the H-LEN can vary, the actual values are not always those assigned by IANA. Replace this: The first set of ULE Type field values comprise the set of values less than 1536 in decimal. These Type field values are IANA assigned (see 4.4.1), and indicate the Next-Header. with this: The first set of ULE Type field values comprise the set of values less than 1536 in decimal. These Type field values are based on IANA assigned codes (see sections 4.4.1, 5, and 15), and indicate the Next-Header. I think the beginning of Section 5 could probably be improved. But, nothing is really wrong. So, I'll skip suggestions for that. Here's a suggested complete rewrite of section 15: 15. IANA Considerations This document will require IANA involvement. The ULE Next-Header type field defined in this document requires creation of two registries: ULE Mandatory Extension Header registry ULE Optional Extension Header registry Each registry allocates a set of values within the range 0-255 (decimal). In combination, these define a set of allowed values in the range 0-1535 for the first part of the ULE Type space (see section 4.1). 15.1 IANA Guidelines The following contains the IANA guidelines for management of the ULE registries. These registries each allocate values 0-255 decimal (0x00-0xFF, hexadecimal). The values in the two registries are independantly allocated. 1) ULE Mandatory Extension Headers: allocations in this registry require "Standards Action" as defined in [RFC2434]. The specification MUST define the value, and the name associated with the Extension Header, together with the procedure for processing the Extension Header. It MUST also define the need for the Mandatory Extension and the intended use. The size of the Extension Header MUST be specified. Assignments made in this document: Type Name Length Reference 0: Test-SNDU 2B Section 4.7.4. 1: Bridged-SNDU 2B Section 4.7.5. 2) ULE Optional Extension Headers: allocations in this registry are "Specification Required" as defined in [RFC2434]/ The specification MUST define the value, and the name associated with the Extension Header, together with the procedure for processing the Extension Header. The entry MUST specify the range of allowable H-LEN values that are permitted (in the range 1-5). It MUST also define the need for the Optional Extension and the intended use. Assignments made in this document: Type Name H-LEN Reference 256: Extension-Padding 1-5 Section 5. ---------------------------------------------------------------- There are also still a lot of minor problems, typos, dangling references, and formatting issues. I'm not going to spend any time cataloging them (some from my first review are still there, and I think I noticed some new ones). Hopefully, the RFC Editor will catch most of them anyway... One comment that I'm not sure how to apply it is that, because of the ordering, I actually managed to completely miss the examples the first time through. I guess I stopped when I hit the boilerplate stuff, knowing I'd already done the review of the IANA section when I did section 5. Unfortunately, the order is somewhat constrained, but I think the IPR and Copyright could come after the Appendices (sorry Annexes :-). But this kind of ordering question is beyond the scope of one Internet Draft...