diff mbox

[net-next] tcp: introduce a per-route knob for quick ack

Message ID 1371030955-6740-1-git-send-email-amwang@redhat.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Amerigo Wang June 12, 2013, 9:55 a.m. UTC
From: Cong Wang <amwang@redhat.com>

According to previous discussion [1], it seems there is no
reasonable heuristics.

Similar to TCP_QUICK_ACK option, but for people who can't
modify the source code and still wants to control
TCP delayed ACK behavior. As people suggested, introduce
a per-route knob for it.

1. http://marc.info/?t=136507071100004&r=1&w=2

Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Rick Jones <rick.jones2@hp.com>
Cc: Stephen Hemminger <stephen@networkplumber.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Thomas Graf <tgraf@suug.ch>
CC: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Cong Wang <amwang@redhat.com>

---
 include/uapi/linux/rtnetlink.h |    2 ++
 net/ipv4/tcp_input.c           |    5 ++++-
 net/ipv4/tcp_output.c          |    6 ++++--
 3 files changed, 10 insertions(+), 3 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Miller June 12, 2013, 10:02 a.m. UTC | #1
From: Cong Wang <amwang@redhat.com>
Date: Wed, 12 Jun 2013 17:55:54 +0800

> According to previous discussion [1], it seems there is no
> reasonable heuristics.
 ...
> 1. http://marc.info/?t=136507071100004&r=1&w=2

You can't just explain the entire essence of why your patch is
necessary by referring to a URL that might disappear some day.

I'm not even going to look there to review your patch, that
should never be necessary.  A patch and it's commit message
should be as self contained as possible.

It is required that you explain, in full detail, and covering
all of the perspectives, the whole issue of ACK behavior in
the commit message.

Otherwise you're making things more difficult for the person who has
to look at this commit in the future.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Amerigo Wang June 13, 2013, 1:56 a.m. UTC | #2
On Wed, 2013-06-12 at 03:02 -0700, David Miller wrote:
> It is required that you explain, in full detail, and covering
> all of the perspectives, the whole issue of ACK behavior in
> the commit message.
> 
> Otherwise you're making things more difficult for the person who has
> to look at this commit in the future. 

Makes sense! I will give a summary of this in the changelog and resend
it.

Thanks.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 7a2144e..eb0f1a5 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -386,6 +386,8 @@  enum {
 #define RTAX_RTO_MIN RTAX_RTO_MIN
 	RTAX_INITRWND,
 #define RTAX_INITRWND RTAX_INITRWND
+	RTAX_QUICKACK,
+#define RTAX_QUICKACK RTAX_QUICKACK
 	__RTAX_MAX
 };
 
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 907311c..51ed9b7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3726,6 +3726,7 @@  void tcp_reset(struct sock *sk)
 static void tcp_fin(struct sock *sk)
 {
 	struct tcp_sock *tp = tcp_sk(sk);
+	const struct dst_entry *dst;
 
 	inet_csk_schedule_ack(sk);
 
@@ -3737,7 +3738,9 @@  static void tcp_fin(struct sock *sk)
 	case TCP_ESTABLISHED:
 		/* Move to CLOSE_WAIT */
 		tcp_set_state(sk, TCP_CLOSE_WAIT);
-		inet_csk(sk)->icsk_ack.pingpong = 1;
+		dst = __sk_dst_get(sk);
+		if (!dst_metric(dst, RTAX_QUICKACK))
+			inet_csk(sk)->icsk_ack.pingpong = 1;
 		break;
 
 	case TCP_CLOSE_WAIT:
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index ec335fa..f840b92 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -160,6 +160,7 @@  static void tcp_event_data_sent(struct tcp_sock *tp,
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	const u32 now = tcp_time_stamp;
+	const struct dst_entry *dst = __sk_dst_get(sk);
 
 	if (sysctl_tcp_slow_start_after_idle &&
 	    (!tp->packets_out && (s32)(now - tp->lsndtime) > icsk->icsk_rto))
@@ -170,8 +171,9 @@  static void tcp_event_data_sent(struct tcp_sock *tp,
 	/* If it is a reply for ato after last received
 	 * packet, enter pingpong mode.
 	 */
-	if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato)
-		icsk->icsk_ack.pingpong = 1;
+	if ((u32)(now - icsk->icsk_ack.lrcvtime) < icsk->icsk_ack.ato &&
+	    !dst_metric(dst, RTAX_QUICKACK))
+			icsk->icsk_ack.pingpong = 1;
 }
 
 /* Account for an ACK we sent. */