Patchwork tcp: should use number of sack blocks instead of -1

login
register
mail settings
Submitter Ilpo Järvinen
Date Oct. 21, 2008, 8:58 a.m.
Message ID <Pine.LNX.4.64.0810211153300.7072@wrl-59.cs.helsinki.fi>
Download mbox | patch
Permalink /patch/5205/
State Accepted
Delegated to: David Miller
Headers show

Comments

Ilpo Järvinen - Oct. 21, 2008, 8:58 a.m.
While looking for the recent "sack issue" I also read all eff_sacks
usage that was played around by some relevant commit. I found
out that there's another thing that is asking for a fix (unrelated
to the "sack issue" though).

This feature has probably very little significance in practice.
Opposite direction timeout with bidirectional tcp comes to me as
the most likely scenario though there might be other cases as
well related to non-data segments we send (e.g., response to the
opposite direction segment). Also some ACK losses or option space
wasted for other purposes is necessary to prevent the earlier
SACK feedback getting to the sender.

This is not too critical fix, so you might want to delay this to
net-next if you're afraid that Linus will get you because of
this...

I suppose we might not need eff_sacks at all but I'll leave
figuring that out to net-next :-).

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---
 net/ipv4/tcp_output.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
David Miller - Oct. 21, 2008, 11:29 p.m.
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Tue, 21 Oct 2008 11:58:05 +0300 (EEST)

> While looking for the recent "sack issue" I also read all eff_sacks
> usage that was played around by some relevant commit. I found
> out that there's another thing that is asking for a fix (unrelated
> to the "sack issue" though).
> 
> This feature has probably very little significance in practice.
> Opposite direction timeout with bidirectional tcp comes to me as
> the most likely scenario though there might be other cases as
> well related to non-data segments we send (e.g., response to the
> opposite direction segment). Also some ACK losses or option space
> wasted for other purposes is necessary to prevent the earlier
> SACK feedback getting to the sender.
...
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>

Applied, thanks Ilpo.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 990a584..de54f02 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -432,7 +432,7 @@  static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp,
 
 		if (tp->rx_opt.dsack) {
 			tp->rx_opt.dsack = 0;
-			tp->rx_opt.eff_sacks--;
+			tp->rx_opt.eff_sacks = tp->rx_opt.num_sacks;
 		}
 	}
 }