* [PATCH] tcp: fix wrong checksum calculation on MTU probing
@ 2016-09-19 21:16 Douglas Caetano dos Santos
2016-09-21 2:57 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Douglas Caetano dos Santos @ 2016-09-19 21:16 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev
With TCP MTU probing enabled and offload TX checksumming disabled,
tcp_mtu_probe() calculated the wrong checksum when a fragment being copied
into the probe's SKB had an odd length. This was caused by the direct use
of skb_copy_and_csum_bits() to calculate the checksum, as it pads the
fragment being copied, if needed. When this fragment was not the last, a
subsequent call used the previous checksum without considering this
padding.
The effect was a stale connection in one way, as even retransmissions
wouldn't solve the problem, because the checksum was never recalculated for
the full SKB length.
Signed-off-by: Douglas Caetano dos Santos <douglascs@taghos.com.br>
---
net/ipv4/tcp_output.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f53d0cc..767135e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1968,10 +1968,12 @@ static int tcp_mtu_probe(struct sock *sk)
copy = min_t(int, skb->len, probe_size - len);
if (nskb->ip_summed)
skb_copy_bits(skb, 0, skb_put(nskb, copy), copy);
- else
- nskb->csum = skb_copy_and_csum_bits(skb, 0,
- skb_put(nskb, copy),
- copy, nskb->csum);
+ else {
+ __wsum csum = skb_copy_and_csum_bits(skb, 0,
+ skb_put(nskb, copy),
+ copy, 0);
+ nskb->csum = csum_block_add(nskb->csum, csum, len);
+ }
if (skb->len <= copy) {
/* We've eaten all the data from this skb.
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] tcp: fix wrong checksum calculation on MTU probing
2016-09-19 21:16 [PATCH] tcp: fix wrong checksum calculation on MTU probing Douglas Caetano dos Santos
@ 2016-09-21 2:57 ` David Miller
2016-09-21 18:26 ` [PATCH v2] " Douglas Caetano dos Santos
0 siblings, 1 reply; 6+ messages in thread
From: David Miller @ 2016-09-21 2:57 UTC (permalink / raw)
To: douglascs; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev
This patch is whitespace damaged by your email client.
Please fix this, email the patch to yourself, and only resubmit this
when you can successfully apply the patch you emailed to yourself.
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] tcp: fix wrong checksum calculation on MTU probing
2016-09-21 2:57 ` David Miller
@ 2016-09-21 18:26 ` Douglas Caetano dos Santos
2016-09-22 11:12 ` Sergei Shtylyov
0 siblings, 1 reply; 6+ messages in thread
From: Douglas Caetano dos Santos @ 2016-09-21 18:26 UTC (permalink / raw)
To: David Miller; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev
With TCP MTU probing enabled and offload TX checksumming disabled,
tcp_mtu_probe() calculated the wrong checksum when a fragment being copied
into the probe's SKB had an odd length. This was caused by the direct use
of skb_copy_and_csum_bits() to calculate the checksum, as it pads the
fragment being copied, if needed. When this fragment was not the last, a
subsequent call used the previous checksum without considering this
padding.
The effect was a stale connection in one way, as even retransmissions
wouldn't solve the problem, because the checksum was never recalculated for
the full SKB length.
Signed-off-by: Douglas Caetano dos Santos <douglascs@taghos.com.br>
---
net/ipv4/tcp_output.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f53d0cc..767135e 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1968,10 +1968,12 @@ static int tcp_mtu_probe(struct sock *sk)
copy = min_t(int, skb->len, probe_size - len);
if (nskb->ip_summed)
skb_copy_bits(skb, 0, skb_put(nskb, copy), copy);
- else
- nskb->csum = skb_copy_and_csum_bits(skb, 0,
- skb_put(nskb, copy),
- copy, nskb->csum);
+ else {
+ __wsum csum = skb_copy_and_csum_bits(skb, 0,
+ skb_put(nskb, copy),
+ copy, 0);
+ nskb->csum = csum_block_add(nskb->csum, csum, len);
+ }
if (skb->len <= copy) {
/* We've eaten all the data from this skb.
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] tcp: fix wrong checksum calculation on MTU probing
2016-09-21 18:26 ` [PATCH v2] " Douglas Caetano dos Santos
@ 2016-09-22 11:12 ` Sergei Shtylyov
2016-09-22 18:52 ` [PATCH v3] " Douglas Caetano dos Santos
0 siblings, 1 reply; 6+ messages in thread
From: Sergei Shtylyov @ 2016-09-22 11:12 UTC (permalink / raw)
To: Douglas Caetano dos Santos, David Miller
Cc: kuznet, jmorris, yoshfuji, kaber, netdev
Hello.
On 9/21/2016 9:26 PM, Douglas Caetano dos Santos wrote:
> With TCP MTU probing enabled and offload TX checksumming disabled,
> tcp_mtu_probe() calculated the wrong checksum when a fragment being copied
> into the probe's SKB had an odd length. This was caused by the direct use
> of skb_copy_and_csum_bits() to calculate the checksum, as it pads the
> fragment being copied, if needed. When this fragment was not the last, a
> subsequent call used the previous checksum without considering this
> padding.
>
> The effect was a stale connection in one way, as even retransmissions
> wouldn't solve the problem, because the checksum was never recalculated for
> the full SKB length.
>
> Signed-off-by: Douglas Caetano dos Santos <douglascs@taghos.com.br>
> ---
> net/ipv4/tcp_output.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index f53d0cc..767135e 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -1968,10 +1968,12 @@ static int tcp_mtu_probe(struct sock *sk)
> copy = min_t(int, skb->len, probe_size - len);
> if (nskb->ip_summed)
> skb_copy_bits(skb, 0, skb_put(nskb, copy), copy);
> - else
> - nskb->csum = skb_copy_and_csum_bits(skb, 0,
> - skb_put(nskb, copy),
> - copy, nskb->csum);
> + else {
CodingStyle: now the first branch needs {} too.
[...]
MBR, Sergei
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v3] tcp: fix wrong checksum calculation on MTU probing
2016-09-22 11:12 ` Sergei Shtylyov
@ 2016-09-22 18:52 ` Douglas Caetano dos Santos
2016-09-23 11:58 ` David Miller
0 siblings, 1 reply; 6+ messages in thread
From: Douglas Caetano dos Santos @ 2016-09-22 18:52 UTC (permalink / raw)
To: Sergei Shtylyov, David Miller; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev
With TCP MTU probing enabled and offload TX checksumming disabled,
tcp_mtu_probe() calculated the wrong checksum when a fragment being copied
into the probe's SKB had an odd length. This was caused by the direct use
of skb_copy_and_csum_bits() to calculate the checksum, as it pads the
fragment being copied, if needed. When this fragment was not the last, a
subsequent call used the previous checksum without considering this
padding.
The effect was a stale connection in one way, as even retransmissions
wouldn't solve the problem, because the checksum was never recalculated for
the full SKB length.
Signed-off-by: Douglas Caetano dos Santos <douglascs@taghos.com.br>
---
net/ipv4/tcp_output.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f53d0cc..2d32952 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1966,12 +1966,14 @@ static int tcp_mtu_probe(struct sock *sk)
len = 0;
tcp_for_write_queue_from_safe(skb, next, sk) {
copy = min_t(int, skb->len, probe_size - len);
- if (nskb->ip_summed)
+ if (nskb->ip_summed) {
skb_copy_bits(skb, 0, skb_put(nskb, copy), copy);
- else
- nskb->csum = skb_copy_and_csum_bits(skb, 0,
- skb_put(nskb, copy),
- copy, nskb->csum);
+ } else {
+ __wsum csum = skb_copy_and_csum_bits(skb, 0,
+ skb_put(nskb, copy),
+ copy, 0);
+ nskb->csum = csum_block_add(nskb->csum, csum, len);
+ }
if (skb->len <= copy) {
/* We've eaten all the data from this skb.
--
2.5.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v3] tcp: fix wrong checksum calculation on MTU probing
2016-09-22 18:52 ` [PATCH v3] " Douglas Caetano dos Santos
@ 2016-09-23 11:58 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2016-09-23 11:58 UTC (permalink / raw)
To: douglascs; +Cc: sergei.shtylyov, kuznet, jmorris, yoshfuji, kaber, netdev
From: Douglas Caetano dos Santos <douglascs@taghos.com.br>
Date: Thu, 22 Sep 2016 15:52:04 -0300
> With TCP MTU probing enabled and offload TX checksumming disabled,
> tcp_mtu_probe() calculated the wrong checksum when a fragment being copied
> into the probe's SKB had an odd length. This was caused by the direct use
> of skb_copy_and_csum_bits() to calculate the checksum, as it pads the
> fragment being copied, if needed. When this fragment was not the last, a
> subsequent call used the previous checksum without considering this
> padding.
>
> The effect was a stale connection in one way, as even retransmissions
> wouldn't solve the problem, because the checksum was never recalculated for
> the full SKB length.
>
> Signed-off-by: Douglas Caetano dos Santos <douglascs@taghos.com.br>
Applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-09-23 11:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-19 21:16 [PATCH] tcp: fix wrong checksum calculation on MTU probing Douglas Caetano dos Santos
2016-09-21 2:57 ` David Miller
2016-09-21 18:26 ` [PATCH v2] " Douglas Caetano dos Santos
2016-09-22 11:12 ` Sergei Shtylyov
2016-09-22 18:52 ` [PATCH v3] " Douglas Caetano dos Santos
2016-09-23 11:58 ` 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).