From owner-freebsd-net@FreeBSD.ORG Fri Aug 1 14:19:02 2014 Return-Path: Delivered-To: freebsd-net@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 9664A1F0; Fri, 1 Aug 2014 14:19:02 +0000 (UTC) Received: from mail-wi0-x230.google.com (mail-wi0-x230.google.com [IPv6:2a00:1450:400c:c05::230]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id E7515263B; Fri, 1 Aug 2014 14:19:01 +0000 (UTC) Received: by mail-wi0-f176.google.com with SMTP id bs8so1461566wib.3 for ; Fri, 01 Aug 2014 07:18:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:hackerspace:user-agent; bh=/Bpx52DghS9IO4yMA1g+4+Uh+5FASbiPYW8/7XTA93M=; b=bBY2CQ0t3hzadd6MHO3MdojqrHUcNMgTjUBK1rdzjwMlNMPVeJfZ/dyG3kdnqsAiQh 0/HomaBrOnC096jTQATDHyAn8COuNQn8QMqCdOtA4VyTHjFoL+lgQyyPa5vKJkkMpLhO OqGzy5zDlxVVgM7zbDF6wWph0rYqFPscGVBxpGwGcZFcT2mx/kWilL/LJzHU8sQp3EU4 zS3uM6fhEWkwb6Og5+V1D2DRObQK/DWf+fDJcrGKWqQbuW3s7HEwyzCyvpat/s5awYPG K/Y+mivQgQU70w0ROocEOHMDCjXrkdOnlkXY8hoJLwPokHsDRi6Dk/Xx5tv3H5BBI+C4 6CPQ== X-Received: by 10.180.102.98 with SMTP id fn2mr7269772wib.39.1406902738317; Fri, 01 Aug 2014 07:18:58 -0700 (PDT) Received: from gmail.com ([2001:630:241:204:d42:3911:638a:37e1]) by mx.google.com with ESMTPSA id fb8sm9123404wib.15.2014.08.01.07.18.57 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 01 Aug 2014 07:18:57 -0700 (PDT) Sender: Tom Jones Date: Fri, 1 Aug 2014 15:19:21 +0100 From: Tom Jones To: Adrian Chadd Subject: Re: [PATCH] Implementation of draft-ietf-tcpm-newcwv-06 Message-ID: <20140801141920.GC75551@gmail.com> References: <20140630170453.GA21404@gmail.com> <20140630205359.GA2221@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="M9NhX3UHpAaciwkO" Content-Disposition: inline In-Reply-To: <20140630205359.GA2221@gmail.com> Hackerspace: 57North Hacklab User-Agent: Mutt/1.5.23 (2014-03-12) Cc: freebsd-net@freebsd.org X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 01 Aug 2014 14:19:02 -0000 --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, Jun 30, 2014 at 09:53:59PM +0100, Tom Jones wrote: > On Mon, Jun 30, 2014 at 12:29:44PM -0700, Adrian Chadd wrote: > > Hi! > > > > Cool! Would you mind throwing it into a bugzilla ticket so it's not > > lost and it can be assigned for some review? > > > > http://bugs.freebsd.org/submit/ > > Done. > https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=191520 > > > Is there any benefit in hanging this state off of it as a pointer to > > some other struct, rather than having it in the tcpcb? Is it always > > going to be used? > > The draft recommends newcwv be used as a default, so there is a chance it will > always be around. What would the impact to performance be by moving the state > out into a struct? I have updated the patch to move the new variable introduced with newcwv into a struct within the tcpcb. -- Tom | I don't see how we are going to build the dystopian megapoli @adventureloop | we were promised in 80s and early 90s cyberpunk fiction if adventurist.me | you guys keep complaining. :wq --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="newcwv-01082014.patch" Index: sys/conf/files =================================================================== --- sys/conf/files (revision 269231) +++ sys/conf/files (working copy) @@ -3383,6 +3383,7 @@ netinet/tcp_sack.c optional inet | inet6 netinet/tcp_subr.c optional inet | inet6 netinet/tcp_syncache.c optional inet | inet6 +netinet/tcp_newcwv.c optional inet | inet6 netinet/tcp_timer.c optional inet | inet6 netinet/tcp_timewait.c optional inet | inet6 netinet/tcp_usrreq.c optional inet | inet6 Index: sys/netinet/tcp_input.c =================================================================== --- sys/netinet/tcp_input.c (revision 269231) +++ sys/netinet/tcp_input.c (working copy) @@ -105,6 +105,7 @@ #include #include #include +#include #ifdef TCPDEBUG #include #endif /* TCPDEBUG */ @@ -174,6 +175,11 @@ &VNET_NAME(tcp_abc_l_var), 2, "Cap the max cwnd increment during slow-start to this number of segments"); +VNET_DEFINE(int, tcp_do_newcwv) = 0; +SYSCTL_VNET_INT(_net_inet_tcp, OID_AUTO, newcwv, CTLFLAG_RW, + &VNET_NAME(tcp_do_newcwv), 0, + "Enable draft-ietf-tcpm-newcwv-06 (New Congestion Window Validation)"); + static SYSCTL_NODE(_net_inet_tcp, OID_AUTO, ecn, CTLFLAG_RW, 0, "TCP ECN"); VNET_DEFINE(int, tcp_do_ecn) = 0; @@ -285,9 +291,12 @@ INP_WLOCK_ASSERT(tp->t_inpcb); tp->ccv->bytes_this_ack = BYTES_THIS_ACK(tp, th); - if (tp->snd_cwnd <= tp->snd_wnd) + /* draft-ietf-tcpm-newcwv relaxes conditions for growing cwnd */ + if (tp->snd_cwnd <= tp->snd_wnd || + (V_tcp_do_newcwv && tp->newcwv.pipeack >= (tp->snd_cwnd >> 1)) ) { tp->ccv->flags |= CCF_CWND_LIMITED; - else + tp->newcwv.cwnd_valid_ts = ticks; + } else tp->ccv->flags &= ~CCF_CWND_LIMITED; if (type == CC_ACK) { @@ -309,6 +318,12 @@ tp->ccv->curack = th->th_ack; CC_ALGO(tp)->ack_received(tp->ccv, type); } + + /* + * update draft-ietf-newcwv-06 pipeack + */ + if(V_tcp_do_newcwv && !IN_FASTRECOVERY(tp->t_flags)) + tcp_newcwv_update_pipeack(tp); } static void inline @@ -378,6 +393,12 @@ tp->snd_cwnd = 4 * tp->t_maxseg; } + /* + * Initialise NewCWV state + */ + tp->newcwv.init_cwnd = tp->snd_cwnd; + tcp_newcwv_reset(tp); + if (CC_ALGO(tp)->conn_init != NULL) CC_ALGO(tp)->conn_init(tp->ccv); } @@ -426,6 +447,11 @@ tp->t_badrxtwin = 0; break; } + + if (V_tcp_do_newcwv && + (type == CC_NDUPACK || type == CC_ECN) && + tp->newcwv.pipeack <= (tp->snd_cwnd >> 1) ) + tcp_newcwv_enter_recovery(tp); if (CC_ALGO(tp)->cong_signal != NULL) { if (th != NULL) @@ -447,6 +473,13 @@ } /* XXXLAS: EXIT_RECOVERY ? */ tp->t_bytes_acked = 0; + + if(V_tcp_do_newcwv) { + if(tp->newcwv.loss_flight_size) + tcp_newcwv_end_recovery(tp); + tcp_newcwv_reset(tp); + } + tp->newcwv.loss_flight_size = 0; } #ifdef TCP_SIGNATURE Index: sys/netinet/tcp_output.c =================================================================== --- sys/netinet/tcp_output.c (revision 269231) +++ sys/netinet/tcp_output.c (working copy) @@ -74,6 +74,7 @@ #include #include #include +#include #ifdef TCPDEBUG #include #endif @@ -691,6 +692,10 @@ #endif hdrlen = sizeof (struct tcpiphdr); + /* Trigger the newcwv timer */ + if(V_tcp_do_newcwv) + tcp_newcwv_datalim_closedown(tp); + /* * Compute options for segment. * We only have to care about SYN and established connection Index: sys/netinet/tcp_subr.c =================================================================== --- sys/netinet/tcp_subr.c (revision 269231) +++ sys/netinet/tcp_subr.c (working copy) @@ -800,6 +800,7 @@ tp->t_flags = (TF_REQ_SCALE|TF_REQ_TSTMP); if (V_tcp_do_sack) tp->t_flags |= TF_SACK_PERMIT; + TAILQ_INIT(&tp->snd_holes); tp->t_inpcb = inp; /* XXX */ /* Index: sys/netinet/tcp_var.h =================================================================== --- sys/netinet/tcp_var.h (revision 269231) +++ sys/netinet/tcp_var.h (working copy) @@ -172,6 +172,22 @@ int t_sndzerowin; /* zero-window updates sent */ u_int t_badrxtwin; /* window for retransmit recovery */ u_char snd_limited; /* segments limited transmitted */ +/* NewCWV releated state */ + struct { + u_int32_t pipeack; + u_int32_t psp; /* pipeack sampling period */ + + u_int32_t head; + u_int32_t psample[4]; /* pipe ack samples */ + u_int32_t time_stamp[4]; /* time stamp samples */ + u_int32_t prev_snd_una; /* previous snd_una in this sampe */ + u_int32_t prev_snd_nxt; /* previous snd_nxt in this sampe */ + + u_int32_t loss_flight_size; /* flightsize at loss detection */ + u_int32_t prior_retrans; /* Retransmission before going into FR */ + u_int32_t cwnd_valid_ts; /*last time cwnd was found valid */ + u_int32_t init_cwnd; /* The inital cwnd */ + } newcwv; /* SACK related state */ int snd_numholes; /* number of holes seen by sender */ TAILQ_HEAD(sackhole_head, sackhole) snd_holes; @@ -605,6 +621,7 @@ VNET_DECLARE(int, path_mtu_discovery); VNET_DECLARE(int, tcp_do_rfc3465); VNET_DECLARE(int, tcp_abc_l_var); +VNET_DECLARE(int, tcp_do_newcwv); #define V_tcb VNET(tcb) #define V_tcbinfo VNET(tcbinfo) #define V_tcp_mssdflt VNET(tcp_mssdflt) @@ -617,6 +634,7 @@ #define V_path_mtu_discovery VNET(path_mtu_discovery) #define V_tcp_do_rfc3465 VNET(tcp_do_rfc3465) #define V_tcp_abc_l_var VNET(tcp_abc_l_var) +#define V_tcp_do_newcwv VNET(tcp_do_newcwv) VNET_DECLARE(int, tcp_do_sack); /* SACK enabled/disabled */ VNET_DECLARE(int, tcp_sc_rst_sock_fail); /* RST on sock alloc failure */ --M9NhX3UHpAaciwkO--