netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Duyck <alexander.duyck@gmail.com>
To: netdev@vger.kernel.org, willemb@google.com, davem@davemloft.net
Subject: [net-next PATCH 2/5] udp: Do not pass checksum or MSS as parameters
Date: Thu, 03 May 2018 17:33:27 -0700	[thread overview]
Message-ID: <20180504003326.4496.54432.stgit@localhost.localdomain> (raw)
In-Reply-To: <20180504002817.4496.64616.stgit@localhost.localdomain>

From: Alexander Duyck <alexander.h.duyck@intel.com>

This patch is meant to be a start at cleaning up some of the UDP GSO
segmentation code. Specifically we were passing mss and a recomputed
checksum when we really didn't need to. The function itself could derive
that information based on the already provided checksum, the length of the
packet stored in the UDP header, and the gso_size.

Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
 include/net/udp.h      |    3 +-
 net/ipv4/udp_offload.c |   61 +++++++++++++++++++++++++++++++-----------------
 net/ipv6/udp_offload.c |    7 +-----
 3 files changed, 41 insertions(+), 30 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 05990746810e..9289b6425032 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -175,8 +175,7 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
 int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
 
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
-				  netdev_features_t features,
-				  unsigned int mss, __sum16 check);
+				  netdev_features_t features);
 
 static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
 {
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 006257092f06..946d06d2aa0c 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -188,19 +188,23 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
 EXPORT_SYMBOL(skb_udp_tunnel_segment);
 
 struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
-				  netdev_features_t features,
-				  unsigned int mss, __sum16 check)
+				  netdev_features_t features)
 {
+	struct sk_buff *seg, *segs = ERR_PTR(-EINVAL);
 	struct sock *sk = gso_skb->sk;
 	unsigned int sum_truesize = 0;
-	struct sk_buff *segs, *seg;
-	unsigned int hdrlen;
 	struct udphdr *uh;
+	unsigned int mss;
+	__sum16 check;
+	__be16 newlen;
 
+	mss = skb_shinfo(gso_skb)->gso_size;
 	if (gso_skb->len <= sizeof(*uh) + mss)
-		return ERR_PTR(-EINVAL);
+		goto out;
+
+	if (!pskb_may_pull(gso_skb, sizeof(*uh)))
+		goto out;
 
-	hdrlen = gso_skb->data - skb_mac_header(gso_skb);
 	skb_pull(gso_skb, sizeof(*uh));
 
 	/* clear destructor to avoid skb_segment assigning it to tail */
@@ -213,24 +217,42 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 		return segs;
 	}
 
-	for (seg = segs; seg; seg = seg->next) {
-		uh = udp_hdr(seg);
-		uh->len = htons(seg->len - hdrlen);
-		uh->check = check;
+	seg = segs;
+	uh = udp_hdr(seg);
 
-		/* last packet can be partial gso_size */
-		if (!seg->next)
-			csum_replace2(&uh->check, htons(mss),
-				      htons(seg->len - hdrlen - sizeof(*uh)));
+	/* compute checksum adjustment based on old length versus new */
+	newlen = htons(sizeof(*uh) + mss);
+	check = ~csum_fold((__force __wsum)((__force u32)uh->check +
+					    ((__force u32)uh->len ^ 0xFFFF) +
+					    (__force u32)newlen));
 
-		uh->check = ~uh->check;
+	for (;;) {
 		seg->destructor = sock_wfree;
 		seg->sk = sk;
 		sum_truesize += seg->truesize;
+
+		if (!seg->next)
+			break;
+
+		uh->len = newlen;
+		uh->check = check;
+
+		seg = seg->next;
+		uh = udp_hdr(seg);
 	}
 
-	refcount_add(sum_truesize - gso_skb->truesize, &sk->sk_wmem_alloc);
+	/* last packet can be partial gso_size, account for that in checksum */
+	newlen = htons(skb_tail_pointer(seg) - skb_transport_header(seg) +
+		       seg->data_len);
+	check = ~csum_fold((__force __wsum)((__force u32)uh->check +
+					    ((__force u32)uh->len ^ 0xFFFF)  +
+					    (__force u32)newlen));
+	uh->len = newlen;
+	uh->check = check;
 
+	/* update refcount for the packet */
+	refcount_add(sum_truesize - gso_skb->truesize, &sk->sk_wmem_alloc);
+out:
 	return segs;
 }
 EXPORT_SYMBOL_GPL(__udp_gso_segment);
@@ -238,15 +260,10 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
 static struct sk_buff *__udp4_gso_segment(struct sk_buff *gso_skb,
 					  netdev_features_t features)
 {
-	const struct iphdr *iph = ip_hdr(gso_skb);
-	unsigned int mss = skb_shinfo(gso_skb)->gso_size;
-
 	if (!can_checksum_protocol(features, htons(ETH_P_IP)))
 		return ERR_PTR(-EIO);
 
-	return __udp_gso_segment(gso_skb, features, mss,
-				 udp_v4_check(sizeof(struct udphdr) + mss,
-					      iph->saddr, iph->daddr, 0));
+	return __udp_gso_segment(gso_skb, features);
 }
 
 static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index f7b85b1e6b3e..61e34f1d2fa2 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -20,15 +20,10 @@
 static struct sk_buff *__udp6_gso_segment(struct sk_buff *gso_skb,
 					  netdev_features_t features)
 {
-	const struct ipv6hdr *ip6h = ipv6_hdr(gso_skb);
-	unsigned int mss = skb_shinfo(gso_skb)->gso_size;
-
 	if (!can_checksum_protocol(features, htons(ETH_P_IPV6)))
 		return ERR_PTR(-EIO);
 
-	return __udp_gso_segment(gso_skb, features, mss,
-				 udp_v6_check(sizeof(struct udphdr) + mss,
-					      &ip6h->saddr, &ip6h->daddr, 0));
+	return __udp_gso_segment(gso_skb, features);
 }
 
 static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,

  parent reply	other threads:[~2018-05-04  0:33 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-04  0:33 [net-next PATCH 0/5] UDP GSO Segmentation clean-ups Alexander Duyck
2018-05-04  0:33 ` [net-next PATCH 1/5] udp: Record gso_segs when supporting UDP segmentation offload Alexander Duyck
2018-05-04  1:50   ` Eric Dumazet
2018-05-04  0:33 ` Alexander Duyck [this message]
2018-05-04  1:56   ` [net-next PATCH 2/5] udp: Do not pass checksum or MSS as parameters Eric Dumazet
2018-05-04 14:27     ` Alexander Duyck
2018-05-04  0:33 ` [net-next PATCH 3/5] udp: Add support for software checksum and GSO_PARTIAL with GSO offload Alexander Duyck
2018-05-04  3:50   ` Eric Dumazet
2018-05-04 14:22     ` Alexander Duyck
2018-05-04  0:33 ` [net-next PATCH 4/5] udp: Do not copy destructor if one is not present Alexander Duyck
2018-05-04  1:58   ` Eric Dumazet
2018-05-04 14:23     ` Alexander Duyck
2018-05-04  0:33 ` [net-next PATCH 5/5] net: Add NETIF_F_GSO_UDP_L4 to list of GSO offloads with fallback Alexander Duyck

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=20180504003326.4496.54432.stgit@localhost.localdomain \
    --to=alexander.duyck@gmail.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=willemb@google.com \
    /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).