Hi, just a bit of caution: i remember getting args-http-slow-consumer.pl to work right in the first place was not easy. If i remember correctly i had quite a lot false positives depending on where i ran it. Alexander made it a bit better later, but i would not be surprised if it can still fail under funny situations.
That said, i have no opinion on the ACK issue, there is probably a valid case to revert the change for the release and figure things out after 6.9. /Benno Alexander Bluhm(alexander.bl...@gmx.net) on 2021.04.15 20:43:02 +0200: > Hi, > > 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. > > As Jan's change send less ACKs that have to be handled, throughput > increases when TCP/IP stack is CPU bound. This can give a 20% > increase. > > Due to the maxburst limitation, throughput may also drop by 70% on > a single connection. I have removed this limitation, so it does > not happen if the other side is a current OpenBSD. But TCP connection > between -current and 6.8 can run into this problem. > > Now I have found another corner case in a regress test where we get > only 10 TCP packets per second. But that may be the result of > unrealistic settings of the socket buffer size. > > Should we back it out for release? Diff below. > > bluhm > > Index: netinet/tcp_input.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v > retrieving revision 1.366 > retrieving revision 1.365 > diff -u -p -r1.366 -r1.365 > --- netinet/tcp_input.c 3 Feb 2021 13:40:06 -0000 1.366 > +++ netinet/tcp_input.c 19 Jun 2020 22:47:22 -0000 1.365 > @@ -165,8 +165,8 @@ do { \ > #endif > > /* > - * Macro to compute ACK transmission behavior. Delay the ACK until > - * a read from the socket buffer or the delayed ACK timer causes one. > + * Macro to compute ACK transmission behavior. Delay the ACK unless > + * we have already delayed an ACK (must send an ACK every two segments). > * We also ACK immediately if we received a PUSH and the ACK-on-PUSH > * option is enabled or when the packet is coming from a loopback > * interface. > @@ -176,7 +176,8 @@ do { \ > struct ifnet *ifp = NULL; \ > if (m && (m->m_flags & M_PKTHDR)) \ > ifp = if_get(m->m_pkthdr.ph_ifidx); \ > - if ((tcp_ack_on_push && (tiflags) & TH_PUSH) || \ > + if (TCP_TIMER_ISARMED(tp, TCPT_DELACK) || \ > + (tcp_ack_on_push && (tiflags) & TH_PUSH) || \ > (ifp && (ifp->if_flags & IFF_LOOPBACK))) \ > tp->t_flags |= TF_ACKNOW; \ > else \ >