On Thu, 2019-12-12 at 13:29 -0800, Ben Greear wrote:

quoted

(*) Hmm. Now I have another idea. Maybe we have some kind of problem
with the medium access configuration, and we transmit all this data
without the AP having a chance to send back all the ACKs? Too bad I
can't put an air sniffer into the setup - it's a conductive setup.

splitter/combiner?

I guess. I haven't looked at it, it's halfway around the world or
something :)

If it is just delayed acks coming back, which would slow down a stream, then
multiple streams would tend to work around that problem?

Only a bit, because it allows somewhat more outstanding data. But each
stream estimates the throughput lower in its congestion control
algorithm, so it would have a smaller window size?

What I was thinking is that if we have some kind of skew in the system
and always/frequently/sometimes make our transmissions have priority
over the AP transmissions, then we'd not get ACKs back, and that might
cause what I see - the queue drains entirely and *then* we get an ACK
back...

That's not a _bad_ theory and I'll have to find a good way to test it,
but I'm not entirely convinced that's the problem.

Oh, actually, I guess I know it's *not* the problem because otherwise
the ss output would show we're blocked on congestion window far more
than it looks like now? I think?

I would actually expect similar speedup with multiple streams if some TCP socket
was blocked on waiting for ACKs too.

Even if you can't sniff the air, you could sniff the wire or just look at packet
in/out counts.  If you have a huge number of ACKs, that would show up in raw pkt
counters.

I know I have a huge number of ACKs, but I also know that's not the
(only) problem. My question/observation was related to the timing of
them.

I'm not sure it matters these days, but this patch greatly helped TCP throughput on
ath10k for a while, and we are still using it.  Maybe your sk_pacing change already
tweaked the same logic:

https://github.com/greearb/linux-ct-5.4/commit/65651d4269eb2b0d4b4952483c56316a7fbe2f48

Yes, you should be able to drop that patch - look at it, it just
multiples the thing there that you have with "sk->sk_pacing_shift",
instead we currently by default set sk->sk_pacing_shift to 7 instead of
10 or something, so that'd be equivalent to setting your sysctl to 8.

		TCP_TSQ=200

Setting it to 200 is way excessive. In particular since you already get
the *8 from the default mac80211 behaviour, so now you effectively have
*1600, which means instead of 1ms you can have 1.6s worth of TCP data on
the queues ... way too much :)

johannes