[rohc] Nits/bugs/Changes to ROHC-TCP-08

[Kristofer Sandlund <kristofer.sandlund at effnet.com>]

Hi all!

Here's another list of nits, bugs and requested changes to the ROHC-TCP spec 
(for older stuff still-to-be-fixed, see 
http://www1.ietf.org/mail-archive/web/rohc/current/msg03072.html)

* There is a field next_header in the tcp_static part of the FN. Should go (I've 
already updated the CVS with this).

* We've managed to set the TCP-ECN stuff so that se transmit the tcp_ecn_flags 
_before_ the tcp_res_flags, but according to RFC3168, they appear in this order 
in the uncompressed header:
      +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
      |               |               | C | E | U | A | P | R | S | F |
      | Header Length |    Reserved   | W | C | R | C | S | S | Y | I |
      |               |               | R | E | G | K | H | T | N | N |
      +---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+---+
 Therefore, I will change the order of these in the FN code to make it easier 
for implementations.

* tcp_dynamic chain item has strange order of fields, so I propose updating the 
order as follows (including the ECN order change from above):
        format_tcp_dynamic =  ecn_used,              %[ 1 ]
                              padding,               %[ 1 ]
                              ack_zero,              %[ 1 ]
                              urp_zero,              %[ 1 ]
                              tcp_res_flags,         %[ 4 ]
                              tcp_ecn_flags,         %[ 2 ]
                              urg_flag,              %[ 1 ]
                              ack_flag,              %[ 1 ]
                              psh_flag,              %[ 1 ]
                              rsf_flags,             %[ 3 ]
                              msn,                   %[ 16 ]
                              seq_number,            %[ 32 ]
                              ack_number,            %  0 or 32 bits
                              window,                %[ 16 ]
                              checksum,              %[ 16 ]
                              urg_ptr,               %  0 or 16 bits
                              options,               %  n bits
Now, all the TCP flags + the reserved and ECN appear on the same order and at 
the same "byte boundaries" as in the uncompressed header, which seems to make a 
lot more sense. Will add this to the CVS.

* The discriminators used for IR and IR-CR seems a bit inconsistent. In 3095, we 
have:
11111101 = IR with dynamic + static chain
11111100 = IR with only static chain
In ROHC-TCP, we have:
11111101 = IR-CR
11111100 = IR with dynamic + static chain

Therefore, the "IR with dynamic + static chain" has different discriminators in 
the two profiles. Therefore I propose to switch places for these two 
discriminators in ROHC-TCP. Will have no functional impact, but will make things 
a bit more consistent.

* tcp_replicate is wrong in the FN code.
        format_tcp_replicate = <snip>
                               tcp_ecn_flags,       %[ 2 ]
                               ecn_used,            %[ 1 ]
			       <snip>
                               tcp_ecn_flags,       %  0 or 2 bits
                               tcp_res_flags,       %  0 or 4 bits
                               options_list,        %  n bits
First of all, it appears that the tcp_ecn_flags could appear twice and there is 
no control bit for the tcp_res_flags presence.
Secondly, the optional part is not octet-aligned, which is bad.

Therefore, I suggest that ecn_used controls if those two fields are present or 
not (since that's how we use it elsewhere) and that I add 2 bits of padding in 
the optional part of the chain. Also, the first place where tcp_ecn_flags is 
located will be replaced by more padding (or we can use it for something else, 
such as ack_stride indicator when/if we add ack scaling).

* section 3.2:
   "This means that on the
   forward path (from server to client), only the sequence number is
   changing while the acknowledgement number remains constant for most
   packets; on the backward path (from client to server), only the
   sequence number is changing and the acknowledgement number remains
   constant for most packets."
I guess that's a cut&paste error, since it says that both directions have a 
changing sequence number and a constant ack. I guess the "backward path" should 
switch places of those two words.

BR,
	Kristofer Sandlund, Effnet AB

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