On 06/17/15 at 03:10P, Jean-Francois HREN wrote: > Hello, while investigating a freeze on a modified FreeBSD 9.3 I stumbled upon > a potential bug in netinet/tcp_output.c > > If an error occurs while processing a TCP segment with some data and the 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=markup#l1360 > and > https://svnweb.freebsd.org/base/head/sys/netinet/tcp_output.c?view=markup#l1439 > ). > > In the case of a transient error, this leads to a retransmitted TCP segment > with > a shifted by 1 sequence number and a missing first byte in the TCP payload. > > In FreeBSD 9.3, it happens only when an error occurs in > netinet/ip_output.c::ip_output() > or netinet6/ip6_output::ip6_output() but in head, R249372 > ( https://svnweb.freebsd.org/base?view=revision&revision=249372 ) now allows > the same behaviour if an ENOBUFS error occurs in netinet/tcp_output.c
Your analysis looks correct to me.
>
> Tentative solutions would be either to remove the back out of the sequence
> number advance completely and to treat transient error cases like real lost
> packets
>
> --- netinet/tcp_output.c
> +++ netinet/tcp_output.c
> @@ -1435,8 +1435,7 @@
> tp->sackhint.sack_bytes_rexmit -= len;
> KASSERT(tp->sackhint.sack_bytes_rexmit >= 0,
> ("sackhint bytes rtx >= 0"));
> - } else
> - tp->snd_nxt -= len;
> + }
> }
> SOCKBUF_UNLOCK_ASSERT(&so->so_snd); /* Check gotos. */
> switch (error) {
>
> or to decrease the sequence number advance by 1 if a FIN flag was sent.
>
> --- netinet/tcp_output.c
> +++ netinet/tcp_output.c
> @@ -1435,8 +1435,11 @@
> tp->sackhint.sack_bytes_rexmit -= len;
> KASSERT(tp->sackhint.sack_bytes_rexmit >= 0,
> ("sackhint bytes rtx >= 0"));
> - } else
> + } else {
> tp->snd_nxt -= len;
> + if (flags & TH_FIN)
> + tp->snd_nxt--;
> + }
> }
> SOCKBUF_UNLOCK_ASSERT(&so->so_snd); /* Check gotos. */
> switch (error) {
I like the second approach better.
Cheers,
Hiren
pgpzITCe9ef4C.pgp
Description: PGP signature
