On Fri, Aug 06, 2021 at 05:22:18PM +0200, Alexandr Nedvedicky wrote:
> > Although I did not obverve it, there seems to be the same problem
> > for snd_wl1 and rcv_up. For rcv_up I copied the comparison with
> > rcv_nxt from urgent processing to keep future urgent data.
Over the weekend I thought about the SEQ_GT(tp->rcv_nxt, tp->rcv_up)
check. I think FreeBSD is right and we don't need it.
In the regular case we may receive a retransmit of an old packet.
This check preserves the rcv_up from packets that we received earlier
but with higher sequence number. But in the header prediction code
we know that TAILQ_EMPTY(&tp->t_segq). So the current packet is
the most recent one and we can blindly take its rcv_nxt as urgent
pointer.
So maybe we want take the simplified diff below.
> can you also share some details about testing you have done?
> (tool + command line options)
That is quite tricky. I do not have a simple test case for that.
One of our OpenBSD based product guarantees unidirectional traffic.
We habe a userland process that receives the data, sends it to
another OpenBSD machine. There it goes to socket splicing and
finaly it ends in a Linux FTP server.
some magic -> user land sending process --> socket splicing --> Linux FTP
I suspect that with all the machines and processes involved, we
have strange timing and run into the race.
bluhm
Index: netinet/tcp_input.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.368
diff -u -p -r1.368 tcp_input.c
--- netinet/tcp_input.c 16 Apr 2021 12:08:25 -0000 1.368
+++ netinet/tcp_input.c 9 Aug 2021 09:41:05 -0000
@@ -966,6 +966,8 @@ findpcb:
tp->t_pmtud_mss_acked = acked;
tp->snd_una = th->th_ack;
+ /* Pull snd_wl2 up to prevent seq wrap. */
+ tp->snd_wl2 = th->th_ack;
/*
* We want snd_last to track snd_una so
* as to avoid sequence wraparound problems
@@ -1015,6 +1017,9 @@ findpcb:
tcp_clean_sackreport(tp);
tcpstat_inc(tcps_preddat);
tp->rcv_nxt += tlen;
+ /* Pull snd_wl1 and rcv_up up to prevent seq wrap. */
+ tp->snd_wl1 = th->th_seq;
+ tp->rcv_up = tp->rcv_nxt;
tcpstat_pkt(tcps_rcvpack, tcps_rcvbyte, tlen);
ND6_HINT(tp);