From: Alexander Duyck <alexander.h.duyck@intel.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, edumazet@google.com, herbert@gondor.apana.org
Subject: [PATCH v3] net: Do not include padding in TCP GRO checksum
Date: Fri, 15 Nov 2013 15:00:34 -0800 [thread overview]
Message-ID: <20131115225856.6988.69733.stgit@ahduyck-fpga.jf.intel.com> (raw)
In some recent tests I found the TCP checksum was being treated as valid
for certain frames with padding on them. On closer inspection I found the
issue was that GRO was using the skb->len instead of the length recorded in
the IP/IPv6 header to determine the number of bytes to checksum. As such
padded frames that actually had invalid checksums generated by adding the
padding to the checksum were being incorrectly tagged as valid.
This change corrects that by using the tot_len from IPv4 headers and the
payload_len from IPv6 headers to compute the total number of bytes to be
included in the checksum.
To address the fact that skb->csum is invalid when a padded frame is
received I have updated the code to fall though to the CHECKSUM_NONE path
for CHECKSUM_COMPLETE frames that contain padding.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v2: Update byte ordering of tot_len and payload_len so it is in host order.
Updated CHECKSUM_COMPLETE path so it falls back through CHECKSUM_NONE for
padded frames since this is how it is handled in ip_rcv.
I have tested and verified the CHECKSUM_NONE path works, but I don't have
any adapters that generate CHECKSUM_COMPLETE to test with.
v3: Added check to handle case where length is greater than skb_gro_len.
net/ipv4/tcp_offload.c | 30 +++++++++++++++++++++---------
net/ipv6/tcpv6_offload.c | 31 +++++++++++++++++++++----------
2 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index a2b68a1..b32f6c3 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -273,26 +273,38 @@ static int tcp_v4_gso_send_check(struct sk_buff *skb)
static struct sk_buff **tcp4_gro_receive(struct sk_buff **head, struct sk_buff *skb)
{
const struct iphdr *iph = skb_gro_network_header(skb);
+ int length = ntohs(iph->tot_len);
__wsum wsum;
__sum16 sum;
+ /* adjust for any offsets */
+ length += skb_network_offset(skb) - skb_gro_offset(skb);
+
+ /* verify the entire packet is here */
+ if (length > skb_gro_len(skb))
+ goto flush;
+
switch (skb->ip_summed) {
case CHECKSUM_COMPLETE:
- if (!tcp_v4_check(skb_gro_len(skb), iph->saddr, iph->daddr,
- skb->csum)) {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- break;
- }
+ if (length == skb_gro_len(skb)) {
+ if (!tcp_v4_check(length, iph->saddr, iph->daddr,
+ skb->csum)) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ break;
+ }
flush:
- NAPI_GRO_CB(skb)->flush = 1;
- return NULL;
+ NAPI_GRO_CB(skb)->flush = 1;
+ return NULL;
+ }
+ /* skb->csum is invalid if frame is padded */
+ skb->ip_summed = CHECKSUM_NONE;
case CHECKSUM_NONE:
wsum = csum_tcpudp_nofold(iph->saddr, iph->daddr,
- skb_gro_len(skb), IPPROTO_TCP, 0);
+ length, IPPROTO_TCP, 0);
sum = csum_fold(skb_checksum(skb,
skb_gro_offset(skb),
- skb_gro_len(skb),
+ length,
wsum));
if (sum)
goto flush;
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index c1097c7..f6047cc 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -36,27 +36,38 @@ static struct sk_buff **tcp6_gro_receive(struct sk_buff **head,
struct sk_buff *skb)
{
const struct ipv6hdr *iph = skb_gro_network_header(skb);
+ int length = ntohs(iph->payload_len);
__wsum wsum;
__sum16 sum;
+ /* adjust for any offset due to extension headers */
+ length += skb_transport_offset(skb) - skb_gro_offset(skb);
+
+ /* verify the entire packet is here */
+ if (length > skb_gro_len(skb))
+ goto flush;
+
switch (skb->ip_summed) {
case CHECKSUM_COMPLETE:
- if (!tcp_v6_check(skb_gro_len(skb), &iph->saddr, &iph->daddr,
- skb->csum)) {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- break;
- }
+ if (length == skb_gro_len(skb)) {
+ if (!tcp_v6_check(length, &iph->saddr, &iph->daddr,
+ skb->csum)) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ break;
+ }
flush:
- NAPI_GRO_CB(skb)->flush = 1;
- return NULL;
+ NAPI_GRO_CB(skb)->flush = 1;
+ return NULL;
+ }
+ /* skb->csum is invalid if frame is padded */
+ skb->ip_summed = CHECKSUM_NONE;
case CHECKSUM_NONE:
wsum = ~csum_unfold(csum_ipv6_magic(&iph->saddr, &iph->daddr,
- skb_gro_len(skb),
- IPPROTO_TCP, 0));
+ length, IPPROTO_TCP, 0));
sum = csum_fold(skb_checksum(skb,
skb_gro_offset(skb),
- skb_gro_len(skb),
+ length,
wsum));
if (sum)
goto flush;
next reply other threads:[~2013-11-15 23:09 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-15 23:00 Alexander Duyck [this message]
2013-11-16 0:47 ` [PATCH v3] net: Do not include padding in TCP GRO checksum Herbert Xu
2013-11-16 1:34 ` David Miller
2013-11-16 1:43 ` Herbert Xu
2013-11-16 2:11 ` David Miller
2013-11-16 1:53 ` Herbert Xu
2013-11-16 4:10 ` Alexander Duyck
2013-11-16 6:46 ` Herbert Xu
2013-11-16 7:23 ` Herbert Xu
2013-11-16 17:02 ` Alexander Duyck
2013-11-17 3:17 ` Herbert Xu
2013-11-17 18:24 ` Alexander Duyck
2013-11-18 0:03 ` Herbert Xu
2013-11-18 17:43 ` Alexander Duyck
2013-11-21 18:35 ` David Miller
2013-11-22 2:30 ` Herbert Xu
2013-11-22 2:31 ` [1/2] gro: Only verify TCP checksums for candidates Herbert Xu
2013-11-22 5:55 ` Eric Dumazet
2013-11-23 22:47 ` David Miller
2013-11-22 2:32 ` [2/2] gro: Clean up tcpX_gro_receive checksum verification Herbert Xu
2013-11-22 5:58 ` Eric Dumazet
2013-11-23 22:47 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20131115225856.6988.69733.stgit@ahduyck-fpga.jf.intel.com \
--to=alexander.h.duyck@intel.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=herbert@gondor.apana.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).