[Gen-art] Gen-ART LC review of draft-ietf-rohc-rtp-impl-guide-21.txt

I have been selected as the General Area Review Team (Gen-ART)
reviewer for this draft (for background on Gen-ART, please see
http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html).

Please resolve these comments along with any other Last Call comments
you may receive.

As an aside, I am really glad to see WGs spending this level of effort on
correcting and clarifying specifications. When you finish with ROHC, I have
some other protocols I'd like to suggest for your consideration :-)

Thanks,

Spencer Dawkins

Document:draft-ietf-rohc-rtp-impl-guide-21.txt
Reviewer:Spencer Dawkins
Review Date:2006-19-26
IETF LC Date:2006-11-01
IESG Telechat date:

Summary: This draft is on the right track for Proposed Standard. There are
some Technical Comments that should be addressed before the document is
published.

Comments:

I included "Editorial" comments in this review for the convenience of
editors, but the Gen-ART review consists only of "Technical Comments".

There are some "Technical Comments" that really do need to be addressed -
for example, I think 4.4.2 is missing 2119 language.

There are several places in the draft (section 3.3 and 4.1 are examples, but
I noted others) where the "INCOMPLETE", "INCORRECT", "INCORRECT AND
INVALIDATED", "CORRECTED", "FORMAL ADDITION" notation is not used, and the
text doesn't say anything using 2119, but it seems to be changing the
protocol. Given that the implementer will have two specifications open at
the same time, being absolutely clear about what the text says NOW seems
pretty important.

Abstract

   RFC 3095 defines the RObust Header Compression (ROHC) framework and
   profiles for IP, UDP, RTP, and ESP. Some parts of the specification

Editorial: IP and UDP are probably OK, but ESP should certainly be expanded
on first use, and RTP is an "almost certainly expand on first use". Just
FYI, there are a LOT of acronyms that aren't expanded on first use in the
body of the document, but I ignored these because anyone reading this
document will have RFC 3095 already open, so I'm assuming the expansions in
RFC 3095 are sufficient in these cases.

   are unclear or contain errors that may lead to misinterpretations
   that may impair interoperability between different implementations.
   This document provides corrections, additions and clarifications to
   RFC 3095; this document thus updates RFC 3095. In addition, other
   clarifications related to RFC 3241 (ROHC over PPP), RFC 3843 (ROHC IP
   profile) and RFC 4109 (ROHC UPD-Lite profiles) are also provided.

1. Introduction and terminology

   RFC 3095 [1] defines the RObust Header Compression (ROHC) framework
   and profiles for IP [8][9], UDP [10], RTP [11], and ESP [12]. During

Editorial: same comment for RTP and ESP expansion on first use as in
abstract.

   implementation and interoperability testing of RFC 3095 some
   ambiguities and common misinterpretations have been identified, as
   well as a few errors.

   This document summarizes identified issues and provides corrections
   needed for implementations of RFC 3095 to interoperate, i.e. it
   constitutes an update to RFC 3095. This document also provides other
   clarifications related to common misinterpretations of the
   specification. When referring to RFC 3095, this document should
   therefore also be referenced.

Editorial: "when referring to RFC 3095, this document should therefore also
be referenced." is probably correct but easy to misinterpret, I think.
Perhaps something like "References to RFC 3095 should also include this
document."?

   When a section of this document makes formal corrections, additions
   or invalidations to text in RFC 3095, this is clearly summarized. The
   text from RFC 3095 that is being addressed is given and labeled
   "INCOMPLETE", "INCORRECT" or "INCORRECT AND INVALIDATED", followed by
   the correct text labeled "CORRECTED", where applicable. When a formal
   addition is provided, it is given with the label "FORMAL ADDITION".

Technical Comment: I'm not sure "formal addition" is a widely used term - at
least, not widely used enough to allow circular self-definition. Perhaps
"When text is added that does not simply correct text appearing in a
previous specification, it is given with the label "FORMAL ADDITION"." No
problem with using the label elsewhere in the document, it just needs to be
described more clearly here.

2.3. CRC coverage in CRC feedback options

   Section RFC3095-5.7.6.3 is incomplete, as it does not mention how the
   "size" field is handled when calculating the 8-bit CRC used in the
   CRC feedback option. Since the "size" field can be considered an

