netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] allow dctcp alpha to drop to zero
@ 2015-10-19  4:59 Andrew Shewmaker
  2015-10-19 14:01 ` Florian Westphal
  2015-10-23  9:47 ` David Miller
  0 siblings, 2 replies; 4+ messages in thread
From: Andrew Shewmaker @ 2015-10-19  4:59 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, Daniel Borkmann, Florian Westphal, Glenn Judd

If alpha is strictly reduced by alpha >> dctcp_shift_g and if alpha is less
than 1 << dctcp_shift_g, then alpha may never reach zero. For example,
given shift_g=4 and alpha=15, alpha >> dctcp_shift_g yields 0 and alpha
remains 15. The effect isn't noticeable in this case below cwnd=137, but
could gradually drive uncongested flows with leftover alpha down to
cwnd=137. A larger dctcp_shift_g would have a greater effect.

This change causes alpha=15 to drop to 0 instead of being decrementing by 1
as it would when alpha=16. However, it requires one less conditional to
implement since it doesn't have to guard against subtracting 1 from 0U. A
decay of 15 is not unreasonable since an equal or greater amount occurs at
alpha >= 240.

Signed-off-by: Andrew G. Shewmaker <agshew@gmail.com>
---
 net/ipv4/tcp_dctcp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index 7092a61..7e538f7 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -209,7 +209,7 @@ static void dctcp_update_alpha(struct sock *sk, u32 flags)
 
 		/* alpha = (1 - g) * alpha + g * F */
 
