RE: [rohc] Fwd: Working group lastcall: draft-ietf-avt-hc-over-mpls-protocol-05.txt

Hi AVT! (keeping ROHC in the cc)

As per Magnus request, I've done a review of this document from a header compression perspective (but due to time constraints, I've been pretty brief and I skipped section 6 altogether, so...).

Generally, I think the document is well written and seems to solve the problem it sets out to do. I have one technical issue with the document which I think either needs to be clarified or maybe "fixed".
Other than that, I just have some comments of the editorial kind.

Technical issue:
---
The document specifies that a PW should, when it is hc-enabled, ONLY send compressed packets. In most cases this is fine, but I wonder how this will work in the case of RFC2507 TCP compression.
All the HC schemes used here are built so that if an non-compressible packet is seen, it should just be sent uncompressed. In the case of ROHC (and *CRTP), the uncompressible packets are usually such that they are not expected to be seen at all (IPv4 options etc). The only packet "normal" packets that are non-compressible is probably IP fragments, and that's not a big issue.
But for IPHC's TCP compression, the HC is built so that non-compressible packets include packets where SYN or FIN is set, and you _will_ see those for every TCP flow. When this compression is used in the MPLS encapsulation, every IPHC-TCP compression instance will require that an PW for uncompressed traffic is also set up to even manage any TCP compression.

I don't know if the intention of this document is that an uncompressed PW should always be present, in that case the issue described above might be a non-issue, but it might be nice to add some short text describing that IPHC TCP compression will always require the use of an uncompressed PW in addition to the HC PW it is assigned to (are there any mapping problems with that?).
If the intention is that such uncompressed PWs will not be setup in the "normal case", then it seems like using IPHC for TCP compression will either have to:
- Specify a way to send uncompressed packets in the same PW as the compressed ones.
- Expicitly set up an uncompressed PW every time the TCP_SPACE is negotiated to a non-zero value.
Either way, some text is needed for this.

By the way, another way that this kind of issue can occur is if MAX_HEADER is configured lower than the longest header chain in which case compression might not be possible in some cases.

Editorial comments:
---
- There seems to be a few formatting problems(such as the footer on page 6), but I guess the RFC editor will fix those....

- There are a lot of abbreviations used in the Abstract, and according to http://www.ietf.org/ietf/1id-guidelines.html, those should be spelled out.

- In section 2 (Contributors), the following is stated:
      Besides the editors, the following people contributed to the
      document:
  And then the editors are listed. It seems a bit redundant to list the same addresses both here and in "Authors' addresses" in the document.

- Section 4:
      In the example scenario, HC therefore takes place between R1 and R4,
      and the MPLS LSP transports data/compressed-header/MPLS-labels
      instead of data/RTP/UDP/IP/MPLS-labels, saving 36 octets per packet
      in the /RTP/UDP/IP/ header.

 I don't think you should quantify to "36 bytes", since that is quite dependent on scheme used (and also you haven't specified if this is IPv4 or IPv6, which of course has an impact on that figure). Maybe write something like "often saving more than 90% of the RTP/UDP/IP overhead"

- section 4, page 8, the term "PVC" is not defined in terminology

- section 5, page 11
      Multiple PWs SHOULD be established in case different QoS requirements
      are needed for different compressed streams.  The QoS received by the
      flow would be determined by the EXP bit marking in the PW label.
      Normally, all RTP packets would get the same EXP marking, equivalent
      to EF treatment in DiffServ.  However, the protocol specified in this

 I think both "EXP marking" and "EF treatment" either need a definition or a reference to another document, they seem to jump out of nowhere in this doc.

- section 5.1
      Any acceptable method of MPLS label distribution MAY be used for
      distributing the MPLS tunnel label.  To assign and distribute the PW
What is an "acceptable method"? Define or re-phrase.

- section 5.1
         a forward equivalence class (FEC) object, a label object, and
  FEC is already defined in terminology, so no need to spell it out here.

- Section 5.2
  There are a lot of MUSTs/MAYs in this section and (at least from a quick reading) some of them seem to say the same thing, such as:
      If HC scheme
      parameters are signaled, the Interface Parameters Sub-TLV MUST be
      used on any unidirectional legs of a PW that will carry HC traffic.
and
      The Interface Parameters Sub-TLV [IANA,
      RFC4447] MUST be used to signal HC channel setup and specify HC
      parameters.

Do these two describe _different_ requirements? If not, maybe one of them could either be removed or it could be written to avoid 2119 language. I'm not keen on having the same requirement listed twice (_if_ that is the case here, there might be some subtle difference that I'm failing to see).
There might be others like these in 5.2 that are overlapping, so maybe a thorough check by of 2119 language in this section could be useful.

- Section 5.2.4
      In RFC 2509 it was not possible to negotiate only TCP HC or only

  Maybe you need to add an informative reference to 2509 since you mention it?

- Section 5.4
  Be consistent and don't mix "misordering" and "reordering", just write "reordering".

- Section 5.4
      CRTP is viewed as
      having risks for a number PW environments due to misordering and
      loss, although commercial issues lead to its choice.
 I don't understand this sentence, what do you mean by "lead to its choice"? Is the intension to say "the inclusion of CRTP into this specification was based on commercial reasons"?
 And if you're going to write "commercial issues" (or reasons) then maybe you should describe those (but I prefer to re-phrase that to something else instead)

 - The reference section might need to be verified a bit, for example [ROHC-REORDER] is now an RFC (RFC4224). There's also some control symbol (=20) in one of the references, so it might be good to review all the references again to make sure they are correct.

/Kristofer Sandlund

Magnus Westerlund wrote:

> Hi,
> 
> As your AD I would really appreciate if someone really would review
> this document and be willing to state that they have reviewed it. We
> need to ensure that cross WG reviews do work.
> 
> Cheers
> 
> Magnus
> 

_______________________________________________
Rohc mailing list
Rohc@ietf.org
https://www1.ietf.org/mailman/listinfo/rohc