Skip to contentSkip to conversation

RTT metrics scaling

RFC In review Stephen Hemminger 3 participants 6 messages Jun 20, 2008 – Jul 19, 2008

Subsystems (3)
Networking Network drivers TCP
lore
Share & tools
lore

Stephen Hemminger

author
#0 lore raw

RFC 1/1 RTT metrics scaling

Some of the metrics (RTT, RTTVAR and RTAX_RTO_MIN) are stored in kernel units (jiffies) and this leaks out through the netlink API to user space where the units for jiffies are unknown.

This patches changes the kernel to convert to/from milliseconds. This changes the ABI, but milliseconds seemed like the most natural unit for these parameters.

This patch is for comment only, just to show how it would be implemented. I will update it after testing more.

include/net/dst.h +12 -0
@@ -128,6 +128,18 @@ static inline u32 dst_mtu(const struct d
 	return mtu;
 }
 
+/* RTT metrics are stored in milliseconds for user ABI, but used as jiffies */
+static inline unsigned long dst_metric_rtt(const struct dst_entry *dst, int metric)
+{
+	return msecs_to_jiffies(dst_metric(dst, metric));
+}
+
+static inline void set_dst_metric_rtt(struct dst_entry *dst, int metric,
+				      unsigned long rtt)
+{
+	dst->metrics[metric-1] = jiffies_to_msecs(rtt);
+}
+
 static inline u32
 dst_allfrag(const struct dst_entry *dst)
 {
net/ipv4/tcp_input.c +14 -18
@@ -602,7 +602,7 @@ static u32 tcp_rto_min(struct sock *sk)
 	u32 rto_min = TCP_RTO_MIN;
 
 	if (dst && dst_metric_locked(dst, RTAX_RTO_MIN))
-		rto_min = dst_metric(dst, RTAX_RTO_MIN);
+		rto_min = dst_metric_rtt(dst, RTAX_RTO_MIN);
 	return rto_min;
 }
 

@@ -740,18 +740,15 @@ void tcp_update_metrics(struct sock *sk)
 			return;
 		}
 
-		m = dst_metric(dst, RTAX_RTT) - tp->srtt;
+		m = dst_metric_rtt(dst, RTAX_RTT) - tp->srtt;
 
 		/* If newly calculated rtt larger than stored one,
 		 * store new one. Otherwise, use EWMA. Remember,
 		 * rtt overestimation is always better than underestimation.
 		 */
