From owner-freebsd-net@FreeBSD.ORG Mon Jun 22 20:07:58 2015 Return-Path: Delivered-To: freebsd-net@nevdull.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 2DAF4472 for ; Mon, 22 Jun 2015 20:07:58 +0000 (UTC) (envelope-from hiren@strugglingcoder.info) Received: from mail.strugglingcoder.info (strugglingcoder.info [65.19.130.35]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 0DA11B35 for ; Mon, 22 Jun 2015 20:07:57 +0000 (UTC) (envelope-from hiren@strugglingcoder.info) Received: from localhost (unknown [10.1.1.3]) (Authenticated sender: hiren@strugglingcoder.info) by mail.strugglingcoder.info (Postfix) with ESMTPSA id BD216D185A; Mon, 22 Jun 2015 13:07:56 -0700 (PDT) Date: Mon, 22 Jun 2015 13:07:56 -0700 From: hiren panchasara To: Jean-Francois HREN Cc: freebsd-net@freebsd.org, Damien DEVILLE , Fabien Thomas Subject: Re: Sequence number handling issue with TCP data and FIN flag with a transient error Message-ID: <20150622200756.GP37728@strugglingcoder.info> References: <1180135344.2814172.1434546351593.JavaMail.zimbra@stormshield.eu> <1176517609.2815392.1434546629748.JavaMail.zimbra@stormshield.eu> <20150617183734.GA53336@strugglingcoder.info> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="o/5eNASeIIpuMggS" Content-Disposition: inline In-Reply-To: <20150617183734.GA53336@strugglingcoder.info> User-Agent: Mutt/1.5.23 (2014-03-12) X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jun 2015 20:07:58 -0000 --o/5eNASeIIpuMggS Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 06/17/15 at 11:37P, hiren panchasara wrote: > On 06/17/15 at 03:10P, Jean-Francois HREN wrote: > > Hello, while investigating a freeze on a modified FreeBSD 9.3 I stumble= d upon > > a potential bug in netinet/tcp_output.c > >=20 > > If an error occurs while processing a TCP segment with some data and th= e FIN flag, > > the back out of the sequence number advance does not take into account > > the increase by 1 due to the FIN flag > > (see https://svnweb.freebsd.org/base/head/sys/netinet/tcp_output.c?view= =3Dmarkup#l1360 > > and https://svnweb.freebsd.org/base/head/sys/netinet/tcp_output.c?view= =3Dmarkup#l1439 ). > >=20 > > In the case of a transient error, this leads to a retransmitted TCP seg= ment with > > a shifted by 1 sequence number and a missing first byte in the TCP payl= oad. > >=20 > > In FreeBSD 9.3, it happens only when an error occurs in netinet/ip_outp= ut.c::ip_output() > > or netinet6/ip6_output::ip6_output() but in head, R249372 > > ( https://svnweb.freebsd.org/base?view=3Drevision&revision=3D249372 ) n= ow allows > > the same behaviour if an ENOBUFS error occurs in netinet/tcp_output.c >=20 > Your analysis looks correct to me. > >=20 > > Tentative solutions would be either to remove the back out of the seque= nce > > number advance completely and to treat transient error cases like real = lost > > packets > >=20 > > --- netinet/tcp_output.c > > +++ netinet/tcp_output.c > > @@ -1435,8 +1435,7 @@ > > tp->sackhint.sack_bytes_rexmit -=3D len; > > KASSERT(tp->sackhint.sack_bytes_rexmit >=3D 0, > > ("sackhint bytes rtx >=3D 0")); > > - } else > > - tp->snd_nxt -=3D len; > > + } > > } > > SOCKBUF_UNLOCK_ASSERT(&so->so_snd); /* Check gotos. */ > > switch (error) { > >=20 > > or to decrease the sequence number advance by 1 if a FIN flag was sent. > >=20 > > --- netinet/tcp_output.c > > +++ netinet/tcp_output.c > > @@ -1435,8 +1435,11 @@ > > tp->sackhint.sack_bytes_rexmit -=3D len; > > KASSERT(tp->sackhint.sack_bytes_rexmit >=3D 0, > > ("sackhint bytes rtx >=3D 0")); > > - } else > > + } else { > > tp->snd_nxt -=3D len; > > + if (flags & TH_FIN) > > + tp->snd_nxt--; > > + } > > } > > SOCKBUF_UNLOCK_ASSERT(&so->so_snd); /* Check gotos. */ > > switch (error) { >=20 > I like the second approach better. Does anyone else have any opinion on this? We should commit this to -head soon to get it in for 10.2 time-frame. Cheers, Hiren --o/5eNASeIIpuMggS Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iQF8BAEBCgBmBQJViGsbXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNEUyMEZBMUQ4Nzg4RjNGMTdFNjZGMDI4 QjkyNTBFMTU2M0VERkU1AAoJEIuSUOFWPt/loNwH/R9nEyiVTcdULkXL3MgmNDdQ DA3roiWDXNrMYN93Q4iAaGluHL0QsGW8zAYOYmpftJO2mDeK6wP1GrocYsqGlY0D 1k9IZFQEcW3tWU5W648/OuhTXyr/4HgQXS0w2omGeSUJhgnqIwym2rLuEfg3iL8w qo+SGRUR26OKb6hF5Cp2GJDCCxtw7iAGbEZ866Sih5gODA80dpn0aOv1sHibf0+q qGL6XgqXPOZp3rJ+ms/Ra0ZV2HLwRT7ltPP4sY/oK3w9SHbgPgYN7YEwqX4ZdWtQ jEzHL94uS/VoJsovg1hh4A4irB08D7lUvNcnXpxVm15k20BcfkFoR6d+k1cjH80= =0nYd -----END PGP SIGNATURE----- --o/5eNASeIIpuMggS--