On Thu, Apr 15, 2021 at 08:43:02PM +0200, Alexander Bluhm wrote:
> I found another regression with Jan's TCP diff that sends less ACK
> packets. relayd run-args-http-slow-consumer.pl fails on i386 due
> to his commit. This test writes a lot of data from the http server,
> but blocks receive for 2 seconds in the client. Relayd between
> these machines should handle the delay. The socket buffer size is
> very small to trigger the situation reliably.
>
> The current TCP stack does not recover after the delay. Packets
> are sent very slowly and the regress test runs in a timeout. When
> I backout the change, the test passes quickly.
>
> Ususally the test runs on localhost loopback. There the problem
> is not triggered. Only my i386 regress setup uses a remote machine.
This issue is caused by another bug in our stack. The Stack calls
tcp_output(), but does not send an ACK with a window update, after the
consuming process empties the receive buffer in soreceive().
In normal conditions, the every other ACK feature hides this problem.
Thus, with my diff, the 200ms ACK timer is the only mechanism that sends
out ACKs. But, this is to slow, to empty the stalled buffer fast
enough.
The following diff removes the every 2nd ACK feature again and ensures
that we send out an ACK if soreceive() empties the receive buffer.
We are so close to 7.0, that I would suggest to commit this after the
release. Thus, we don't risk another last minute regression.
OK?
bye,
Jan
Index: netinet/tcp_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_input.c,v
retrieving revision 1.370
diff -u -p -r1.370 tcp_input.c
--- netinet/tcp_input.c 9 Aug 2021 17:03:08 -0000 1.370
+++ netinet/tcp_input.c 18 Sep 2021 07:53:45 -0000
@@ -176,8 +176,7 @@ do { \
struct ifnet *ifp = NULL; \
if (m && (m->m_flags & M_PKTHDR)) \
ifp = if_get(m->m_pkthdr.ph_ifidx); \
- if (TCP_TIMER_ISARMED(tp, TCPT_DELACK) || \
- (tcp_ack_on_push && (tiflags) & TH_PUSH) || \
+ if ((tcp_ack_on_push && (tiflags) & TH_PUSH) || \
(ifp && (ifp->if_flags & IFF_LOOPBACK))) \
tp->t_flags |= TF_ACKNOW; \
else \
Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /cvs/src/sys/netinet/tcp_usrreq.c,v
retrieving revision 1.181
diff -u -p -r1.181 tcp_usrreq.c
--- netinet/tcp_usrreq.c 30 Apr 2021 13:52:48 -0000 1.181
+++ netinet/tcp_usrreq.c 18 Sep 2021 07:53:45 -0000
@@ -329,8 +329,15 @@ tcp_usrreq(struct socket *so, int req, s
* template for a listening socket and hence the kernel
* will panic.
*/
- if ((so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING)) != 0)
+ if ((so->so_state & (SS_ISCONNECTED|SS_ISCONNECTING)) != 0) {
+ /*
+ * If soreceive() empty the receive buffer, we have to
+ * send a window update.
+ */
+ if (so->so_rcv.sb_cc == 0)
+ tp->t_flags |= TF_ACKNOW;
(void) tcp_output(tp);
+ }
break;
/*
