Skip to content

Commit

Permalink
TCP: CR: fix corner case for Context Replication
Browse files Browse the repository at this point in the history
Corner case is:
1/ Send IR packet for stream CID=0
2/ Receive ACK for CID=0, making the stream 'established'
3/ Send IR-CR packet for stream CID=1, with base CID=0 (as CID=0 is
   established), but lose the packet in the way towards decompressor
4/ Send a new packet for CID=0, but with TTL changed (+1)
5/ Send a new packet for CID=1, IR-CR is repeated since previous IR-CR
   was lost and no ACK was received
6/ Decompressor fails to decompress packet IR-CR

The problem was located at compressor. At step 5, the compressor used a
copy of context CID=0 as base context instead of the real context CID=0.

The problem was fixed by using the base context as reference while:
a/ finding differences between the context and the packet,
b/ determining the best packet type to build, and
c/ while building the packet.
The real context is only used while updating the context.

To achieve that, all steps a/ b/ c/ were reworked not to modify the
context. 49 commits were required. Once rework was achieved, the fix is
quite simple.
  • Loading branch information
didier-barvaux committed Sep 24, 2019
1 parent bc12b88 commit b863678
Show file tree
Hide file tree
Showing 12 changed files with 560 additions and 474 deletions.
941 changes: 510 additions & 431 deletions src/comp/c_tcp.c

Large diffs are not rendered by default.

12 changes: 7 additions & 5 deletions src/comp/c_tcp_dynamic.c
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ static int tcp_code_dynamic_tcp_part(const struct rohc_comp_ctxt *const context,
/**
* @brief Code the dynamic part of an IR or IR-DYN packet
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ref_ctxt The reference compression context to detect changes
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for compressed packet
* @param rohc_pkt OUT: The ROHC packet
Expand All @@ -76,12 +77,13 @@ static int tcp_code_dynamic_tcp_part(const struct rohc_comp_ctxt *const context,
* -1 otherwise
*/
int tcp_code_dyn_part(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_pkt,
const size_t rohc_pkt_max_len)
{
const struct sc_tcp_context *const tcp_context = context->specific;
const struct sc_tcp_context *const tcp_context = ref_ctxt->specific;

uint8_t *rohc_remain_data = rohc_pkt;
size_t rohc_remain_len = rohc_pkt_max_len;
Expand Down Expand Up @@ -173,7 +175,7 @@ int tcp_code_dyn_part(const struct rohc_comp_ctxt *const context,
/**
* @brief Build the dynamic part of the IPv4 header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ip_context The specific IP compression context
* @param ipv4 The IPv4 header
* @param ip_id_behavior The IP-ID behavior of the IPv4 header
Expand Down Expand Up @@ -254,7 +256,7 @@ static int tcp_code_dynamic_ipv4_part(const struct rohc_comp_ctxt *const context
/**
* @brief Build the dynamic part of the IPv6 header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ip_context The specific IP compression context
* @param ipv6 The IPv6 header
* @param[out] rohc_data The ROHC packet being built
Expand Down Expand Up @@ -366,7 +368,7 @@ TODO
\endverbatim
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for compressed packet
* @param[out] rohc_data The ROHC packet being built
Expand Down
3 changes: 2 additions & 1 deletion src/comp/c_tcp_dynamic.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@
#include <stdlib.h>

int tcp_code_dyn_part(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_pkt,
const size_t rohc_pkt_max_len)
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4)));
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4, 5)));

#endif /* ROHC_COMP_TCP_DYNAMIC_H */

27 changes: 16 additions & 11 deletions src/comp/c_tcp_irregular.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,20 @@ static int tcp_code_irregular_ipv6_opt_part(const struct rohc_comp_ctxt *const c
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4)));

static int tcp_code_irregular_tcp_part(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
const uint8_t ip_inner_ecn,
uint8_t *const rohc_data,
const size_t rohc_max_len)
__attribute__((warn_unused_result, nonnull(1, 2, 3, 5)));
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4, 6)));


