Unidirectional Lightweight Encapsulation (ULE) for Transmission of IP Datagrams over an MPEG-2 Transport Stream (TS)
draft-ietf-ipdvb-ule-06

[Ballot comment]
Editorial comments on -06 version:

Gorry's latest statement is that the PP field is not formally part
of the TS header, and there are still a few placess where this needs to be made
consistent.

In 6.2 I found that

>    (The standard rules require the
>    header of this new TS Packet to carry a PUSI value of 1 and a
>    Payload Pointer value of 0x00.)

occurs three times and needs slight fixing, e.g.

  (The standard rules require the
  header of this new TS Packet to carry a PUSI value of 1
  followed by a Payload Pointer value of 0x00.)

Gorry also proposed adding this somewhere:

  If the PUSI bit is set to a value of 1, there is one additional field
  following the TS packet header:
        *8b            Payload Pointer (PP)

(And there are formatting nits.)



-------------------------------------------------
from review of -05 version by Michael Patton:

The first use of "TS" in the Intro should probably be expanded.  It
was previously expanded in the Abstract, but you probably shouldn't
rely on readers having seen that recently...

Throughout the document there are references that get split across
line boundaries.  This should be avoided.

Section 1 has a reference to [draft-ipdvb-arch] which is not in the
references section.

At the end of Section 1 is a reference to [draft-ipdvb-ar] which is
not in the references section.

Section 2, in the def for AFC is a reference to [ISO_MPEG] which is
not in the references section.

Section 2, in the def of MPEG-2 there's mention of H.220 which could
usefully have a reference.

Section 2, in the def of PID the reference to "all 1s" is easy to
misread because the 1 looks like an l in some fonts.  I'd suggest
writing it as "all ones" to avoid potential confusion.  This
construction also appears in other places (Section 4.3 at least) which
should also be adjusted.

Section 2, has two slightly different definitions for PSI.

Section 4.6 has a ref to [ITU3563] which is not listed in the
references section.

Is [ISO-8802-2] really 8802.2 rather than 802.2?

RFC3667 and RFC3668 appear in the references section, but are not
actually referenced (although 3668 is mentioned in boilerplate that
will go away in the RFC).  I don't think these belong here and they're
certainly not normative.

For IEEE 802.3 you use the IEEE ref and mention the ISO one, for 802.2
you only show ISO.  I think it might be better to make these
references consistent.


Typos:

Section 1, third line has a double open bracket ("[[").

Section 2, in the def of AFC: "ISO_MPEG" => "ISO-MPEG"

Section 2, in the def of SI Table, "that is been" => "that has been"

Section 2, in the def of TS Header there is a formatting error in the
table.

In Section 2 the last paragraph (def of ULE Stream) is indented an
extra space.

Section 2, in the def of ULE Stream: "ISO_MPEG2" => "ISO-MPEG2"

Section 4.4 "[IEEE 802.3;" => "[IEEE-802.3;"

In Section 5.1 "is of a Mandatory Extension Header"
=> "is a Mandatory Extension Header"

In the diagram at the start of Section 6, the marks on the left and
right don't line up quite the same with the lower part.

[Ballot comment]
One needs to read the MPEG2 context in the intro to understand what "ULE" signifiies but that's the
nature of a spec.  I think it's quite a well-done spec, given its environment.

[Ballot discuss]
this one should be relatively easy to clear:

The name of this protocol is very misleading.

It needs 48 pages to explain an 'Ultra Lightweight Encapsulation'.

In addition, this protocol is only used for dvb and is not a generic ultra lightweight encapsulation that is useful in other contexts than dvb.

I suggest that the authors come up with a name that better covers the content.

[Ballot comment]
ANNEX B, page 41 states:

  Source IPv6:                  2001:660:3008:1789::5
  Destination IPv6:            2001:660:3008:1789::6

while RFC3849 has reserved
 
    prefix  2001:DB8::/32 as a documentation-only prefix  in the IPv6
    address registry.  No end party is to be assigned this address.

