* [net-next PATCH 01/10] net: Drop unecessary enc_features variable from tunnel segmentation functions
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
@ 2016-02-05 23:27 ` Alexander Duyck
2016-02-06 20:38 ` Tom Herbert
2016-02-05 23:27 ` [net-next PATCH 02/10] net: Move GSO csum into SKB_GSO_CB Alexander Duyck
` (9 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:27 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
The enc_features variable isn't necessary since features isn't used
anywhere after we create enc_features so instead just use a destructive AND
on features itself and save ourselves the variable declaration.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/ipv4/gre_offload.c | 6 +++---
net/ipv4/udp_offload.c | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 5a8ee3282550..02cb1a416c7d 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -19,7 +19,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
netdev_features_t features)
{
struct sk_buff *segs = ERR_PTR(-EINVAL);
- netdev_features_t enc_features;
int ghl;
struct gre_base_hdr *greh;
u16 mac_offset = skb->mac_header;
@@ -68,9 +67,10 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
skb_set_network_header(skb, skb_inner_network_offset(skb));
skb->mac_len = skb_inner_network_offset(skb);
+ features &= skb->dev->hw_enc_features;
+
/* segment inner packet. */
- enc_features = skb->dev->hw_enc_features & features;
- segs = skb_mac_gso_segment(skb, enc_features);
+ segs = skb_mac_gso_segment(skb, features);
if (IS_ERR_OR_NULL(segs)) {
skb_gso_error_unwind(skb, protocol, ghl, mac_offset, mac_len);
goto out;
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 4c519c1dc161..ce64c2b7ba55 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -37,7 +37,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
int mac_len = skb->mac_len;
int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
__be16 protocol = skb->protocol;
- netdev_features_t enc_features;
int udp_offset, outer_hlen;
unsigned int oldlen;
bool need_csum = !!(skb_shinfo(skb)->gso_type &
@@ -65,9 +64,10 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
(skb->dev->features & (is_ipv6 ?
NETIF_F_IPV6_CSUM : NETIF_F_IP_CSUM))));
+ features &= skb->dev->hw_enc_features;
+
/* segment inner packet. */
- enc_features = skb->dev->hw_enc_features & features;
- segs = gso_inner_segment(skb, enc_features);
+ segs = gso_inner_segment(skb, features);
if (IS_ERR_OR_NULL(segs)) {
skb_gso_error_unwind(skb, protocol, tnl_hlen, mac_offset,
mac_len);
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 01/10] net: Drop unecessary enc_features variable from tunnel segmentation functions
2016-02-05 23:27 ` [net-next PATCH 01/10] net: Drop unecessary enc_features variable from tunnel segmentation functions Alexander Duyck
@ 2016-02-06 20:38 ` Tom Herbert
0 siblings, 0 replies; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:38 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:27 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> The enc_features variable isn't necessary since features isn't used
> anywhere after we create enc_features so instead just use a destructive AND
> on features itself and save ourselves the variable declaration.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Acked-by: Tom Herbert <tom@herbertland.com>
> ---
> net/ipv4/gre_offload.c | 6 +++---
> net/ipv4/udp_offload.c | 6 +++---
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index 5a8ee3282550..02cb1a416c7d 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -19,7 +19,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> netdev_features_t features)
> {
> struct sk_buff *segs = ERR_PTR(-EINVAL);
> - netdev_features_t enc_features;
> int ghl;
> struct gre_base_hdr *greh;
> u16 mac_offset = skb->mac_header;
> @@ -68,9 +67,10 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> skb_set_network_header(skb, skb_inner_network_offset(skb));
> skb->mac_len = skb_inner_network_offset(skb);
>
> + features &= skb->dev->hw_enc_features;
> +
> /* segment inner packet. */
> - enc_features = skb->dev->hw_enc_features & features;
> - segs = skb_mac_gso_segment(skb, enc_features);
> + segs = skb_mac_gso_segment(skb, features);
> if (IS_ERR_OR_NULL(segs)) {
> skb_gso_error_unwind(skb, protocol, ghl, mac_offset, mac_len);
> goto out;
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 4c519c1dc161..ce64c2b7ba55 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -37,7 +37,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> int mac_len = skb->mac_len;
> int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> __be16 protocol = skb->protocol;
> - netdev_features_t enc_features;
> int udp_offset, outer_hlen;
> unsigned int oldlen;
> bool need_csum = !!(skb_shinfo(skb)->gso_type &
> @@ -65,9 +64,10 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> (skb->dev->features & (is_ipv6 ?
> NETIF_F_IPV6_CSUM : NETIF_F_IP_CSUM))));
>
> + features &= skb->dev->hw_enc_features;
> +
> /* segment inner packet. */
> - enc_features = skb->dev->hw_enc_features & features;
> - segs = gso_inner_segment(skb, enc_features);
> + segs = gso_inner_segment(skb, features);
> if (IS_ERR_OR_NULL(segs)) {
> skb_gso_error_unwind(skb, protocol, tnl_hlen, mac_offset,
> mac_len);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 02/10] net: Move GSO csum into SKB_GSO_CB
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
2016-02-05 23:27 ` [net-next PATCH 01/10] net: Drop unecessary enc_features variable from tunnel segmentation functions Alexander Duyck
@ 2016-02-05 23:27 ` Alexander Duyck
2016-02-06 20:41 ` Tom Herbert
2016-02-05 23:27 ` [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum Alexander Duyck
` (8 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:27 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
This patch moves the checksum maintained by GSO out of skb->csum and into
the GSO context block in order to allow for us to work on outer checksums
while maintaining the inner checksum offsets in the case of the inner
checksum being offloaded, while the outer checksums will be computed.
While updating the code I also did a minor cleanu-up on gso_make_checksum.
The change is mostly to make it so that we store the values and compute the
checksum instead of computing the checksum and then storing the values we
needed to update.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
include/linux/skbuff.h | 14 +++++++-------
net/core/skbuff.c | 16 +++++++++-------
2 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 61b8cef73296..33c3807b618a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3549,6 +3549,7 @@ static inline struct sec_path *skb_sec_path(struct sk_buff *skb)
struct skb_gso_cb {
int mac_offset;
int encap_level;
+ __wsum csum;
__u16 csum_start;
};
#define SKB_SGO_CB_OFFSET 32
@@ -3585,15 +3586,14 @@ static inline int gso_pskb_expand_head(struct sk_buff *skb, int extra)
*/
static inline __sum16 gso_make_checksum(struct sk_buff *skb, __wsum res)
{
- int plen = SKB_GSO_CB(skb)->csum_start - skb_headroom(skb) -
- skb_transport_offset(skb);
- __wsum partial;
+ unsigned char *csum_start = skb_transport_header(skb);
+ int plen = (skb->head + SKB_GSO_CB(skb)->csum_start) - csum_start;
+ __wsum partial = SKB_GSO_CB(skb)->csum;
- partial = csum_partial(skb_transport_header(skb), plen, skb->csum);
- skb->csum = res;
- SKB_GSO_CB(skb)->csum_start -= plen;
+ SKB_GSO_CB(skb)->csum = res;
+ SKB_GSO_CB(skb)->csum_start = csum_start - skb->head;
- return csum_fold(partial);
+ return csum_fold(csum_partial(csum_start, plen, partial));
}
static inline bool skb_is_gso(const struct sk_buff *skb)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index b2df375ec9c2..02c638a643ea 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3100,11 +3100,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
if (!sg && !nskb->remcsum_offload) {
nskb->ip_summed = CHECKSUM_NONE;
- nskb->csum = skb_copy_and_csum_bits(head_skb, offset,
- skb_put(nskb, len),
- len, 0);
+ SKB_GSO_CB(nskb)->csum =
+ skb_copy_and_csum_bits(head_skb, offset,
+ skb_put(nskb, len),
+ len, 0);
SKB_GSO_CB(nskb)->csum_start =
- skb_headroom(nskb) + doffset;
+ skb_headroom(nskb) + doffset;
continue;
}
@@ -3171,11 +3172,12 @@ skip_fraglist:
perform_csum_check:
if (!csum && !nskb->remcsum_offload) {
- nskb->csum = skb_checksum(nskb, doffset,
- nskb->len - doffset, 0);
nskb->ip_summed = CHECKSUM_NONE;
+ SKB_GSO_CB(nskb)->csum =
+ skb_checksum(nskb, doffset,
+ nskb->len - doffset, 0);
SKB_GSO_CB(nskb)->csum_start =
- skb_headroom(nskb) + doffset;
+ skb_headroom(nskb) + doffset;
}
} while ((offset += len) < head_skb->len);
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 02/10] net: Move GSO csum into SKB_GSO_CB
2016-02-05 23:27 ` [net-next PATCH 02/10] net: Move GSO csum into SKB_GSO_CB Alexander Duyck
@ 2016-02-06 20:41 ` Tom Herbert
0 siblings, 0 replies; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:41 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:27 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch moves the checksum maintained by GSO out of skb->csum and into
> the GSO context block in order to allow for us to work on outer checksums
> while maintaining the inner checksum offsets in the case of the inner
> checksum being offloaded, while the outer checksums will be computed.
>
> While updating the code I also did a minor cleanu-up on gso_make_checksum.
> The change is mostly to make it so that we store the values and compute the
> checksum instead of computing the checksum and then storing the values we
> needed to update.
>
Typo in commit log (cleanu-up)
Acked-by: Tom Herbert <tom@herbertland.com>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> include/linux/skbuff.h | 14 +++++++-------
> net/core/skbuff.c | 16 +++++++++-------
> 2 files changed, 16 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 61b8cef73296..33c3807b618a 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -3549,6 +3549,7 @@ static inline struct sec_path *skb_sec_path(struct sk_buff *skb)
> struct skb_gso_cb {
> int mac_offset;
> int encap_level;
> + __wsum csum;
> __u16 csum_start;
> };
> #define SKB_SGO_CB_OFFSET 32
> @@ -3585,15 +3586,14 @@ static inline int gso_pskb_expand_head(struct sk_buff *skb, int extra)
> */
> static inline __sum16 gso_make_checksum(struct sk_buff *skb, __wsum res)
> {
> - int plen = SKB_GSO_CB(skb)->csum_start - skb_headroom(skb) -
> - skb_transport_offset(skb);
> - __wsum partial;
> + unsigned char *csum_start = skb_transport_header(skb);
> + int plen = (skb->head + SKB_GSO_CB(skb)->csum_start) - csum_start;
> + __wsum partial = SKB_GSO_CB(skb)->csum;
>
> - partial = csum_partial(skb_transport_header(skb), plen, skb->csum);
> - skb->csum = res;
> - SKB_GSO_CB(skb)->csum_start -= plen;
> + SKB_GSO_CB(skb)->csum = res;
> + SKB_GSO_CB(skb)->csum_start = csum_start - skb->head;
>
> - return csum_fold(partial);
> + return csum_fold(csum_partial(csum_start, plen, partial));
> }
>
> static inline bool skb_is_gso(const struct sk_buff *skb)
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index b2df375ec9c2..02c638a643ea 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3100,11 +3100,12 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
>
> if (!sg && !nskb->remcsum_offload) {
> nskb->ip_summed = CHECKSUM_NONE;
> - nskb->csum = skb_copy_and_csum_bits(head_skb, offset,
> - skb_put(nskb, len),
> - len, 0);
> + SKB_GSO_CB(nskb)->csum =
> + skb_copy_and_csum_bits(head_skb, offset,
> + skb_put(nskb, len),
> + len, 0);
> SKB_GSO_CB(nskb)->csum_start =
> - skb_headroom(nskb) + doffset;
> + skb_headroom(nskb) + doffset;
> continue;
> }
>
> @@ -3171,11 +3172,12 @@ skip_fraglist:
>
> perform_csum_check:
> if (!csum && !nskb->remcsum_offload) {
> - nskb->csum = skb_checksum(nskb, doffset,
> - nskb->len - doffset, 0);
> nskb->ip_summed = CHECKSUM_NONE;
> + SKB_GSO_CB(nskb)->csum =
> + skb_checksum(nskb, doffset,
> + nskb->len - doffset, 0);
> SKB_GSO_CB(nskb)->csum_start =
> - skb_headroom(nskb) + doffset;
> + skb_headroom(nskb) + doffset;
> }
> } while ((offset += len) < head_skb->len);
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
2016-02-05 23:27 ` [net-next PATCH 01/10] net: Drop unecessary enc_features variable from tunnel segmentation functions Alexander Duyck
2016-02-05 23:27 ` [net-next PATCH 02/10] net: Move GSO csum into SKB_GSO_CB Alexander Duyck
@ 2016-02-05 23:27 ` Alexander Duyck
2016-02-06 20:44 ` Tom Herbert
2016-02-05 23:27 ` [net-next PATCH 04/10] net: Store checksum result for offloaded GSO checksums Alexander Duyck
` (7 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:27 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
This patch addresses two main issues.
First in the case of remote checksum offload we were avoiding dealing with
scatter-gather issues. As a result it would be possible to assemble a
series of frames that used frags instead of being linearized as they should
have if remote checksum offload was enabled.
Second I have updated the code so that we now let GSO take care of doing
the checksum on the data itself and drop the special case that was added
for remote checksum offload.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/skbuff.c | 10 ++++++----
net/ipv4/udp_offload.c | 22 ++++++++++------------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 02c638a643ea..9c065ac72e87 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3098,8 +3098,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
if (nskb->len == len + doffset)
goto perform_csum_check;
- if (!sg && !nskb->remcsum_offload) {
- nskb->ip_summed = CHECKSUM_NONE;
+ if (!sg) {
+ if (!nskb->remcsum_offload)
+ nskb->ip_summed = CHECKSUM_NONE;
SKB_GSO_CB(nskb)->csum =
skb_copy_and_csum_bits(head_skb, offset,
skb_put(nskb, len),
@@ -3171,8 +3172,9 @@ skip_fraglist:
nskb->truesize += nskb->data_len;
perform_csum_check:
- if (!csum && !nskb->remcsum_offload) {
- nskb->ip_summed = CHECKSUM_NONE;
+ if (!csum) {
+ if (!nskb->remcsum_offload)
+ nskb->ip_summed = CHECKSUM_NONE;
SKB_GSO_CB(nskb)->csum =
skb_checksum(nskb, doffset,
nskb->len - doffset, 0);
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index ce64c2b7ba55..86687f58d613 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -66,6 +66,16 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
features &= skb->dev->hw_enc_features;
+ /* The only checksum offload we care about from here on out is the
+ * outer one so strip the existing checksum feature flags and
+ * instead set the flag based on our outer checksum offload value.
+ */
+ if (remcsum) {
+ features &= ~NETIF_F_CSUM_MASK;
+ if (offload_csum)
+ features |= NETIF_F_HW_CSUM;
+ }
+
/* segment inner packet. */
segs = gso_inner_segment(skb, features);
if (IS_ERR_OR_NULL(segs)) {
@@ -116,18 +126,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
skb->ip_summed = CHECKSUM_PARTIAL;
skb->csum_start = skb_transport_header(skb) - skb->head;
skb->csum_offset = offsetof(struct udphdr, check);
- } else if (remcsum) {
- /* Need to calculate checksum from scratch,
- * inner checksums are never when doing
- * remote_checksum_offload.
- */
-
- skb->csum = skb_checksum(skb, udp_offset,
- skb->len - udp_offset,
- 0);
- uh->check = csum_fold(skb->csum);
- if (uh->check == 0)
- uh->check = CSUM_MANGLED_0;
} else {
uh->check = gso_make_checksum(skb, ~uh->check);
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum
2016-02-05 23:27 ` [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum Alexander Duyck
@ 2016-02-06 20:44 ` Tom Herbert
2016-02-07 10:13 ` Alexander Duyck
0 siblings, 1 reply; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:44 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:27 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch addresses two main issues.
>
> First in the case of remote checksum offload we were avoiding dealing with
> scatter-gather issues. As a result it would be possible to assemble a
> series of frames that used frags instead of being linearized as they should
> have if remote checksum offload was enabled.
>
> Second I have updated the code so that we now let GSO take care of doing
> the checksum on the data itself and drop the special case that was added
> for remote checksum offload.
>
Did you test this (with vxlan or GUE)?
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> net/core/skbuff.c | 10 ++++++----
> net/ipv4/udp_offload.c | 22 ++++++++++------------
> 2 files changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 02c638a643ea..9c065ac72e87 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3098,8 +3098,9 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> if (nskb->len == len + doffset)
> goto perform_csum_check;
>
> - if (!sg && !nskb->remcsum_offload) {
> - nskb->ip_summed = CHECKSUM_NONE;
> + if (!sg) {
> + if (!nskb->remcsum_offload)
> + nskb->ip_summed = CHECKSUM_NONE;
> SKB_GSO_CB(nskb)->csum =
> skb_copy_and_csum_bits(head_skb, offset,
> skb_put(nskb, len),
> @@ -3171,8 +3172,9 @@ skip_fraglist:
> nskb->truesize += nskb->data_len;
>
> perform_csum_check:
> - if (!csum && !nskb->remcsum_offload) {
> - nskb->ip_summed = CHECKSUM_NONE;
> + if (!csum) {
> + if (!nskb->remcsum_offload)
> + nskb->ip_summed = CHECKSUM_NONE;
> SKB_GSO_CB(nskb)->csum =
> skb_checksum(nskb, doffset,
> nskb->len - doffset, 0);
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index ce64c2b7ba55..86687f58d613 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -66,6 +66,16 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
>
> features &= skb->dev->hw_enc_features;
>
> + /* The only checksum offload we care about from here on out is the
> + * outer one so strip the existing checksum feature flags and
> + * instead set the flag based on our outer checksum offload value.
> + */
> + if (remcsum) {
> + features &= ~NETIF_F_CSUM_MASK;
> + if (offload_csum)
> + features |= NETIF_F_HW_CSUM;
> + }
> +
> /* segment inner packet. */
> segs = gso_inner_segment(skb, features);
> if (IS_ERR_OR_NULL(segs)) {
> @@ -116,18 +126,6 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> skb->ip_summed = CHECKSUM_PARTIAL;
> skb->csum_start = skb_transport_header(skb) - skb->head;
> skb->csum_offset = offsetof(struct udphdr, check);
> - } else if (remcsum) {
> - /* Need to calculate checksum from scratch,
> - * inner checksums are never when doing
> - * remote_checksum_offload.
> - */
> -
> - skb->csum = skb_checksum(skb, udp_offset,
> - skb->len - udp_offset,
> - 0);
> - uh->check = csum_fold(skb->csum);
> - if (uh->check == 0)
> - uh->check = CSUM_MANGLED_0;
> } else {
> uh->check = gso_make_checksum(skb, ~uh->check);
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum
2016-02-06 20:44 ` Tom Herbert
@ 2016-02-07 10:13 ` Alexander Duyck
0 siblings, 0 replies; 24+ messages in thread
From: Alexander Duyck @ 2016-02-07 10:13 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers, Edward Cree,
David S. Miller
On Sat, Feb 6, 2016 at 2:44 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, Feb 5, 2016 at 3:27 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch addresses two main issues.
>>
>> First in the case of remote checksum offload we were avoiding dealing with
>> scatter-gather issues. As a result it would be possible to assemble a
>> series of frames that used frags instead of being linearized as they should
>> have if remote checksum offload was enabled.
>>
>> Second I have updated the code so that we now let GSO take care of doing
>> the checksum on the data itself and drop the special case that was added
>> for remote checksum offload.
>>
> Did you test this (with vxlan or GUE)?
I was testing with VXLAN.
- Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 04/10] net: Store checksum result for offloaded GSO checksums
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (2 preceding siblings ...)
2016-02-05 23:27 ` [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum Alexander Duyck
@ 2016-02-05 23:27 ` Alexander Duyck
2016-02-05 23:27 ` [net-next PATCH 05/10] net: Move skb_has_shared_frag check out of GRE code and into segmentation Alexander Duyck
` (6 subsequent siblings)
10 siblings, 0 replies; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:27 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
This patch makes it so that we can offload the checksums for a packet up
to a certain point and then begin computing the checksums via software.
Setting this up is fairly straight forward as all we need to do is reset
the values stored in csum and csum_start for the GSO context block.
One complication for this is remote checksum offload. In order to allow
the inner checksums to be offloaded while computing the outer checksum
manually we needed to have some way of indicating that the offload wasn't
real. In order to do that I replaced CHECKSUM_PARTIAL with
CHECKSUM_UNNECESSARY in the case of us computing checksums for the outer
header while skipping computing checksums for the inner headers. We clean
up the ip_summed flag and set it to either CHECKSUM_PARTIAL or
CHECKSUM_NONE once we hand the packet off to the next lower level.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
include/linux/skbuff.h | 15 +++++++++++++++
net/ipv4/tcp_offload.c | 8 ++++++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 33c3807b618a..77ebb61e2352 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2161,6 +2161,11 @@ static inline int skb_checksum_start_offset(const struct sk_buff *skb)
return skb->csum_start - skb_headroom(skb);
}
+static inline unsigned char *skb_checksum_start(const struct sk_buff *skb)
+{
+ return skb->head + skb->csum_start;
+}
+
static inline int skb_transport_offset(const struct sk_buff *skb)
{
return skb_transport_header(skb) - skb->data;
@@ -3576,6 +3581,16 @@ static inline int gso_pskb_expand_head(struct sk_buff *skb, int extra)
return 0;
}
+static inline void gso_reset_checksum(struct sk_buff *skb, __wsum res)
+{
+ /* Do not update partial checksums if remote checksum is enabled. */
+ if (skb->remcsum_offload)
+ return;
+
+ SKB_GSO_CB(skb)->csum = res;
+ SKB_GSO_CB(skb)->csum_start = skb_checksum_start(skb) - skb->head;
+}
+
/* Compute the checksum for a gso segment. First compute the checksum value
* from the start of transport header to SKB_GSO_CB(skb)->csum_start, and
* then add in skb->csum (checksum from csum_start to end of packet).
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 9864a2dbadce..773083b7f1e9 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -135,7 +135,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
th->fin = th->psh = 0;
th->check = newcheck;
- if (skb->ip_summed != CHECKSUM_PARTIAL)
+ if (skb->ip_summed == CHECKSUM_PARTIAL)
+ gso_reset_checksum(skb, ~th->check);
+ else
th->check = gso_make_checksum(skb, ~th->check);
seq += mss;
@@ -169,7 +171,9 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
skb->data_len);
th->check = ~csum_fold((__force __wsum)((__force u32)th->check +
(__force u32)delta));
- if (skb->ip_summed != CHECKSUM_PARTIAL)
+ if (skb->ip_summed == CHECKSUM_PARTIAL)
+ gso_reset_checksum(skb, ~th->check);
+ else
th->check = gso_make_checksum(skb, ~th->check);
out:
return segs;
^ permalink raw reply related [flat|nested] 24+ messages in thread* [net-next PATCH 05/10] net: Move skb_has_shared_frag check out of GRE code and into segmentation
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (3 preceding siblings ...)
2016-02-05 23:27 ` [net-next PATCH 04/10] net: Store checksum result for offloaded GSO checksums Alexander Duyck
@ 2016-02-05 23:27 ` Alexander Duyck
2016-02-06 20:45 ` Tom Herbert
2016-02-05 23:28 ` [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags Alexander Duyck
` (5 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:27 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
The call skb_has_shared_frag is used in the GRE path and skb_checksum_help
to verify that no frags can be modified by an external entity. This check
really doesn't belong in the GRE path but in the skb_segment function
itself. This way any protocol that might be segmented will be performing
this check before attempting to offload a checksum to software.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/skbuff.c | 5 +++++
net/ipv4/gre_offload.c | 11 -----------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 9c065ac72e87..88262c82b96a 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3173,6 +3173,11 @@ skip_fraglist:
perform_csum_check:
if (!csum) {
+ if (skb_has_shared_frag(nskb)) {
+ err = __skb_linearize(nskb);
+ if (err)
+ goto err;
+ }
if (!nskb->remcsum_offload)
nskb->ip_summed = CHECKSUM_NONE;
SKB_GSO_CB(nskb)->csum =
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 02cb1a416c7d..35a8dd35ed4e 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -83,17 +83,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
if (csum) {
__be32 *pcsum;
- if (skb_has_shared_frag(skb)) {
- int err;
-
- err = __skb_linearize(skb);
- if (err) {
- kfree_skb_list(segs);
- segs = ERR_PTR(err);
- goto out;
- }
- }
-
skb_reset_transport_header(skb);
greh = (struct gre_base_hdr *)
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 05/10] net: Move skb_has_shared_frag check out of GRE code and into segmentation
2016-02-05 23:27 ` [net-next PATCH 05/10] net: Move skb_has_shared_frag check out of GRE code and into segmentation Alexander Duyck
@ 2016-02-06 20:45 ` Tom Herbert
0 siblings, 0 replies; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:45 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:27 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> The call skb_has_shared_frag is used in the GRE path and skb_checksum_help
> to verify that no frags can be modified by an external entity. This check
> really doesn't belong in the GRE path but in the skb_segment function
> itself. This way any protocol that might be segmented will be performing
> this check before attempting to offload a checksum to software.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Acked-by: Tom Herbert <tom@herbertland.com>
> ---
> net/core/skbuff.c | 5 +++++
> net/ipv4/gre_offload.c | 11 -----------
> 2 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 9c065ac72e87..88262c82b96a 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3173,6 +3173,11 @@ skip_fraglist:
>
> perform_csum_check:
> if (!csum) {
> + if (skb_has_shared_frag(nskb)) {
> + err = __skb_linearize(nskb);
> + if (err)
> + goto err;
> + }
> if (!nskb->remcsum_offload)
> nskb->ip_summed = CHECKSUM_NONE;
> SKB_GSO_CB(nskb)->csum =
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index 02cb1a416c7d..35a8dd35ed4e 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -83,17 +83,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> if (csum) {
> __be32 *pcsum;
>
> - if (skb_has_shared_frag(skb)) {
> - int err;
> -
> - err = __skb_linearize(skb);
> - if (err) {
> - kfree_skb_list(segs);
> - segs = ERR_PTR(err);
> - goto out;
> - }
> - }
> -
> skb_reset_transport_header(skb);
>
> greh = (struct gre_base_hdr *)
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (4 preceding siblings ...)
2016-02-05 23:27 ` [net-next PATCH 05/10] net: Move skb_has_shared_frag check out of GRE code and into segmentation Alexander Duyck
@ 2016-02-05 23:28 ` Alexander Duyck
2016-02-06 20:52 ` Tom Herbert
2016-02-05 23:28 ` [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol Alexander Duyck
` (4 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:28 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
This patch updates the gre checksum path to follow something much closer to
the UDP checksum path. By doing this we can avoid needing to do as much
header inspection and can just make use of the fields we were already
reading in the sk_buff structure.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/ipv4/gre_offload.c | 64 +++++++++++++++++++++++-------------------------
1 file changed, 30 insertions(+), 34 deletions(-)
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index 35a8dd35ed4e..c15441b5ff61 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -18,14 +18,14 @@
static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
netdev_features_t features)
{
+ int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
struct sk_buff *segs = ERR_PTR(-EINVAL);
- int ghl;
struct gre_base_hdr *greh;
u16 mac_offset = skb->mac_header;
- int mac_len = skb->mac_len;
__be16 protocol = skb->protocol;
- int tnl_hlen;
- bool csum;
+ u16 mac_len = skb->mac_len;
+ int gre_offset, outer_hlen;
+ bool need_csum;
if (unlikely(skb_shinfo(skb)->gso_type &
~(SKB_GSO_TCPV4 |
@@ -42,64 +42,60 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
if (!skb->encapsulation)
goto out;
- if (unlikely(!pskb_may_pull(skb, sizeof(*greh))))
+ if (unlikely(tnl_hlen < sizeof(struct gre_base_hdr)))
goto out;
- greh = (struct gre_base_hdr *)skb_transport_header(skb);
-
- ghl = skb_inner_mac_header(skb) - skb_transport_header(skb);
- if (unlikely(ghl < sizeof(*greh)))
+ if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
goto out;
- csum = !!(greh->flags & GRE_CSUM);
- if (csum)
- skb->encap_hdr_csum = 1;
+ greh = (struct gre_base_hdr *)skb_transport_header(skb);
/* setup inner skb. */
skb->protocol = greh->protocol;
skb->encapsulation = 0;
-
- if (unlikely(!pskb_may_pull(skb, ghl)))
- goto out;
-
- __skb_pull(skb, ghl);
+ __skb_pull(skb, tnl_hlen);
skb_reset_mac_header(skb);
skb_set_network_header(skb, skb_inner_network_offset(skb));
skb->mac_len = skb_inner_network_offset(skb);
+ need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
+ skb->encap_hdr_csum = need_csum;
+
features &= skb->dev->hw_enc_features;
/* segment inner packet. */
segs = skb_mac_gso_segment(skb, features);
if (IS_ERR_OR_NULL(segs)) {
- skb_gso_error_unwind(skb, protocol, ghl, mac_offset, mac_len);
+ skb_gso_error_unwind(skb, protocol, tnl_hlen, mac_offset,
+ mac_len);
goto out;
}
+ outer_hlen = skb_tnl_header_len(skb);
+ gre_offset = outer_hlen - tnl_hlen;
skb = segs;
- tnl_hlen = skb_tnl_header_len(skb);
do {
- __skb_push(skb, ghl);
- if (csum) {
- __be32 *pcsum;
-
- skb_reset_transport_header(skb);
-
- greh = (struct gre_base_hdr *)
- skb_transport_header(skb);
- pcsum = (__be32 *)(greh + 1);
- *pcsum = 0;
- *(__sum16 *)pcsum = gso_make_checksum(skb, 0);
- }
- __skb_push(skb, tnl_hlen - ghl);
+ __be32 *pcsum;
skb_reset_inner_headers(skb);
skb->encapsulation = 1;
- skb_reset_mac_header(skb);
- skb_set_network_header(skb, mac_len);
skb->mac_len = mac_len;
skb->protocol = protocol;
+
+ __skb_push(skb, outer_hlen);
+ skb_reset_mac_header(skb);
+ skb_set_network_header(skb, mac_len);
+ skb_set_transport_header(skb, gre_offset);
+
+ if (!need_csum)
+ continue;
+
+ greh = (struct gre_base_hdr *)skb_transport_header(skb);
+ pcsum = (__be32 *)(greh + 1);
+
+ *pcsum = 0;
+ *(__sum16 *)pcsum = gso_make_checksum(skb, 0);
} while ((skb = skb->next));
out:
return segs;
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags
2016-02-05 23:28 ` [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags Alexander Duyck
@ 2016-02-06 20:52 ` Tom Herbert
2016-02-07 10:06 ` Alexander Duyck
0 siblings, 1 reply; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:52 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch updates the gre checksum path to follow something much closer to
> the UDP checksum path. By doing this we can avoid needing to do as much
> header inspection and can just make use of the fields we were already
> reading in the sk_buff structure.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> net/ipv4/gre_offload.c | 64 +++++++++++++++++++++++-------------------------
> 1 file changed, 30 insertions(+), 34 deletions(-)
>
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index 35a8dd35ed4e..c15441b5ff61 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -18,14 +18,14 @@
> static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> netdev_features_t features)
> {
> + int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> struct sk_buff *segs = ERR_PTR(-EINVAL);
> - int ghl;
> struct gre_base_hdr *greh;
> u16 mac_offset = skb->mac_header;
> - int mac_len = skb->mac_len;
> __be16 protocol = skb->protocol;
> - int tnl_hlen;
> - bool csum;
> + u16 mac_len = skb->mac_len;
> + int gre_offset, outer_hlen;
> + bool need_csum;
>
> if (unlikely(skb_shinfo(skb)->gso_type &
> ~(SKB_GSO_TCPV4 |
> @@ -42,64 +42,60 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> if (!skb->encapsulation)
> goto out;
>
> - if (unlikely(!pskb_may_pull(skb, sizeof(*greh))))
> + if (unlikely(tnl_hlen < sizeof(struct gre_base_hdr)))
> goto out;
>
> - greh = (struct gre_base_hdr *)skb_transport_header(skb);
> -
> - ghl = skb_inner_mac_header(skb) - skb_transport_header(skb);
> - if (unlikely(ghl < sizeof(*greh)))
> + if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
> goto out;
>
> - csum = !!(greh->flags & GRE_CSUM);
> - if (csum)
> - skb->encap_hdr_csum = 1;
> + greh = (struct gre_base_hdr *)skb_transport_header(skb);
>
> /* setup inner skb. */
> skb->protocol = greh->protocol;
> skb->encapsulation = 0;
> -
> - if (unlikely(!pskb_may_pull(skb, ghl)))
> - goto out;
> -
> - __skb_pull(skb, ghl);
> + __skb_pull(skb, tnl_hlen);
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, skb_inner_network_offset(skb));
> skb->mac_len = skb_inner_network_offset(skb);
>
> + need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
Should we do a WARN_ON(need_csum && !(greh->flags & GRE_CSUM)) just to
be conservative?
> + skb->encap_hdr_csum = need_csum;
> +
> features &= skb->dev->hw_enc_features;
>
> /* segment inner packet. */
> segs = skb_mac_gso_segment(skb, features);
> if (IS_ERR_OR_NULL(segs)) {
> - skb_gso_error_unwind(skb, protocol, ghl, mac_offset, mac_len);
> + skb_gso_error_unwind(skb, protocol, tnl_hlen, mac_offset,
> + mac_len);
> goto out;
> }
>
> + outer_hlen = skb_tnl_header_len(skb);
> + gre_offset = outer_hlen - tnl_hlen;
> skb = segs;
> - tnl_hlen = skb_tnl_header_len(skb);
> do {
> - __skb_push(skb, ghl);
> - if (csum) {
> - __be32 *pcsum;
> -
> - skb_reset_transport_header(skb);
> -
> - greh = (struct gre_base_hdr *)
> - skb_transport_header(skb);
> - pcsum = (__be32 *)(greh + 1);
> - *pcsum = 0;
> - *(__sum16 *)pcsum = gso_make_checksum(skb, 0);
> - }
> - __skb_push(skb, tnl_hlen - ghl);
> + __be32 *pcsum;
>
> skb_reset_inner_headers(skb);
> skb->encapsulation = 1;
>
> - skb_reset_mac_header(skb);
> - skb_set_network_header(skb, mac_len);
> skb->mac_len = mac_len;
> skb->protocol = protocol;
> +
> + __skb_push(skb, outer_hlen);
> + skb_reset_mac_header(skb);
> + skb_set_network_header(skb, mac_len);
> + skb_set_transport_header(skb, gre_offset);
> +
> + if (!need_csum)
> + continue;
> +
> + greh = (struct gre_base_hdr *)skb_transport_header(skb);
> + pcsum = (__be32 *)(greh + 1);
> +
> + *pcsum = 0;
> + *(__sum16 *)pcsum = gso_make_checksum(skb, 0);
> } while ((skb = skb->next));
> out:
> return segs;
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags
2016-02-06 20:52 ` Tom Herbert
@ 2016-02-07 10:06 ` Alexander Duyck
0 siblings, 0 replies; 24+ messages in thread
From: Alexander Duyck @ 2016-02-07 10:06 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers, Edward Cree,
David S. Miller
On Sat, Feb 6, 2016 at 2:52 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> This patch updates the gre checksum path to follow something much closer to
>> the UDP checksum path. By doing this we can avoid needing to do as much
>> header inspection and can just make use of the fields we were already
>> reading in the sk_buff structure.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>> net/ipv4/gre_offload.c | 64 +++++++++++++++++++++++-------------------------
>> 1 file changed, 30 insertions(+), 34 deletions(-)
>>
>> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
>> index 35a8dd35ed4e..c15441b5ff61 100644
>> --- a/net/ipv4/gre_offload.c
>> +++ b/net/ipv4/gre_offload.c
>> @@ -18,14 +18,14 @@
>> static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>> netdev_features_t features)
>> {
>> + int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
>> struct sk_buff *segs = ERR_PTR(-EINVAL);
>> - int ghl;
>> struct gre_base_hdr *greh;
>> u16 mac_offset = skb->mac_header;
>> - int mac_len = skb->mac_len;
>> __be16 protocol = skb->protocol;
>> - int tnl_hlen;
>> - bool csum;
>> + u16 mac_len = skb->mac_len;
>> + int gre_offset, outer_hlen;
>> + bool need_csum;
>>
>> if (unlikely(skb_shinfo(skb)->gso_type &
>> ~(SKB_GSO_TCPV4 |
>> @@ -42,64 +42,60 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>> if (!skb->encapsulation)
>> goto out;
>>
>> - if (unlikely(!pskb_may_pull(skb, sizeof(*greh))))
>> + if (unlikely(tnl_hlen < sizeof(struct gre_base_hdr)))
>> goto out;
>>
>> - greh = (struct gre_base_hdr *)skb_transport_header(skb);
>> -
>> - ghl = skb_inner_mac_header(skb) - skb_transport_header(skb);
>> - if (unlikely(ghl < sizeof(*greh)))
>> + if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
>> goto out;
>>
>> - csum = !!(greh->flags & GRE_CSUM);
>> - if (csum)
>> - skb->encap_hdr_csum = 1;
>> + greh = (struct gre_base_hdr *)skb_transport_header(skb);
>>
>> /* setup inner skb. */
>> skb->protocol = greh->protocol;
>> skb->encapsulation = 0;
>> -
>> - if (unlikely(!pskb_may_pull(skb, ghl)))
>> - goto out;
>> -
>> - __skb_pull(skb, ghl);
>> + __skb_pull(skb, tnl_hlen);
>> skb_reset_mac_header(skb);
>> skb_set_network_header(skb, skb_inner_network_offset(skb));
>> skb->mac_len = skb_inner_network_offset(skb);
>>
>> + need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
>
> Should we do a WARN_ON(need_csum && !(greh->flags & GRE_CSUM)) just to
> be conservative?
>
We could but what would be the point?
If there is an issue like that present in the kernel we should
probably fix it in build_header in ip_gre.c instead of trying to push
validation for the case into the GSO code.
- Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (5 preceding siblings ...)
2016-02-05 23:28 ` [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags Alexander Duyck
@ 2016-02-05 23:28 ` Alexander Duyck
2016-02-06 20:55 ` Tom Herbert
2016-02-05 23:28 ` [net-next PATCH 08/10] udp: Clean up the use of flags in UDP segmentation offload Alexander Duyck
` (3 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:28 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
Instead of parsing headers to determine the inner protocol we can just pull
the value from inner_proto.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/ipv4/gre_offload.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
index c15441b5ff61..003b0ebbcfdd 100644
--- a/net/ipv4/gre_offload.c
+++ b/net/ipv4/gre_offload.c
@@ -20,7 +20,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
{
int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
struct sk_buff *segs = ERR_PTR(-EINVAL);
- struct gre_base_hdr *greh;
u16 mac_offset = skb->mac_header;
__be16 protocol = skb->protocol;
u16 mac_len = skb->mac_len;
@@ -48,15 +47,13 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
goto out;
- greh = (struct gre_base_hdr *)skb_transport_header(skb);
-
/* setup inner skb. */
- skb->protocol = greh->protocol;
skb->encapsulation = 0;
__skb_pull(skb, tnl_hlen);
skb_reset_mac_header(skb);
skb_set_network_header(skb, skb_inner_network_offset(skb));
skb->mac_len = skb_inner_network_offset(skb);
+ skb->protocol = skb->inner_protocol;
need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
skb->encap_hdr_csum = need_csum;
@@ -75,6 +72,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
gre_offset = outer_hlen - tnl_hlen;
skb = segs;
do {
+ struct gre_base_hdr *greh;
__be32 *pcsum;
skb_reset_inner_headers(skb);
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol
2016-02-05 23:28 ` [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol Alexander Duyck
@ 2016-02-06 20:55 ` Tom Herbert
2016-02-07 10:11 ` Alexander Duyck
0 siblings, 1 reply; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:55 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> Instead of parsing headers to determine the inner protocol we can just pull
> the value from inner_proto.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
> ---
> net/ipv4/gre_offload.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
> index c15441b5ff61..003b0ebbcfdd 100644
> --- a/net/ipv4/gre_offload.c
> +++ b/net/ipv4/gre_offload.c
> @@ -20,7 +20,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> {
> int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> struct sk_buff *segs = ERR_PTR(-EINVAL);
> - struct gre_base_hdr *greh;
> u16 mac_offset = skb->mac_header;
> __be16 protocol = skb->protocol;
> u16 mac_len = skb->mac_len;
> @@ -48,15 +47,13 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
> goto out;
>
> - greh = (struct gre_base_hdr *)skb_transport_header(skb);
> -
> /* setup inner skb. */
> - skb->protocol = greh->protocol;
> skb->encapsulation = 0;
> __skb_pull(skb, tnl_hlen);
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, skb_inner_network_offset(skb));
> skb->mac_len = skb_inner_network_offset(skb);
> + skb->protocol = skb->inner_protocol;
Is this going to be a problem if we try to perform GSO with nested
encapsulations? (I suppose that's something not supported).
>
> need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_GRE_CSUM);
> skb->encap_hdr_csum = need_csum;
> @@ -75,6 +72,7 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
> gre_offset = outer_hlen - tnl_hlen;
> skb = segs;
> do {
> + struct gre_base_hdr *greh;
> __be32 *pcsum;
>
> skb_reset_inner_headers(skb);
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol
2016-02-06 20:55 ` Tom Herbert
@ 2016-02-07 10:11 ` Alexander Duyck
0 siblings, 0 replies; 24+ messages in thread
From: Alexander Duyck @ 2016-02-07 10:11 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexander Duyck, Linux Kernel Network Developers, Edward Cree,
David S. Miller
On Sat, Feb 6, 2016 at 2:55 PM, Tom Herbert <tom@herbertland.com> wrote:
> On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
>> Instead of parsing headers to determine the inner protocol we can just pull
>> the value from inner_proto.
>>
>> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
>> ---
>> net/ipv4/gre_offload.c | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv4/gre_offload.c b/net/ipv4/gre_offload.c
>> index c15441b5ff61..003b0ebbcfdd 100644
>> --- a/net/ipv4/gre_offload.c
>> +++ b/net/ipv4/gre_offload.c
>> @@ -20,7 +20,6 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>> {
>> int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
>> struct sk_buff *segs = ERR_PTR(-EINVAL);
>> - struct gre_base_hdr *greh;
>> u16 mac_offset = skb->mac_header;
>> __be16 protocol = skb->protocol;
>> u16 mac_len = skb->mac_len;
>> @@ -48,15 +47,13 @@ static struct sk_buff *gre_gso_segment(struct sk_buff *skb,
>> if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
>> goto out;
>>
>> - greh = (struct gre_base_hdr *)skb_transport_header(skb);
>> -
>> /* setup inner skb. */
>> - skb->protocol = greh->protocol;
>> skb->encapsulation = 0;
>> __skb_pull(skb, tnl_hlen);
>> skb_reset_mac_header(skb);
>> skb_set_network_header(skb, skb_inner_network_offset(skb));
>> skb->mac_len = skb_inner_network_offset(skb);
>> + skb->protocol = skb->inner_protocol;
>
> Is this going to be a problem if we try to perform GSO with nested
> encapsulations? (I suppose that's something not supported).
Right tunnels don't offload tunnels so there is no need to worry about
a tunnel nested in a tunnel. We might get there someday, but I don't
see it happening any time soon because we would end up likely needing
yet another level of header pointers for middle mac, network, and
transport in such a case.
- Alex
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 08/10] udp: Clean up the use of flags in UDP segmentation offload
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (6 preceding siblings ...)
2016-02-05 23:28 ` [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol Alexander Duyck
@ 2016-02-05 23:28 ` Alexander Duyck
2016-02-06 20:57 ` Tom Herbert
2016-02-05 23:28 ` [net-next PATCH 09/10] udp: Use uh->len instead of skb->len to compute checksum in segmentation Alexander Duyck
` (2 subsequent siblings)
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:28 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
This patch goes though and cleans up the logic related to several of the
control flags used in UDP segmentation. Specifically the use of dont_encap
isn't really needed as we can just check the skb for CHECKSUM_PARTIAL and
if it isn't set then we don't need to update the internal headers. As such
we can just drop that value.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/ipv4/udp_offload.c | 37 ++++++++++++++++++-------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 86687f58d613..9e4816fc9927 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -33,16 +33,13 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
__be16 new_protocol, bool is_ipv6)
{
struct sk_buff *segs = ERR_PTR(-EINVAL);
+ bool remcsum, need_csum, offload_csum;
u16 mac_offset = skb->mac_header;
int mac_len = skb->mac_len;
int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
__be16 protocol = skb->protocol;
int udp_offset, outer_hlen;
unsigned int oldlen;
- bool need_csum = !!(skb_shinfo(skb)->gso_type &
- SKB_GSO_UDP_TUNNEL_CSUM);
- bool remcsum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TUNNEL_REMCSUM);
- bool offload_csum = false, dont_encap = (need_csum || remcsum);
oldlen = (u16)~skb->len;
@@ -55,14 +52,18 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
skb_set_network_header(skb, skb_inner_network_offset(skb));
skb->mac_len = skb_inner_network_offset(skb);
skb->protocol = new_protocol;
+
+ need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM);
skb->encap_hdr_csum = need_csum;
+
+ remcsum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TUNNEL_REMCSUM);
skb->remcsum_offload = remcsum;
/* Try to offload checksum if possible */
offload_csum = !!(need_csum &&
- ((skb->dev->features & NETIF_F_HW_CSUM) ||
- (skb->dev->features & (is_ipv6 ?
- NETIF_F_IPV6_CSUM : NETIF_F_IP_CSUM))));
+ (skb->dev->features &
+ (is_ipv6 ? (NETIF_F_HW_CSUM | NETIF_F_IPV6_CSUM) :
+ (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM))));
features &= skb->dev->hw_enc_features;
@@ -92,13 +93,11 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
int len;
__be32 delta;
- if (dont_encap) {
- skb->encapsulation = 0;
+ if (remcsum)
skb->ip_summed = CHECKSUM_NONE;
- } else {
- /* Only set up inner headers if we might be offloading
- * inner checksum.
- */
+
+ /* Set up inner headers if we are offloading inner checksum */
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
skb_reset_inner_headers(skb);
skb->encapsulation = 1;
}
@@ -122,15 +121,15 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
uh->check = ~csum_fold((__force __wsum)
((__force u32)uh->check +
(__force u32)delta));
- if (offload_csum) {
- skb->ip_summed = CHECKSUM_PARTIAL;
- skb->csum_start = skb_transport_header(skb) - skb->head;
- skb->csum_offset = offsetof(struct udphdr, check);
- } else {
- uh->check = gso_make_checksum(skb, ~uh->check);
+ if (skb->encapsulation || !offload_csum) {
+ uh->check = gso_make_checksum(skb, ~uh->check);
if (uh->check == 0)
uh->check = CSUM_MANGLED_0;
+ } else {
+ skb->ip_summed = CHECKSUM_PARTIAL;
+ skb->csum_start = skb_transport_header(skb) - skb->head;
+ skb->csum_offset = offsetof(struct udphdr, check);
}
} while ((skb = skb->next));
out:
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 08/10] udp: Clean up the use of flags in UDP segmentation offload
2016-02-05 23:28 ` [net-next PATCH 08/10] udp: Clean up the use of flags in UDP segmentation offload Alexander Duyck
@ 2016-02-06 20:57 ` Tom Herbert
0 siblings, 0 replies; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:57 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch goes though and cleans up the logic related to several of the
> control flags used in UDP segmentation. Specifically the use of dont_encap
> isn't really needed as we can just check the skb for CHECKSUM_PARTIAL and
> if it isn't set then we don't need to update the internal headers. As such
> we can just drop that value.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Acked-by: Tom Herbert <tom@herbertland.com>
> ---
> net/ipv4/udp_offload.c | 37 ++++++++++++++++++-------------------
> 1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 86687f58d613..9e4816fc9927 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -33,16 +33,13 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> __be16 new_protocol, bool is_ipv6)
> {
> struct sk_buff *segs = ERR_PTR(-EINVAL);
> + bool remcsum, need_csum, offload_csum;
> u16 mac_offset = skb->mac_header;
> int mac_len = skb->mac_len;
> int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> __be16 protocol = skb->protocol;
> int udp_offset, outer_hlen;
> unsigned int oldlen;
> - bool need_csum = !!(skb_shinfo(skb)->gso_type &
> - SKB_GSO_UDP_TUNNEL_CSUM);
> - bool remcsum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TUNNEL_REMCSUM);
> - bool offload_csum = false, dont_encap = (need_csum || remcsum);
>
> oldlen = (u16)~skb->len;
>
> @@ -55,14 +52,18 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> skb_set_network_header(skb, skb_inner_network_offset(skb));
> skb->mac_len = skb_inner_network_offset(skb);
> skb->protocol = new_protocol;
> +
> + need_csum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL_CSUM);
> skb->encap_hdr_csum = need_csum;
> +
> + remcsum = !!(skb_shinfo(skb)->gso_type & SKB_GSO_TUNNEL_REMCSUM);
> skb->remcsum_offload = remcsum;
>
> /* Try to offload checksum if possible */
> offload_csum = !!(need_csum &&
> - ((skb->dev->features & NETIF_F_HW_CSUM) ||
> - (skb->dev->features & (is_ipv6 ?
> - NETIF_F_IPV6_CSUM : NETIF_F_IP_CSUM))));
> + (skb->dev->features &
> + (is_ipv6 ? (NETIF_F_HW_CSUM | NETIF_F_IPV6_CSUM) :
> + (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM))));
>
> features &= skb->dev->hw_enc_features;
>
> @@ -92,13 +93,11 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> int len;
> __be32 delta;
>
> - if (dont_encap) {
> - skb->encapsulation = 0;
> + if (remcsum)
> skb->ip_summed = CHECKSUM_NONE;
> - } else {
> - /* Only set up inner headers if we might be offloading
> - * inner checksum.
> - */
> +
> + /* Set up inner headers if we are offloading inner checksum */
> + if (skb->ip_summed == CHECKSUM_PARTIAL) {
> skb_reset_inner_headers(skb);
> skb->encapsulation = 1;
> }
> @@ -122,15 +121,15 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> uh->check = ~csum_fold((__force __wsum)
> ((__force u32)uh->check +
> (__force u32)delta));
> - if (offload_csum) {
> - skb->ip_summed = CHECKSUM_PARTIAL;
> - skb->csum_start = skb_transport_header(skb) - skb->head;
> - skb->csum_offset = offsetof(struct udphdr, check);
> - } else {
> - uh->check = gso_make_checksum(skb, ~uh->check);
>
> + if (skb->encapsulation || !offload_csum) {
> + uh->check = gso_make_checksum(skb, ~uh->check);
> if (uh->check == 0)
> uh->check = CSUM_MANGLED_0;
> + } else {
> + skb->ip_summed = CHECKSUM_PARTIAL;
> + skb->csum_start = skb_transport_header(skb) - skb->head;
> + skb->csum_offset = offsetof(struct udphdr, check);
> }
> } while ((skb = skb->next));
> out:
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 09/10] udp: Use uh->len instead of skb->len to compute checksum in segmentation
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (7 preceding siblings ...)
2016-02-05 23:28 ` [net-next PATCH 08/10] udp: Clean up the use of flags in UDP segmentation offload Alexander Duyck
@ 2016-02-05 23:28 ` Alexander Duyck
2016-02-06 20:59 ` Tom Herbert
2016-02-05 23:28 ` [net-next PATCH 10/10] net: Allow tunnels to use inner checksum offloads with outer checksums needed Alexander Duyck
2016-02-11 14:32 ` [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads David Miller
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:28 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
The segmentation code was having to do a bunch of work to pull the
skb->len and strip the udp header offset before the value could be used to
adjust the checksum. Instead of doing all this work we can just use the
value that goes into uh->len since that is the correct value with the
correct byte order that we need anyway. By using this value we can save
ourselves a bunch of pain as there is no need to do multiple byte swaps.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/ipv4/udp_offload.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index 9e4816fc9927..56c4c8b88b28 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -32,20 +32,23 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
netdev_features_t features),
__be16 new_protocol, bool is_ipv6)
{
+ int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
struct sk_buff *segs = ERR_PTR(-EINVAL);
bool remcsum, need_csum, offload_csum;
+ struct udphdr *uh = udp_hdr(skb);
u16 mac_offset = skb->mac_header;
- int mac_len = skb->mac_len;
- int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
__be16 protocol = skb->protocol;
+ u16 mac_len = skb->mac_len;
int udp_offset, outer_hlen;
- unsigned int oldlen;
-
- oldlen = (u16)~skb->len;
+ u32 partial;
if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
goto out;
+ /* adjust partial header checksum to negate old length */
+ partial = (__force u32)uh->check + (__force u16)~uh->len;
+
+ /* setup inner skb. */
skb->encapsulation = 0;
__skb_pull(skb, tnl_hlen);
skb_reset_mac_header(skb);
@@ -89,9 +92,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
udp_offset = outer_hlen - tnl_hlen;
skb = segs;
do {
- struct udphdr *uh;
- int len;
- __be32 delta;
+ __be16 len;
if (remcsum)
skb->ip_summed = CHECKSUM_NONE;
@@ -105,22 +106,19 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
skb->mac_len = mac_len;
skb->protocol = protocol;
- skb_push(skb, outer_hlen);
+ __skb_push(skb, outer_hlen);
skb_reset_mac_header(skb);
skb_set_network_header(skb, mac_len);
skb_set_transport_header(skb, udp_offset);
- len = skb->len - udp_offset;
+ len = htons(skb->len - udp_offset);
uh = udp_hdr(skb);
- uh->len = htons(len);
+ uh->len = len;
if (!need_csum)
continue;
- delta = htonl(oldlen + len);
-
uh->check = ~csum_fold((__force __wsum)
- ((__force u32)uh->check +
- (__force u32)delta));
+ ((__force u32)len + partial));
if (skb->encapsulation || !offload_csum) {
uh->check = gso_make_checksum(skb, ~uh->check);
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 09/10] udp: Use uh->len instead of skb->len to compute checksum in segmentation
2016-02-05 23:28 ` [net-next PATCH 09/10] udp: Use uh->len instead of skb->len to compute checksum in segmentation Alexander Duyck
@ 2016-02-06 20:59 ` Tom Herbert
0 siblings, 0 replies; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 20:59 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> The segmentation code was having to do a bunch of work to pull the
> skb->len and strip the udp header offset before the value could be used to
> adjust the checksum. Instead of doing all this work we can just use the
> value that goes into uh->len since that is the correct value with the
> correct byte order that we need anyway. By using this value we can save
> ourselves a bunch of pain as there is no need to do multiple byte swaps.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Acked-by: Tom Herbert <tom@herbertland.com>
> ---
> net/ipv4/udp_offload.c | 28 +++++++++++++---------------
> 1 file changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index 9e4816fc9927..56c4c8b88b28 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -32,20 +32,23 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> netdev_features_t features),
> __be16 new_protocol, bool is_ipv6)
> {
> + int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> struct sk_buff *segs = ERR_PTR(-EINVAL);
> bool remcsum, need_csum, offload_csum;
> + struct udphdr *uh = udp_hdr(skb);
> u16 mac_offset = skb->mac_header;
> - int mac_len = skb->mac_len;
> - int tnl_hlen = skb_inner_mac_header(skb) - skb_transport_header(skb);
> __be16 protocol = skb->protocol;
> + u16 mac_len = skb->mac_len;
> int udp_offset, outer_hlen;
> - unsigned int oldlen;
> -
> - oldlen = (u16)~skb->len;
> + u32 partial;
>
> if (unlikely(!pskb_may_pull(skb, tnl_hlen)))
> goto out;
>
> + /* adjust partial header checksum to negate old length */
> + partial = (__force u32)uh->check + (__force u16)~uh->len;
> +
> + /* setup inner skb. */
> skb->encapsulation = 0;
> __skb_pull(skb, tnl_hlen);
> skb_reset_mac_header(skb);
> @@ -89,9 +92,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> udp_offset = outer_hlen - tnl_hlen;
> skb = segs;
> do {
> - struct udphdr *uh;
> - int len;
> - __be32 delta;
> + __be16 len;
>
> if (remcsum)
> skb->ip_summed = CHECKSUM_NONE;
> @@ -105,22 +106,19 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,
> skb->mac_len = mac_len;
> skb->protocol = protocol;
>
> - skb_push(skb, outer_hlen);
> + __skb_push(skb, outer_hlen);
> skb_reset_mac_header(skb);
> skb_set_network_header(skb, mac_len);
> skb_set_transport_header(skb, udp_offset);
> - len = skb->len - udp_offset;
> + len = htons(skb->len - udp_offset);
> uh = udp_hdr(skb);
> - uh->len = htons(len);
> + uh->len = len;
>
> if (!need_csum)
> continue;
>
> - delta = htonl(oldlen + len);
> -
> uh->check = ~csum_fold((__force __wsum)
> - ((__force u32)uh->check +
> - (__force u32)delta));
> + ((__force u32)len + partial));
>
> if (skb->encapsulation || !offload_csum) {
> uh->check = gso_make_checksum(skb, ~uh->check);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [net-next PATCH 10/10] net: Allow tunnels to use inner checksum offloads with outer checksums needed
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (8 preceding siblings ...)
2016-02-05 23:28 ` [net-next PATCH 09/10] udp: Use uh->len instead of skb->len to compute checksum in segmentation Alexander Duyck
@ 2016-02-05 23:28 ` Alexander Duyck
2016-02-06 21:00 ` Tom Herbert
2016-02-11 14:32 ` [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads David Miller
10 siblings, 1 reply; 24+ messages in thread
From: Alexander Duyck @ 2016-02-05 23:28 UTC (permalink / raw)
To: netdev, alexander.duyck; +Cc: ecree, tom, davem
This patch enables us to use inner checksum offloads if provided by
hardware with outer checksums computed by software.
It basically reduces encap_hdr_csum to an advisory flag for now, but based
on the fact that SCTP may be getting segmentation support before long I
thought we may want to keep it as it is possible we may need to support
CRC32c and 1's compliment checksum in the same packet at some point in the
future.
Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
---
net/core/skbuff.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 88262c82b96a..b0cce744e2a0 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3004,8 +3004,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
if (unlikely(!proto))
return ERR_PTR(-EINVAL);
- csum = !head_skb->encap_hdr_csum &&
- !!can_checksum_protocol(features, proto);
+ csum = !!can_checksum_protocol(features, proto);
headroom = skb_headroom(head_skb);
pos = skb_headlen(head_skb);
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [net-next PATCH 10/10] net: Allow tunnels to use inner checksum offloads with outer checksums needed
2016-02-05 23:28 ` [net-next PATCH 10/10] net: Allow tunnels to use inner checksum offloads with outer checksums needed Alexander Duyck
@ 2016-02-06 21:00 ` Tom Herbert
0 siblings, 0 replies; 24+ messages in thread
From: Tom Herbert @ 2016-02-06 21:00 UTC (permalink / raw)
To: Alexander Duyck
Cc: Linux Kernel Network Developers, Alexander Duyck, Edward Cree,
David S. Miller
On Fri, Feb 5, 2016 at 3:28 PM, Alexander Duyck <aduyck@mirantis.com> wrote:
> This patch enables us to use inner checksum offloads if provided by
> hardware with outer checksums computed by software.
>
> It basically reduces encap_hdr_csum to an advisory flag for now, but based
> on the fact that SCTP may be getting segmentation support before long I
> thought we may want to keep it as it is possible we may need to support
> CRC32c and 1's compliment checksum in the same packet at some point in the
> future.
>
> Signed-off-by: Alexander Duyck <aduyck@mirantis.com>
Acked-by: Tom Herbert <tom@herbertland.com>
> ---
> net/core/skbuff.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 88262c82b96a..b0cce744e2a0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -3004,8 +3004,7 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
> if (unlikely(!proto))
> return ERR_PTR(-EINVAL);
>
> - csum = !head_skb->encap_hdr_csum &&
> - !!can_checksum_protocol(features, proto);
> + csum = !!can_checksum_protocol(features, proto);
>
> headroom = skb_headroom(head_skb);
> pos = skb_headlen(head_skb);
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads
2016-02-05 23:27 [net-next PATCH 00/10] Add GSO support for outer checksum w/ inner checksum offloads Alexander Duyck
` (9 preceding siblings ...)
2016-02-05 23:28 ` [net-next PATCH 10/10] net: Allow tunnels to use inner checksum offloads with outer checksums needed Alexander Duyck
@ 2016-02-11 14:32 ` David Miller
10 siblings, 0 replies; 24+ messages in thread
From: David Miller @ 2016-02-11 14:32 UTC (permalink / raw)
To: aduyck; +Cc: netdev, alexander.duyck, ecree, tom
From: Alexander Duyck <aduyck@mirantis.com>
Date: Fri, 05 Feb 2016 15:27:25 -0800
> This patch series updates the existing segmentation offload code for
> tunnels to make better use of existing and updated GSO checksum
> computation. This is done primarily through two mechanisms. First we
> maintain a separate checksum in the GSO context block of the sk_buff. This
> allows us to maintain two checksum values, one offloaded with values stored
> in csum_start and csum_offset, and one computed and tracked in
> SKB_GSO_CB(skb)->csum. By maintaining these two values we are able to take
> advantage of the same sort of math used in local checksum offload so that
> we can provide both inner and outer checksums with minimal overhead.
>
> Below is the performance for a netperf session between an ixgbe PF and VF
> on the same host but in different namespaces. As can be seen a significant
> gain in performance can be had from allowing the use of Tx checksum offload
> on the inner headers while performing a software offload on the outer
> header computation:
>
> Recv Send Send Utilization Service Demand
> Socket Socket Message Elapsed Send Recv Send Recv
> Size Size Size Time Throughput local remote local remote
> bytes bytes bytes secs. 10^6bits/s % S % U us/KB us/KB
>
> Before:
> 87380 16384 16384 10.00 12844.38 9.30 -1.00 0.712 -1.00
> After:
> 87380 16384 16384 10.00 13216.63 6.78 -1.00 0.504 -1.000
>
> Changes from v1:
> * Dropped use of CHECKSUM_UNNECESSARY for remote checksum offload
> * Left encap_hdr_csum as it will likely be needed in future for SCTP GSO
> * Broke the changes out over many more patches
> * Updated GRE segmentation to more closely match UDP tunnel segmentation
Series applied, thanks Alex.
^ permalink raw reply [flat|nested] 24+ messages in thread