-		if (!(dst_metric_locked(dst, RTAX_RTT))) {
-			if (m <= 0)
-				dst->metrics[RTAX_RTT - 1] = tp->srtt;
-			else
-				dst->metrics[RTAX_RTT - 1] -= (m >> 3);
-		}
+		if (!(dst_metric_locked(dst, RTAX_RTT)))
+			set_dst_metric_rtt(dst, RTAX_RTT,
+					   m <= 0 ? tp->srtt : (m >>3));
 
 		if (!(dst_metric_locked(dst, RTAX_RTTVAR))) {
 			if (m < 0)

@@ -762,11 +759,10 @@ void tcp_update_metrics(struct sock *sk)
 			if (m < tp->mdev)
 				m = tp->mdev;
 
-			if (m >= dst_metric(dst, RTAX_RTTVAR))
-				dst->metrics[RTAX_RTTVAR - 1] = m;
-			else
-				dst->metrics[RTAX_RTTVAR-1] -=
-					(dst_metric(dst, RTAX_RTTVAR) - m)>>2;
+			set_dst_metric(dst, RTAX_RTTVAR,
+				       (m >= dst_metric_rtt(dst, RTAX_RTTVAR))
+				       ? m
+				       : (dst_metric(dst, RTAX_RTTVAR) - m)>>2);
 		}
 
 		if (tp->snd_ssthresh >= 0xFFFF) {

@@ -897,7 +893,7 @@ static void tcp_init_metrics(struct sock
 	if (dst_metric(dst, RTAX_RTT) == 0)
 		goto reset;
 
-	if (!tp->srtt && dst_metric(dst, RTAX_RTT) < (TCP_TIMEOUT_INIT << 3))
+	if (!tp->srtt && dst_metric_rtt(dst, RTAX_RTT) < (TCP_TIMEOUT_INIT << 3))
 		goto reset;
 
 	/* Initial rtt is determined from SYN,SYN-ACK.

@@ -914,12 +910,12 @@ static void tcp_init_metrics(struct sock
 	 * to low value, and then abruptly stops to do it and starts to delay
 	 * ACKs, wait for troubles.
 	 */
-	if (dst_metric(dst, RTAX_RTT) > tp->srtt) {
-		tp->srtt = dst_metric(dst, RTAX_RTT);
+	if (dst_metric_rtt(dst, RTAX_RTT) > tp->srtt) {
+		tp->srtt = dst_metric_rtt(dst, RTAX_RTT);
 		tp->rtt_seq = tp->snd_nxt;
 	}
-	if (dst_metric(dst, RTAX_RTTVAR) > tp->mdev) {
-		tp->mdev = dst_metric(dst, RTAX_RTTVAR);
+	if (dst_metric_rtt(dst, RTAX_RTTVAR) > tp->mdev) {
+		tp->mdev = dst_metric_rtt(dst, RTAX_RTTVAR);
 		tp->mdev_max = tp->rttvar = max(tp->mdev, tcp_rto_min(sk));
 	}
 	tcp_set_rto(sk);
maintainer
#1 lore raw

From: Stephen Hemminger <[email protected]>
Date: Fri, 20 Jun 2008 16:22:34 -0700

Quoted 9 lines

Some of the metrics (RTT, RTTVAR and RTAX_RTO_MIN) are stored in kernel units (jiffies) and this leaks out through the netlink API to user space where the units for jiffies are unknown.

This patches changes the kernel to convert to/from milliseconds. This changes the ABI, but milliseconds seemed like the most natural unit for these parameters.

This patch is for comment only, just to show how it would be implemented. I will update it after testing more.

No real objection.

But aren't we approaching microsecond RTT's soon? :-) (yes, I know in TCP we min clamp at 200ms RTT, but that clamp we probably want to remove soon)

If you don't think that's worth worrying about, I'll apply this patch.

Thanks.

#2 lore raw
Quoted 10 lines · On Fri, 27 Jun 2008 19:50:13 -0700 (PDT) David Miller <[email protected]>…

From: Stephen Hemminger <[email protected]>
Date: Fri, 20 Jun 2008 16:22:34 -0700

Quoted 7 lines

Some of the metrics (RTT, RTTVAR and RTAX_RTO_MIN) are stored in kernel units (jiffies) and this leaks out through the netlink API to user space where the units for jiffies are unknown.

This patches changes the kernel to convert to/from milliseconds. This changes the ABI, but milliseconds seemed like the most natural unit for these parameters.

Quoted 8 lines
Quoted 2 lines

This patch is for comment only, just to show how it would be implemented. I will update it after testing more.

No real objection.

But aren't we approaching microsecond RTT's soon? :-) (yes, I know in TCP we min clamp at 200ms RTT, but that clamp we probably want to remove soon)

We could put in microseconds, but since it is computer off of jiffies (ie clock ticks), the resolution wouldn't be worth it.

Quoted 1 line

If you don't think that's worth worrying about, I'll apply this patch.

Hold off it needs more testing.

maintainer
#3 lore raw

From: Stephen Hemminger <[email protected]>
Date: Fri, 27 Jun 2008 22:30:07 -0700

Quoted 9 lines
Quoted 3 lines · On Fri, 27 Jun 2008 19:50:13 -0700 (PDT) David Miller <[email protected]>…

But aren't we approaching microsecond RTT's soon? :-) (yes, I know in TCP we min clamp at 200ms RTT, but that clamp we probably want to remove soon)

We could put in microseconds, but since it is computer off of jiffies (ie clock ticks), the resolution wouldn't be worth it.

Fair enough.

Quoted 4 lines
Quoted 2 lines

If you don't think that's worth worrying about, I'll apply this patch.

Hold off it needs more testing.

Ok.

author
#4 lore raw

PATCH 1/1 tcp: RTT metrics scaling

In reply to Stephen Hemminger

Some of the metrics (RTT, RTTVAR and RTAX_RTO_MIN) are stored in kernel units (jiffies) and this leaks out through the netlink API to user space where the units for jiffies are unknown.

This patches changes the kernel to convert to/from milliseconds. This changes the ABI, but milliseconds seemed like the most natural unit for these parameters. Values available via syscall in /proc/net/rt_cache and netlink will be in milliseconds.

Signed-off-by Stephen Hemminger <[email protected]>
---
 include/net/dst.h    |   14 +++++++++++++-
 net/ipv4/tcp_input.c |   31 ++++++++++++++++++-------------
 2 files changed, 31 insertions(+), 14 deletions(-)
include/net/dst.h +12 -0
@@ -128,6 +128,18 @@ static inline u32 dst_mtu(const struct d
 	return mtu;
 }
 
+/* RTT metrics are stored in milliseconds for user ABI, but used as jiffies */
+static inline unsigned long dst_metric_rtt(const struct dst_entry *dst, int metric)
+{
+	return msecs_to_jiffies(dst_metric(dst, metric));
+}
+
+static inline void set_dst_metric_rtt(struct dst_entry *dst, int metric,
+				      unsigned long rtt)
+{
+	dst->metrics[metric-1] = jiffies_to_msecs(rtt);
+}
+
 static inline u32
 dst_allfrag(const struct dst_entry *dst)
 {
net/ipv4/tcp_input.c +18 -13
@@ -602,7 +602,7 @@ static u32 tcp_rto_min(struct sock *sk)
 	u32 rto_min = TCP_RTO_MIN;
 
 	if (dst && dst_metric_locked(dst, RTAX_RTO_MIN))
-		rto_min = dst_metric(dst, RTAX_RTO_MIN);
+		rto_min = dst_metric_rtt(dst, RTAX_RTO_MIN);
 	return rto_min;
 }
 

@@ -729,6 +729,7 @@ void tcp_update_metrics(struct sock *sk)
 	if (dst && (dst->flags & DST_HOST)) {
 		const struct inet_connection_sock *icsk = inet_csk(sk);
 		int m;
+		unsigned long rtt;
 
 		if (icsk->icsk_backoff || !tp->srtt) {
 			/* This session failed to estimate rtt. Why?

@@ -740,7 +741,8 @@ void tcp_update_metrics(struct sock *sk)
 			return;
 		}
 
-		m = dst_metric(dst, RTAX_RTT) - tp->srtt;
+		rtt = dst_metric_rtt(dst, RTAX_RTT);
+		m = rtt - tp->srtt;
 
 		/* If newly calculated rtt larger than stored one,
 		 * store new one. Otherwise, use EWMA. Remember,

@@ -748,12 +750,13 @@ void tcp_update_metrics(struct sock *sk)
 		 */
 		if (!(dst_metric_locked(dst, RTAX_RTT))) {
 			if (m <= 0)
-				dst->metrics[RTAX_RTT - 1] = tp->srtt;
+				set_dst_metric_rtt(dst, RTAX_RTT, tp->srtt);
 			else
-				dst->metrics[RTAX_RTT - 1] -= (m >> 3);
+				set_dst_metric_rtt(dst, RTAX_RTT, rtt - (m >> 3));
 		}
 
 		if (!(dst_metric_locked(dst, RTAX_RTTVAR))) {
+			unsigned long var;
 			if (m < 0)
 				m = -m;
 

@@ -762,11 +765,13 @@ void tcp_update_metrics(struct sock *sk)
 			if (m < tp->mdev)
 				m = tp->mdev;
 
-			if (m >= dst_metric(dst, RTAX_RTTVAR))
-				dst->metrics[RTAX_RTTVAR - 1] = m;
+			var = dst_metric_rtt(dst, RTAX_RTTVAR);
+			if (m >= var)
+				var = m;
 			else
-				dst->metrics[RTAX_RTTVAR-1] -=
-					(dst_metric(dst, RTAX_RTTVAR) - m)>>2;
+				var -= (var - m) >> 2;
+
+			set_dst_metric_rtt(dst, RTAX_RTTVAR, var);
 		}
 
 		if (tp->snd_ssthresh >= 0xFFFF) {

@@ -897,7 +902,7 @@ static void tcp_init_metrics(struct sock
 	if (dst_metric(dst, RTAX_RTT) == 0)
 		goto reset;
 
-	if (!tp->srtt && dst_metric(dst, RTAX_RTT) < (TCP_TIMEOUT_INIT << 3))
+	if (!tp->srtt && dst_metric_rtt(dst, RTAX_RTT) < (TCP_TIMEOUT_INIT << 3))
 		goto reset;
 
 	/* Initial rtt is determined from SYN,SYN-ACK.

@@ -914,12 +919,12 @@ static void tcp_init_metrics(struct sock
 	 * to low value, and then abruptly stops to do it and starts to delay
 	 * ACKs, wait for troubles.
 	 */
-	if (dst_metric(dst, RTAX_RTT) > tp->srtt) {
-		tp->srtt = dst_metric(dst, RTAX_RTT);
+	if (dst_metric_rtt(dst, RTAX_RTT) > tp->srtt) {
+		tp->srtt = dst_metric_rtt(dst, RTAX_RTT);
 		tp->rtt_seq = tp->snd_nxt;
 	}
-	if (dst_metric(dst, RTAX_RTTVAR) > tp->mdev) {
-		tp->mdev = dst_metric(dst, RTAX_RTTVAR);
+	if (dst_metric_rtt(dst, RTAX_RTTVAR) > tp->mdev) {
+		tp->mdev = dst_metric_rtt(dst, RTAX_RTTVAR);
 		tp->mdev_max = tp->rttvar = max(tp->mdev, tcp_rto_min(sk));
 	}
 	tcp_set_rto(sk);
maintainer
#5 lore raw

Re: [PATCH] tcp: RTT metrics scaling

From: Stephen Hemminger <[email protected]>
Date: Fri, 18 Jul 2008 20:34:31 -0700

Quoted 9 lines

Some of the metrics (RTT, RTTVAR and RTAX_RTO_MIN) are stored in kernel units (jiffies) and this leaks out through the netlink API to user space where the units for jiffies are unknown.

This patches changes the kernel to convert to/from milliseconds. This changes the ABI, but milliseconds seemed like the most natural unit for these parameters. Values available via syscall in /proc/net/rt_cache and netlink will be in milliseconds.

Signed-off-by Stephen Hemminger <[email protected]>

Looks good, applied.