From: Gerrit Renker <gerrit@erg.abdn.ac.uk>
To: dccp@vger.kernel.org
Subject: [PATCH 7/7]: Debug timeval operations
Date: Tue, 05 Dec 2006 20:38:48 +0000 [thread overview]
Message-ID: <200612052038.48522@strip-the-willow> (raw)
[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>
---
net/dccp/ccids/ccid3.c | 56 ++++++++++++++++++++++++++++---------------------
net/dccp/dccp.h | 1
2 files changed, 33 insertions(+), 24 deletions(-)
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -434,6 +434,7 @@ static inline void timeval_sub_usecs(str
tv->tv_sec--;
tv->tv_usec += USEC_PER_SEC;
}
+ DCCP_BUG_ON(tv->tv_sec < 0);
}
#ifdef CONFIG_SYSCTL
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -128,8 +128,8 @@ static void ccid3_hc_tx_update_x(struct
hctx->ccid3hctx_x = max_t(u64, hctx->ccid3hctx_x,
(hctx->ccid3hctx_s << 6)/TFRC_T_MBI);
- } else if (timeval_delta(now, &hctx->ccid3hctx_t_ld) >- hctx->ccid3hctx_rtt) {
+ } else if (timeval_delta(now, &hctx->ccid3hctx_t_ld) -
+ (suseconds_t)hctx->ccid3hctx_rtt >= 0 ) {
hctx->ccid3hctx_x = max(2 * min(hctx->ccid3hctx_x,
hctx->ccid3hctx_x_recv),
@@ -271,7 +271,7 @@ static int ccid3_hc_tx_send_packet(struc
struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);
struct dccp_tx_hist_entry *new_packet;
struct timeval now;
- long delay;
+ suseconds_t delay;
BUG_ON(hctx = NULL);
@@ -331,7 +331,7 @@ static int ccid3_hc_tx_send_packet(struc
* else
* // send the packet in (t_nom - t_now) milliseconds.
*/
- if (delay - (long)hctx->ccid3hctx_delta >= 0)
+ if (delay - (suseconds_t)hctx->ccid3hctx_delta >= 0)
return delay / 1000L;
break;
case TFRC_SSTATE_TERM:
@@ -353,7 +353,7 @@ static void ccid3_hc_tx_packet_sent(stru
const struct dccp_sock *dp = dccp_sk(sk);
struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);
struct timeval now;
- unsigned long quarter_rtt;
+ suseconds_t quarter_rtt;
struct dccp_tx_hist_entry *packet;
BUG_ON(hctx = NULL);
@@ -450,10 +450,8 @@ static void ccid3_hc_tx_packet_recv(stru
r_sample = timeval_delta(&now, &packet->dccphtx_tstamp);
t_elapsed = dp->dccps_options_received.dccpor_elapsed_time * 10;
- if (unlikely(r_sample <= 0)) {
- DCCP_WARN("WARNING: R_sample (%ld) <= 0!\n", r_sample);
- r_sample = 0;
- } else if (unlikely(r_sample <= t_elapsed))
+ DCCP_BUG_ON(r_sample < 0);
+ if (unlikely(r_sample <= t_elapsed))
DCCP_WARN("WARNING: r_sample=%ldus <= t_elapsed=%ldus\n",
r_sample, t_elapsed);
else
@@ -698,6 +696,7 @@ static void ccid3_hc_rx_send_feedback(st
struct dccp_sock *dp = dccp_sk(sk);
struct dccp_rx_hist_entry *packet;
struct timeval now;
+ suseconds_t delta;
ccid3_pr_debug("%s, sk=%p\n", dccp_role(sk), sk);
@@ -707,12 +706,12 @@ static void ccid3_hc_rx_send_feedback(st
case TFRC_RSTATE_NO_DATA:
hcrx->ccid3hcrx_x_recv = 0;
break;
- case TFRC_RSTATE_DATA: {
- const u32 delta = timeval_delta(&now,
- &hcrx->ccid3hcrx_tstamp_last_feedback);
+ case TFRC_RSTATE_DATA:
+ delta = timeval_delta(&now,
+ &hcrx->ccid3hcrx_tstamp_last_feedback);
+ DCCP_BUG_ON(delta < 0);
hcrx->ccid3hcrx_x_recv scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta);
- }
break;
case TFRC_RSTATE_TERM:
DCCP_BUG("Illegal %s state TERM, sk=%p", dccp_role(sk), sk);
@@ -730,9 +729,10 @@ static void ccid3_hc_rx_send_feedback(st
hcrx->ccid3hcrx_ccval_last_counter = packet->dccphrx_ccval;
hcrx->ccid3hcrx_bytes_recv = 0;
- /* Convert to multiples of 10us */
- hcrx->ccid3hcrx_elapsed_time - timeval_delta(&now, &packet->dccphrx_tstamp) / 10;
+ /* Elapsed time information [RFC 4340, 13.2] in units of 10 * usecs */
+ delta = timeval_delta(&now, &packet->dccphrx_tstamp);
+ DCCP_BUG_ON(delta < 0);
+ hcrx->ccid3hcrx_elapsed_time = delta / 10;
if (hcrx->ccid3hcrx_p = 0)
hcrx->ccid3hcrx_pinv = ~0U; /* see RFC 4342, 8.5 */
@@ -785,7 +785,8 @@ static u32 ccid3_hc_rx_calc_first_li(str
{
struct ccid3_hc_rx_sock *hcrx = ccid3_hc_rx_sk(sk);
struct dccp_rx_hist_entry *entry, *next, *tail = NULL;
- u32 rtt, delta, x_recv, p;
+ u32 x_recv, p;
+ suseconds_t rtt, delta;
struct timeval tstamp = { 0, };
int interval = 0;
int win_count = 0;
@@ -830,8 +831,12 @@ found:
DCCP_CRIT("tail is null\n");
return ~0;
}
- rtt = timeval_delta(&tstamp, &tail->dccphrx_tstamp) * 4 / interval;
- ccid3_pr_debug("%s, sk=%p, approximated RTT to %uus\n",
+
+ delta = timeval_delta(&tstamp, &tail->dccphrx_tstamp);
+ DCCP_BUG_ON(delta < 0);
+
+ rtt = delta * 4 / interval;
+ ccid3_pr_debug("%s, sk=%p, approximated RTT to %ldus\n",
dccp_role(sk), sk, rtt);
/*
@@ -849,8 +854,9 @@ found:
dccp_timestamp(sk, &tstamp);
delta = timeval_delta(&tstamp, &hcrx->ccid3hcrx_tstamp_last_feedback);
- x_recv = scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta);
+ DCCP_BUG_ON(delta <= 0);
+ x_recv = scaled_div32(hcrx->ccid3hcrx_bytes_recv, delta);
if (x_recv = 0) { /* would also trigger divide-by-zero */
DCCP_WARN("X_recv=0\n");
if((x_recv = hcrx->ccid3hcrx_x_recv) = 0) {
@@ -977,7 +983,8 @@ static void ccid3_hc_rx_packet_recv(stru
const struct dccp_options_received *opt_recv;
struct dccp_rx_hist_entry *packet;
struct timeval now;
- u32 p_prev, rtt_prev, r_sample, t_elapsed;
+ u32 p_prev, rtt_prev;
+ suseconds_t r_sample, t_elapsed;
int loss, payload_size;
BUG_ON(hcrx = NULL);
@@ -997,8 +1004,9 @@ static void ccid3_hc_rx_packet_recv(stru
r_sample = timeval_usecs(&now);
t_elapsed = opt_recv->dccpor_elapsed_time * 10;
+ DCCP_BUG_ON(r_sample < 0);
if (unlikely(r_sample <= t_elapsed))
- DCCP_WARN("r_sample=%uus, t_elapsed=%uus\n",
+ DCCP_WARN("r_sample=%ldus, t_elapsed=%ldus\n",
r_sample, t_elapsed);
else
r_sample -= t_elapsed;
@@ -1051,8 +1059,8 @@ static void ccid3_hc_rx_packet_recv(stru
break;
dccp_timestamp(sk, &now);
- if (timeval_delta(&now, &hcrx->ccid3hcrx_tstamp_last_ack) >- hcrx->ccid3hcrx_rtt) {
+ if (timeval_delta(&now, &hcrx->ccid3hcrx_tstamp_last_ack) -
+ (suseconds_t)hcrx->ccid3hcrx_rtt >= 0) {
hcrx->ccid3hcrx_tstamp_last_ack = now;
ccid3_hc_rx_send_feedback(sk);
}
next reply other threads:[~2006-12-05 20:38 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-12-05 20:38 Gerrit Renker [this message]
2006-12-05 20:52 ` [PATCH 7/7]: Debug timeval operations Ian McDonald
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200612052038.48522@strip-the-willow \
--to=gerrit@erg.abdn.ac.uk \
--cc=dccp@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.