Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 07 Jan 2014 19:07:11 -0800
From:      Peter Wemm <peter@wemm.org>
To:        Mike Tancsa <mike@sentex.net>, freebsd-net@freebsd.org,  eadler@freebsd.org, rrs@freebsd.org
Subject:   Re: TCP question: Is this simultaneous close handling broken?
Message-ID:  <52CCC0DF.1020007@wemm.org>
In-Reply-To: <52CC903C.5090706@sentex.net>
References:  <52CB3AE9.3030107@wemm.org> <52CC5F2E.5030201@wemm.org> <52CC8246.7080609@wemm.org> <52CC903C.5090706@sentex.net>

next in thread | previous in thread | raw e-mail | index | archive | help
This is an OpenPGP/MIME signed message (RFC 4880 and 3156)
--lnlDPe6q6u8Qe90FEFXPpNMAbTlf7Tclu
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

On 1/7/14, 3:39 PM, Mike Tancsa wrote:
> On 1/7/2014 5:40 PM, Peter Wemm wrote:
>=20
>> The packet may be dropped without processing the FIN flag.
>=20
>> MFC after:	never
>=20
> Hi,
> 	Are there any potential side effects to this fix ? The original
> author said they were not going to MFC due to possible regressions. I
> know you probably see more FreeBSD traffic then most at Y!, and so are
> very sensitive to this, but thought I would ask for clarification.
>=20
> 	---Mike

Actually, I'm very troubled by that entire chunk of code.

This is the actual fix:
------------------------------------------------------------------------
r239672 | rrs | 2012-08-25 02:26:37 -0700 (Sat, 25 Aug 2012) | 12 lines

This small change takes care of a race condition
that can occur when both sides close at the same time.
If that occurs, without this fix the connection enters
FIN1 on both sides and they will forever send FIN|ACK at
each other until the connection times out. This is because
we stopped processing the FIN|ACK and thus did not advance
the sequence and so never ACK'd each others FIN. This
fix adjusts it so we *do* process the FIN properly and
the race goes away ;-)

MFC after:      1 month
------------------------------------------------------------------------

It was MFC'ed all the way to stable/6 in Feb 2013.

r258821 from the PR does not handle the case where a fin arrives while we=

are in a retransmit state ourselves due to packet loss and interferes wit=
h
the retransmits.  r258821 needs to be backed out.

I'm not convinced that r239672 is complete, or even entirely correct.

For completeness I'm concerned about the by the if (tp->t_flags &
TF_SACK_PERMIT) .. goto drop; scenario.

Shouldn't the TH_FIN checks be moved so that it looks more like this?

@@ -2534,16 +2535,6 @@
                        }
                        tp->snd_nxt =3D th->th_ack;
                        tp->snd_cwnd =3D tp->t_maxseg;
-                       if ((thflags & TH_FIN) &&
-                           (TCPS_HAVERCVDFIN(tp->t_state) =3D=3D 0)) {
-                               /*
-                                * If its a fin we need to process
-                                * it to avoid a race where both
-                                * sides enter FIN-WAIT and send FIN|ACK
-                                * at the same time.
-                                */
-                               break;
-                       }
                        (void) tcp_output(tp);
                        KASSERT(tp->snd_limited <=3D 2,
                            ("%s: tp->snd_limited too big",
@@ -2553,7 +2544,11 @@
                             (tp->t_dupacks - tp->snd_limited);
                        if (SEQ_GT(onxt, tp->snd_nxt))
                                tp->snd_nxt =3D onxt;
-                       goto drop;
+                       if ((thflags & TH_FIN) &&
+                           (TCPS_HAVERCVDFIN(tp->t_state) =3D=3D 0))
+                               break;  /* respond to incoming FIN */
+                       else
+                               goto drop;
                } else if (V_tcp_do_rfc3042) {
                        cc_ack_received(tp, th, CC_DUPACK);
                        u_long oldcwnd =3D tp->snd_cwnd;

ie: respond to it as we normally would, but check to see if it's a new FI=
N
state before discarding it?

NetBSD handle this differently.  They seem to check to see if it's a
genuinely redundant/duplicate segment earlier and leverage it here.

It seems like our handling of this is all wrong.
--=20
Peter Wemm - peter@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com; KI6F=
JV
UTF-8: for when a ' just won\342\200\231t do.


--lnlDPe6q6u8Qe90FEFXPpNMAbTlf7Tclu
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlLMwOMACgkQFRKuUnJ3cX9fbACgh2tebjkpETkbd4MAIZCjSFng
wOEAn3x8QeIsEoI7W29UXd67FtC/0u3D
=kzOO
-----END PGP SIGNATURE-----

--lnlDPe6q6u8Qe90FEFXPpNMAbTlf7Tclu--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?52CCC0DF.1020007>