diff options
author | Gerrit Renker <gerrit@erg.abdn.ac.uk> | 2006-12-09 21:03:51 -0500 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2006-12-11 17:34:45 -0500 |
commit | 0f9e5b573f7249b0e04a03457b55081d1f60f2bf (patch) | |
tree | fa33e3252d5df520348bb22d2a14aeac0e6b2bfc | |
parent | bfe24a6cc222d27e1491f850802fa6932232b8ef (diff) |
[DCCP]: Debug timeval operations
Problem:
Most target types in the CCID3 code are u32, so subtle conversion errors
can occur if signed time calculations yield negative results: the original
values are lost in the conversion to unsigned, calculation errors go undetected.
This patch therefore
* sets all critical time types from unsigned to suseconds_t
* avoids comparison between signed/unsigned via type-casting
* provides ample warning messages in case time calculations are negative
These warning messages can be removed at a later stage when the code
has undergone more testing.
Signed-off-by: Gerrit Renker <gerrit@erg.abdn.ac.uk>
Acked-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
-rw-r--r-- | net/dccp/ccids/ccid3.c | 56 | ||||
-rw-r--r-- | net/dccp/dccp.h | 1 |
2 files changed, 33 insertions, 24 deletions
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c index 89ef1183e48c..820bc25e14dc 100644 --- a/net/dccp/ccids/ccid3.c +++ b/net/dccp/ccids/ccid3.c | |||
@@ -128,8 +128,8 @@ static void ccid3_hc_tx_update_x(struct sock *sk, struct timeval *now) | |||
128 | hctx->ccid3hctx_x = max_t(u64, hctx->ccid3hctx_x, | 128 | hctx->ccid3hctx_x = max_t(u64, hctx->ccid3hctx_x, |
129 | (hctx->ccid3hctx_s << 6)/TFRC_T_MBI); | 129 | (hctx->ccid3hctx_s << 6)/TFRC_T_MBI); |
130 | 130 | ||
131 | } else if (timeval_delta(now, &hctx->ccid3hctx_t_ld) >= | 131 | } else if (timeval_delta(now, &hctx->ccid3hctx_t_ld) - |
132 | hctx->ccid3hctx_rtt) { | 132 | (suseconds_t)hctx->ccid3hctx_rtt >= 0 ) { |
133 | 133 | ||
134 | hctx->ccid3hctx_x = max(2 * min(hctx->ccid3hctx_x, | 134 | hctx->ccid3hctx_x = max(2 * min(hctx->ccid3hctx_x, |
135 | hctx->ccid3hctx_x_recv), | 135 | hctx->ccid3hctx_x_recv), |
@@ -271,7 +271,7 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) | |||
271 | struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); | 271 | struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); |
272 | struct dccp_tx_hist_entry *new_packet; | 272 | struct dccp_tx_hist_entry *new_packet; |
273 | struct timeval now; | 273 | struct timeval now; |
274 | long delay; | 274 | suseconds_t delay; |
275 | 275 | ||
276 | BUG_ON(hctx == NULL); | 276 | BUG_ON(hctx == NULL); |
277 | 277 | ||
@@ -331,7 +331,7 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb) | |||
331 | * else | 331 | * else |
332 | * // send the packet in (t_nom - t_now) milliseconds. | 332 | * // send the packet in (t_nom - t_now) milliseconds. |
333 | */ | 333 | */ |
334 | if (delay - (long)hctx->ccid3hctx_delta >= 0) | 334 | if (delay - (suseconds_t)hctx->ccid3hctx_delta >= 0) |
335 | return delay / 1000L; | 335 | return delay / 1000L; |
336 | break; | 336 | break; |
337 | case TFRC_SSTATE_TERM: | 337 | case TFRC_SSTATE_TERM: |
@@ -353,7 +353,7 @@ static void ccid3_hc_tx_packet_sent(struct sock *sk, int more, unsigned int len) | |||
353 | const struct dccp_sock *dp = dccp_sk(sk); | 353 | const struct dccp_sock *dp = dccp_sk(sk); |
354 | struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); | 354 | struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk); |
355 | struct timeval now; | 355 | struct timeval now; |
356 | unsigned long quarter_rtt; | 356 | suseconds_t quarter_rtt; |
357 | struct dccp_tx_hist_entry *packet; | 357 | struct dccp_tx_hist_entry *packet; |
358 | 358 | ||
359 | BUG_ON(hctx == NULL); | 359 | BUG_ON(hctx == NULL); |
@@ -450,10 +450,8 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb) | |||
450 | r_sample = timeval_delta(&now, &packet->dccphtx_tstamp); | 450 | r_sample = timeval_delta(&now, &packet->dccphtx_tstamp); |
451 | t_elapsed = dp->dccps_options_received.dccpor_elapsed_time * 10; | 451 | t_elapsed = dp->dccps_options_received.dccpor_elapsed_time * 10; |
452 | 452 | ||
453 | if (unlikely(r_sample <= 0)) { | 453 | DCCP_BUG_ON(r_sample < 0); |
454 | DCCP_WARN("WARNING: R_sample (%ld) <= 0!\n", r_sample); | 454 | if (unlikely(r_sample <= t_elapsed)) |
455 | r_sample = 0; | ||
456 | } else if (unlikely(r_sample <= t_elapsed)) | ||
457 | DCCP_WARN("WARNING: r_sample=%ldus <= t_elapsed=%ldus\n", | 455 | DCCP_WARN("WARNING: r_sample=%ldus <= t_elapsed=%ldus\n", |
458 | r_sample, t_elapsed); | 456 | r_sample, t_elapsed); |
459 | else | 457 | else |
@@ -698,6 +696,7 @@ static void ccid3_hc_rx_send_feedback(struct sock *sk) | |||
698 | struct dccp_sock *dp = dccp_sk(sk); | 696 | struct dccp_sock *dp = dccp_sk(sk); |
699 | struct dccp_rx_hist_entry *packet; | 697 | struct dccp_rx_hist_entry *packet; |
700 | struct timeval now; | 698 | struct timeval now; |
699 | suseconds_t delta; | ||
701 | 700 | ||
702 | ccid3_pr_debug("%s, sk=%p\n", dccp_role(sk), sk); | 701 | ccid3_pr_debug("%s, sk=%p\n", dccp_role(sk), sk); |
703 | 702 | ||
@@ -707,12 +706,12 @@ static void ccid3_hc_rx_send_feedback(struct sock *sk) | |||
707 | case TFRC_RSTATE_NO_DATA: | 706 | case TFRC_RSTATE_NO_DATA: |
708 | hcrx->ccid3hcrx_x_recv = 0; | 707 | hcrx->ccid3hcrx_x_recv = 0; |
709 | break; | 708 | break; |
710 | case TFRC_RSTATE_DATA: { | 709 | case TFRC_RSTATE_DATA: |
711 | const u32 delta = timeval_delta(&now, | 710 | delta = timeval_delta(&now, |
712 | &hcrx->ccid3hcrx_tstamp_last_feedback); | 711 | &hcrx->ccid3hcrx_tstamp_last_feedback); |
712 | DCCP_BUG_ON(delta < 0); | ||
713 | hcrx->ccid3hcrx_x_recv = | 713 | hcrx->ccid3hcrx_x_recv = |
714 | scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta); | 714 | scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta); |
715 | } | ||
716 | break; | 715 | break; |
717 | case TFRC_RSTATE_TERM: | 716 | case TFRC_RSTATE_TERM: |
718 | DCCP_BUG("Illegal %s state TERM, sk=%p", dccp_role(sk), sk); | 717 | DCCP_BUG("Illegal %s state TERM, sk=%p", dccp_role(sk), sk); |
@@ -730,9 +729,10 @@ static void ccid3_hc_rx_send_feedback(struct sock *sk) | |||
730 | hcrx->ccid3hcrx_ccval_last_counter = packet->dccphrx_ccval; | 729 | hcrx->ccid3hcrx_ccval_last_counter = packet->dccphrx_ccval; |
731 | hcrx->ccid3hcrx_bytes_recv = 0; | 730 | hcrx->ccid3hcrx_bytes_recv = 0; |
732 | 731 | ||
733 | /* Convert to multiples of 10us */ | 732 | /* Elapsed time information [RFC 4340, 13.2] in units of 10 * usecs */ |
734 | hcrx->ccid3hcrx_elapsed_time = | 733 | delta = timeval_delta(&now, &packet->dccphrx_tstamp); |
735 | timeval_delta(&now, &packet->dccphrx_tstamp) / 10; | 734 | DCCP_BUG_ON(delta < 0); |
735 | hcrx->ccid3hcrx_elapsed_time = delta / 10; | ||
736 | 736 | ||
737 | if (hcrx->ccid3hcrx_p == 0) | 737 | if (hcrx->ccid3hcrx_p == 0) |
738 | hcrx->ccid3hcrx_pinv = ~0U; /* see RFC 4342, 8.5 */ | 738 | hcrx->ccid3hcrx_pinv = ~0U; /* see RFC 4342, 8.5 */ |
@@ -785,7 +785,8 @@ static u32 ccid3_hc_rx_calc_first_li(struct sock *sk) | |||
785 | { | 785 | { |
786 | struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk); | 786 | struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk); |
787 | struct dccp_rx_hist_entry *entry, *next, *tail = NULL; | 787 | struct dccp_rx_hist_entry *entry, *next, *tail = NULL; |
788 | u32 rtt, delta, x_recv, p; | 788 | u32 x_recv, p; |
789 | suseconds_t rtt, delta; | ||
789 | struct timeval tstamp = { 0, }; | 790 | struct timeval tstamp = { 0, }; |
790 | int interval = 0; | 791 | int interval = 0; |
791 | int win_count = 0; | 792 | int win_count = 0; |
@@ -830,8 +831,12 @@ found: | |||
830 | DCCP_CRIT("tail is null\n"); | 831 | DCCP_CRIT("tail is null\n"); |
831 | return ~0; | 832 | return ~0; |
832 | } | 833 | } |
833 | rtt = timeval_delta(&tstamp, &tail->dccphrx_tstamp) * 4 / interval; | 834 | |
834 | ccid3_pr_debug("%s, sk=%p, approximated RTT to %uus\n", | 835 | delta = timeval_delta(&tstamp, &tail->dccphrx_tstamp); |
836 | DCCP_BUG_ON(delta < 0); | ||
837 | |||
838 | rtt = delta * 4 / interval; | ||
839 | ccid3_pr_debug("%s, sk=%p, approximated RTT to %ldus\n", | ||
835 | dccp_role(sk), sk, rtt); | 840 | dccp_role(sk), sk, rtt); |
836 | 841 | ||
837 | /* | 842 | /* |
@@ -849,8 +854,9 @@ found: | |||
849 | 854 | ||
850 | dccp_timestamp(sk, &tstamp); | 855 | dccp_timestamp(sk, &tstamp); |
851 | delta = timeval_delta(&tstamp, &hcrx->ccid3hcrx_tstamp_last_feedback); | 856 | delta = timeval_delta(&tstamp, &hcrx->ccid3hcrx_tstamp_last_feedback); |
852 | x_recv = scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta); | 857 | DCCP_BUG_ON(delta <= 0); |
853 | 858 | ||
859 | x_recv = scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta); | ||
854 | if (x_recv == 0) { /* would also trigger divide-by-zero */ | 860 | if (x_recv == 0) { /* would also trigger divide-by-zero */ |
855 | DCCP_WARN("X_recv==0\n"); | 861 | DCCP_WARN("X_recv==0\n"); |
856 | if ((x_recv = hcrx->ccid3hcrx_x_recv) == 0) { | 862 | if ((x_recv = hcrx->ccid3hcrx_x_recv) == 0) { |
@@ -977,7 +983,8 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) | |||
977 | const struct dccp_options_received *opt_recv; | 983 | const struct dccp_options_received *opt_recv; |
978 | struct dccp_rx_hist_entry *packet; | 984 | struct dccp_rx_hist_entry *packet; |
979 | struct timeval now; | 985 | struct timeval now; |
980 | u32 p_prev, rtt_prev, r_sample, t_elapsed; | 986 | u32 p_prev, rtt_prev; |
987 | suseconds_t r_sample, t_elapsed; | ||
981 | int loss, payload_size; | 988 | int loss, payload_size; |
982 | 989 | ||
983 | BUG_ON(hcrx == NULL); | 990 | BUG_ON(hcrx == NULL); |
@@ -997,8 +1004,9 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) | |||
997 | r_sample = timeval_usecs(&now); | 1004 | r_sample = timeval_usecs(&now); |
998 | t_elapsed = opt_recv->dccpor_elapsed_time * 10; | 1005 | t_elapsed = opt_recv->dccpor_elapsed_time * 10; |
999 | 1006 | ||
1007 | DCCP_BUG_ON(r_sample < 0); | ||
1000 | if (unlikely(r_sample <= t_elapsed)) | 1008 | if (unlikely(r_sample <= t_elapsed)) |
1001 | DCCP_WARN("r_sample=%uus, t_elapsed=%uus\n", | 1009 | DCCP_WARN("r_sample=%ldus, t_elapsed=%ldus\n", |
1002 | r_sample, t_elapsed); | 1010 | r_sample, t_elapsed); |
1003 | else | 1011 | else |
1004 | r_sample -= t_elapsed; | 1012 | r_sample -= t_elapsed; |
@@ -1051,8 +1059,8 @@ static void ccid3_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb) | |||
1051 | break; | 1059 | break; |
1052 | 1060 | ||
1053 | dccp_timestamp(sk, &now); | 1061 | dccp_timestamp(sk, &now); |
1054 | if (timeval_delta(&now, &hcrx->ccid3hcrx_tstamp_last_ack) >= | 1062 | if (timeval_delta(&now, &hcrx->ccid3hcrx_tstamp_last_ack) - |
1055 | hcrx->ccid3hcrx_rtt) { | 1063 | (suseconds_t)hcrx->ccid3hcrx_rtt >= 0) { |
1056 | hcrx->ccid3hcrx_tstamp_last_ack = now; | 1064 | hcrx->ccid3hcrx_tstamp_last_ack = now; |
1057 | ccid3_hc_rx_send_feedback(sk); | 1065 | ccid3_hc_rx_send_feedback(sk); |
1058 | } | 1066 | } |
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h index 44829d636c93..a0900bf98e6b 100644 --- a/net/dccp/dccp.h +++ b/net/dccp/dccp.h | |||
@@ -432,6 +432,7 @@ static inline void timeval_sub_usecs(struct timeval *tv, | |||
432 | tv->tv_sec--; | 432 | tv->tv_sec--; |
433 | tv->tv_usec += USEC_PER_SEC; | 433 | tv->tv_usec += USEC_PER_SEC; |
434 | } | 434 | } |
435 | DCCP_BUG_ON(tv->tv_sec < 0); | ||
435 | } | 436 | } |
436 | 437 | ||
437 | #ifdef CONFIG_SYSCTL | 438 | #ifdef CONFIG_SYSCTL |