[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[rohc] RE: Comments on tcp-08



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.

> 
> - 5.2.7
>       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?

>      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?

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.
 
> - 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
>     :                               :
>     +---+---+---+---+---+---+---+---+

Fixed from our offline discussion.
 
> - 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".

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

Fixed.
 
> - 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)
> 

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

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