tcp: introduce a per-route knob for quick ack
Subsystems (3)
PATCH 1/1 tcp: introduce a per-route knob for quick ack
From: Cong Wang <[email protected]>
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
--- 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(-)
include/uapi/linux/rtnetlink.h +2 -0
@@ -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 };
net/ipv4/tcp_input.c +4 -1
@@ -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 :
net/ipv4/tcp_output.c +4 -2
@@ -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. */
PATCH 1/1 add quickack option to ip route
From: Cong Wang <[email protected]>
---
include/linux/rtnetlink.h +2 -0
@@ -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 };
ip/iproute.c +11 -0
@@ -52,6 +52,7 @@ static const char *mx_names[RTAX_MAX+1] = { [RTAX_FEATURES] = "features", [RTAX_RTO_MIN] = "rto_min", [RTAX_INITRWND] = "initrwnd", + [RTAX_QUICKACK] = "quickack", };static void usage (void ) __attribute__((noreturn));
@@ -79,6 +80,7 @@ static void usage(void)fprintf (stderr," [ window NUMBER] [ cwnd NUMBER ] [ initcwnd NUMBER ]\n" );fprintf (stderr," [ ssthresh NUMBER ] [ realms REALM ] [ src ADDRESS ]\n" );fprintf (stderr," [ rto_min TIME ] [ hoplimit NUMBER ] [ initrwnd NUMBER ]\n" ); +fprintf (stderr," [ quickack BOOL ]\n" );fprintf (stderr,"TYPE := [ unicast | local | broadcast | multicast | throw |\n" );fprintf (stderr," unreachable | prohibit | blackhole | nat ]\n" );fprintf (stderr,"TABLE_ID := [ local | main | default | all | NUMBER ]\n" );
@@ -86,6 +88,7 @@ static void usage(void)fprintf (stderr,"NHFLAGS := [ onlink | pervasive ]\n" );fprintf (stderr,"RTPROTO := [ kernel | boot | static | NUMBER ]\n" );fprintf (stderr,"TIME := NUMBER[s|ms]\n" ); +fprintf (stderr,"BOOL := [1|0]\n" );exit (-1 ); }
@@ -885,6 +888,14 @@ static int iproute_modify(int cmd, unsigned flags, int argc, char **argv)if (get_unsigned (&win, *argv,0 ))invarg ("\"initrwnd\" value is invalid\n" , *argv);rta_addattr32 (mxrta,sizeof (mxbuf),RTAX_INITRWND , win); + }else if (matches (*argv,"quickack" ) ==0 ) { +unsigned quickack; +NEXT_ARG (); +if (get_unsigned (&quickack, *argv,0 )) +invarg ("\"quickack\" value is invalid\n" , *argv); +if (quickack !=1 && quickack !=0 ) +invarg ("\"quickack\" value should be 0 or 1\n" , *argv); +rta_addattr32 (mxrta,sizeof (mxbuf),RTAX_QUICKACK , quickack); }else if (matches (*argv,"rttvar" ) ==0 ) {unsigned win;NEXT_ARG ();
man/man8/ip-route.8.in +6 -0
@@ -111,6 +111,8 @@ replace " } " .IR NUMBER " ] [ " .B initrwnd .IR NUMBER " ]" +.B quickack +.IR BOOL " ]" .ti -8 .IR TYPE " := [ "
@@ -401,6 +403,10 @@ Actual window size is this value multiplied by the MSS of the connection. The default value is zero, meaning to use Slow Start value. .TP +.BI quickack " BOOL " "(3.11+ only)" +Enable or disable quick ack for connections to this destination. + +.TP .BI advmss " NUMBER " "(2.3.15+ only)" the MSS ('Maximal Segment Size') to advertise to these destinations when establishing TCP connections. If it is not given,
David Miller
maintainerFrom: Cong Wang <[email protected]>
Date: Wed, 12 Jun 2013 17:55:54 +0800
Quoted 2 lines
According to previous discussion [1], it seems there is no reasonable heuristics.
...
Quoted 1 line
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.
Quoted 6 lines · David Miller
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.