[Ballot comment]
from review by Michael Patton:

The first use of "TS" in the Intro should probably be expanded.  It
was previously expanded in the Abstract, but you probably shouldn't
rely on readers having seen that recently...

Throughout the document there are references that get split across
line boundaries.  This should be avoided.

Section 1 has a reference to [draft-ipdvb-arch] which is not in the
references section.

At the end of Section 1 is a reference to [draft-ipdvb-ar] which is
not in the references section.

Section 2, in the def for AFC is a reference to [ISO_MPEG] which is
not in the references section.

Section 2, in the def of MPEG-2 there's mention of H.220 which could
usefully have a reference.

Section 2, in the def of PID the reference to "all 1s" is easy to
misread because the 1 looks like an l in some fonts.  I'd suggest
writing it as "all ones" to avoid potential confusion.  This
construction also appears in other places (Section 4.3 at least) which
should also be adjusted.

Section 2, has two slightly different definitions for PSI.

Section 4.6 has a ref to [ITU3563] which is not listed in the
references section.

Is [ISO-8802-2] really 8802.2 rather than 802.2?

RFC3667 and RFC3668 appear in the references section, but are not
actually referenced (although 3668 is mentioned in boilerplate that
will go away in the RFC).  I don't think these belong here and they're
certainly not normative.

For IEEE 802.3 you use the IEEE ref and mention the ISO one, for 802.2
you only show ISO.  I think it might be better to make these
references consistent.


Typos:

Section 1, third line has a double open bracket ("[[").

Section 2, in the def of AFC: "ISO_MPEG" => "ISO-MPEG"

Section 2, in the def of SI Table, "that is been" => "that has been"

Section 2, in the def of TS Header there is a formatting error in the
table.

In Section 2 the last paragraph (def of ULE Stream) is indented an
extra space.

Section 2, in the def of ULE Stream: "ISO_MPEG2" => "ISO-MPEG2"

Section 4.4 "[IEEE 802.3;" => "[IEEE-802.3;"

In Section 5.1 "is of a Mandatory Extension Header"
=> "is a Mandatory Extension Header"

In the diagram at the start of Section 6, the marks on the left and
right don't line up quite the same with the lower part.

[Ballot discuss]
There seem to be internal inconsistencies that would make life hard for
an implementor. From review by Michael Patton:

There appears to be a technical inconsistency about the meaning of the
value in a PP.  In the definition of PP in Section 2 says that if the
PUSI bit is set, then the PP byte follows the TS header and indicates
how many bytes between the end of the header and the start of a PU.
Since there's at least the PP byte, it would seem that the minimum
value would be 1.  But, in Section 3 the next to last paragraph talks
about a PP value of 0x00 when the Payload Unit immediately follows the
header, and 6.1 seems to repeat that.  But it can't because the PP has
to be in there.  I'm sure there's actually a consistent definition for
these fields, but what's in the document isn't quite it.  These
definitions need to be cleaned up and made more explicit.  When I
finally got to Section 7.1.1 I find what appears to be the complete
definition, which clears it up, but the earlier sections should be
cleaned up to be consistent with it.

Section 4.4.1 reserves values 0 through 1535 and declares an IANA
registry for assigned values.  However, in the IANA Consideration
section it only talks about values from 0 through 511.  I'd suggest
adding a paragraph to the IANA Considerations reserving 512 through
1535 for future IETF Standards action.  Not that I expect these would
ever get used, just that it should probably be explicitly stated.
However, Section 5 defines a format for this field that has potential
values above 511.  So, ultimately I'm completely confused about this
field.

There is an inconsistency between 4.1 which says D=0 except in an End
Indicator.  However, 4.5 ascribes meaning for both D=0 and D=1.  At
first this seems to be a hard inconsistency, however I think (but I'm
not the expert here, the authors are supposed to be) what they mean is
that in most usage D would be 0, but there may be some cases where D=1
could be needed and that D=0 should be used except when D=1 is
absolutely needed.  If that's the case, I think a little more
explanation in 4.1 could clear up the confusion.  I think more
explicit description of the distinction in 4.5 would also help.

