Subject: [rohc] Comments on tcp-08

From: Kristofer Sandlund <[email-masked]>
Date: Tue, 07 Dec 2004 10:33:27 +0100

Hi!

Have collected a few minor comments on draft-ietf-rohc-tcp-08.txt.

- General comments:
     The structure is a bit strange right now, I think that the list compression 
section (4.6) should move to somewhere inside section 6, since it is closely 
related to TCP itself. Now, it appears before the basic things such as the state 
machine operations have been defined.
     Also, I think I'd prefer to move section 6.8 to being before all the 
ROHC-FN-code, so that the FN definitions are put last in the document.

- 5.2.7
      In NC state:

         The decompressor SHOULD send a STATIC-NACK if a packet of a type
         other than IR is received, or if an IR packet has failed the CRC
         check.

     I think that either, the rate limiting text below this should be moved up, 
or this text should refer that text for clarify. As it is now, one can easily 
read that one SHOULD send feedback for each failed packet and then one reads the 
rate limiting text which kind of contradicts this. So maybe something like this 
would improve readability:

         The decompressor SHOULD send a STATIC-NACK (subject to the feedback
         rate limiting considerations below/above/somewhere) if a packet of
         a type other than IR is received, or if an IR packet has failed the CRC
         check.

     Similar reference for SC and FC state might also help.
     Also:

      In FC state:

         The decompressor SHOULD send a NACK when decompression of any
         packet type fails and if context damage is assumed (see also
         section 5.2.3).

     This text seems a bit hard to interpret. Maybe it would be better to write:

         When context damage is assumed and the decompression of any
         compressed packet fails, the decompressor SHOULD send a NACK (see also
         section 5.2.3).

     Because that's the intention, right?

- 6.5 "ends with the static part of a TCP packet" -> "packet" is not a good word 
here. Maybe "ends with a TCP replication item" or maybe even "ends with a TCP 
static chain item". Now sure.

- 6.6 "30 packet formats" - not that many in there right now. Maybe remove this 
sentence altogether.

- 6.6.2 The coding of small CIDs in the "Base CID" field is not obvious, since 
in other CID fields, the small CID is encoded as an add-CID with 0xe<value>, but 
here I guess it should be 0x0<value> instead. Maybe draw something like:

    +---+---+---+---+---+---+---+---+
    :      0        | Base CID Small: 1 octet, if small CID and B=1
    +---+---+---+---+---+---+---+---+
    :                               :
    /        Base CID Large         / 1-2 octets, if large CIDs and B=1
    :                               :
    +---+---+---+---+---+---+---+---+

- 6.6.3 "variable number of bits" (base header field). Since this will always be 
a multiple of 8 bits, it might be better to say "variable number of octets".

- 6.7 The two constants defined are not used any longer so they should be 
removed and also the sentence above them.

- 6.8.2 Some references to 3095 are for the wrong section:
	"section 5.7.6.6" should be "section 5.7.6.7" (CLOCK option)
	"section 5.7.6.7" should be "section 5.7.6.8" (JITTER option)

BR,
	Kristofer Sandlund, Effnet AB

_______________________________________________
Rohc mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/rohc

---

From: "Ghyslain Pelletier (LU/EAB)" <[email-masked]>
Date: Tue, 14 Dec 2004 20:21:32 +0100

Hi Kristofer,

Thanks for your comments. See inline.

/Ghyslain

> -----Original Message-----
> - General comments:

> section (4.6) should move to somewhere inside section 6, 

Will do.

> Also, I think I'd prefer to move section 6.8 to being 
> before all the ROHC-FN-code, so that the FN definitions

Agree.

>       In NC state:

I suggest to write this pointer in the sentence above, as follow:

  "Once the feedback channel is established, the decompressor is
   REQUIRED to continue sending feedback (subject to the feedback
   rate limiting considerations later in this section) for the
   lifetime of the packet stream as follow:

   In NC state:
  "

Ok?

>    compressed packet fails, the decompressor SHOULD 
>    send a NACK (see also section 5.2.3).
> 

Actually, the intention of the text is to say that "if you don't have assumed context damage yet, and if the decompression of any packet type fails which leads you to assume context damage, the decompressor should send a NACK". Which is why the text uses "when" and then "if". That is, the condition that context damage is already assumed from the failure of a previous packet is no pre-requisite, which is how I interpret your suggested text. So I wouldn't make changes...
 
> - 6.5 "ends with the static part of a TCP packet" -> "packet" 
> is not a good word here. Maybe "ends with a TCP replication
> item" or maybe even "ends with a TCP static chain item". Not sure.

How about:

   6.5.  Initialization

   The static context of ROHC TCP streams can be initialized in either
   two ways:

   1) By using an IR packet as in section 6.6.1, where the profile is
   six (6) and the static chain ends with the static part of a TCP
   header. 

   2) By replicating an existing context using the mechanism defined
   by ROHC-CR. This is done with the IR-CR packet defined in section
   6.6.2, where the profile number is six (6). 

Note that I have simply replaced TCP packet with TCP header in 1), and removed the last bit in 2) since it seems to be actually sufficient that the packet type be IR-CR with profile number being ROHC-TCP. It seems we don't need to have more text there.
 
> - 6.6 "30 packet formats" - not that many in there right now. 
> Maybe remove this sentence altogether.

I'll leave it in for now and make sure that the numbers are right once all packet formats are set in stone.
 
> here I guess it should be 0x0<value> instead. Maybe draw 

Fixed from our offline discussion.
 
> a multiple of 8 bits, it might be better to say "variable 

Fixed.
 
> removed and also the sentence above them.

Fixed.
 
> 	"section 5.7.6.7" should be "section 5.7.6.8" (JITTER option)

Actually, I have removed those bullets and instead included explicitly all relevant 3095 options to the ROHC-TCP profile in the draft.