Skip to contentSkip to conversation

Make ccid3_hc_tx_update_x get a timestamp if needed

PATCH In review Arnaldo Carvalho de Melo 2 participants 2 messages Aug 19, 2007 – Aug 21, 2007 Networking

lore
Share & tools
lore

Arnaldo Carvalho de Melo

author
#0 lore raw

PATCH 2/15 Make ccid3_hc_tx_update_x get a timestamp if needed

The code was too complicated, if p > 0 in ccid3_hc_tx_no_feedback_timer the timestamp was being obtained to be passed to ccid3_hc_tx_update_x, where only if p > 0 the timestamp was needed, so just leave it to ccid3_hc_tx_update_x to obtain the timestamp if needed.

This will help in the upcoming changesets where we'll convert t_ld to ktime_t. We'll eventually try to reuse ktime_get_real() calls again.

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 net/dccp/ccids/ccid3.c |   30 ++++++++++++++++--------------
 1 files changed, 16 insertions(+), 14 deletions(-)
net/dccp/ccids/ccid3.c +15 -13
index e91c2b9..d0763ad 100644
@@ -128,7 +128,7 @@ static inline void ccid3_update_send_interval(struct ccid3_hc_tx_sock *hctx)
  *       throughout the code. Only X_calc is unscaled (in bytes/second).
  *
  */
-static void ccid3_hc_tx_update_x(struct sock *sk, struct timeval *now)
+static void ccid3_hc_tx_update_x(struct sock *sk)
 
 {
 	struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);

@@ -153,14 +153,20 @@ static void ccid3_hc_tx_update_x(struct sock *sk, struct timeval *now)
 					(((__u64)hctx->ccid3hctx_s) << 6) /
 								TFRC_T_MBI);
 
-	} else if (timeval_delta(now, &hctx->ccid3hctx_t_ld) -
-			(suseconds_t)hctx->ccid3hctx_rtt >= 0) {
+	} else {
+		struct timeval now;
 
-		hctx->ccid3hctx_x -			max(min(2 * hctx->ccid3hctx_x, min_rate),
-			    scaled_div(((__u64)hctx->ccid3hctx_s) << 6,
-				       hctx->ccid3hctx_rtt));
-		hctx->ccid3hctx_t_ld = *now;
+		dccp_timestamp(sk, &now);
+
+		if ((timeval_delta(&now, &hctx->ccid3hctx_t_ld) -
+		     (suseconds_t)hctx->ccid3hctx_rtt) >= 0) {
+
+			hctx->ccid3hctx_x +				max(min(2 * hctx->ccid3hctx_x, min_rate),
+				    scaled_div(((__u64)hctx->ccid3hctx_s) << 6,
+					       hctx->ccid3hctx_rtt));
+			hctx->ccid3hctx_t_ld = now;
+		}
 	}
 
 	if (hctx->ccid3hctx_x != old_x) {

@@ -214,7 +220,6 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long data)
 {
 	struct sock *sk = (struct sock *)data;
 	struct ccid3_hc_tx_sock *hctx = ccid3_hc_tx_sk(sk);
-	struct timeval now;
 	unsigned long t_nfb = USEC_PER_SEC / 5;
 
 	bh_lock_sock(sk);

@@ -265,15 +270,12 @@ static void ccid3_hc_tx_no_feedback_timer(unsigned long data)
 				max(hctx->ccid3hctx_x_recv / 2,
 				    (((__u64)hctx->ccid3hctx_s) << 6) /
 							      (2 * TFRC_T_MBI));
-
-			if (hctx->ccid3hctx_p = 0)
-				dccp_timestamp(sk, &now);
 		} else {
 			hctx->ccid3hctx_x_recv = hctx->ccid3hctx_x_calc;
 			hctx->ccid3hctx_x_recv <<= 4;
 		}
 		/* Now recalculate X [RFC 3448, 4.3, step (4)] */
-		ccid3_hc_tx_update_x(sk, &now);
+		ccid3_hc_tx_update_x(sk);
 		/*
 		 * Schedule no feedback timer to expire in
 		 * max(t_RTO, 2 * s/X)  =  max(t_RTO, 2 * t_ipi)

@@ -496,7 +498,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
 					tfrc_calc_x(hctx->ccid3hctx_s,
 						    hctx->ccid3hctx_rtt,
 						    hctx->ccid3hctx_p);
-			ccid3_hc_tx_update_x(sk, &now);
+			ccid3_hc_tx_update_x(sk);
 
 			ccid3_pr_debug("%s(%p), RTT=%uus (sample=%uus), s=%u, "
 				       "p=%u, X_calc=%u, X_recv=%u, X=%u\n",
Signature · 1 line
1.5.2.2
#1 lore raw
|  The code was too complicated, if p > 0 in ccid3_hc_tx_no_feedback_timer the
|  timestamp was being obtained to be passed to ccid3_hc_tx_update_x, where only
|  if p > 0 the timestamp was needed, so just leave it to ccid3_hc_tx_update_x to
|  obtain the timestamp if needed.

The `too complicated' is there for a reason, it served to reduce the number of timestamps per packet.

With your patch, the code is now back at taking two timestamps per each received packet:

* first a timestamp is taken for the RTT sample
* whenever p = 0 (normal condition without loss), the second timestamp is now taken in ccid3_hc_tx_update_x()

Since this code is in the receive path, it is executed for each received packet, i.e. two timestamps per each incoming packet.

The original code recycled the first timestamp; the timestamp in no_feedback_timer() (which is only executed in exception cases, when there is no feedback for over 4 RTTs) was taken only when absolutely necessary, if two `if' conditions held.

But this is not a big deal - I will send a patch shortly.