/**
* @brief Code the irregular chain of one CO packet
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ref_ctxt The reference compression context to detect changes
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for the compressed packet
* @param rohc_pkt OUT: The ROHC packet
Expand All @@ -83,12 +85,13 @@ static int tcp_code_irregular_tcp_part(const struct rohc_comp_ctxt *const contex
* -1 otherwise
*/
int tcp_code_irreg_chain(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_pkt,
const size_t rohc_pkt_max_len)
{
const struct sc_tcp_context *const tcp_context = context->specific;
const struct sc_tcp_context *const tcp_context = ref_ctxt->specific;
uint8_t *rohc_remain_data = rohc_pkt;
size_t rohc_remain_len = rohc_pkt_max_len;
uint8_t ip_inner_ecn;
Expand Down Expand Up @@ -177,8 +180,8 @@ int tcp_code_irreg_chain(const struct rohc_comp_ctxt *const context,
}

/* TCP part (base header + options) of the irregular chain */
ret = tcp_code_irregular_tcp_part(context, uncomp_pkt_hdrs, tmp, ip_inner_ecn,
rohc_remain_data, rohc_remain_len);
ret = tcp_code_irregular_tcp_part(context, ref_ctxt, uncomp_pkt_hdrs, tmp,
ip_inner_ecn, rohc_remain_data, rohc_remain_len);
if(ret < 0)
{
rohc_comp_warn(context, "failed to build the TCP header part "
Expand All @@ -202,7 +205,7 @@ int tcp_code_irreg_chain(const struct rohc_comp_ctxt *const context,
*
* See RFC 4996 page 63
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ip_context The specific IP compression context
* @param ipv4 The IPv4 header
* @param ip_id_behavior The IP-ID behavior of the IPv4 header
Expand Down Expand Up @@ -305,7 +308,7 @@ static int tcp_code_irregular_ipv4_part(const struct rohc_comp_ctxt *const conte
*
* See RFC 4996 page 63
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ip_context The specific IP compression context
* @param ipv6 The IPv6 header
* @param is_innermost True if IP header is the innermost of the packet
Expand Down Expand Up @@ -387,15 +390,15 @@ static int tcp_code_irregular_ipv6_part(const struct rohc_comp_ctxt *const conte
/**
* @brief Build the irregular part of the IPv6 option header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param opt_ctxt The compression context of the IPv6 option
* @param ext The IPv6 extension header
* @param[out] rohc_data The ROHC packet being built
* @param rohc_max_len The max remaining length in the ROHC buffer
* @return The length appended in the ROHC buffer if positive,
* -1 in case of error
*/
static int tcp_code_irregular_ipv6_opt_part(const struct rohc_comp_ctxt *const context __attribute__((unused)),
static int tcp_code_irregular_ipv6_opt_part(const struct rohc_comp_ctxt *const context,
const ip_option_context_t *const opt_ctxt __attribute__((unused)),
const struct rohc_pkt_ip_ext_hdr *const ext,
uint8_t *const rohc_data __attribute__((unused)),
Expand Down Expand Up @@ -424,7 +427,8 @@ static int tcp_code_irregular_ipv6_opt_part(const struct rohc_comp_ctxt *const c
/**
* @brief Build the irregular part of the TCP header.
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ref_ctxt The reference compression context to detect changes
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for compressed packet
* @param ip_inner_ecn The ECN flags of the innermost IP header
Expand All @@ -434,13 +438,14 @@ static int tcp_code_irregular_ipv6_opt_part(const struct rohc_comp_ctxt *const c
* -1 in case of error
*/
static int tcp_code_irregular_tcp_part(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
const uint8_t ip_inner_ecn,
uint8_t *const rohc_data,
const size_t rohc_max_len)
{
const struct sc_tcp_context *const tcp_context = context->specific;
const struct sc_tcp_context *const tcp_context = ref_ctxt->specific;
const struct tcphdr *const tcp = (struct tcphdr *) uncomp_pkt_hdrs->tcp;
uint8_t *rohc_remain_data = rohc_data;
size_t rohc_remain_len = rohc_max_len;
Expand Down
3 changes: 2 additions & 1 deletion src/comp/c_tcp_irregular.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,12 @@
#include <stdlib.h>

int tcp_code_irreg_chain(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_pkt,
const size_t rohc_pkt_max_len)
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4)));
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4, 5)));

#endif /* ROHC_COMP_TCP_IRREGULAR_H */

2 changes: 1 addition & 1 deletion src/comp/c_tcp_opts_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ void tcp_detect_options_changes(const struct rohc_comp_ctxt *const context,
* - the replicate chain of the IR-CR packets,
* - at the end of the rnd_8, seq_8, and co_common packets.
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for compressed TCP options
* @param items_needed Whether items shall be transmitted or not
Expand Down
23 changes: 14 additions & 9 deletions src/comp/c_tcp_replicate.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,19 @@ static int tcp_code_replicate_ipv6_opt_part(const struct rohc_comp_ctxt *const c
__attribute__((warn_unused_result, nonnull(1, 2, 3)));

static int tcp_code_replicate_tcp_part(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_data,
const size_t rohc_max_len)
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4)));
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4, 5)));


