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 \
>
