From owner-freebsd-hackers@FreeBSD.ORG  Fri Jan 11 07:18:09 2013
Return-Path: <owner-freebsd-hackers@FreeBSD.ORG>
Delivered-To: freebsd-hackers@freebsd.org
Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115])
 by hub.freebsd.org (Postfix) with ESMTP id AFA001EA
 for <freebsd-hackers@freebsd.org>; Fri, 11 Jan 2013 07:18:09 +0000 (UTC)
 (envelope-from jacques.fourie@gmail.com)
Received: from mail-la0-f41.google.com (mail-la0-f41.google.com
 [209.85.215.41]) by mx1.freebsd.org (Postfix) with ESMTP id 42695643
 for <freebsd-hackers@freebsd.org>; Fri, 11 Jan 2013 07:18:08 +0000 (UTC)
Received: by mail-la0-f41.google.com with SMTP id em20so1469236lab.14
 for <freebsd-hackers@freebsd.org>; Thu, 10 Jan 2013 23:18:07 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;
 h=mime-version:date:message-id:subject:from:to:content-type;
 bh=vQKdA3GnEmtKgZ0K7XcdgMI5gMZjwwlLpFfYIkwycFY=;
 b=wtD+xHDAItq51SX0omYjzdYUltaemXQQ29wkdJLsvQB7RznnFPlGpMLMZaU4EPlKal
 CMS5RvXLG+RRj6AEh/XLPeFMriqz7rI4HrijV5THul3Otr+enJdlQjehtsL8diGNK3v6
 rsY1gRVIMCAO0ONHMYSew12+T0299avHYIeR8LeGyOG6LkcVNfEIWMPnJvsAkRvHEn4s
 P0aCbx6s8/eKEZ/R3L1bMIT2cNevna3t4kFKuaP5UqJbZHsKoq6jrlQmPKrDuFNlBbyu
 7NZIRilN4pg0E0vBH4BhD7Zt09bSCCzdvVoXlizpD9O0m2GdCXl0vd8/MEwCk29B4lUO
 QW4A==
MIME-Version: 1.0
Received: by 10.152.111.166 with SMTP id ij6mr72349645lab.47.1357888687812;
 Thu, 10 Jan 2013 23:18:07 -0800 (PST)
Received: by 10.152.13.36 with HTTP; Thu, 10 Jan 2013 23:18:07 -0800 (PST)
Date: Fri, 11 Jan 2013 09:18:07 +0200
Message-ID: <CALX0vxAKNMdP6ZvXf0mvk5MKdRL45D82AYBy8Ras-zRDmixa3Q@mail.gmail.com>
Subject: Inconsistent TCP state in tcp_do_segment()
From: Jacques Fourie <jacques.fourie@gmail.com>
To: Hackers freeBSD <freebsd-hackers@freebsd.org>
Content-Type: text/plain; charset=ISO-8859-1
X-Content-Filtered-By: Mailman/MimeDel 2.1.14
X-BeenThere: freebsd-hackers@freebsd.org
X-Mailman-Version: 2.1.14
Precedence: list
List-Id: Technical Discussions relating to FreeBSD
 <freebsd-hackers.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/options/freebsd-hackers>, 
 <mailto:freebsd-hackers-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/freebsd-hackers>
List-Post: <mailto:freebsd-hackers@freebsd.org>
List-Help: <mailto:freebsd-hackers-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/freebsd-hackers>,
 <mailto:freebsd-hackers-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 11 Jan 2013 07:18:09 -0000

Hi,

I've been using the kernel socket API and noticed that every once in a
while the data received at the remote end of a TCP connection doesn't match
the data sent locally using sosend(). I tracked it down to the piece of
code in tcp_do_segment() starting at line 2665: (svn rev 245286)

               if (acked > so->so_snd.sb_cc) {
                        tp->snd_wnd -= so->so_snd.sb_cc;
                        sbdrop_locked(&so->so_snd, (int)so->so_snd.sb_cc);
                        ourfinisacked = 1;
                } else {
                        sbdrop_locked(&so->so_snd, acked);
                        tp->snd_wnd -= acked;
                        ourfinisacked = 0;
                }
                sowwakeup_locked(so);
                /* Detect una wraparound. */
                if (!IN_RECOVERY(tp->t_flags) &&
                    SEQ_GT(tp->snd_una, tp->snd_recover) &&
                    SEQ_LEQ(th->th_ack, tp->snd_recover))
                        tp->snd_recover = th->th_ack - 1;
                /* XXXLAS: Can this be moved up into cc_post_recovery? */
                if (IN_RECOVERY(tp->t_flags) &&
                    SEQ_GEQ(th->th_ack, tp->snd_recover)) {
                        EXIT_RECOVERY(tp->t_flags);
                }
                tp->snd_una = th->th_ack;
                if (tp->t_flags & TF_SACK_PERMIT) {
                        if (SEQ_GT(tp->snd_una, tp->snd_recover))
                                tp->snd_recover = tp->snd_una;
                }
                if (SEQ_LT(tp->snd_nxt, tp->snd_una))
                        tp->snd_nxt = tp->snd_una;

The issue is that sowwakeup_locked() is called before all the tcpcb fields
are updated to account for the current ACK processing as can be seen from
the tp->snd_una = th->th_ack in line 2686. My code that uses the kernel
socket API calls sosend() in the upcall path resulting from
sowwakeup_locked() which in turn can lead to tcp_output() being called with
inconsistent TCP state. If I move the call to sowwakeup_locked() to after
the 'if (SEQ_LT(tp->snd_nxt, tp->snd_una))' line in the code snippet above
the data corruption issue seems to be fixed.