-		alpha -= alpha >> dctcp_shift_g;
+		alpha -= min_not_zero(alpha, alpha >> dctcp_shift_g);
 		if (bytes_ecn) {
 			/* If dctcp_shift_g == 1, a 32bit value would overflow
 			 * after 8 Mbytes.
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] allow dctcp alpha to drop to zero
  2015-10-19  4:59 [PATCH] allow dctcp alpha to drop to zero Andrew Shewmaker
@ 2015-10-19 14:01 ` Florian Westphal
  2015-10-20 21:30   ` Andrew Shewmaker
  2015-10-23  9:47 ` David Miller
  1 sibling, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2015-10-19 14:01 UTC (permalink / raw)
  To: Andrew Shewmaker
  Cc: netdev, David S. Miller, Daniel Borkmann, Florian Westphal,
	Glenn Judd

Andrew Shewmaker <agshew@gmail.com> wrote:
> If alpha is strictly reduced by alpha >> dctcp_shift_g and if alpha is less
> than 1 << dctcp_shift_g, then alpha may never reach zero. For example,
> given shift_g=4 and alpha=15, alpha >> dctcp_shift_g yields 0 and alpha
> remains 15. The effect isn't noticeable in this case below cwnd=137, but
> could gradually drive uncongested flows with leftover alpha down to
> cwnd=137. A larger dctcp_shift_g would have a greater effect.
> 
> This change causes alpha=15 to drop to 0 instead of being decrementing by 1
> as it would when alpha=16. However, it requires one less conditional to
> implement since it doesn't have to guard against subtracting 1 from 0U. A
> decay of 15 is not unreasonable since an equal or greater amount occurs at
> alpha >= 240.
> 
> Signed-off-by: Andrew G. Shewmaker <agshew@gmail.com>

Acked-by: Florian Westphal <fw@strlen.de>

[ cwnd=137 is quite large so I don't think its important enough for
-stable ].

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] allow dctcp alpha to drop to zero
  2015-10-19 14:01 ` Florian Westphal
@ 2015-10-20 21:30   ` Andrew Shewmaker
  0 siblings, 0 replies; 4+ messages in thread
From: Andrew Shewmaker @ 2015-10-20 21:30 UTC (permalink / raw)
  To: Florian Westphal
  Cc: netdev@vger.kernel.org, David S. Miller, Glenn Judd,
	Daniel Borkmann

On Mon, Oct 19, 2015 at 8:01 AM, Florian Westphal <fw@strlen.de> wrote:
> Andrew Shewmaker <agshew@gmail.com> wrote:
>> If alpha is strictly reduced by alpha >> dctcp_shift_g and if alpha is less
>> than 1 << dctcp_shift_g, then alpha may never reach zero. For example,
>> given shift_g=4 and alpha=15, alpha >> dctcp_shift_g yields 0 and alpha
>> remains 15. The effect isn't noticeable in this case below cwnd=137, but
>> could gradually drive uncongested flows with leftover alpha down to
>> cwnd=137. A larger dctcp_shift_g would have a greater effect.
>>
>> This change causes alpha=15 to drop to 0 instead of being decrementing by 1
>> as it would when alpha=16. However, it requires one less conditional to
>> implement since it doesn't have to guard against subtracting 1 from 0U. A
>> decay of 15 is not unreasonable since an equal or greater amount occurs at
>> alpha >= 240.
>>
>> Signed-off-by: Andrew G. Shewmaker <agshew@gmail.com>
>
> Acked-by: Florian Westphal <fw@strlen.de>
>
> [ cwnd=137 is quite large so I don't think its important enough for
> -stable ].

If my math is correct (please double check it), then my patch will
make a difference in scenarios such as:

data center: ~20+ Gbps and 500 microsecond RTT
137 * 9000 * 8 / (500 * pow(10, -6)) / pow(10, 9) = 19.728 Gbps
[pkt / W] * [bits / pkt] * [W / sec]

regional: ~33+ Mbps and 50 millisecond RTT
137 * 1500 * 8 / (50 * pow(10, -3)) / pow(10, 6) = 32.88 Mbps

international: ~11+ Mbps and 150 millisecond RTT
137 * 1500 * 8 / (150 * pow(10, -3)) / pow(10, 6) = 10.96 Mbps


Even if the broader internet does not ever configure ECN appropriately
for use with DCTCP, I care about the latter two cases because I'm
using an RTT-based congestion ratio in the same way as DCTCP uses its
ECN-based congestion ratio. A tech report describing use of RTTs in
this way, with some preliminary Mininet results, is available:

https://www.soe.ucsc.edu/research/technical-reports/UCSC-SOE-15-20

I plan on submitting some RFC patches to DCTCP based on my prototype soon.

-- 
Andrew Shewmaker

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] allow dctcp alpha to drop to zero
  2015-10-19  4:59 [PATCH] allow dctcp alpha to drop to zero Andrew Shewmaker
  2015-10-19 14:01 ` Florian Westphal
@ 2015-10-23  9:47 ` David Miller
  1 sibling, 0 replies; 4+ messages in thread
From: David Miller @ 2015-10-23  9:47 UTC (permalink / raw)
  To: agshew; +Cc: netdev, dborkman, fw, glenn.judd

From: Andrew Shewmaker <agshew@gmail.com>
Date: Sun, 18 Oct 2015 21:59:08 -0700

> If alpha is strictly reduced by alpha >> dctcp_shift_g and if alpha is less
> than 1 << dctcp_shift_g, then alpha may never reach zero. For example,
> given shift_g=4 and alpha=15, alpha >> dctcp_shift_g yields 0 and alpha
> remains 15. The effect isn't noticeable in this case below cwnd=137, but
> could gradually drive uncongested flows with leftover alpha down to
> cwnd=137. A larger dctcp_shift_g would have a greater effect.
> 
> This change causes alpha=15 to drop to 0 instead of being decrementing by 1
> as it would when alpha=16. However, it requires one less conditional to
> implement since it doesn't have to guard against subtracting 1 from 0U. A
> decay of 15 is not unreasonable since an equal or greater amount occurs at
> alpha >= 240.
> 
> Signed-off-by: Andrew G. Shewmaker <agshew@gmail.com>

Applied, thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-10-23  9:30 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-19  4:59 [PATCH] allow dctcp alpha to drop to zero Andrew Shewmaker
2015-10-19 14:01 ` Florian Westphal
2015-10-20 21:30   ` Andrew Shewmaker
2015-10-23  9:47 ` David Miller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).