: Clarify causes of feedback, compute X_recv more accurately
PATCH 1/3 : Clarify causes of feedback, compute X_recv more accurately
[CCID3]: Clarify causes of feedback, compute X_recv more accurately
The computation of X_recv does currently not conform to TFRC/RFC 3448, since this specification requires that X_recv be computed over the last R_m seconds (sec. 6.2).
The code, in contrast, computes X_recv over the time the last feedback was sent. This results in sending wrong estimates of X_recv, making it impossible for the sender to find its allowed sending rate.
The patch addresses this problem and implements a clearer interface; it
* explicitly distinguishes the types of feedback (using an enum);
* uses previous value of X_recv when sending feedback due to a parameter change;
* makes all state changes local to ccid3_hc_tx_packet_recv;
* assigns feedback type according to incident (previously only used flag `do_feedback').
Further and detailed information is at
http://www.erg.abdn.ac.uk/users/gerrit/dccp/notes/ccid3_packet_reception/#8._Computing_X_recv_
--- net/dccp/ccids/ccid3.c | 39 +++++++++++++++++++++++++++------------ net/dccp/ccids/ccid3.h | 10 +++++++++- 2 files changed, 36 insertions(+), 13 deletions(-)
net/dccp/ccids/ccid3.h +9 -1
@@ -124,13 +124,21 @@ static inline struct ccid3_hc_tx_sock *creturn ccid3_tx_priv; } -/* TFRC receiver states */ +/* CCID3 receiver states */ enum ccid3_hc_rx_states {TFRC_RSTATE_NO_DATA =1 ,TFRC_RSTATE_DATA ,TFRC_RSTATE_TERM =127 , }; +/* CCID3 feedback types */ +enum ccid3_fback_type { +FBACK_NONE =0 , +FBACK_INITIAL , +FBACK_PERIODIC , +FBACK_PARAM_CHANGE +}; +/** struct ccid3_hc_rx_sock - CCID3 receiver half-connection socket * * @ccid3hcrx_x_recv - Receiver estimate of send rate (RFC 3448 4.3)
net/dccp/ccids/ccid3.c +27 -12
@@ -665,7 +665,8 @@ static inline void ccid3_hc_rx_update_s( hcrx->ccid3hcrx_s =tfrc_ewma (hcrx->ccid3hcrx_s, len,9 ); } -static void ccid3_hc_rx_send_feedback (struct sock *sk,struct sk_buff *skb) +static void ccid3_hc_rx_send_feedback (struct sock *sk,struct sk_buff *skb, +enum ccid3_fback_type fbtype) {struct ccid3_hc_rx_sock *hcrx =ccid3_hc_rx_sk (sk);struct dccp_sock *dp =dccp_sk (sk);
@@ -675,13 +676,26 @@ static void ccid3_hc_rx_send_feedback(stif (unlikely (hcrx->ccid3hcrx_state =TFRC_RSTATE_TERM ))return ; -switch (hcrx->ccid3hcrx_state) { -case TFRC_RSTATE_NO_DATA : +switch (fbtype) { +case FBACK_INITIAL : hcrx->ccid3hcrx_x_recv =0 ; hcrx->ccid3hcrx_pinv = ~0U ;/* see RFC 4342, 8.5 */ -ccid3_hc_rx_set_state (sk,TFRC_RSTATE_DATA );break ; -case TFRC_RSTATE_DATA : +case FBACK_PARAM_CHANGE : +/* +* When parameters change (new loss or p > p_prev), we do not +* have a reliable estimate for R_m of [RFC 3448, 6.2] and so +* need to reuse the previous value of X_recv. However, when +* X_recv was 0 (due to early loss), this would kill X down to +* s/t_mbi (i.e. one packet in 64 seconds). +* To avoid such drastic reduction, we approximate X_recv as +* the number of bytes since last feedback. +* This is a safe fallback, since X is bounded above by X_calc. +*/ +if (hcrx->ccid3hcrx_x_recv >0 ) +break ; +/* fall through */ +case FBACK_PERIODIC : delta =ktime_us_delta (now, hcrx->ccid3hcrx_last_feedback);if (delta <=0 )DCCP_BUG ("delta (%ld) <= 0" , (long )delta);
@@ -769,16 +783,17 @@ static u32 ccid3_first_li(struct sock *sstatic void ccid3_hc_rx_packet_recv (struct sock *sk,struct sk_buff *skb) {struct ccid3_hc_rx_sock *hcrx =ccid3_hc_rx_sk (sk); +enum ccid3_fback_type do_feedback =FBACK_NONE ;u32 sample, ndp =dccp_sk (sk)->dccps_options_received.dccpor_ndp, payload_size = skb->len -dccp_hdr (skb)->dccph_doff *4 ; -u8 is_data_packet =dccp_data_packet (skb), do_feedback =0 ; +u8 is_data_packet =dccp_data_packet (skb);spin_lock (&hcrx->ccid3hcrx_hist.lock);if (unlikely (hcrx->ccid3hcrx_state =TFRC_RSTATE_NO_DATA )) { -/* Handle initial feedback */ if (is_data_packet) { - do_feedback =1 ; + do_feedback =FBACK_INITIAL ; +ccid3_hc_rx_set_state (sk,TFRC_RSTATE_DATA );ccid3_hc_rx_update_s (hcrx, payload_size); } goto update_records;
@@ -799,7 +814,7 @@ static void ccid3_hc_rx_packet_recv(stru tfrc_rx_handle_loss(&hcrx->ccid3hcrx_hist, &hcrx->ccid3hcrx_li_hist, skb, ndp, ccid3_first_li, sk) ) { - do_feedback =1 ; + do_feedback =FBACK_PARAM_CHANGE ; goto done_receiving; }
@@ -828,11 +843,11 @@ static void ccid3_hc_rx_packet_recv(stru* Step (3) of [RFC 3448, 6.1]: Recompute I_mean and, if I_mean * has decreased (resp. p has increased), send feedback now. */ - do_feedback =1 ; + do_feedback =FBACK_PARAM_CHANGE ;/* check if the periodic once-per-RTT feedback is due; RFC 4342, 10.3 */ if (SUB16 (dccp_hdr (skb)->dccph_ccval, hcrx->ccid3hcrx_last_counter) >3 ) - do_feedback =1 ; + do_feedback =FBACK_PERIODIC ; update_records:tfrc_rx_hist_update (&hcrx->ccid3hcrx_hist, skb, ndp);
@@ -841,7 +856,7 @@ done_receiving:spin_unlock (&hcrx->ccid3hcrx_hist.lock);if (do_feedback) -ccid3_hc_rx_send_feedback (sk, skb); +ccid3_hc_rx_send_feedback (sk, skb, do_feedback); }static int ccid3_hc_rx_init (struct ccid *ccid,struct sock *sk)