Editorial: if the specification is being corrected based on this
consideration, perhaps it should be "field is considered an extension".

   extension of the "code" field, it must be treated in the same way.

2.4. CRC coverage of the ESP NULL header

   Section RFC3095-5.8.7 gives the CRC coverage of the ESP NULL [13]
   header as "Entire ESP header". This must be interpreted as including
   only the initial part of the header (i.e. SPI and Sequence number),
   and not the trailer part at the end of the payload. Therefore, the
   ESP NULL header has the same CRC coverage as the ESP header used in
   the ESP profile (section RFC3095-5.7.7.7).

Technical Comment: Doesn't this actually need a text correction? I'm reading
this section as saying ["Entire" must be interpreted as "only the initial
part"], and if I'm intrpreting this correctly, the statement doesn't work
for me.

3.1. Feedback during mode transition to U- and O-mode

   These enhanced procedures should be considered only as a possible
   improvement to a decompressor implementation, since interoperability
   is not affected in any way. A decompressor implemented according to
   the optimized procedures will interoperate with an RFC3095
   compressor, as well as a decompressor implemented according to the
   procedures described in RFC3095 does.

Editorial: The last sentence reads oddly to me. It would read more clearly
if the sentence ended "will interoperate with an RFC3095 compressor.",
without the last clause, but even removing the final "does" would be
clearer.

3.1.1. Mode transition procedures allowing sparse feedback

   This enhanced transition, where feedback need not be sent for every
   decompressed packet, does however introduce some considerations in
   case feedback messages would be lost. Specifically, there is a risk
   for a deadlock situation when a transition from R-mode is performed
   in case no feedback message successfully reaches the compressor and

Editorial: s/in case/in cases where/, or even "performed, no feedback
message successfully reaches".

   the transition is not complete. For transition between U-mode and O-
   mode, there is also a small risk for reduced compression efficiency.

   To avoid this, the decompressor MUST continue to send feedback at
   least periodically, also when in Pending transition state. This is

Editorial: I THINK the "also" can be deleted, but would appreciate author
confirmation.

   equivalent to enhancing the definition of the D_TRANS parameter in
   section RFC3095-5.6.1, to include the definition of a Pending state:

   - D_TRANS:
      Possible values for the D_TRANS parameter are (I)NITIATED,
      (P)ENDING and (D)ONE. D_TRANS MUST be initialized to D, and a mode
      transition can be initiated only when D_TRANS is D. While D_TRANS
      is I, the decompressor sends a NACK or ACK carrying a CRC option
      for each packet received. When D_TRANS is set to P, the
      decompressor do not have to send a NACK or ACK for each packet
      received, but it MUST continue to send feedback with some
      periodicity, and all feedback packets sent MUST include the CRC

Technical Comment: I understand that the appropriate periodicity can be very
application-dependent, but is it possible to place any maximum on
periodicity? ("MUST send feedback every two thousand years" seems to violate
the spirit of this description :-)

      option. This ensures that all mode transitions will be completed
      also in case of feedback losses.

3.1.2. Transition from Reliable to Optimistic mode

   The enhanced procedure for transition from Reliable to Optimistic
   mode is shown below:

Editorial: I didn't see any text that described what the enhancement
actually was - just a sentence would keep the reader from having to dig the
difference out without guidance.

3.1.3. Transition to Unidirectional mode

   The enhanced procedure for transition to Unidirectional mode is shown
   on the following figure:

Editorial: Again, a one-sentence summary of the enhancement would be nice
here.

3.3. Packet decoding during mode transition

   The purpose of a mode transition is to ensure that the compressor and
   the decompressor coherently move from one mode of operation to
   another using a three-way handshake. At one point during the mode
   transition, the decompressor acknowledges the reception of one (or
   more) IR, IR-DYN or UOR-2 packet(s) that have mode bits set to the
   new mode. Packets of type 0 or 1 that are received up to this point
   are decompressed using the old mode, while afterwards they are
   decompressed using the new mode. If the enhanced transition
   procedures described in section 3.1 are used, the setting of the
   D_TRANS parameter to P represents this breakpoint. The successful
   decompression of a packet of type 0 or type 1 completes the mode
   transition.

Technical Comment: Section 3.3 seems very reasonable, but does it clarify,
or change, or ... - what is the relationship between this text and text in
previous Proposed Standards?

4. Timestamp encoding

4.1. Encoding used for compressed TS bits

   RTP Timestamp values (TS) are always encoded using W-LSB encoding,
   both when sent scaled and when sent unscaled. When no TS bits are
   transmitted in a compressed packet, TS is always scaled. If a
   compressed packet carries an extension 3 and field(Tsc)=0, the
   compressed packet must thus always carry unscaled TS bits. For TS
   values sent in Extension 3, W-LSB encoded values are sent using the
   self-describing variable-length format (section RFC3095-4.5.6), and
   this applies to both scaled and unscaled values.

Technical Comment: Again, Section 4.1 seems very reasonable, but does it
clarify, or change, or ... - what is the relationship between this text and
text in previous Proposed Standards?

4.2. (De)compression of TS without transmitted TS bits

   When ROHC RTP operate using its most efficient packet types, apart

Editorial: s/operate/operates/

   from packet type identification and the error detection CRC, only RTP
   sequence number (SN) bits have to be transmitted in RTP compressed

Editorial: "must be transmitted", or "are transmitted"?

   headers. All other fields are then omitted either because they are
   unchanged or because they can be reconstructed through a function
   from the SN (i.e. by combining the transmitted SN bits with state
   information from the context). Fields that can be inferred from the
   SN are the IP Identification (IP-ID) and the RTP Timestamp (TS).

   IP-ID compression and decompression, both with and without
   transmitted IP-ID bits in the compressed header, are well defined in
   section RFC3095-4.5.5 (see section 8.2). However, for TS it is only
   defined how to decompress based on actual TS bits in the compressed
   header, either scaled or unscaled, but not how to infer the TS from
   the SN. This section specifies how the scaled TS is decompressed when
   no TS bits are received in the compressed header.

Editorial: the last two sentences weren't easy for me to understand.
Perhaps, "RFC3095-4.5.5 defines decompression when TS bits are received in
the compressed header, but does not specify how to infer the TS from the SN
when no TS bits are received in the compressed header."?

4.4.1. TS_STRIDE for scaled timestamp encoding

   The relation between TS and TS_SCALED, given by the following
   equality in section RFC3095-4.5.3, defines the mathematical meaning
   of TS_STRIDE:

      TS = TS_SCALED * TS_STRIDE + TS_OFFSET

   TS_SCALED is incorrectly written as TS / TS_STRIDE in the compression
   step following the above core equality. This formula is incorrect
   both because it excludes TS_OFFSET and because it would prevent a
   TS_STRIDE value of 0, which is an alternative not excluded by the
   definition or by the core equality above. If "/" were a generally
   unambiguously defined operation meaning "the integral part of the
   result from dividing X by Y", the absence of TS_OFFSET could be
   explained, but the formula would still lack a proper output for
   TS_STRIDE equal to 0. The formula of "2. Compression" is thus
   invalid.

Technical Comment: I could probably figure out the "correct formula in the
compression step following the core equality", but that would require me to
look at both documents and try to figure it out. Since you guys already know
the right answer, could you say "the CORRECTED formula for the compression
step is (whatever it is)" in the text?

4.4.2. TS wraparound with scaled timestamp encoding

   Section RFC3095-4.5.3 states in point 4 and 5 that the compressor is
   not required to initialize TS_OFFSET at wraparound, but that it is
   required to increase the number of bits sent for the scaled TS value
   when there is a TS wraparound. The decompressor is also required to
   detect and cope with TS wraparound, including updating TS_OFFSET.

   This method is not interoperable and not robust. The gain is also
   insignificant, as TS wraparound happens very seldom. Therefore, the
   compressor reinitializes TS_OFFSET upon TS wraparound, by sending
   unscaled TS.

Technical Comment: is this "the compressor MUST reinitialize TS_OFFSET"?

4.7. Using timer-based compression

   Whether to use timer-based compression or not is controlled by the
   TIME_STRIDE control field, which can be set either by an IR, an IR-
   DYN, or by a compressed packet with extension 3. Before timer-based
   compression can be used, the decompressor has to inform the
   compressor (on a per-channel basis) about its clock resolution by
   sending a CLOCK feedback option for any CID on the channel. The
   compressor can then initiate timer-based compression by sending (on a
   per-context basis) a non-zero TIME_STRIDE to the decompressor. First

Editorial: s/First when/When/

   when the compressor is confident that the decompressor has received
   the TIME_STRIDE value, it can switch to timer-based compression.

5.3. Bit masks in list compression

   The decompressor MUST NOT modify items with an index not covered by
   the 7-bit mask, when a 7-bit mask is received for a reference list
   that contains more than 7 items.

Technical Comment: is this adding a MUST NOT that isn't in the original
specification? If so, should it be flagged as a FORMAL ADDITION?

5.6. Translation tables and indexes for IP extension headers

   When content of extension headers changes, an implementation can
   choose to either use a different index, or update the existing one.
   Some extensions can be compressed away also when some fields change,

Editorial: s/be compressed away also/also be compressed away/

   as those changes can be conveyed to the decompressor implicitly (e.g.
   sequence numbers in extension headers that can be inferred from the
   RTP SN) or explicitly (e.g. as part of the 'IP extension header(s)'
   field in extension 3).

6.1. Implicit updates

   A context updating packet that contains compressed sequence number
   information may also carry information about other fields; in such
   case, these fields are updated according to the content of the

Editorial: s/case/cases/

   packet. The updating packet also implicitly updates inferred fields
   (e.g. RTP timestamp) according to the current mode and the
   appropriate mapping function of the updated and the inferred fields.

   An updating packet thus updates the reference values of all header
   fields, either explicitly or implicitly, with an exception for the

Editorial: s/with an exception for/except for/

   UO-1-ID packet (see section 6.2 of this document). In UO-mode, all
   packets are updating packets, while in R-mode all packets with a CRC
   are updating packets.

7.2. CID/context re-use

   As part of the channel negotiation, the maximal number of active
   contexts supported is negotiated between the compressor and the
   decompressor through the MAX_CID parameter. The value of MAX_CID can
   differ significantly from one link application to another, as well as
   the load in terms of the number of packet streams to compress. The
   lifetime of a ROHC channel can also vary, from almost permanent to
   rather short-lived. However, in general it is not expected that
   resources will be allocated for more contexts than what can
   reasonably be expected to be active concurrently over the link. As a
   consequence hereof, context identifiers (CIDs) and context memory are
   resources that will have to be re-used by the compressor as part of
   what can be considered normal operation.

   How context resources are re-used is in RFC 3095 [1] and subsequent
   ROHC standards left unspecified and up to implementation. This

Editorial: s/in RFC 3095 [1] and subsequent ROHC standards left unspecified/
left unspecified in RFC 3095 [1] and subsequent ROHC standards/

   document does not intends to change that, i.e. ROHC resource
   management is still considered an implementation detail. However, re-
   using a CID and its allocated memory is not always as simple as
   initiating a context with a previously unused CID. Because some
   profiles can be operating in various modes where packet formats vary
   depending on current mode, care has to be taken to ensure that the
   old context data will be completely and safely overwritten,
   eliminating the risk of undesired side effects from interactions
   between old and new context data. This document therefore points out
   some important core aspects to consider when implementing resource
   management in ROHC compressors and decompressors.

8.6. Multiple CRC options in one feedback packet

   Although it is not useful to have more than one single CRC option in
   a feedback packet, having multiple CRC options is still allowed. If
   multiple CRC options are included, all such CRC options MUST be
   identical, as they will be calculated over the same header, the
   compressor SHOULD otherwise discard the feedback packet.

Technical Comment: Why SHOULD here? Is there a reason besides "not
supporting this specification" that the compressor would not discard the
feedback packet? If so, providing the reason why this is not a MUST would be
nice.

8.10. Expecting UOR-2 ACKs in O-mode

   It is thus RECOMMENDED to not use of the optional ACK mechanism in

Editorial: s/RECOMMENDED to not/NOT RECOMMENDED to/

   O-mode, neither in compressor nor in decompressor implementations.

_______________________________________________
Gen-art mailing list
Gen-art@ietf.org
https://www1.ietf.org/mailman/listinfo/gen-art