Add GSO support for outer checksum w/ inner checksum offloads
Subsystems (3)
cover Add GSO support for outer checksum w/ inner checksum offloads
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
---
Alexander Duyck (10):
net: Drop unecessary enc_features variable from tunnel segmentation functions
net: Move GSO csum into SKB_GSO_CB
net: Update remote checksum segmentation to support use of GSO checksum
net: Store checksum result for offloaded GSO checksums
net: Move skb_has_shared_frag check out of GRE code and into segmentation
gre: Use GSO flags to determine csum need instead of GRE flags
gre: Use inner_proto to obtain inner header protocol
udp: Clean up the use of flags in UDP segmentation offload
udp: Use uh->len instead of skb->len to compute checksum in segmentation
net: Allow tunnels to use inner checksum offloads with outer checksums needed
include/linux/skbuff.h | 29 +++++++++++---- net/core/skbuff.c | 34 +++++++++++------- net/ipv4/gre_offload.c | 85 ++++++++++++++++++-------------------------- net/ipv4/tcp_offload.c | 8 +++- net/ipv4/udp_offload.c | 93 +++++++++++++++++++++++------------------------- 5 files changed, 127 insertions(+), 122 deletions(-)
PATCH 1/10 net: Drop unecessary enc_features variable from tunnel segmentation functions
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.
--- net/ipv4/gre_offload.c | 6 +++--- net/ipv4/udp_offload.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
net/ipv4/gre_offload.c +3 -3
@@ -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;
net/ipv4/udp_offload.c +3 -3
@@ -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);
Re: [net-next PATCH 01/10] net: Drop unecessary enc_features variable from tunnel segmentation functions
Quoted 5 lines · Alexander Duyck
The
enc_featuresvariable isn't necessary since features isn't used anywhere after we createenc_featuresso instead just use a destructive AND on features itself and save ourselves the variable declaration.Signed-off-by Alexander Duyck <[email protected]>
Quoted 56 lines
--- net/ipv4/gre_offload.c | 6 +++--- net/ipv4/udp_offload.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)net/ipv4/gre_offload.c +3
-3@@ -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;net/ipv4/udp_offload.c +3
-3@@ -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);
PATCH 2/10 net: Move GSO csum into SKB_GSO_CB
In reply to Alexander DuyckThis 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.
--- include/linux/skbuff.h | 14 +++++++------- net/core/skbuff.c | 16 +++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-)
include/linux/skbuff.h +7 -7
@@ -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)
net/core/skbuff.c +9 -7
@@ -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);
Re: [net-next PATCH 02/10] net: Move GSO csum into SKB_GSO_CB
Quoted 10 lines · Alexander Duyck
This patch moves the checksum maintained by GSO out of
skb->csumand 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)
Quoted 78 lines
Signed-off-by Alexander Duyck <[email protected]>--- include/linux/skbuff.h | 14 +++++++------- net/core/skbuff.c | 16 +++++++++------- 2 files changed, 16 insertions(+), 14 deletions(-)include/linux/skbuff.h +7
-7@@ -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)net/core/skbuff.c +9
-7@@ -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);
PATCH 3/10 net: Update remote checksum segmentation to support use of GSO checksum
In reply to Alexander DuyckThis 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.
--- net/core/skbuff.c | 10 ++++++---- net/ipv4/udp_offload.c | 22 ++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-)
net/core/skbuff.c +6 -4
@@ -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 );
net/ipv4/udp_offload.c +10 -12
@@ -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);
Re: [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum
Quoted 11 lines · Alexander Duyck
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)?
Quoted 75 lines
Signed-off-by Alexander Duyck <[email protected]>--- net/core/skbuff.c | 10 ++++++---- net/ipv4/udp_offload.c | 22 ++++++++++------------ 2 files changed, 16 insertions(+), 16 deletions(-)net/core/skbuff.c +6
-4@@ -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 );net/ipv4/udp_offload.c +10
-12@@ -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);
Re: [net-next PATCH 03/10] net: Update remote checksum segmentation to support use of GSO checksum
Quoted 8 lines · Tom Herbert
Quoted 7 lines · Alexander Duyck
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.
Quoted 5 lines
Quoted 4 lines
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
PATCH 4/10 net: Store checksum result for offloaded GSO checksums
In reply to Alexander DuyckThis 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.
--- include/linux/skbuff.h | 15 +++++++++++++++ net/ipv4/tcp_offload.c | 8 ++++++-- 2 files changed, 21 insertions(+), 2 deletions(-)
include/linux/skbuff.h +15 -0
@@ -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).
net/ipv4/tcp_offload.c +6 -2
@@ -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;
PATCH 5/10 net: Move skb_has_shared_frag check out of GRE code and into segmentation
In reply to Alexander DuyckThe 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.
--- net/core/skbuff.c | 5 +++++ net/ipv4/gre_offload.c | 11 ----------- 2 files changed, 5 insertions(+), 11 deletions(-)
net/core/skbuff.c +5 -0
@@ -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 =
net/ipv4/gre_offload.c +0 -11
@@ -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 *)
Re: [net-next PATCH 05/10] net: Move skb_has_shared_frag check out of GRE code and into segmentation
Quoted 7 lines · Alexander Duyck
The call
skb_has_shared_fragis used in the GRE path andskb_checksum_helpto verify that no frags can be modified by an external entity. This check really doesn't belong in the GRE path but in theskb_segmentfunction 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 <[email protected]>
Quoted 44 lines
--- net/core/skbuff.c | 5 +++++ net/ipv4/gre_offload.c | 11 ----------- 2 files changed, 5 insertions(+), 11 deletions(-)net/core/skbuff.c +5
-0@@ -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 =net/ipv4/gre_offload.c +0
-11@@ -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 *)
PATCH 6/10 gre: Use GSO flags to determine csum need instead of GRE flags
In reply to Alexander DuyckThis 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.
--- net/ipv4/gre_offload.c | 64 +++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 34 deletions(-)
net/ipv4/gre_offload.c +30 -34
@@ -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;
Re: [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags
Quoted 57 lines · Alexander Duyck
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_buffstructure.Signed-off-by Alexander Duyck <[email protected]>--- net/ipv4/gre_offload.c | 64 +++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 34 deletions(-)net/ipv4/gre_offload.c +7
-13@@ -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 ; -
net/ipv4/gre_offload.c static struct sk_buff *gre_gso_segment(struct sk_buff *skb, · line 61
Quoted 10 lines
net/ipv4/gre_offload.c +2
-4-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?
Quoted 58 lines
net/ipv4/gre_offload.c +21
-17+ 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;
Re: [net-next PATCH 06/10] gre: Use GSO flags to determine csum need instead of GRE flags
Quoted 67 lines · Tom Herbert
Quoted 66 lines · Alexander Duyck
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_buffstructure.Signed-off-by Alexander Duyck <[email protected]>--- net/ipv4/gre_offload.c | 64 +++++++++++++++++++++++------------------------- 1 file changed, 30 insertions(+), 34 deletions(-)net/ipv4/gre_offload.c +8
-17@@ -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);
Quoted 5 lines
net/ipv4/gre_offload.c static struct sk_buff *gre_gso_segment(struct sk_buff *skb, · line 61
Quoted 1 line
net/ipv4/gre_offload.c +1
-0+ 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
PATCH 7/10 gre: Use inner_proto to obtain inner header protocol
In reply to Alexander DuyckInstead of parsing headers to determine the inner protocol we can just pull the value from inner_proto.
--- net/ipv4/gre_offload.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
net/ipv4/gre_offload.c +2 -4
@@ -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);
Re: [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol
Quoted 24 lines · Alexander Duyck
Instead of parsing headers to determine the inner protocol we can just pull the value from
inner_proto.Signed-off-by Alexander Duyck <[email protected]>--- net/ipv4/gre_offload.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)net/ipv4/gre_offload.c +0
-1@@ -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;
net/ipv4/gre_offload.c static struct sk_buff *gre_gso_segment(struct sk_buff *skb, · line 56
Quoted 10 lines
net/ipv4/gre_offload.c +1
-3- 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).
Quoted 12 lines
net/ipv4/gre_offload.c +1
-0need_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);
Re: [net-next PATCH 07/10] gre: Use inner_proto to obtain inner header protocol
Quoted 25 lines · Tom Herbert
Quoted 24 lines · Alexander Duyck
Instead of parsing headers to determine the inner protocol we can just pull the value from
inner_proto.Signed-off-by Alexander Duyck <[email protected]>--- net/ipv4/gre_offload.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)net/ipv4/gre_offload.c +0
-1@@ -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;
Quoted 13 lines
net/ipv4/gre_offload.c static struct sk_buff *gre_gso_segment(struct sk_buff *skb, · line 56
Quoted 10 lines
net/ipv4/gre_offload.c +1
-3- 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
PATCH 8/10 udp: Clean up the use of flags in UDP segmentation offload
In reply to Alexander DuyckThis 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.
--- net/ipv4/udp_offload.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)
net/ipv4/udp_offload.c +18 -19
@@ -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:
Re: [net-next PATCH 08/10] udp: Clean up the use of flags in UDP segmentation offload
Quoted 7 lines · Alexander Duyck
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_encapisn't really needed as we can just check the skb forCHECKSUM_PARTIALand 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 <[email protected]>
Quoted 89 lines
--- net/ipv4/udp_offload.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)net/ipv4/udp_offload.c +18
-19@@ -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:
PATCH 9/10 udp: Use uh->len instead of skb->len to compute checksum in segmentation
In reply to Alexander DuyckThe 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.
--- net/ipv4/udp_offload.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)
net/ipv4/udp_offload.c +13 -15
@@ -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);
Re: [net-next PATCH 09/10] udp: Use uh->len instead of skb->len to compute checksum in segmentation
Quoted 8 lines · Alexander Duyck
The segmentation code was having to do a bunch of work to pull the
skb->lenand 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 intouh->lensince 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 <[email protected]>
Quoted 76 lines
--- net/ipv4/udp_offload.c | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-)net/ipv4/udp_offload.c +13
-15@@ -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);
PATCH 10/10 net: Allow tunnels to use inner checksum offloads with outer checksums needed
In reply to Alexander DuyckThis 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.
--- net/core/skbuff.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
net/core/skbuff.c +1 -2
@@ -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);
Re: [net-next PATCH 10/10] net: Allow tunnels to use inner checksum offloads with outer checksums needed
Quoted 10 lines · Alexander Duyck
This patch enables us to use inner checksum offloads if provided by hardware with outer checksums computed by software.
It basically reduces
encap_hdr_csumto 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 <[email protected]>
Quoted 19 lines
--- net/core/skbuff.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)net/core/skbuff.c +1
-2@@ -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);
David Miller
maintainerFrom: Alexander Duyck <[email protected]>
Date: Fri, 05 Feb 2016 15:27:25 -0800
Quoted 26 lines
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 incsum_startandcsum_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/KBBefore:
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
Quoted 5 lines
Changes from v1:
* Dropped use ofCHECKSUM_UNNECESSARYfor remote checksum offload
* Leftencap_hdr_csumas 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.