Comment 2 for bug 1496023

Revision history for this message
Klaus Warnke (k-warnke) wrote :

I agree, this patch does not work, if a SACK option disappears for one packet and appears again.
This occurs in the 4th packet.
This next try works for me.
It uses blocks_nr as an "is_static" indicator.
To be more explicit, you can introduce it
and set it in the _dyn and _irreg functions accordingly.
However, this patch uses the context values while building,
if no bits are received:

diff --git a/src/decomp/d_tcp_opts_list.c b/src/decomp/d_tcp_opts_list.c
index 0fef714..5b84b74 100644
--- a/src/decomp/d_tcp_opts_list.c
+++ b/src/decomp/d_tcp_opts_list.c
@@ -1176,10 +1184,26 @@ static bool d_tcp_build_sack(const struct rohc_decomp_ctxt *const context,
                              struct rohc_buf *const uncomp_packet,
                              size_t *const opt_len)
 {
- const sack_block_t *const blocks = decoded->opt_sack_blocks.blocks;
- const size_t blocks_nr = decoded->opt_sack_blocks.blocks_nr;
- const size_t blocks_len = sizeof(sack_block_t) * blocks_nr;
- const size_t sack_len = 2 + blocks_len;
+ sack_block_t const *blocks;
+ size_t blocks_nr;
+ size_t blocks_len;
+ size_t sack_len;
+
+ if (decoded->opt_sack_blocks.blocks_nr > 0)
+ {
+ blocks = decoded->opt_sack_blocks.blocks;
+ blocks_nr = decoded->opt_sack_blocks.blocks_nr;
+ }
+ else
+ {
+ const struct d_tcp_context *const tcp_context = context->persist_ctxt;
+
+ blocks = tcp_context->opt_sack_blocks.blocks;
+ blocks_nr = tcp_context->opt_sack_blocks.blocks_nr;
+ }
+
+ blocks_len = sizeof(sack_block_t) * blocks_nr;
+ sack_len = 2 + blocks_len;

  if(rohc_buf_avail_len(*uncomp_packet) < sack_len)
  {

And this avoid overwriting the context with empty values,
if no received.

diff --git a/src/decomp/d_tcp.c b/src/decomp/d_tcp.c
index 9ededdc..94ac4b9 100644
--- a/src/decomp/d_tcp.c
+++ b/src/decomp/d_tcp.c
@@ -3389,8 +3390,11 @@ static void d_tcp_update_ctxt(struct rohc_decomp_ctxt *const context,
   }
   else if(opt_index == TCP_INDEX_SACK)
   {
- memcpy(&tcp_context->opt_sack_blocks, &decoded->opt_sack_blocks,
- sizeof(struct d_tcp_opt_sack));
+ if (decoded->opt_sack_blocks.blocks_nr > 0)
+ {
+ memcpy(&tcp_context->opt_sack_blocks, &decoded->opt_sack_blocks,
+ sizeof(struct d_tcp_opt_sack));
+ }
   }
  }
 }

And don't use the d_tcp_sack_parse() hack.

Feel free to replace blocks_nr with a is_static variable.
These are the 7 RoHC packet for testing, if you want:

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 50-byte packet #1
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (50 bytes):
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] fd 06 34 00 06 c0 a8 02 02 5b 79 32 85 be 63 00
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 50 06 00 40 45 35 30 02 00 01 b7 00 cb 35 39 08
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 50 d7 05 ad e8 b0 05 b4 01 ff ff d6 8e 00 a9 72
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] 00 07

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 10-byte packet #2
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (10 bytes):
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] fa 22 00 04 9f 45 35 50 d7 00

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 27-byte packet #3
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (27 bytes):
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] fa 83 76 08 86 36 2e cc 82 68 00 73 45 36 03 00
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] c0 41 d6 8e 04 56 95 b6 05 50 cf

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 126-byte packet #4
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (100 bytes):
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] b3 be 48 02 68 0b 36 03 00 c0 41 d6 8e 04 02 00
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 04 00 0b 43 00 00 00 00 00 00 00 00 ff 7f 00 00
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 01 00 00 00 b3 20 88 50 50 ab 04 00 4a 00 00 00
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 4a 00 00 00 00 0b 6b 7e 54 71 00 19 7e b2 12 af
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 08 00 45 00 00 3c 45 35 40 00 40 06 a4 de c0 a8
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 02 02 5b 79 32 85 be 63 00 50 b7 00 cb 35 00 00
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] 00 00 a0 02

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 20-byte packet #5
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (20 bytes):
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] fa 25 74 08 9f 35 00 00 00 00 39 08 30 05 25 60
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] 30 50 d7 00

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 26-byte packet #6
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (26 bytes):
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] fa 86 74 08 86 36 2e cc 82 68 00 73 30 03 00 c0
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] 41 d6 8e 04 56 95 b6 05 50 cf

[DEBUG] [rohc_decomp.c:779 rohc_decompress3()] decompress the 118-byte packet #7
[DEBUG] [rohc_traces_internal.c:100 rohc_dump_buf()] compressed data, max 100 bytes (100 bytes):
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] fa 27 74 08 b3 35 00 00 00 00 39 08 30 05 25 60
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 30 50 d7 00 b3 20 88 50 e7 31 05 00 4a 00 00 00
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 4a 00 00 00 00 0b 6b 7e 54 71 00 19 7e b2 12 af
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 08 00 45 00 00 3c 45 35 40 00 40 06 a4 de c0 a8
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 02 02 5b 79 32 85 be 63 00 50 b7 00 cb 35 00 00
[DEBUG] [rohc_traces_internal.c:109 rohc_dump_buf()] 00 00 a0 02 39 08 50 d7 00 00 02 04 05 b4 04 02
[DEBUG] [rohc_traces_internal.c:129 rohc_dump_buf()] 6f 6e 6e 65

Best,
Klaus Warnke