/**
* @brief Code the replicate chain of an IR packet
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ref_ctxt The reference compression context to detect changes
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for the compressed packet
* @param[out] rohc_pkt The ROHC packet being built
Expand All @@ -73,12 +75,13 @@ static int tcp_code_replicate_tcp_part(const struct rohc_comp_ctxt *const contex
* -1 otherwise
*/
int tcp_code_replicate_chain(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_pkt,
const size_t rohc_pkt_max_len)
{
const struct sc_tcp_context *const tcp_context = context->specific;
const struct sc_tcp_context *const tcp_context = ref_ctxt->specific;

uint8_t *rohc_remain_data = rohc_pkt;
size_t rohc_remain_len = rohc_pkt_max_len;
Expand Down Expand Up @@ -148,7 +151,7 @@ int tcp_code_replicate_chain(const struct rohc_comp_ctxt *const context,
}

/* add TCP replicate part */
ret = tcp_code_replicate_tcp_part(context, uncomp_pkt_hdrs, tmp,
ret = tcp_code_replicate_tcp_part(context, ref_ctxt, uncomp_pkt_hdrs, tmp,
rohc_remain_data, rohc_remain_len);
if(ret < 0)
{
Expand All @@ -171,7 +174,7 @@ int tcp_code_replicate_chain(const struct rohc_comp_ctxt *const context,
/**
* @brief Build the replicate part of the IPv4 header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ip_context The specific IP compression context
* @param ipv4 The IPv4 header
* @param ip_id_behavior The IP-ID behavior of the IPv4 header
Expand Down Expand Up @@ -261,7 +264,7 @@ static int tcp_code_replicate_ipv4_part(const struct rohc_comp_ctxt *const conte
/**
* @brief Build the replicate part of the IPv6 header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ip_context The specific IP compression context
* @param ipv6 The IPv6 header
* @param[out] rohc_data The ROHC packet being built
Expand Down Expand Up @@ -332,7 +335,7 @@ static int tcp_code_replicate_ipv6_part(const struct rohc_comp_ctxt *const conte
/**
* @brief Build the replicate part of the IPv6 option header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ext The IPv6 extension header
* @param[out] rohc_data The ROHC packet being built
* @param rohc_max_len The max remaining length in the ROHC buffer
Expand Down Expand Up @@ -390,7 +393,8 @@ static int tcp_code_replicate_ipv6_opt_part(const struct rohc_comp_ctxt *const c
/**
* @brief Build the replicate part of the TCP header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ref_ctxt The reference compression context to detect changes
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param tmp The temporary state for the compressed packet
* @param[out] rohc_data The ROHC packet being built
Expand All @@ -399,13 +403,14 @@ static int tcp_code_replicate_ipv6_opt_part(const struct rohc_comp_ctxt *const c
* -1 in case of error
*/
static int tcp_code_replicate_tcp_part(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_data,
const size_t rohc_max_len)
{
const uint8_t oa_repetitions_nr = context->compressor->oa_repetitions_nr;
const struct sc_tcp_context *const tcp_context = context->specific;
const struct sc_tcp_context *const tcp_context = ref_ctxt->specific;
const struct tcphdr *const tcp = (struct tcphdr *) uncomp_pkt_hdrs->tcp;

uint8_t *rohc_remain_data = rohc_data;
Expand Down
3 changes: 2 additions & 1 deletion src/comp/c_tcp_replicate.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@
#include <stdlib.h>

int tcp_code_replicate_chain(const struct rohc_comp_ctxt *const context,
const struct rohc_comp_ctxt *const ref_ctxt,
const struct rohc_pkt_hdrs *const uncomp_pkt_hdrs,
const struct tcp_tmp_variables *const tmp,
uint8_t *const rohc_pkt,
const size_t rohc_pkt_max_len)
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4)));
__attribute__((warn_unused_result, nonnull(1, 2, 3, 4, 5)));

#endif /* ROHC_COMP_TCP_REPLICATE_H */

10 changes: 5 additions & 5 deletions src/comp/c_tcp_static.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ static int tcp_code_static_tcp_part(const struct rohc_comp_ctxt *const context,
/**
* @brief Code the static part of an IR packet
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param uncomp_pkt_hdrs The uncompressed headers to encode
* @param rohc_pkt OUT: The ROHC packet
* @param rohc_pkt_max_len The maximum length of the ROHC packet
Expand Down Expand Up @@ -158,7 +158,7 @@ int tcp_code_static_part(const struct rohc_comp_ctxt *const context,
/**
* @brief Build the static part of the IPv4 header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ipv4 The IPv4 header
* @param[out] rohc_data The ROHC packet being built
* @param rohc_max_len The max remaining length in the ROHC buffer
Expand Down Expand Up @@ -200,7 +200,7 @@ static int tcp_code_static_ipv4_part(const struct rohc_comp_ctxt *const context,
/**
* @brief Build the static part of the IPv6 header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ipv6 The IPv6 header
* @param[out] rohc_data The ROHC packet being built
* @param rohc_max_len The max remaining length in the ROHC buffer
Expand Down Expand Up @@ -271,7 +271,7 @@ static int tcp_code_static_ipv6_part(const struct rohc_comp_ctxt *const context,
/**
* @brief Build the static part of the IPv6 option header
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param ext The IPv6 extension header
* @param[out] rohc_data The ROHC packet being built
* @param rohc_max_len The max remaining length in the ROHC buffer
Expand Down Expand Up @@ -357,7 +357,7 @@ static int tcp_code_static_ipv6_opt_part(const struct rohc_comp_ctxt *const cont
\endverbatim
*
* @param context The compression context
* @param context The real compression context for traces and update
* @param tcp The TCP header
* @param[out] rohc_data The ROHC packet being built
* @param rohc_max_len The max remaining length in the ROHC buffer
Expand Down
5 changes: 1 addition & 4 deletions src/comp/rohc_comp.c
Original file line number Diff line number Diff line change
Expand Up @@ -3460,15 +3460,13 @@ static struct rohc_comp_ctxt *

/* copy the base context, then reset some parts of it */
memcpy(c, base_ctxt, sizeof(struct rohc_comp_ctxt));
c->do_ctxt_replication = true;
c->cr_base_cid = base_ctxt->cid;
c->state = ROHC_COMP_STATE_CR;
}
else
{
rohc_debug(comp, ROHC_TRACE_COMP, ROHC_PROFILE_GENERAL,
"create context with CID %u without replication", cid_to_use);
c->do_ctxt_replication = false;
c->state = ROHC_COMP_STATE_IR;
}

Expand Down Expand Up @@ -3500,7 +3498,7 @@ static struct rohc_comp_ctxt *
c->compressor = comp;

/* create profile-specific context */
if(c->do_ctxt_replication)
if(c->state == ROHC_COMP_STATE_CR)
{
if(!profile->clone(c, base_ctxt))
{
Expand Down Expand Up @@ -3755,7 +3753,6 @@ static struct rohc_comp_ctxt *
* is in action, check that the base context didn't change too much */
if(context != NULL &&
profile->id == ROHCv1_PROFILE_IP_TCP && /* TODO: replace TCP by CR capacity */
context->do_ctxt_replication &&
context->state == ROHC_COMP_STATE_CR &&
context->state_oa_repeat_nr < comp->oa_repetitions_nr)
{
Expand Down
2 changes: 0 additions & 2 deletions src/comp/rohc_comp_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,6 @@ struct rohc_comp_ctxt
/** Profile-specific data, defined by the profiles */
void *specific;

/** Whether Context Replication (CR) may be used */
bool do_ctxt_replication;
/** The base context for Context Replication (CR) */
rohc_cid_t cr_base_cid;

Expand Down
Loading

0 comments on commit b863678

Please sign in to comment.