* [PATCH net-2.6.24 0/4]: TCP fixes @ 2007-10-01 12:29 Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 1/4] [TCP]: No fackets_out/highest_sack tuning when SACK isn't enabled Ilpo Järvinen 2007-10-01 22:29 ` [PATCH net-2.6.24 0/4]: TCP fixes David Miller 0 siblings, 2 replies; 6+ messages in thread From: Ilpo Järvinen @ 2007-10-01 12:29 UTC (permalink / raw) To: David Miller; +Cc: netdev Hi Dave, This fixes the newreno fackets_out case, which turned out to be not related to the Cedric's case being under investigation. Two trivial comment patches, and frto with high-speed seqno wrap-around protection. Compile tested. Please apply to net-2.6.24. -- i. ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/4] [TCP]: No fackets_out/highest_sack tuning when SACK isn't enabled 2007-10-01 12:29 [PATCH net-2.6.24 0/4]: TCP fixes Ilpo Järvinen @ 2007-10-01 12:29 ` Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 2/4] [TCP]: fix comments that got messed up during code move Ilpo Järvinen 2007-10-01 22:29 ` [PATCH net-2.6.24 0/4]: TCP fixes David Miller 1 sibling, 1 reply; 6+ messages in thread From: Ilpo Järvinen @ 2007-10-01 12:29 UTC (permalink / raw) To: David Miller; +Cc: netdev This was found due to bug report from Cedric Le Goater though it turned this turned out to be unrelated bug. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_output.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 94c8011..6199abe 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -660,7 +660,7 @@ static void tcp_set_skb_tso_segs(struct sock *sk, struct sk_buff *skb, unsigned static void tcp_adjust_fackets_out(struct tcp_sock *tp, struct sk_buff *skb, int decr) { - if (!tp->sacked_out) + if (!tp->sacked_out || tcp_is_reno(tp)) return; if (!before(tp->highest_sack, TCP_SKB_CB(skb)->seq)) @@ -712,7 +712,8 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss TCP_SKB_CB(buff)->end_seq = TCP_SKB_CB(skb)->end_seq; TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(buff)->seq; - if (tp->sacked_out && (TCP_SKB_CB(skb)->seq == tp->highest_sack)) + if (tcp_is_sack(tp) && tp->sacked_out && + (TCP_SKB_CB(skb)->seq == tp->highest_sack)) tp->highest_sack = TCP_SKB_CB(buff)->seq; /* PSH and FIN should only be set in the second packet. */ @@ -1718,7 +1719,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m BUG_ON(tcp_skb_pcount(skb) != 1 || tcp_skb_pcount(next_skb) != 1); - if (WARN_ON(tp->sacked_out && + if (WARN_ON(tcp_is_sack(tp) && tp->sacked_out && (TCP_SKB_CB(next_skb)->seq == tp->highest_sack))) return; -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] [TCP]: fix comments that got messed up during code move 2007-10-01 12:29 ` [PATCH 1/4] [TCP]: No fackets_out/highest_sack tuning when SACK isn't enabled Ilpo Järvinen @ 2007-10-01 12:29 ` Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 3/4] [TCP]: Update comment of SACK block validator Ilpo Järvinen 0 siblings, 1 reply; 6+ messages in thread From: Ilpo Järvinen @ 2007-10-01 12:29 UTC (permalink / raw) To: David Miller; +Cc: netdev Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_input.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 2286361..135f046 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -1467,8 +1467,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_ return flag; } -/* F-RTO can only be used if TCP has never retransmitted anything other than - * head (SACK enhanced variant from Appendix B of RFC4138 is more robust here) +/* If we receive more dupacks than we expected counting segments + * in assumption of absent reordering, interpret this as reordering. + * The only another reason could be bug in receiver TCP. */ static void tcp_check_reno_reordering(struct sock *sk, const int addend) { @@ -1516,6 +1517,9 @@ static inline void tcp_reset_reno_sack(struct tcp_sock *tp) tp->sacked_out = 0; } +/* F-RTO can only be used if TCP has never retransmitted anything other than + * head (SACK enhanced variant from Appendix B of RFC4138 is more robust here) + */ int tcp_use_frto(struct sock *sk) { const struct tcp_sock *tp = tcp_sk(sk); -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] [TCP]: Update comment of SACK block validator 2007-10-01 12:29 ` [PATCH 2/4] [TCP]: fix comments that got messed up during code move Ilpo Järvinen @ 2007-10-01 12:29 ` Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 4/4] [TCP]: Wrap-safed reordering detection FRTO check Ilpo Järvinen 0 siblings, 1 reply; 6+ messages in thread From: Ilpo Järvinen @ 2007-10-01 12:29 UTC (permalink / raw) To: David Miller; +Cc: netdev Just came across what RFC2018 states about generation of valid SACK blocks in case of reneging. Alter comment a bit to point out clearly. IMHO, there isn't any reason to change code because the validation is there for a purpose (counters will inform user about decision TCP made if this case ever surfaces). Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_input.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 135f046..cec2611 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -1027,8 +1027,15 @@ static void tcp_update_reordering(struct sock *sk, const int metric, * SACK block range validation checks that the received SACK block fits to * the expected sequence limits, i.e., it is between SND.UNA and SND.NXT. * Note that SND.UNA is not included to the range though being valid because - * it means that the receiver is rather inconsistent with itself (reports - * SACK reneging when it should advance SND.UNA). + * it means that the receiver is rather inconsistent with itself reporting + * SACK reneging when it should advance SND.UNA. Such SACK block this is + * perfectly valid, however, in light of RFC2018 which explicitly states + * that "SACK block MUST reflect the newest segment. Even if the newest + * segment is going to be discarded ...", not that it looks very clever + * in case of head skb. Due to potentional receiver driven attacks, we + * choose to avoid immediate execution of a walk in write queue due to + * reneging and defer head skb's loss recovery to standard loss recovery + * procedure that will eventually trigger (nothing forbids us doing this). * * Implements also blockage to start_seq wrap-around. Problem lies in the * fact that though start_seq (s) is before end_seq (i.e., not reversed), -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] [TCP]: Wrap-safed reordering detection FRTO check 2007-10-01 12:29 ` [PATCH 3/4] [TCP]: Update comment of SACK block validator Ilpo Järvinen @ 2007-10-01 12:29 ` Ilpo Järvinen 0 siblings, 0 replies; 6+ messages in thread From: Ilpo Järvinen @ 2007-10-01 12:29 UTC (permalink / raw) To: David Miller; +Cc: netdev In case somebody has a suggestion about a better place for this check, which must guarantee execution "early enough" (i.e, before the wrap can occur), I'm very open to them. Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> --- net/ipv4/tcp_input.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index cec2611..e22ffe7 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -3024,6 +3024,9 @@ static int tcp_ack(struct sock *sk, struct sk_buff *skb, int flag) /* See if we can take anything off of the retransmit queue. */ flag |= tcp_clean_rtx_queue(sk, &seq_rtt); + /* Guarantee sacktag reordering detection against wrap-arounds */ + if (before(tp->frto_highmark, tp->snd_una)) + tp->frto_highmark = 0; if (tp->frto_counter) frto_cwnd = tcp_process_frto(sk, flag); -- 1.5.0.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-2.6.24 0/4]: TCP fixes 2007-10-01 12:29 [PATCH net-2.6.24 0/4]: TCP fixes Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 1/4] [TCP]: No fackets_out/highest_sack tuning when SACK isn't enabled Ilpo Järvinen @ 2007-10-01 22:29 ` David Miller 1 sibling, 0 replies; 6+ messages in thread From: David Miller @ 2007-10-01 22:29 UTC (permalink / raw) To: ilpo.jarvinen; +Cc: netdev From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Mon, 1 Oct 2007 15:29:40 +0300 > This fixes the newreno fackets_out case, which turned out to be > not related to the Cedric's case being under investigation. Two > trivial comment patches, and frto with high-speed seqno > wrap-around protection. Compile tested. Please apply to > net-2.6.24. I've applied them all to net-2.6.24, thanks Ilpo! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-10-01 22:29 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2007-10-01 12:29 [PATCH net-2.6.24 0/4]: TCP fixes Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 1/4] [TCP]: No fackets_out/highest_sack tuning when SACK isn't enabled Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 2/4] [TCP]: fix comments that got messed up during code move Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 3/4] [TCP]: Update comment of SACK block validator Ilpo Järvinen 2007-10-01 12:29 ` [PATCH 4/4] [TCP]: Wrap-safed reordering detection FRTO check Ilpo Järvinen 2007-10-01 22:29 ` [PATCH net-2.6.24 0/4]: TCP fixes 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).