TCP ACK number scaling

Bug #1507675 reported by Klaus Warnke
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
rohc
Status tracked in Rohc-main
Rohc-1.7.x
Won't Fix
Medium
Didier Barvaux
Rohc-main
Fix Released
Medium
Didier Barvaux

Bug Description

Didier,

I compared the compressor output
to see how my compression ratio is.

I found some differences...

To see what is going on, I feed the RoHC
pcap from the test directory into your deomp.
You have a great diagnostic debug output.

And I found this:

[DEBUG] [d_tcp.c:2353 d_tcp_decode_bits()] TCP ACK number = 0x26abe294 (re-used from previous packet)
[DEBUG] [d_tcp.c:2365 d_tcp_decode_bits()] TCP ACK number (0x7589a264) = scaled (0xf175ea) * payload size (41) + residue (0x1a)

I looks like, that you are using the payload size for scaling.
This values is used for the seq_number scaling, but not for the ack_number:

   The compressor MAY use the scaled acknowledgment number encoding;
   what value it will use as the scaling factor is up to the compressor
   implementation. In the case where there is a co-located decompressor
   processing packets of the same TCP flow in the opposite direction,
   the scaling factor for the sequence number used for that flow can be
   used by the compressor to determine a suitable scaling factor for the
   TCP Acknowledgment number for this flow.

If I understand this correct, I can use the seq_number scaling factor
from the opposite direction compression, if available. This could be
the payload size. But it looks like, that the compressor transmits the
default value 0 for the ack_stride (for what reason ever).

  INITIAL {
    ack_stride =:= uncompressed_value(16, 0);
  }

[DEBUG] [d_tcp.c:1549 d_tcp_parse_CO()] found 16 bits of sequence number encoded on 2 bytes
[DEBUG] [d_tcp.c:1570 d_tcp_parse_CO()] found 0 bits of acknowledgment number encoded on 0 bytes
[DEBUG] [d_tcp.c:1585 d_tcp_parse_CO()] found 16 bits of ACK stride encoded on 2 bytes
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] fa 89 8c 00 6b 68 c0 00 00 01 5e 25

[DEBUG] [d_tcp.c:2257 d_tcp_decode_bits()] TCP sequence number = 0x758968c0 (decoded from 16-bit 0x68c0 with p = 16383)
[DEBUG] [d_tcp.c:2276 d_tcp_decode_bits()] TCP sequence number (0x758968c0) = scaled (0x153838) * payload size (1418) + residue (0x290)
[DEBUG] [d_tcp.c:2353 d_tcp_decode_bits()] TCP ACK number = 0x26abe242 (re-used from previous packet)
[DEBUG] [d_tcp.c:2365 d_tcp_decode_bits()] TCP ACK number (0x758968c0) = scaled (0x6fb48) * payload size (1418) + residue (0x572)

This is the rohc packet, where immediately after
the seq_num the ack_stride follows:

[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] fa 89 8c 00 6b
68 c0 <- seq_number 16bit 0x68c0
00 00 <- ack_stride 00 00
01 5e 25

but

[DEBUG] [d_tcp.c:2365 d_tcp_decode_bits()] TCP ACK number (0x758968c0) = scaled (0x6fb48) * payload size (1418) + residue (0x572)

Code:

  /* compute scaled acknowledgement number & residue */
  if(payload_len != 0)
  {
   decoded->ack_num_scaled = decoded->ack_num / payload_len;
   decoded->ack_num_residue = decoded->ack_num % payload_len;
   rohc_decomp_debug(context, " TCP ACK number (0x%08x) = scaled (0x%x) "
         "* payload size (%zu) + residue (0x%x)",
                     decoded->seq_num, decoded->ack_num_scaled, payload_len,
                     decoded->ack_num_residue);
  }

This look wrong for me. (The "decoded->seq_num" in rohc_decomp_debug
also, copy/paste)
If have no test, because I have no ack_stride value, because I have no
opposite decomp associated.
Instead of payload_len there should be decoded->ack_stride or
bits->ack_stride.bits?

In short:
Maybe compressor bug, sending ack_stride 0.
decomp uses payload_len for scaling ACK num as default instead of ack_stride.
Looks like a copy/paste issue from seq_number handling...

Could you please comment?

Best,
Klaus Warnke

Tags: library tcp
Revision history for this message
Didier Barvaux (didier-barvaux) wrote :

Klaus,

Scaled ACK number was not fully implemented. Some of the code was indeed copy/paste from the scaled sequence number.

I fixed the scaled ACK implementation in the dev_fuzzing_decompressor on the 11th of October. Your bug report looks like you tested with the master branch or maybe a prior version of the same branch. Could you tell me if the fix works for you?

The fix is located there on branch dev_fuzzing_decompressor:
https://github.com/didier-barvaux/rohc/commit/0a708940627ad9e20ff7f35df9290015ec017b3e

Regards,
Didier

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.