Date: Mon, 29 Apr 2013 04:04:04 +0000 From: Barry Spinney <spinney@tilera.com> To: Mark Johnston <markjdb@gmail.com> Cc: "freebsd-net@freebsd.org" <freebsd-net@freebsd.org>, "juli@clockworksquid.com" <juli@clockworksquid.com> Subject: RE: TF_NEEDSYN flag never set. Message-ID: <73BC01C897E9F642AC962BF311A45ACB100B0081@USMAExch1.tad.internal.tilera.com> In-Reply-To: <20130428230454.GA31215@gloom> References: <73BC01C897E9F642AC962BF311A45ACB100B0057@USMAExch1.tad.internal.tilera.com> <20130428230454.GA31215@gloom>
next in thread | previous in thread | raw e-mail | index | archive | help
Yes, I switched over to using src_current.tar.gz, and that specific bug see=
ms to be gone. Thanx a lot.
BUT, the original reason I was looking so closely at this code, still remai=
ns a mystery to me.
In particular, the comment on lines 2716-2717 of tcp_input.c, along with th=
e following statement,
seems to be wrong to me.
Specifically:
/*
* If no data (only SYN) was ACK'd,
* skip rest of ACK processing.
*/
if (acked =3D=3D 0)
goto step6;
This comment implies to me that if the current rcvd pkt is a (say pure) AC=
K,
that happens to ACK my SYN-ACK (here it is assumed that the stack is operat=
ing
as a server, and we previously rcvd a SYN, sent a SYN-ACK, and are now rcvi=
ng the
subsequent ACK), then acked should be 0 and we should "goto step6".
However my analysis of the code seems to imply that in this case "acked" wi=
ll have
the value 1, which will proceed to execute in the following code, most of w=
hich seems
to expect some real data bytes to have been ack'd. Now, I am not sure tha=
t
continuing past this goto necessarily causes harm, but it sure seems unexpe=
cted, and
in the future someone else might add code here assuming that the current AC=
K acks
some real data bytes.
Regarding my analysis, that acked is 1 in this case, one can examine syncac=
he_socket
(called by syncache_expand when we promote a syncache entry to a full entry=
upon receipt
of the aforementioned ACK pkt), on line 821 of tcp_syncache.c. This invoke=
s the
tcp_sendseqinit macro (defined on line 62 of tcp_seq.h), which sets "snd_un=
a" to be "iss".
Also line 817 of tcp_syncache.c sets the t_state to be TCPS_SYN_RECEIVED.
One can also see that snd_una MUST be iss, at least by noticing that the in=
coming ackNum
must of course be equal to "iss + 1" (since it acks the SYN which takes a s=
equence number),
and seeing that line 1916 of tcp_input.c would drop the pkt if ackNum were =
<=3D snd_una=20
(assuming the t_state is still TCPS_SYN_RECEIVED - which more boring analys=
is seems to
confirm).
By the way line 2398 of tcp_input.c will change the t_state from TCPS_SYN_R=
ECEIVED to
TCPS_ESTABLISHED. Also notice line 2438 of tcp_input.c, which would cause =
the code to
go down the duplicate ACK pathway if the ackNum were <=3D snd_una (the equa=
lity be
the key here).
Finally we reach line 2661 of tcp_input.c which is:
acked =3D BYTES_THIS_ACK(tp, th);
Where BYTES_THIS_ACK is just "ackNum - snd_una" (see line 261 of tcp_var.h)=
.
But since snd_una=3D=3Diss and ackNum=3Diss+1, "acked" must be 1.
Thanx Barry.
(p.s. the reason I am curious is I have been doing some work with a tcp sta=
ck that
was original derived from some FreeBSD software, though with non-trivial ch=
anges,
and I have been trying to understand its workings).
=20
-----Original Message-----
From: Mark Johnston [mailto:markjdb@gmail.com]=20
Sent: Sunday, April 28, 2013 7:05 PM
To: Barry Spinney
Cc: freebsd-net@freebsd.org
Subject: Re: TF_NEEDSYN flag never set.
On Sun, Apr 28, 2013 at 10:31:48PM +0000, Barry Spinney wrote:
> I am sorry if this is a dumb question, but I was trying to understand=20
> the FreeBSD TCP stack, and In particular I was trying to understand=20
> the use of the TF_NEEDSYN flag. This flag is referenced a number of=20
> times in tcp_input.c and tcp_output.c, but I don't think that it can ever=
be set.
>=20
> In particular grepping through the "../src/sys/netinet", one discovers=20
> that the only code that can set this flag is lines 1018 and 1020 of=20
> tcp_input.c. But, it appears to me that none of the lines in=20
> tcp_input.c from 999 thru 1023 are even reachable! The reason they are n=
ot reachable is because just ahead of them are the following lines:
>=20
> if (!syncache_add(&inc, &to, th, &so, m))
> goto drop;
> if (so =3D=3D NULL) {
> ... // uninteresting lines, but no gotos
> return;
> }
> ... /unreachable code here
>=20
>=20
> Studying syncache_add (in file tcp_syncache.c), reveals three return st=
atements.
> One of the returns, returns the value 0, which will cause the "goto dro=
p" to be executed.
> The other two returns, return both the value 1 AND set "*sop =3D NULL",=
which should cause
> the following "if (so =3D=3D NULL)" to execute the subsequent return st=
atement.
>=20
> Is this intentional? (i.e. dead code awaiting future development?), or a =
bug?
> Or I am going blind to something obvious?
>=20
> Thanx Barry Spinney.
>=20
> (p.s. I doubt it matters which version of code, but to be precise this=20
> is from the /pub/FreeBSD/development/tarballs named=20
> "src_stable_6.tar.gz" dated "4/21/2013 01:15 AM", gotten from=20
> ftp1.us.freebsd.org<ftp://ftp1.us.freebsd.org>)
That tarball presumably contains sources for the stable branch of FreeBSD 6=
, which probably isn't what you're looking for - the last release from that=
branch was in 2008.
The relevant code in FreeBSD-CURRENT is different and your observations don=
't seem to apply there. Based on a comment added in r156125, you seem to be=
correct though. :)
http://svnweb.freebsd.org/base?view=3Drevision&revision=3D156125
I'd suggest fetching src_current.tar.gz from the FTP same directory and loo=
king at that instead - you're more likely to get replies to questions about=
the current tip of development.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?73BC01C897E9F642AC962BF311A45ACB100B0081>