I'm not sure I've completely wrapped my head around the whole thing,
but from Section 7.2.1 it looks like this encapsulation assumes that
no SNDU will ever need to be spread across more than 2 TS Packets.
But, given the length that IP and Ethernet packets can be and that TS
Packets are 188 bytes, more than 2 TS packets for each SNDU would
probably be the norm.  So, I guess I don't understand how the three
cases (start of SNDU, middle of SNDU, end of SNDU) are distinguished.
So, I think a bit more explanation of that is needed somewhere.

Follow-up IANA comments:
Below is the response from the Author. 
IANA to follow-up with Margaret Wasserman about the IANA Considerations.

I think we spoke (very briefly) about this at the IANA desk last IETF.

Here's my justification:

1) The ULE spec. is a Layer-2 specification, and correct implementation is
required to guarentee the working of the network-layer IP Internet service.
(e.g. this is in contrast to TCP-port allocations). It is important requests
are reviewed by an IETF Expert, to ensure they do not impose unreasonable
impact on implementors (i.e. in their firmware and drivers). - EXPERT REVIEW
is required.

2) The spec. of a header extension MUST fully specify the format of the header
and its usage. My interpretation is that this MUST require an RFC of some
form. Prefably STD - standards track (with a full spec of the extension
protocol), although INFO - informational could be sufficient in cases where
this did not raise interoperability issues. That is "Specification Required in
form of a RFC".

So, that said, would it be sensible to combine the two?
"RFC and Expert Review"
- or does Expert Review imply also (2)?

It would be wise also to "ping" Margaret Wasserman to confirm her current
advice (offered two IETF's ago) after you reply.

IANA Last Call Comments:
Upon approval of this document the IANA will create the ULE Next-Header registry and will include the initial registrations described in this document.
For both ranges within the ULE Next-Header registry, the registration procedures are "expert review leading to prior issue of an IETF RFC". 
Will the IANA just list this as "Expert Review" as the procedure or will this also be specification required?  Please clarify.

AD Review Comments:

4. SNDU Format
   
  PDUs are encapsulated using ULE to form an SNDU. (Each SNDU is an
  MPEG-2 Payload Unit.) The encapsulation format to be used for PDUs
  is illustrated below:
   
  < ----------------------------- SNDU ----------------------------- >
  +-+-------------------------------------------------------+--------+
  |D| Length | Type |                PDU                  | CRC-32 |
  +-+-------------------------------------------------------+--------+
   
  Figure 1: SNDU Encapsulation
   
  All multi-byte values in ULE (including Length, Type, and
  Destination fields) are transmitted in network byte order (most
  significant byte first). The most significant bit of each byte is
  placed in the left-most position of the 8-bit field. Appendix A
  provides informative examples of usage.

>> A destination field is mentioned in the text, but not shown in
>> the diagram?  It might make sense to how/where the destination
>> may be included before mentioning it.

  5.1 Test SNDU
   
  A Test SNDU (figure 10) is of a Mandatory Extension Header of Type
  1. This header must be the final (or only) extension header
  specified in the header chain of a SNDU.

[...]
   
  5.2 Bridge Frame SNDU Encapsulation
   
  A bridged SNDU is a Mandatory Extension Header of Type 1. It must be
  the final (or only) extension header specified in the header chain
  of a SNDU. 

>> Is it intentional that the Test SNDU and the Bridge Frame SNDU
>> headers cannot be used in the same SNDU?

3. Description of the Method
   
[...]
   
  The ULE encapsulation is limited to TS private streams only. The
  header of each TS Packet carries a one bit Payload Unit Start
  Indicator (PUSI) field. A PUSI field with a value of 1 indicates the
  presence of at least one Payload Unit (SNDU) within the TS Packet
  payload.

>> s/the presence of at least one/the start of at least one/  ??
>>
>> If I understand this correctly, the PUSI field will be 0 if
>> the TS does not include the start of an SNDU.