From: Eric Dumazet <hidden> Date: 2015-02-06 13:53:05
On Fri, 2015-02-06 at 05:40 -0800, Eric Dumazet wrote:
tcp_wfree() could maintain in tp->tx_completion_delay_ms an EWMA
of TX completion delay. But this would require yet another expensive
call to ktime_get() if HZ < 1000.
Then tcp_write_xmit() could use it to adjust :
limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 9);
to
amount = (2 + tp->tx_completion_delay_ms) * sk->sk_pacing_rate
limit = max(2 * skb->truesize, amount / 1000);
I'll cook a patch.
Hmm... doing this in all protocols would be too expensive,
and we do not want to include time spent in qdiscs.
wifi could eventually do that, providing in skb->tx_completion_delay_us
the time spent in wifi driver.
This way, we would have no penalty for network devices doing normal skb
orphaning (loopback interface, ethernet, ...)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Michal Kazior <hidden> Date: 2015-02-06 14:09:38
On 6 February 2015 at 14:53, Eric Dumazet [off-list ref] wrote:
On Fri, 2015-02-06 at 05:40 -0800, Eric Dumazet wrote:
quoted
tcp_wfree() could maintain in tp->tx_completion_delay_ms an EWMA
of TX completion delay. But this would require yet another expensive
call to ktime_get() if HZ < 1000.
Then tcp_write_xmit() could use it to adjust :
limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 9);
to
amount = (2 + tp->tx_completion_delay_ms) * sk->sk_pacing_rate
limit = max(2 * skb->truesize, amount / 1000);
I'll cook a patch.
Hmm... doing this in all protocols would be too expensive,
and we do not want to include time spent in qdiscs.
wifi could eventually do that, providing in skb->tx_completion_delay_us
the time spent in wifi driver.
This way, we would have no penalty for network devices doing normal skb
orphaning (loopback interface, ethernet, ...)
I'll play around with this idea and report back later.
Michał
From: Eric Dumazet <hidden> Date: 2015-02-06 14:10:44
On Fri, 2015-02-06 at 05:53 -0800, Eric Dumazet wrote:
wifi could eventually do that, providing in skb->tx_completion_delay_us
the time spent in wifi driver.
This way, we would have no penalty for network devices doing normal skb
orphaning (loopback interface, ethernet, ...)
Another way would be that wifi does an automatic orphaning after 1 or
2ms.
From: David Laight <hidden> Date: 2015-02-06 14:31:59
From: Eric Dumazet
On Fri, 2015-02-06 at 05:53 -0800, Eric Dumazet wrote:
quoted
wifi could eventually do that, providing in skb->tx_completion_delay_us
the time spent in wifi driver.
This way, we would have no penalty for network devices doing normal skb
orphaning (loopback interface, ethernet, ...)
Another way would be that wifi does an automatic orphaning after 1 or
2ms.
Couldn't you do byte counting?
So orphan enough packets to keep a few ms of tx traffic (at the current
tx rate) orphaned.
You might need to give the hardware both orphaned and non-orphaned (parented?)
packets and orphan some when you get a tx complete for an orphaned packet.
David
From: Michal Kazior <hidden> Date: 2015-02-09 13:47:58
On 6 February 2015 at 15:09, Michal Kazior [off-list ref] wrote:
On 6 February 2015 at 14:53, Eric Dumazet [off-list ref] wrote:
quoted
On Fri, 2015-02-06 at 05:40 -0800, Eric Dumazet wrote:
quoted
tcp_wfree() could maintain in tp->tx_completion_delay_ms an EWMA
of TX completion delay. But this would require yet another expensive
call to ktime_get() if HZ < 1000.
Then tcp_write_xmit() could use it to adjust :
limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 9);
to
amount = (2 + tp->tx_completion_delay_ms) * sk->sk_pacing_rate
limit = max(2 * skb->truesize, amount / 1000);
I'll cook a patch.
Hmm... doing this in all protocols would be too expensive,
and we do not want to include time spent in qdiscs.
wifi could eventually do that, providing in skb->tx_completion_delay_us
the time spent in wifi driver.
This way, we would have no penalty for network devices doing normal skb
orphaning (loopback interface, ethernet, ...)
I'll play around with this idea and report back later.
I'm able to get 600mbps with 5 flows and 250mbps with 1 flow, i.e.
same as before the regression. I'm attaching the patch at the end of
my mail - is this approach viable?
I wonder if there's anything that can be done to allow 600mbps (line
rate) on 1 flow with ath10k without tweaking tcp_limit_output_bytes
(you can't expect end-users to tweak this).
Perhaps tcp_limit_output_bytes should also consider tx_completion_delay, e.g.:
amount = sk->sk_tx_completion_delay_us;
amount *= sk->sk_pacing_rate >> 10;
limit = max(2 * skb->truesize, amount >> 10);
max_limit = sysctl_tcp_limit_output_bytes;
max_limit *= 1 + (sk->sk_tx_completion_delay_us / USEC_PER_MSEC);
limit = min(u32, limit, max_limit);
With this I get ~400mbps on 1 flow. If I add the original 1ms extra
delay from your formula to tx_completion_delay I fill in ath10k I get
nearly line rate in 1 flow (almost 600mbps; it hops between 570-620).
Decreasing tcp_limit_output_bytes decreases throughput (e.g. 64K gives
300mbps, 32K gives 180mbps, 16K gives 110mbps). Multiple flows in
iperf seem unbalanced with 128K limit, but look okay with 32K).
Michał
unsigned int mss_now, int nonagle,
* of queued bytes to ensure line rate.
* One example is wifi aggregation (802.11 AMPDU)
*/
- limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 10);
+ amount = sk->sk_tx_completion_delay_us *
+ (sk->sk_pacing_rate >> 10);
+ limit = max(2 * skb->truesize, amount >> 10);
limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
if (atomic_read(&sk->sk_wmem_alloc) > limit) {
This is not what I suggested.
If you test this on any other network device, you'll have
sk->sk_tx_completion_delay_us == 0
amount = 0 * (sk->sk_pacing_rate >> 10); --> 0
limit = max(2 * skb->truesize, amount >> 10); --> 2 * skb->truesize
So non TSO/GSO NIC will not be able to queue more than 2 MSS (one MSS
per skb)
Then if you store only the last tx completion, you have the possibility
of having a last packet of a train (say a retransmit) to make it very
low.
Ideally the formula would be in TCP something very fast to compute :
amount = (sk->sk_pacing_rate >> 10) + sk->tx_completion_delay_cushion;
limit = max(2 * skb->truesize, amount);
limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
So a 'problematic' driver would have to do the math (64 bit maths) like
this :
sk->tx_completion_delay_cushion = ewma_tx_delay * sk->sk_pacing_rate;
From: Michal Kazior <hidden> Date: 2015-02-10 10:33:49
On 9 February 2015 at 16:11, Eric Dumazet [off-list ref] wrote:
On Mon, 2015-02-09 at 14:47 +0100, Michal Kazior wrote:
[...]
This is not what I suggested.
If you test this on any other network device, you'll have
sk->sk_tx_completion_delay_us == 0
amount = 0 * (sk->sk_pacing_rate >> 10); --> 0
limit = max(2 * skb->truesize, amount >> 10); --> 2 * skb->truesize
You're right. Sorry for mixing up.
So non TSO/GSO NIC will not be able to queue more than 2 MSS (one MSS
per skb)
Then if you store only the last tx completion, you have the possibility
of having a last packet of a train (say a retransmit) to make it very
low.
Ideally the formula would be in TCP something very fast to compute :
amount = (sk->sk_pacing_rate >> 10) + sk->tx_completion_delay_cushion;
limit = max(2 * skb->truesize, amount);
limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
So a 'problematic' driver would have to do the math (64 bit maths) like
this :
sk->tx_completion_delay_cushion = ewma_tx_delay * sk->sk_pacing_rate;
Hmm. So I've done like you suggested (hopefully I didn't mix anything
up this time around).
I now get pre-regression performance, ~250mbps on 1 flow, 600mbps on 5
flows (vs 250mbps whatever number of flows).
Michał
Hi Michal
This is almost it ;)
As I said you must do this using u64 arithmetics, we still support 32bit
kernels.
Also, >> 20 instead of / 1000000 introduces a 5% error, I would use a
plain divide, as the compiler will use a reciprocal divide (ie : a
multiply)
We use >> 10 instead of /1000 because a 2.4 % error is probably okay.
ewma_add(&ar->tx_delay_us,
ktime_to_ns(ktime_sub(ktime_get(),
skb_cb->stamp)) /
NSEC_PER_USEC);
u64 val = (u64)ewma_read(&ar->tx_delay_us) *
msdu->sk->sk_pacing_rate;
do_div(val, USEC_PER_SEC);
ACCESS_ONCE(msdu->sk->sk_tx_completion_delay_cushion) =
(u32)val;
(WRITE_ONCE() would be better for new kernels, but ACCESS_ONCE() is ok
since we probably want to backport to stable kernels)
Thanks
From: Eric Dumazet <hidden> Date: 2015-02-10 13:05:18
On Tue, 2015-02-10 at 04:54 -0800, Eric Dumazet wrote:
Hi Michal
This is almost it ;)
As I said you must do this using u64 arithmetics, we still support 32bit
kernels.
Also, >> 20 instead of / 1000000 introduces a 5% error, I would use a
plain divide, as the compiler will use a reciprocal divide (ie : a
multiply)
We use >> 10 instead of /1000 because a 2.4 % error is probably okay.
ewma_add(&ar->tx_delay_us,
ktime_to_ns(ktime_sub(ktime_get(),
skb_cb->stamp)) /
NSEC_PER_USEC);
btw I suspect this wont compile on 32 bit kernel
You need to use do_div() as well :
u64 val = ktime_to_ns(ktime_sub(ktime_get(),
skb_cb->stamp));
do_div(val, NSEC_PER_USEC);
ewma_add(&ar->tx_delay_us, val);
1) 16384 factor might be too big.
2) a weight of 8 seems too low given aggregation values used in wifi.
On 32bit arches, the max range for ewma value would be 262144 usec,
a quarter of a second...
You could use a factor of 64 instead, and a weight of 16.
To some extent, every wifi driver is going to have this problem. Perhaps
we should do this in mac80211?
I'll provide the TCP patch.
sk->sk_tx_completion_delay_cushion is probably a wrong name, as the
units here are in bytes, since it is really number of bytes in the
network driver that accommodate for tx completions delays.
tx_completion_delay * pacing_rate
sk_tx_completion_cushion maybe.
1) 16384 factor might be too big.
2) a weight of 8 seems too low given aggregation values used in wifi.
On 32bit arches, the max range for ewma value would be 262144 usec,
a quarter of a second...
You could use a factor of 64 instead, and a weight of 16.
64/16 seems to work fine as well.
On a related note: I still wonder how to get single TCP flow to reach
line rate with ath10k (it still doesn't; I reach line rate with
multiple flows only). Isn't the tcp_limit_output_bytes just too small
for devices like Wi-Fi where you can send aggregates of even 64*3*1500
bytes long in a single shot and you can't expect even a single
tx-completion of it to come in before its transmitted entirely? You
effectively operate with bursts of traffic.
Some numbers:
ath10k w/o cushion w/o aggregation 1 flow: UDP 65mbps, TCP 30mbps
ath10k w/ cushion w/o aggregation 1 flow: UDP 65mbps, TCP 59mbps
ath10k w/o cushion w/ aggregation 1 flow: UDP 650mbps, TCP 250mbps
ath10k w/ cushion w/ aggregation 1 flow: UDP 650mbps, TCP 250mbps
ath10k w/o cushion w/ aggregation 5 flows: UDP 650mbps, TCP 250mbps
ath10k w/ cushion w/ aggregation 5 flows: UDP 650mbps, TCP 600mbps
"w/o aggregation" means forcing ath10k to use 1 A-MSDU and 1 A-MPDU
per aggregate so latencies due to aggregation itself should be pretty
much nil.
If I set tcp_limit_output_bytes to 700K+ I can get ath10k w/ cushion
w/ aggregation to reach 600mbps on a single flow.
Michał
From: Eric Dumazet <hidden> Date: 2015-02-11 13:17:52
On Wed, 2015-02-11 at 09:33 +0100, Michal Kazior wrote:
If I set tcp_limit_output_bytes to 700K+ I can get ath10k w/ cushion
w/ aggregation to reach 600mbps on a single flow.
You know, there is a reason this sysctl exists in the first place ;)
The first suggestion I made to you was to raise it.
The default setting must stay as is as long default Qdisc is pfifo_fast.
I believe I already mentioned skb->truesize tricks for drivers willing
to adjust the TSQ given their constraints.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Johannes Berg <johannes@sipsolutions.net> Date: 2015-03-31 11:08:37
To revive an old thread ...
On Mon, 2015-02-09 at 07:11 -0800, Eric Dumazet wrote:
Ideally the formula would be in TCP something very fast to compute :
amount = (sk->sk_pacing_rate >> 10) + sk->tx_completion_delay_cushion;
limit = max(2 * skb->truesize, amount);
limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
So a 'problematic' driver would have to do the math (64 bit maths) like
this :
sk->tx_completion_delay_cushion = ewma_tx_delay * sk->sk_pacing_rate;
The whole notion with "ewma_tx_delay" seems very wrong to me. Measuring
something while also trying to control it (or something closely related)
seems a bit strange, but perhaps you meant to measure something
different than what Michal implemented.
What he implemented was measuring the time it takes from submission to
the hardware queues, but that seems to create a bad feedback cycle:
Allowing it as extra transmit "cushion" will, IIUC, cause TCP to submit
more data to the queues, which will in turn cause the next packets to be
potentially delayed more (since there are still waiting ones) thus
causing a longer delay to be measured, which in turn allows even more
data to be submitted etc.
IOW, while traffic is flowing this will likely cause feedback that
completely removes the point of this, no? Leaving only
sysctl_tcp_limit_output_bytes as the limit (*).
It seems it'd be better to provide a calculated estimate, perhaps based
on current transmit rate and (if available) CCA/TXOP acquisition time.
johannes
(*) Which, btw, isn't all that big given that a maximally sized A-MPDU
is like 1MB containing close to that much actual data! Don't think that
can actually be done at current transmit rates though.