Local Checksum Offload
Subsystems (5)
v2 cover Local Checksum Offload
Tested with a VXLAN tunnel over a device that doesn't support inner checksum
offload (so the checksum will have been done in sw by validate_xmit_skb()).
Changes from v1:
* Enabled support in more encapsulation protocols.
I think it now covers everything except GRE.
* Wrote up some documentation covering TX checksum offload, LCO and RCO.
Edward Cree (5):
net: local checksum offload for encapsulation
net: enable LCO for udp_tunnel_handle_offloads() users
net: vxlan: enable local checksum offload
fou: enable LCO in FOU and GUE Documentation/networking: add tx-offloads.txt to explain LCO
Documentation/networking/00-INDEX | 2 + Documentation/networking/tx-offloads.txt | 122 +++++++++++++++++++++++++++++++ drivers/net/vxlan.c | 4 +- include/linux/skbuff.h | 26 +++++++ include/net/udp_tunnel.h | 3 +- net/ipv4/fou.c | 5 +- net/ipv4/ip_tunnel_core.c | 4 + net/ipv4/udp.c | 29 +++----- net/ipv6/ip6_checksum.c | 24 ++---- 9 files changed, 178 insertions(+), 41 deletions(-) create mode 100644 Documentation/networking/tx-offloads.txt
PATCH v2 1/5 net: local checksum offload for encapsulation
The arithmetic properties of the ones-complement checksum mean that a
correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.
--- include/linux/skbuff.h | 26 ++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 ++++ net/ipv4/udp.c | 29 ++++++++++------------------- net/ipv6/ip6_checksum.c | 24 ++++++++---------------- 4 files changed, 48 insertions(+), 35 deletions(-)
include/linux/skbuff.h +26 -0
@@ -3665,5 +3665,31 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)return hdr_len +skb_gso_transport_seglen (skb); } +/* Local Checksum Offload. +* Compute outer checksum based on the assumption that the +* inner checksum will be offloaded later. +* See Documentation/networking/tx-offloads.txt for +* explanation of how this works. +* Fill in outer checksum adjustment (e.g. with sum of outer +* pseudo-header) before calling. +* Also ensure that inner checksum is in linear data area. +*/ +static inline __wsum lco_csum (struct sk_buff *skb) +{ +char *inner_csum_field; +__wsum csum; + +/* Start with complement of inner checksum adjustment */ + inner_csum_field = skb->data +skb_checksum_start_offset (skb) + + skb->csum_offset; + csum = ~csum_unfold (*(__force __sum16 *)inner_csum_field); +/* Add in checksum of our headers (incl. outer checksum +* adjustment filled in by caller) +*/ + csum =skb_checksum (skb,0 ,skb_checksum_start_offset (skb), csum); +/* The result is the outer checksum */ +return csum; +} +#endif /* __KERNEL__ */ #endif /* _LINUX_SKBUFF_H */
net/ipv4/ip_tunnel_core.c +4 -0
@@ -146,6 +146,10 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md, }EXPORT_SYMBOL_GPL (iptunnel_metadata_reply); +/* csum_help should only be ever true if the protocol doesn't support LCO. +* If the tunnel uses udp_tunnel_xmit_skb(), then it gets LCO for free, and +* should always set csum_help to false. +*/ struct sk_buff *iptunnel_handle_offloads (struct sk_buff *skb,bool csum_help,int gso_type_mask)
net/ipv4/udp.c +10 -19
@@ -767,32 +767,23 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb, {struct udphdr *uh =udp_hdr (skb); -if (nocheck) +if (nocheck) { uh->check =0 ; -else if (skb_is_gso (skb)) + }else if (skb_is_gso (skb)) { uh->check = ~udp_v4_check (len, saddr, daddr,0 ); -else if (skb_dst (skb) &&skb_dst (skb)->dev && - (skb_dst (skb)->dev->features & - (NETIF_F_IP_CSUM |NETIF_F_HW_CSUM ))) { - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); + }else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum; + uh->check = ~udp_v4_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +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); uh->check = ~udp_v4_check (len, saddr, daddr,0 ); - }else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v4_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp_set_csum);
net/ipv6/ip6_checksum.c +8 -16
@@ -98,27 +98,19 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb, uh->check =0 ; else if (skb_is_gso(skb)) uh->check = ~udp_v6_check(len, saddr, daddr, 0); - else if (skb_dst(skb) && skb_dst(skb)->dev && - (skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) { - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); + else if (skb->ip_summed == CHECKSUM_PARTIAL) { +__wsum csum; + uh->check = ~udp_v6_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +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); uh->check = ~udp_v6_check (len, saddr, daddr,0 ); - } else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v6_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp6_set_csum);
RE: [PATCH v2 net-next 1/5] net: local checksum offload for encapsulation
From: Edward Cree
Quoted 10 lines
Sent: 07 January 2016 17:12
The arithmetic properties of the ones-complement checksum mean that a
correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.
Isn't it even simpler than that?
The checksum of the inner packet (including its header) is ~0 (ie 0). So the checksum of the whole packet (for the outer header) is the same as that of the packet down to the start of the inner header.
David
Re: [PATCH v2 net-next 1/5] net: local checksum offload for encapsulation
Quoted 4 lines · David Laight
Isn't it even simpler than that?
The checksum of the inner packet (including its header) is ~0 (ie 0). So the checksum of the whole packet (for the outer header) is the same as that of the packet down to the start of the inner header.
Not quite. The inner pseudo-header is included in the inner checksum
calculation, but does not actually appear in the packet. Thus the checksum of the inner packet is ~sum_of_inner_pseudo_header.
-ed
Re: [PATCH v2 net-next 1/5] net: local checksum offload for encapsulation
In reply to Edward CreeQuoted 39 lines · Edward Cree
The arithmetic properties of the ones-complement checksum mean that a
correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.Signed-off-by Edward Cree <[email protected]>--- include/linux/skbuff.h | 26 ++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 ++++ net/ipv4/udp.c | 29 ++++++++++------------------- net/ipv6/ip6_checksum.c | 24 ++++++++---------------- 4 files changed, 48 insertions(+), 35 deletions(-)include/linux/skbuff.h +13
-0@@ -3665,5 +3665,31 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)return hdr_len +skb_gso_transport_seglen (skb); } +/* Local Checksum Offload. +* Compute outer checksum based on the assumption that the +* inner checksum will be offloaded later. +* See Documentation/networking/tx-offloads.txt for +* explanation of how this works. +* Fill in outer checksum adjustment (e.g. with sum of outer +* pseudo-header) before calling. +* Also ensure that inner checksum is in linear data area. +*/ +static inline __wsum lco_csum (struct sk_buff *skb) +{ +char *inner_csum_field; +__wsum csum;
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3690
Quoted 10 lines
include/linux/skbuff.h +10
-0+ +/* Start with complement of inner checksum adjustment */ + inner_csum_field = skb->data +skb_checksum_start_offset (skb) + + skb->csum_offset; + csum = ~csum_unfold (*(__force __sum16 *)inner_csum_field); +/* Add in checksum of our headers (incl. outer checksum +* adjustment filled in by caller) +*/ + csum =skb_checksum (skb,0 ,skb_checksum_start_offset (skb), csum); +/* The result is the outer checksum */
Specifically packet checksum starting from skb->data.
Quoted 10 lines
include/linux/skbuff.h +3
-0+return csum; +} +#endif /* __KERNEL__ */ #endif /* _LINUX_SKBUFF_H */ net/ipv4/ip_tunnel_core.c +0
-0
net/ipv4/ip_tunnel_core.c struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md, · line 152
Quoted 7 lines
net/ipv4/ip_tunnel_core.c +4
-0@@ -146,6 +146,10 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md, }EXPORT_SYMBOL_GPL (iptunnel_metadata_reply); +/* csum_help should only be ever true if the protocol doesn't support LCO. +* If the tunnel uses udp_tunnel_xmit_skb(), then it gets LCO for free, and +* should always set csum_help to false. +*/
This comment probably isn't necessary.
Quoted 11 lines
net/ipv4/ip_tunnel_core.c +0
-0struct sk_buff *iptunnel_handle_offloads (struct sk_buff *skb,bool csum_help,int gso_type_mask)net/ipv4/udp.c +0
-0@@ -767,32 +767,23 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb, {struct udphdr *uh =udp_hdr (skb);
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 777
Quoted 10 lines
net/ipv4/udp.c +2
-6-if (nocheck) +if (nocheck) { uh->check =0 ; -else if (skb_is_gso (skb)) + }else if (skb_is_gso (skb)) { uh->check = ~udp_v4_check (len, saddr, daddr,0 ); -else if (skb_dst (skb) &&skb_dst (skb)->dev && - (skb_dst (skb)->dev->features & - (NETIF_F_IP_CSUM |NETIF_F_HW_CSUM ))) { -
I think getting rid of this check is correct, but maybe it should be in a separate patch?
Quoted 37 lines
net/ipv4/udp.c +8
-13-BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); + }else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum; + uh->check = ~udp_v4_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +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); uh->check = ~udp_v4_check (len, saddr, daddr,0 ); - }else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v4_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp_set_csum);net/ipv6/ip6_checksum.c +0
-0@@ -98,27 +98,19 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb, uh->check =0 ; else if (skb_is_gso(skb)) uh->check = ~udp_v6_check(len, saddr, daddr, 0);
net/ipv6/ip6_checksum.c void udp6_set_csum(bool nocheck, struct sk_buff *skb, · line 106
Quoted 10 lines
net/ipv6/ip6_checksum.c +5
-4-else if (skb_dst (skb) &&skb_dst (skb)->dev && - (skb_dst (skb)->dev->features &NETIF_F_IPV6_CSUM )) { - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); +else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum; + uh->check = ~udp_v6_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum);
Just do uh->check = csum_fold(loc)csum(skb)) to eliminate need for local csum variable.
Quoted 24 lines
net/ipv6/ip6_checksum.c +3
-12+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); uh->check = ~udp_v6_check (len, saddr, daddr,0 ); - }else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v6_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp6_set_csum);The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
Re: [PATCH v2 net-next 1/5] net: local checksum offload for encapsulation
In reply to Edward CreeQuoted 34 lines · Edward Cree
The arithmetic properties of the ones-complement checksum mean that a
correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.Signed-off-by Edward Cree <[email protected]>--- include/linux/skbuff.h | 26 ++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 ++++ net/ipv4/udp.c | 29 ++++++++++------------------- net/ipv6/ip6_checksum.c | 24 ++++++++---------------- 4 files changed, 48 insertions(+), 35 deletions(-)include/linux/skbuff.h +7
-0@@ -3665,5 +3665,31 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)return hdr_len +skb_gso_transport_seglen (skb); } +/* Local Checksum Offload. +* Compute outer checksum based on the assumption that the +* inner checksum will be offloaded later. +* See Documentation/networking/tx-offloads.txt for +* explanation of how this works. +* Fill in outer checksum adjustment (e.g. with sum of outer +* pseudo-header) before calling.
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3684
Quoted 10 lines
include/linux/skbuff.h +10
-0+* Also ensure that inner checksum is in linear data area. +*/ +static inline __wsum lco_csum (struct sk_buff *skb) +{ +char *inner_csum_field; +__wsum csum; + +/* Start with complement of inner checksum adjustment */ + inner_csum_field = skb->data +skb_checksum_start_offset (skb) + + skb->csum_offset;
You would probably benefit from caching off the result of skb_checksum_start_offset into a local variable so the compiler doesn't go through and recompute it when you call it again below.
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3685
Quoted 1 line
include/linux/skbuff.h +1
-0+ csum = ~csum_unfold (*(__force __sum16 *)inner_csum_field);
This seems like a lot of work, couldn't you get away with just bit-flipping this and moving it into uh->check on the outer header?
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3693
Quoted 8 lines
include/linux/skbuff.h +8
-0+/* Add in checksum of our headers (incl. outer checksum +* adjustment filled in by caller) +*/ + csum =skb_checksum (skb,0 ,skb_checksum_start_offset (skb), csum); +/* The result is the outer checksum */ +return csum; +} +
The more I think about it I am not sure how much there is to be gained by having this as a separate function anyway since I think you might be able to better exploit things with a few changes to the ordering of operations. See my notes below in the IPv4 section.
Quoted 29 lines
include/linux/skbuff.h +0
-0#endif /* __KERNEL__ */ #endif /* _LINUX_SKBUFF_H */ net/ipv4/ip_tunnel_core.c +4
-0@@ -146,6 +146,10 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md, }EXPORT_SYMBOL_GPL (iptunnel_metadata_reply); +/* csum_help should only be ever true if the protocol doesn't support LCO. +* If the tunnel uses udp_tunnel_xmit_skb(), then it gets LCO for free, and +* should always set csum_help to false. +*/ struct sk_buff *iptunnel_handle_offloads (struct sk_buff *skb,bool csum_help,int gso_type_mask)net/ipv4/udp.c +1
-2@@ -767,32 +767,23 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb, {struct udphdr *uh =udp_hdr (skb); -if (nocheck) +if (nocheck) { uh->check =0 ; -else if (skb_is_gso (skb))
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 776
Quoted 10 lines
net/ipv4/udp.c +3
-5+ }else if (skb_is_gso (skb)) { uh->check = ~udp_v4_check (len, saddr, daddr,0 ); - else if (skb_dst(skb) && skb_dst(skb)->dev && - (skb_dst(skb)->dev->features & - (NETIF_F_IP_CSUM |NETIF_F_HW_CSUM ))) { - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); + }else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum;
I wonder if this shouldn't be made a check that is in addition to the two options below instead of completely replacing them. The question I would have is if there are any cases where we need to follow the path that results in the CHECKSUM_UNNECESSARY being set.
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 781
Quoted 6 lines
net/ipv4/udp.c +5
-0+ uh->check = ~udp_v4_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +if (uh->check ==0 ) + uh->check =CSUM_MANGLED_0 ;
You would probably benefit from reordering this to something like what we have in the last block below this one. The idea is then you only halve to fold things once and can avoid some unnecessary duplication.
So you could code it up with something like:
__wsum csum;
int start_offset;
start_offset = skb_checksum_start_offset(skb);
uh->check = ~(*(__sum16 *)(skb->data + start_offset + skb->csum_offset));
csum = skb_checksum(skb, 0, start_offset, 0);
uh->check = udp_v4_check(len, saddr, daddr, csum);
if (uh->check == 0)
uh->check = CSUM_MANGLED_0;Forgive the formatting, my email client mangles tabs badly. By using the pseudo header checksum from the inner header for the starting value and then computing the udp_v4_check for the outer header last you save yourself a few cycles since you only have to fold the checksum once instead of once for the pseudo-header and again for the final result.
Quoted 31 lines
net/ipv4/udp.c +1
-12+ }else { skb->ip_summed =CHECKSUM_PARTIAL ; skb->csum_start =skb_transport_header (skb) - skb->head; skb->csum_offset = offsetof(struct udphdr , check); uh->check = ~udp_v4_check (len, saddr, daddr,0 ); - }else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v4_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp_set_csum);net/ipv6/ip6_checksum.c +0
-2@@ -98,27 +98,19 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb, uh->check =0 ; else if (skb_is_gso(skb)) uh->check = ~udp_v6_check(len, saddr, daddr, 0); - else if (skb_dst(skb) && skb_dst(skb)->dev && - (skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) {
net/ipv6/ip6_checksum.c void udp6_set_csum(bool nocheck, struct sk_buff *skb, · line 108
Quoted 10 lines
net/ipv6/ip6_checksum.c +7
-2- -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); +else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum; + uh->check = ~udp_v6_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +if (uh->check ==0 ) + uh->check =CSUM_MANGLED_0 ;
Same here for the IPv6 block. Instead of initializing uh->check using the outer pseudo-header checksum you would probably be better off using the inner header and then folding that into the outer pseudo-header checksum last.
Quoted 26 lines
net/ipv6/ip6_checksum.c +1
-13+ }else { skb->ip_summed =CHECKSUM_PARTIAL ; skb->csum_start =skb_transport_header (skb) - skb->head; skb->csum_offset = offsetof(struct udphdr , check); uh->check = ~udp_v6_check (len, saddr, daddr,0 ); - }else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v6_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp6_set_csum); --To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 net-next 1/5] net: local checksum offload for encapsulation
Quoted 39 lines · Alexander Duyck
Quoted 38 lines · Edward Cree
The arithmetic properties of the ones-complement checksum mean that a
correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.Signed-off-by Edward Cree <[email protected]>--- include/linux/skbuff.h | 26 ++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 ++++ net/ipv4/udp.c | 29 ++++++++++------------------- net/ipv6/ip6_checksum.c | 24 ++++++++---------------- 4 files changed, 48 insertions(+), 35 deletions(-)include/linux/skbuff.h +12
-0@@ -3665,5 +3665,31 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)return hdr_len +skb_gso_transport_seglen (skb); } +/* Local Checksum Offload. +* Compute outer checksum based on the assumption that the +* inner checksum will be offloaded later. +* See Documentation/networking/tx-offloads.txt for +* explanation of how this works. +* Fill in outer checksum adjustment (e.g. with sum of outer +* pseudo-header) before calling. +* Also ensure that inner checksum is in linear data area. +*/ +static inline __wsum lco_csum (struct sk_buff *skb) +{ +char *inner_csum_field;
Quoted 8 lines
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3684
Quoted 5 lines
include/linux/skbuff.h +5
-0+__wsum csum; + +/* Start with complement of inner checksum adjustment */ + inner_csum_field = skb->data +skb_checksum_start_offset (skb) + + skb->csum_offset;You would probably benefit from caching off the result of
skb_checksum_start_offsetinto a local variable so the compiler doesn't go through and recompute it when you call it again below.
It's a nearly-trivial inline function; won't the compiler be smart enough to
cache that result itself?
Quoted 3 lines
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3685
Quoted 1 line
include/linux/skbuff.h +1
-0+ csum = ~csum_unfold (*(__force __sum16 *)inner_csum_field);This seems like a lot of work, couldn't you get away with just bit-flipping this and moving it into
uh->checkon the outer header?
It's not a lot of work: all this does is zero-extend to 32 bits and flip. It looks like more, but most of it is just a cast; it's written in this way
to pacify sparse while using as little __force as possible.lco_csum can't move it into uh->check, because it doesn't have uh. In fact,
the skb might not even be UDP - this function is intended to be used also for GRE, which has an IP-style checksum but in a different place. (Next version of the patch series will implement that btw)
Quoted 48 lines
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3693
Quoted 8 lines
include/linux/skbuff.h +8
-0+/* Add in checksum of our headers (incl. outer checksum +* adjustment filled in by caller) +*/ + csum =skb_checksum (skb,0 ,skb_checksum_start_offset (skb), csum); +/* The result is the outer checksum */ +return csum; +} +The more I think about it I am not sure how much there is to be gained by having this as a separate function anyway since I think you might be able to better exploit things with a few changes to the ordering of operations. See my notes below in the IPv4 section.
Quoted 35 lines
include/linux/skbuff.h +0
-0#endif /* __KERNEL__ */ #endif /* _LINUX_SKBUFF_H */ net/ipv4/ip_tunnel_core.c +4
-0@@ -146,6 +146,10 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md, }EXPORT_SYMBOL_GPL (iptunnel_metadata_reply); +/* csum_help should only be ever true if the protocol doesn't support LCO. +* If the tunnel uses udp_tunnel_xmit_skb(), then it gets LCO for free, and +* should always set csum_help to false. +*/ struct sk_buff *iptunnel_handle_offloads (struct sk_buff *skb,bool csum_help,int gso_type_mask)net/ipv4/udp.c +2
-6@@ -767,32 +767,23 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb, {struct udphdr *uh =udp_hdr (skb); -if (nocheck) +if (nocheck) { uh->check =0 ; -else if (skb_is_gso (skb)) + }else if (skb_is_gso (skb)) { uh->check = ~udp_v4_check (len, saddr, daddr,0 ); -else if (skb_dst (skb) &&skb_dst (skb)->dev && - (skb_dst (skb)->dev->features & - (NETIF_F_IP_CSUM |NETIF_F_HW_CSUM ))) { -
Quoted 8 lines
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 776
Quoted 4 lines
net/ipv4/udp.c +2
-1-BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); + }else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum;I wonder if this shouldn't be made a check that is in addition to the two options below instead of completely replacing them. The question I would have is if there are any cases where we need to follow the path that results in the
CHECKSUM_UNNECESSARYbeing set.
I don't think there can be such a case.
Either: we've already set up PARTIAL for an inner header, so we can
definitely do LCO.
Or: we haven't set up PARTIAL yet, so we can use that now. If the device
doesn't support it, it'll get fixed up later when we validate_xmit_skb().
So there's no way (AFAICT) that we'd ever not be able to use PARTIAL. Unless - hmmm - what happens if we've set up PARTIAL for a CRC rather than
an IP checksum? However, it looks to me as if in that case the old code would have screwed up when iptunnel_handle_offloads() would do the inner csum in skb_checksum_help() and would do it as an IP checksum. So I'm guessing this probably can't happen. Or it's already broken and so my patch won't make it any worse ;)
However, the next version of the patch series will split this change out
from the rest of the patch, as Tom Herbert suggested.
Quoted 22 lines
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 781
Quoted 6 lines
net/ipv4/udp.c +5
-0+ uh->check = ~udp_v4_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +if (uh->check ==0 ) + uh->check =CSUM_MANGLED_0 ;You would probably benefit from reordering this to something like what we have in the last block below this one. The idea is then you only halve to fold things once and can avoid some unnecessary duplication.
So you could code it up with something like:
__wsumcsum;
intstart_offset;start_offset = skb_checksum_start_offset(skb); uh->check = ~(*(__sum16 *)(skb->data + start_offset + skb->csum_offset)); csum = skb_checksum(skb, 0, start_offset, 0); uh->check = udp_v4_check(len, saddr, daddr, csum); if (uh->check == 0) uh->check = CSUM_MANGLED_0;
Quoted 6 lines
Forgive the formatting, my email client mangles tabs badly. By using the pseudo header checksum from the inner header for the starting value and then computing the
udp_v4_checkfor the outer header last you save yourself a few cycles since you only have to fold the checksum once instead of once for the pseudo-header and again for the final result.
Hmm, I think we can do this without losing the helper function (which will
be shared not just by UDPv4 and UDPv6 but also GRE).
Something like this:
uh->check = 0;
uh->check = ~udp_v4_check(len, saddr, daddr, lco_csum(skb)); if (uh->check == 0)
uh->check = CSUM_MANGLED_0;
Now the only fold is the one udp_v4_check() does.
Would that shave off enough cycles to satisfy?
Re: [PATCH v2 net-next 1/5] net: local checksum offload for encapsulation
Quoted 48 lines · Edward Cree
Quoted 39 lines · Alexander Duyck
Quoted 38 lines · Edward Cree
The arithmetic properties of the ones-complement checksum mean that a
correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented).
Consequently, if we are going to offload the inner checksum with
CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.Signed-off-by Edward Cree <[email protected]>--- include/linux/skbuff.h | 26 ++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 ++++ net/ipv4/udp.c | 29 ++++++++++------------------- net/ipv6/ip6_checksum.c | 24 ++++++++---------------- 4 files changed, 48 insertions(+), 35 deletions(-)include/linux/skbuff.h +12
-0@@ -3665,5 +3665,31 @@ static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb)return hdr_len +skb_gso_transport_seglen (skb); } +/* Local Checksum Offload. +* Compute outer checksum based on the assumption that the +* inner checksum will be offloaded later. +* See Documentation/networking/tx-offloads.txt for +* explanation of how this works. +* Fill in outer checksum adjustment (e.g. with sum of outer +* pseudo-header) before calling. +* Also ensure that inner checksum is in linear data area. +*/ +static inline __wsum lco_csum (struct sk_buff *skb) +{ +char *inner_csum_field;Quoted 8 lines
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3684
Quoted 5 lines
include/linux/skbuff.h +5
-0+__wsum csum; + +/* Start with complement of inner checksum adjustment */ + inner_csum_field = skb->data +skb_checksum_start_offset (skb) + + skb->csum_offset;You would probably benefit from caching off the result of
skb_checksum_start_offsetinto a local variable so the compiler doesn't go through and recompute it when you call it again below.
Quoted 2 lines
It's a nearly-trivial inline function; won't the compiler be smart enough to
cache that result itself?
Maybe, but maybe not. I can't say for sure. The issue is that you are reading memory locations of a possibly shared structure so it could do one of several things. From my perspective it is just kind of pain to see since the function name is so long and you are having to call it a couple times. One thing that might almost be useful would be to consider looking at adding a shortcut for getting the pointer. Maybe a function like skb_csum_start_ptr(skb), that could just give you the result of "skb->head + skb->csum_start". Then it saves you the trouble of having to rely on the compiler to cancel out skb->data in an expression such as this one.
For now I am probably just engaging in a bit of premature optimization. I can probably go over the code and clean it up once the patches are actually in net-next.
Quoted 10 lines
Quoted 3 lines
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3685
Quoted 1 line
include/linux/skbuff.h +1
-0+ csum = ~csum_unfold (*(__force __sum16 *)inner_csum_field);This seems like a lot of work, couldn't you get away with just bit-flipping this and moving it into
uh->checkon the outer header?It's not a lot of work: all this does is zero-extend to 32 bits and flip. It looks like more, but most of it is just a cast; it's written in this way
to pacify sparse while using as little__forceas possible.lco_csumcan't move it intouh->check, because it doesn't have uh. In fact,
the skb might not even be UDP - this function is intended to be used also for GRE, which has an IP-style checksum but in a different place. (Next version of the patch series will implement that btw)
Actually one minor thing you might want to change here is to do the bit-flipping first, and then zero extend the value instead of the other way around. That way it makes this a bit easier to use the resultant value since you could use simple addition on it with any other 16 bit checksum without having to worry about needing to carry anything over from the addition.
Quoted 69 lines
Quoted 48 lines
include/linux/skbuff.h static inline unsigned int skb_gso_network_seglen(const struct sk_buff *skb) · line 3693
Quoted 8 lines
include/linux/skbuff.h +8
-0+/* Add in checksum of our headers (incl. outer checksum +* adjustment filled in by caller) +*/ + csum =skb_checksum (skb,0 ,skb_checksum_start_offset (skb), csum); +/* The result is the outer checksum */ +return csum; +} +The more I think about it I am not sure how much there is to be gained by having this as a separate function anyway since I think you might be able to better exploit things with a few changes to the ordering of operations. See my notes below in the IPv4 section.
Quoted 35 lines
include/linux/skbuff.h +0
-0#endif /* __KERNEL__ */ #endif /* _LINUX_SKBUFF_H */ net/ipv4/ip_tunnel_core.c +4
-0@@ -146,6 +146,10 @@ struct metadata_dst *iptunnel_metadata_reply(struct metadata_dst *md, }EXPORT_SYMBOL_GPL (iptunnel_metadata_reply); +/* csum_help should only be ever true if the protocol doesn't support LCO. +* If the tunnel uses udp_tunnel_xmit_skb(), then it gets LCO for free, and +* should always set csum_help to false. +*/ struct sk_buff *iptunnel_handle_offloads (struct sk_buff *skb,bool csum_help,int gso_type_mask)net/ipv4/udp.c +2
-6@@ -767,32 +767,23 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb, {struct udphdr *uh =udp_hdr (skb); -if (nocheck) +if (nocheck) { uh->check =0 ; -else if (skb_is_gso (skb)) + }else if (skb_is_gso (skb)) { uh->check = ~udp_v4_check (len, saddr, daddr,0 ); -else if (skb_dst (skb) &&skb_dst (skb)->dev && - (skb_dst (skb)->dev->features & - (NETIF_F_IP_CSUM |NETIF_F_HW_CSUM ))) { -Quoted 8 lines
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 776
Quoted 4 lines
net/ipv4/udp.c +2
-1-BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); + }else if (skb->ip_summed ==CHECKSUM_PARTIAL ) { +__wsum csum;I wonder if this shouldn't be made a check that is in addition to the two options below instead of completely replacing them. The question I would have is if there are any cases where we need to follow the path that results in the
CHECKSUM_UNNECESSARYbeing set.I don't think there can be such a case.
Either: we've already set up PARTIAL for an inner header, so we can
definitely do LCO.
Or: we haven't set up PARTIAL yet, so we can use that now. If the device
doesn't support it, it'll get fixed up later when wevalidate_xmit_skb().
So there's no way (AFAICT) that we'd ever not be able to use PARTIAL. Unless - hmmm - what happens if we've set up PARTIAL for a CRC rather than
an IP checksum? However, it looks to me as if in that case the old code would have screwed up wheniptunnel_handle_offloads()would do the inner csum inskb_checksum_help()and would do it as an IP checksum. So I'm guessing this probably can't happen. Or it's already broken and so my patch won't make it any worse ;)
Quoted 2 lines
However, the next version of the patch series will split this change out
from the rest of the patch, as Tom Herbert suggested.
Works for me.
Quoted 34 lines
Quoted 22 lines
net/ipv4/udp.c void udp_set_csum(bool nocheck, struct sk_buff *skb, · line 781
Quoted 6 lines
net/ipv4/udp.c +5
-0+ uh->check = ~udp_v4_check (len, saddr, daddr,0 ); + csum =lco_csum (skb); + uh->check =csum_fold (csum); +if (uh->check ==0 ) + uh->check =CSUM_MANGLED_0 ;You would probably benefit from reordering this to something like what we have in the last block below this one. The idea is then you only halve to fold things once and can avoid some unnecessary duplication.
So you could code it up with something like:
__wsumcsum;
intstart_offset;start_offset = skb_checksum_start_offset(skb); uh->check = ~(*(__sum16 *)(skb->data + start_offset + skb->csum_offset)); csum = skb_checksum(skb, 0, start_offset, 0); uh->check = udp_v4_check(len, saddr, daddr, csum); if (uh->check == 0) uh->check = CSUM_MANGLED_0;Quoted 6 lines
Forgive the formatting, my email client mangles tabs badly. By using the pseudo header checksum from the inner header for the starting value and then computing the
udp_v4_checkfor the outer header last you save yourself a few cycles since you only have to fold the checksum once instead of once for the pseudo-header and again for the final result.Hmm, I think we can do this without losing the helper function (which will
be shared not just by UDPv4 and UDPv6 but also GRE).
Something like this:
uh->check= 0;
uh->check= ~udp_v4_check(len, saddr, daddr,lco_csum(skb)); if (uh->check== 0)
Quoted 3 lines
uh->check = CSUM_MANGLED_0;Now the only fold is the one
udp_v4_check()does.
Would that shave off enough cycles to satisfy?
Yeah, that should work. When I was messing around with the code I realized we cannot assume uh->check to contain the inner checksum anyway since the GSO path has already populated uh->check with the partial checksum for the outer headers. This would mean we can reuse the lco_csum function in the GSO path.
- Alex
PATCH v2 2/5 net: enable LCO for udp_tunnel_handle_offloads() users
In reply to Edward CreeThe only protocol affected at present is Geneve.
--- include/net/udp_tunnel.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
include/net/udp_tunnel.h +2 -1
@@ -103,7 +103,8 @@ static inline struct sk_buff *udp_tunnel_handle_offloads(struct sk_buff *skb, {int type = udp_csum ?SKB_GSO_UDP_TUNNEL_CSUM :SKB_GSO_UDP_TUNNEL ; -return iptunnel_handle_offloads (skb, udp_csum, type); +/* As we're a UDP tunnel, we support LCO, so don't need csum_help */ +return iptunnel_handle_offloads (skb, false, type); }static inline void udp_tunnel_gro_complete (struct sk_buff *skb,int nhoff)
PATCH v2 3/5 net: vxlan: enable local checksum offload
In reply to Edward Cree--- drivers/net/vxlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/vxlan.c +2 -2
@@ -1733,7 +1733,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk, goto err; } - skb =iptunnel_handle_offloads (skb, udp_sum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb)) { err = -EINVAL ; goto err;
@@ -1814,7 +1814,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skif (WARN_ON (!skb))return -ENOMEM ; - skb =iptunnel_handle_offloads (skb, udp_sum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb))return PTR_ERR (skb);
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
drivers/net/vxlan.c static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk, · line 1735
Quoted 13 lines · Edward Cree
Signed-off-by Edward Cree <[email protected]>--- drivers/net/vxlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/net/vxlan.c +0
-0@@ -1733,7 +1733,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk, goto err; }
A few lines up from here there is a "udp_sum = false" that can be dropped for the remote checksum stuff.
drivers/net/vxlan.c static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk · line 1816
Quoted 9 lines
drivers/net/vxlan.c +1
-1- skb =iptunnel_handle_offloads (skb, udp_sum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb)) { err = -EINVAL ; goto err;@@ -1814,7 +1814,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skif (WARN_ON (!skb))return -ENOMEM ;
Same thing for this one. If you aren't passing it there is no point in updating it a few lines above here.
Quoted 5 lines
drivers/net/vxlan.c +1
-1- skb =iptunnel_handle_offloads (skb, udp_sum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb))return PTR_ERR (skb);
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 6 lines · Alexander Duyck
Quoted 5 lines · Edward Cree
Signed-off-by Edward Cree <[email protected]>--- drivers/net/vxlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Quoted 10 lines
drivers/net/vxlan.c static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk, · line 1735
Quoted 8 lines
drivers/net/vxlan.c +0
-0@@ -1733,7 +1733,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk, goto err; }A few lines up from here there is a "
udp_sum= false" that can be dropped for the remote checksum stuff.
Good catch. Fixed up these (and equivalents in the FOU/GUE patch) for the
nextversion of the patch series.
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
In reply to Edward CreeQuoted 20 lines · Edward Cree
Signed-off-by Edward Cree <[email protected]>--- drivers/net/vxlan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/net/vxlan.c +1
-1@@ -1733,7 +1733,7 @@ static int vxlan6_xmit_skb(struct dst_entry *dst, struct sock *sk, goto err; } - skb =iptunnel_handle_offloads (skb, udp_sum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb)) { err = -EINVAL ; goto err;
drivers/net/vxlan.c static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *sk · line 1820
Quoted 8 lines
drivers/net/vxlan.c +1
-1@@ -1814,7 +1814,7 @@ static int vxlan_xmit_skb(struct rtable *rt, struct sock *sk, struct sk_buff *skif (WARN_ON (!skb))return -ENOMEM ; - skb =iptunnel_handle_offloads (skb, udp_sum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb))return PTR_ERR (skb);
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to udp_tunnel_xmit_skb and udp_tunnel6_xmit skb need to be updated so that you pass false for the last parameter and allow the use of your udp_set_csum modifications.
- Alex
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 7 lines · Alexander Duyck
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to
udp_tunnel_xmit_skbandudp_tunnel6_xmitskb need to be updated so that you pass false for the last parameter and allow the use of yourudp_set_csummodifications.- Alex
Are you remembering to enable outer checksums on your vxlan tunnel? It's
the vxflags & VXLAN_F_UDP_CSUM check, and to enable it you need a recent iproute2 so you can use the 'udpcsum' option when creating the vxlan device.
It would be nice if that could just default to enabled now that we have LCO,
but I don't think we can change that now (ABIs set in stone and all that), so I think it would have to be iproute2 that turned it on by default.
-ed
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 11 lines · Edward Cree
Quoted 7 lines · Alexander Duyck
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to
udp_tunnel_xmit_skbandudp_tunnel6_xmitskb need to be updated so that you pass false for the last parameter and allow the use of yourudp_set_csummodifications.- Alex
Are you remembering to enable outer checksums on your vxlan tunnel? It's
the vxflags &VXLAN_F_UDP_CSUMcheck, and to enable it you need a recent iproute2 so you can use the 'udpcsum' option when creating the vxlan device.
No I hadn't done that. I kind of assumed that the checksums were just being enabled by default. That might explain the some of the issues I had.. ;-)
Quoted 3 lines
It would be nice if that could just default to enabled now that we have LCO,
but I don't think we can change that now (ABIs set in stone and all that), so I think it would have to be iproute2 that turned it on by default.
If anything we might want to expose the capabilities of the kernel so that iproute2 could make an informed decision on if it want to enable the outer checksum by default or not.
- Alex
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 17 lines · Alexander Duyck
Quoted 11 lines · Edward Cree
Quoted 7 lines · Alexander Duyck
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to
udp_tunnel_xmit_skbandudp_tunnel6_xmitskb need to be updated so that you pass false for the last parameter and allow the use of yourudp_set_csummodifications.- Alex
Are you remembering to enable outer checksums on your vxlan tunnel? It's
the vxflags &VXLAN_F_UDP_CSUMcheck, and to enable it you need a recent iproute2 so you can use the 'udpcsum' option when creating the vxlan device.No I hadn't done that. I kind of assumed that the checksums were just being enabled by default. That might explain the some of the issues I had.. ;-)
Quoted 7 lines
Quoted 3 lines
It would be nice if that could just default to enabled now that we have LCO,
but I don't think we can change that now (ABIs set in stone and all that), so I think it would have to be iproute2 that turned it on by default.If anything we might want to expose the capabilities of the kernel so that iproute2 could make an informed decision on if it want to enable the outer checksum by default or not.
Keep in mind that protocols like VXLAN specify that the UDP checksum should be set to zero. As a result, enabling the checksum is not a purely local decision and it probably shouldn't be on by default.
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 23 lines · Jesse Gross
Quoted 21 lines · Alexander Duyck
Quoted 11 lines · Edward Cree
Quoted 7 lines · Alexander Duyck
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to
udp_tunnel_xmit_skbandudp_tunnel6_xmitskb need to be updated so that you pass false for the last parameter and allow the use of yourudp_set_csummodifications.- Alex
Are you remembering to enable outer checksums on your vxlan tunnel? It's
the vxflags &VXLAN_F_UDP_CSUMcheck, and to enable it you need a recent iproute2 so you can use the 'udpcsum' option when creating the vxlan device.No I hadn't done that. I kind of assumed that the checksums were just being enabled by default. That might explain the some of the issues I had.. ;-)
Quoted 3 lines
It would be nice if that could just default to enabled now that we have LCO,
but I don't think we can change that now (ABIs set in stone and all that), so I think it would have to be iproute2 that turned it on by default.
Quoted 7 lines
Quoted 3 lines
If anything we might want to expose the capabilities of the kernel so that iproute2 could make an informed decision on if it want to enable the outer checksum by default or not.
Keep in mind that protocols like VXLAN specify that the UDP checksum should be set to zero. As a result, enabling the checksum is not a purely local decision and it probably shouldn't be on by default.
Yes, but the RFC allows for a non-zero checksum. The key difference here is the wording between SHOULD and MUST. We aren't requiring the other endpoint to enable outer checksum on its end, and we don't require the remote end to validate the outer checksum. We have made that purely optional. If however the other end is also a Linux client it can make use of the outer checksum and hardware offloads to greatly improve the performance.
It is essentially all the goodness of remote checksum offload without the compatibility negatives, assuming of course everyone implemented things according to the RFC and didn't make the 0 checksum mandatory..
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 1 line · Alexander Duyck
and we don't require the remote end to validate the outer checksum.
Doesn't the UDP specification require validating a non-zero checksum on receipt?
rick jones
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 8 lines · Rick Jones
Quoted 2 lines · Alexander Duyck
and we don't require the remote end to validate the outer checksum.
Doesn't the UDP specification require validating a non-zero checksum on receipt?
Yes, VXLAN RFC is broken in this regard since it allows a receiver to ignore a non-zero checksums. In Linux we always validate non-zero checksums.
For VXLAN, UDP checksums are not enabled for IPv4 by default, but are enabled by default for IPv6. Checksums are required for UDP/IPv6, however RFC6935 and RFC6936 relax this requirement a bit for tunnels, although IMO not enough to make disabling UDP checksums for VXLAN/IPv6 the default.
Tom
Quoted 2 lines
rick jones
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
In reply to Alexander DuyckQuoted 40 lines · Alexander Duyck
Quoted 23 lines · Jesse Gross
Quoted 21 lines · Alexander Duyck
Quoted 11 lines · Edward Cree
Quoted 7 lines · Alexander Duyck
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to
udp_tunnel_xmit_skbandudp_tunnel6_xmitskb need to be updated so that you pass false for the last parameter and allow the use of yourudp_set_csummodifications.- Alex
Are you remembering to enable outer checksums on your vxlan tunnel? It's
the vxflags &VXLAN_F_UDP_CSUMcheck, and to enable it you need a recent iproute2 so you can use the 'udpcsum' option when creating the vxlan device.No I hadn't done that. I kind of assumed that the checksums were just being enabled by default. That might explain the some of the issues I had.. ;-)
Quoted 3 lines
It would be nice if that could just default to enabled now that we have LCO,
but I don't think we can change that now (ABIs set in stone and all that), so I think it would have to be iproute2 that turned it on by default.Quoted 7 lines
Quoted 3 lines
If anything we might want to expose the capabilities of the kernel so that iproute2 could make an informed decision on if it want to enable the outer checksum by default or not.
Keep in mind that protocols like VXLAN specify that the UDP checksum should be set to zero. As a result, enabling the checksum is not a purely local decision and it probably shouldn't be on by default.
Yes, but the RFC allows for a non-zero checksum. The key difference here is the wording between SHOULD and MUST. We aren't requiring the other endpoint to enable outer checksum on its end, and we don't require the remote end to validate the outer checksum. We have made that purely optional. If however the other end is also a Linux client it can make use of the outer checksum and hardware offloads to greatly improve the performance.
Quoted 3 lines
It is essentially all the goodness of remote checksum offload without the compatibility negatives, assuming of course everyone implemented things according to the RFC and didn't make the 0 checksum mandatory..
It's hard to say what other types of endpoints might do if they receive a VXLAN packet with a checksum set and, of course, there are other protocols that don't specify that received UDP checksums can be ignored.
I'm all in favor of enabling support for the checksum but it doesn't seem right to make the default behavior go against the RFC recommendation and common implementation.
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 42 lines · Jesse Gross
Quoted 40 lines · Alexander Duyck
Quoted 23 lines · Jesse Gross
Quoted 21 lines · Alexander Duyck
Quoted 11 lines · Edward Cree
Quoted 7 lines · Alexander Duyck
So I just tried testing your patches and as it turns out you are still missing some bits. In this patch for instance the calls to
udp_tunnel_xmit_skbandudp_tunnel6_xmitskb need to be updated so that you pass false for the last parameter and allow the use of yourudp_set_csummodifications.- Alex
Are you remembering to enable outer checksums on your vxlan tunnel? It's
the vxflags &VXLAN_F_UDP_CSUMcheck, and to enable it you need a recent iproute2 so you can use the 'udpcsum' option when creating the vxlan device.No I hadn't done that. I kind of assumed that the checksums were just being enabled by default. That might explain the some of the issues I had.. ;-)
Quoted 3 lines
It would be nice if that could just default to enabled now that we have LCO,
but I don't think we can change that now (ABIs set in stone and all that), so I think it would have to be iproute2 that turned it on by default.Quoted 7 lines
Quoted 3 lines
If anything we might want to expose the capabilities of the kernel so that iproute2 could make an informed decision on if it want to enable the outer checksum by default or not.
Keep in mind that protocols like VXLAN specify that the UDP checksum should be set to zero. As a result, enabling the checksum is not a purely local decision and it probably shouldn't be on by default.
Yes, but the RFC allows for a non-zero checksum. The key difference here is the wording between SHOULD and MUST. We aren't requiring the other endpoint to enable outer checksum on its end, and we don't require the remote end to validate the outer checksum. We have made that purely optional. If however the other end is also a Linux client it can make use of the outer checksum and hardware offloads to greatly improve the performance.
Quoted 9 lines
Quoted 3 lines
It is essentially all the goodness of remote checksum offload without the compatibility negatives, assuming of course everyone implemented things according to the RFC and didn't make the 0 checksum mandatory..
It's hard to say what other types of endpoints might do if they receive a VXLAN packet with a checksum set and, of course, there are other protocols that don't specify that received UDP checksums can be ignored.
RFC1122 is very clear on the subject of UDP checksums:
"If a UDP datagram is received with a checksum that is non- zero and invalid, UDP MUST silently discard the datagram."
So if a VXLAN receiver accepts a UDP packet without checking a non-zero UDP checksum for validity they are breaking the UDP protocol. If they blow up because they don't expect to receive non-zero checksums that is entirely their problem.
Quoted 3 lines
I'm all in favor of enabling support for the checksum but it doesn't seem right to make the default behavior go against the RFC recommendation and common implementation.
RFC2460 requires UDP checksums for IPv6 which IMO supersedes what is recommended in RFC7348 which says that the checksum SHOULD be transmitted as zero. RFC6935 and RFC 6936 permit zero checksums for tunneled packets over IPv6 under specific conditions, but we cannot say that those conditions are meant in every deployment to make that a default. This is interpretation is consistent with other foo/UDP, like MPLS/UDP RFC7510 which states:
"When UDP is used over IPv6, the UDP checksum is relied upon to protect both the IPv6 and UDP headers from corruption and MUST be used unless the requirements in [RFC6935] and [RFC6936] for use of UDP zero-checksum mode with a tunnel protocol are satisfied."
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 8 lines · Tom Herbert
Quoted 5 lines · Jesse Gross
It's hard to say what other types of endpoints might do if they receive a VXLAN packet with a checksum set and, of course, there are other protocols that don't specify that received UDP checksums can be ignored.
RFC1122 is very clear on the subject of UDP checksums:
Quoted 7 lines
"If a UDP datagram is received with a checksum that is non- zero and invalid, UDP MUST silently discard the datagram."
So if a VXLAN receiver accepts a UDP packet without checking a non-zero UDP checksum for validity they are breaking the UDP protocol. If they blow up because they don't expect to receive non-zero checksums that is entirely their problem.
More to the point, RFC7348 says that if the outer checksum isvalid, the
receiver MUST accept it.
"If the decapsulating destination chooses not to perform the verification,
or performs it successfully, the packet MUST be accepted for decapsulation."
Therefore, if they blow up because they don't expect to receive non-zero
checksums, they're not compliant with the spec.
On the other hand, there is always 'be conservative in what you emit'... how
far should you go to interoperate with broken implementations.
On the gripping hand, I suspect RFC7348 only recommends against outer csums
in the first place because the authors believed it was expensive.
-ed
Re: [PATCH v2 net-next 3/5] net: vxlan: enable local checksum offload
Quoted 12 lines · Edward Cree
Quoted 11 lines · Tom Herbert
Quoted 5 lines · Jesse Gross
It's hard to say what other types of endpoints might do if they receive a VXLAN packet with a checksum set and, of course, there are other protocols that don't specify that received UDP checksums can be ignored.
RFC1122 is very clear on the subject of UDP checksums:
"If a UDP datagram is received with a checksum that is non- zero and invalid, UDP MUST silently discard the datagram."
Quoted 6 lines
Quoted 4 lines
So if a VXLAN receiver accepts a UDP packet without checking a non-zero UDP checksum for validity they are breaking the UDP protocol. If they blow up because they don't expect to receive non-zero checksums that is entirely their problem.
More to the point, RFC7348 says that if the outer checksum isvalid, the
receiver MUST accept it.
But the only way to know if a checksum is valid is to actually do the computation. :-)
Quoted 9 lines
"If the decapsulating destination chooses not to perform the verification,
or performs it successfully, the packet MUST be accepted for decapsulation."
Therefore, if they blow up because they don't expect to receive non-zero
checksums, they're not compliant with the spec.
On the other hand, there is always 'be conservative in what you emit'... how
far should you go to interoperate with broken implementations.
On the gripping hand, I suspect RFC7348 only recommends against outer csums
in the first place because the authors believed it was expensive.
Right, I've probably mentioned it at least ten times in nvo3 that we usually get *better* performance by enabling the outer checksums due to use of checksum offload. The reason Cisco doesn't want payload checksums is because switch hardware typically doesn't support it, but this doesn't justify changing fundamental UDP requirements for their purposes. Making enabling of transmitting UDP checksums per tunnel is sufficient to allow interoperability with such devices.
Quoted 1 line
-ed
PATCH v2 4/5 fou: enable LCO in FOU and GUE
In reply to Edward Cree--- net/ipv4/fou.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
net/ipv4/fou.c +2 -3
@@ -773,7 +773,6 @@ static void fou_build_udp(struct sk_buff *skb, struct ip_tunnel_encap *e, uh->dest = e->dport; uh->source = sport; uh->len =htons (skb->len); - uh->check =0 ;udp_set_csum (!(e->flags &TUNNEL_ENCAP_FLAG_CSUM ), skb, fl4->saddr, fl4->daddr, skb->len);
@@ -787,7 +786,7 @@ int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,int type = csum ?SKB_GSO_UDP_TUNNEL_CSUM :SKB_GSO_UDP_TUNNEL ;__be16 sport; - skb =iptunnel_handle_offloads (skb, csum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb))return PTR_ERR (skb);
@@ -821,7 +820,7 @@ int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e, optlen += need_priv ?GUE_LEN_PRIV :0 ; - skb =iptunnel_handle_offloads (skb, csum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb))return PTR_ERR (skb);
Re: [PATCH v2 net-next 4/5] fou: enable LCO in FOU and GUE
Quoted 18 lines · Edward Cree
Signed-off-by Edward Cree <[email protected]>--- net/ipv4/fou.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)net/ipv4/fou.c +0
-1@@ -773,7 +773,6 @@ static void fou_build_udp(struct sk_buff *skb, struct ip_tunnel_encap *e, uh->dest = e->dport; uh->source = sport; uh->len =htons (skb->len); - uh->check =0 ;udp_set_csum (!(e->flags &TUNNEL_ENCAP_FLAG_CSUM ), skb, fl4->saddr, fl4->daddr, skb->len);
net/ipv4/fou.c int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e, · line 789
Quoted 5 lines
net/ipv4/fou.c +1
-1@@ -787,7 +786,7 @@ int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,int type = csum ?SKB_GSO_UDP_TUNNEL_CSUM :SKB_GSO_UDP_TUNNEL ;__be16 sport; - skb =iptunnel_handle_offloads (skb, csum, type); + skb =iptunnel_handle_offloads (skb, false, type);
It might good to have another iptunnel_handle_offloads (lifke iptunnel_handle_offloads_nocsum) function that doesn't have csum parameter for readability.
Quoted 14 lines
net/ipv4/fou.c +1
-1if (IS_ERR (skb))return PTR_ERR (skb);@@ -821,7 +820,7 @@ int gue_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e, optlen += need_priv ?GUE_LEN_PRIV :0 ; - skb =iptunnel_handle_offloads (skb, csum, type); + skb =iptunnel_handle_offloads (skb, false, type);if (IS_ERR (skb))return PTR_ERR (skb);The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
Re: [PATCH v2 net-next 4/5] fou: enable LCO in FOU and GUE
Quoted 10 lines · Tom Herbert
net/ipv4/fou.c int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e, · line 789
Quoted 6 lines · Edward Cree
net/ipv4/fou.c +1
-1@@ -787,7 +786,7 @@ int fou_build_header(struct sk_buff *skb, struct ip_tunnel_encap *e,int type = csum ?SKB_GSO_UDP_TUNNEL_CSUM :SKB_GSO_UDP_TUNNEL ;__be16 sport; - skb =iptunnel_handle_offloads (skb, csum, type); + skb =iptunnel_handle_offloads (skb, false, type);It might good to have another
iptunnel_handle_offloads(lifkeiptunnel_handle_offloads_nocsum) function that doesn't have csum parameter for readability.
I hope to follow up soon with a patch to use LCO for GRE, at which point I
believe that all callers will be passing false anyway, so the csum_help parameter can be removed from the function entirely.
-ed
PATCH v2 5/5 Documentation/networking: add tx-offloads.txt to explain LCO
In reply to Edward CreeIt would also be a good place to explain GSO if someone wants to do that...
--- Documentation/networking/00-INDEX | 2 + Documentation/networking/tx-offloads.txt | 122 +++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 Documentation/networking/tx-offloads.txt
Documentation/networking/00-INDEX +2 -0
@@ -216,6 +216,8 @@ tproxy.txt - Transparent proxy support user guide. tuntap.txt - TUN/TAP device driver, allowing user space Rx/Tx of packets. +tx-offloads.txt + - Explanation of Transmit Offloads: Checksums, LCO, RCO udplite.txt - UDP-Lite protocol (RFC 3828) introduction. vortex.txt
Documentation/networking/tx-offloads.txt new file +122 -0
122 lines not on this page · raw message
Re: [PATCH v2 net-next 5/5] Documentation/networking: add tx-offloads.txt to explain LCO
Quoted 2 lines · Edward Cree
It would also be a good place to explain GSO if someone wants to do that...
This looks great! Maybe the title should be checksum-offload.txt though. We can add the material from skbuff.h to describe all about RX and TX checksum later on. Segmentation offload could be another doc in itself.
Tom
Quoted 149 lines
Signed-off-by Edward Cree <[email protected]>--- Documentation/networking/00-INDEX | 2 + Documentation/networking/tx-offloads.txt | 122 +++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 Documentation/networking/tx-offloads.txtDocumentation/networking/00-INDEX +2
-0@@ -216,6 +216,8 @@ tproxy.txt - Transparent proxy support user guide. tuntap.txt - TUN/TAP device driver, allowing user space Rx/Tx of packets. +tx-offloads.txt + - Explanation of Transmit Offloads: Checksums, LCO, RCO udplite.txt - UDP-Lite protocol (RFC 3828) introduction. vortex.txtDocumentation/networking/tx-offloads.txt new file +122
-0@@ -0,0 +1,122 @@ +Transmit Offloads in the Linux Networking Stack + + +Introduction +============ + +This document describes a set of techniques in the Linux networking stack + to take advantage of offload capabilities of various NICs. + +The following technologies are described: + * TX Checksum Offload + * LCO: Local Checksum Offload + * RCO: Remote Checksum Offload + +Things that should be documented here but aren't yet: + * Encapsulation offloads + - hardware VLAN acceleration + - hardware VXLAN/GRE/GENEVE/etc. encapsulation + * Segmentation offloads + - GSO: Generic Segmentation Offload + - TSO: TCP Segmentation Offload + + +TX Checksum Offload +=================== + +The interface for offloading a transmit checksum to a device is explained + in detail in comments near the top of include/linux/skbuff.h. +In brief, it allows to request the device fill in a single ones-complement + checksum defined by the sk_buff fields skb->csum_start and + skb->csum_offset. The device should compute the 16-bit ones-complement + checksum (i.e. the 'IP-style' checksum) from csum_start to the end of the + packet, and fill in the result at (csum_start + csum_offset). +Because csum_offset cannot be negative, this ensures that the previous + value of the checksum field is included in the checksum computation, thus + it can be used to supply any needed corrections to the checksum (such as + the sum of the pseudo-header for UDP or TCP). +This interface only allows a single checksum to be offloaded. Where + encapsulation is used, the packet may have multiple checksum fields in + different header layers, and the rest will have to be handled by another + mechanism such as LCO or RCO. +No offloading of the IP header checksum is performed; it is always done in + software. This is OK because when we build the IP header, we obviously + have it in cache, so summing it isn't expensive. It's also rather short. +The requirements for GSO are more complicated, because when segmenting an + encapsulated packet both the inner and outer checksums may need to be + edited or recomputed for each resulting segment. See the skbuff.h comment + (section 'E') for more details. + +A driver declares its offload capabilities in netdev->hw_features; see + Documentation/networking/netdev-features for more. Note that a device + which only advertises NETIF_F_IP[V6]_CSUM must still obey the csum_start + and csum_offset given in the SKB; if it tries to deduce these itself in + hardware (as some NICs do) the driver should check that the values in the + SKB match those which the hardware will deduce, and if not, fall back to + checksumming in software instead (with skb_checksum_help or one of the + skb_csum_off_chk* functions as mentioned in include/linux/skbuff.h). This + is a pain, but that's what you get when hardware tries to be clever. + +The stack should, for the most part, assume that checksum offload is + supported by the underlying device. The only place that should check is + validate_xmit_skb(), and the functions it calls directly or indirectly. + That function compares the offload features requested by the SKB (which + may include other offloads besides TX Checksum Offload) and, if they are + not supported or enabled on the device (determined by netdev->features), + performs the corresponding offload in software. In the case of TX + Checksum Offload, that means calling skb_checksum_help(skb). + + +LCO: Local Checksum Offload +=========================== + +LCO is a technique for efficiently computing the outer checksum of an + encapsulated datagram when the inner checksum is due to be offloaded. +The ones-complement sum of a correctly checksummed TCP or UDP packet is + equal to the sum of the pseudo header, because everything else gets + 'cancelled out' by the checksum field. This is because the sum was + complemented before being written to the checksum field. +More generally, this holds in any case where the 'IP-style' ones complement + checksum is used, and thus any checksum that TX Checksum Offload supports. +That is, if we have set up TX Checksum Offload with a start/offset pair, we + know that _after the device has filled in that checksum_, the ones + complement sum from csum_start to the end of the packet will be equal to + _whatever value we put in the checksum field beforehand_. This allows us + to compute the outer checksum without looking at the payload: we simply + stop summing when we get to csum_start, then add the 16-bit word at + (csum_start + csum_offset). +Then, when the true inner checksum is filled in (either by hardware or by + skb_checksum_help()), the outer checksum will become correct by virtue of + the arithmetic. + +LCO is performed by the stack when constructing an outer UDP header for an + encapsulation such as VXLAN or GENEVE, in udp_set_csum(). Similarly for + the IPv6 equivalents, in udp6_set_csum(). +It is *not* currently performed when constructing a GRE header; the + GRE checksum is computed over the whole packet in either + net/ipv4/ip_gre.c:build_header() or net/ipv6/ip6_gre.c:ip6gre_xmit2(), but + it should be possible to use LCO here as GRE uses an IP-style checksum. +There is a helper function lco_csum(), in include/linux/skbuff.h, which may + be useful for this. + +LCO can safely be used for nested encapsulations; in this case, the outer + encapsulation layer will sum over both its own header and the 'middle' + header. This does mean that the 'middle' header will get summed multiple + times, but there doesn't seem to be a way to avoid that without incurring + bigger costs (e.g. in SKB bloat). + + +RCO: Remote Checksum Offload +============================ + +RCO is a technique for eliding the inner checksum of an encapsulated + datagram, allowing the outer checksum to be offloaded. It does, however, + involve a change to the encapsulation protocols, which the receiver must + also support. For this reason, it is disabled by default. +RCO is detailed in the following Internet-Drafts: +https://tools.ietf.org/html/draft-herbert-remotecsumoffload-00 +https://tools.ietf.org/html/draft-herbert-vxlan-rco-00 +In Linux, RCO is implemented individually in each encapsulation protocol, + and most tunnel types have flags controlling its use. For instance, VXLAN + has the flag VXLAN_F_REMCSUM_TX (per struct vxlan_rdst) to indicate that + RCO should be used when transmitting to a given remote destination.The information contained in this message is confidential and is intended for the addressee(s) only. If you have received this message in error, please notify the sender immediately and delete the message. Unless you are an addressee (or authorized to receive for an addressee), you may not use, copy or disclose to anyone this message or any information contained in this message. The unauthorized use, disclosure, copying or alteration of this message is strictly prohibited.
cover Rework of "net: local checksum offload for encapsulation"
In reply to Edward CreeEdward,
Attached is your first patch with a couple of modifications in order to improve performance and enable the use of local checksum offloag for GSO. The second patch is my UDP local checksum offload patch for UDP tunnel segmentation offload.
Feel free to incorporate the GSO patch into your series if you want, or borrow liberally if you can apply some of this to GRE as I haven't gotten around to working on that yet as I started doing a clean-up on the Intel drivers to enable NETIF_F_HW_CSUM for a number of parts.
- Alex
---
Alexander Duyck (1):
net: Add support for UDP local checksum offload as a part of tunnel segmentation
Edward Cree (1):
net: local checksum offload for encapsulation
include/linux/skbuff.h | 30 ++++++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 +++- net/ipv4/udp.c | 24 ++++++------------------ net/ipv4/udp_offload.c | 38 +++++++++++++++++++++++--------------- net/ipv6/ip6_checksum.c | 23 ++++++----------------- 5 files changed, 68 insertions(+), 51 deletions(-)
RFC PATCH 1/2 net: local checksum offload for encapsulation
From: Edward Cree <[email protected]>
The arithmetic properties of the ones-complement checksum mean that a correctly checksummed inner packet, including its checksum, has a ones complement sum depending only on whatever value was used to initialise the checksum field before checksumming (in the case of TCP and UDP, this is the ones complement sum of the pseudo header, complemented). Consequently, if we are going to offload the inner checksum with CHECKSUM_PARTIAL, we can compute the outer checksum based only on the packed data not covered by the inner checksum, and the initial value of the inner checksum field.
--- include/linux/skbuff.h | 30 ++++++++++++++++++++++++++++++ net/ipv4/ip_tunnel_core.c | 4 +++- net/ipv4/udp.c | 24 ++++++------------------ net/ipv6/ip6_checksum.c | 23 ++++++----------------- 4 files changed, 45 insertions(+), 36 deletions(-)
include/linux/skbuff.h +30 -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;
@@ -3578,6 +3583,31 @@ static inline __sum16 gso_make_checksum(struct sk_buff *skb, __wsum res)return csum_fold (partial); } +/* Local Checksum Offload. +* Compute outer checksum based on the assumption that the +* inner checksum will be offloaded later. +* See Documentation/networking/tx-offloads.txt for +* explanation of how this works. +* Fill in outer checksum adjustment (e.g. with sum of outer +* pseudo-header) before calling. +* Also ensure that inner checksum is in linear data area. +*/ +static inline __wsum lco_csum (struct sk_buff *skb) +{ +unsigned char *csum_start =skb_checksum_start (skb); +unsigned char *l4_hdr =skb_transport_header (skb); +__wsum partial; + +/* Start with complement of inner checksum adjustment */ + partial = ~csum_unfold (*(__force __sum16 *)(csum_start + + skb->csum_offset)); + +/* Add in checksum of our headers (incl. outer checksum +* adjustment filled in by caller) and return result. +*/ +return csum_partial (l4_hdr, csum_start - l4_hdr, partial); +} +static inline bool skb_is_gso (const struct sk_buff *skb) {return skb_shinfo (skb)->gso_size;
net/ipv4/ip_tunnel_core.c +3 -1
@@ -178,8 +178,10 @@ struct sk_buff *iptunnel_handle_offloads(struct sk_buff *skb, err =skb_checksum_help (skb);if (unlikely (err)) goto error; - }else if (skb->ip_summed !=CHECKSUM_PARTIAL ) + }else if (skb->ip_summed !=CHECKSUM_PARTIAL ) { + skb->encapsulation =0 ; skb->ip_summed =CHECKSUM_NONE ; + }return skb; error:
net/ipv4/udp.c +6 -18
@@ -842,28 +842,16 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb, uh->check =0 ; else if (skb_is_gso(skb)) uh->check = ~udp_v4_check(len, saddr, daddr, 0); - else if (skb_dst(skb) && skb_dst(skb)->dev && - (skb_dst(skb)->dev->features & - (NETIF_F_IP_CSUM |NETIF_F_HW_CSUM ))) { - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - + else if (skb->ip_summed == CHECKSUM_PARTIAL) { + uh->check =0 ; + uh->check =udp_v4_check (len, saddr, daddr,lco_csum (skb)); +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); uh->check = ~udp_v4_check (len, saddr, daddr,0 ); - } else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v4_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp_set_csum);
net/ipv6/ip6_checksum.c +6 -17
@@ -98,27 +98,16 @@ void udp6_set_csum(bool nocheck, struct sk_buff *skb, uh->check =0 ; else if (skb_is_gso(skb)) uh->check = ~udp_v6_check(len, saddr, daddr, 0); - else if (skb_dst(skb) && skb_dst(skb)->dev && - (skb_dst(skb)->dev->features & NETIF_F_IPV6_CSUM)) { - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - + else if (skb->ip_summed == CHECKSUM_PARTIAL) { + uh->check =0 ; + uh->check =udp_v6_check (len, saddr, daddr,lco_csum (skb)); +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); uh->check = ~udp_v6_check (len, saddr, daddr,0 ); - } else { -__wsum csum; - -BUG_ON (skb->ip_summed ==CHECKSUM_PARTIAL ); - - uh->check =0 ; - csum =skb_checksum (skb,0 , len,0 ); - uh->check =udp_v6_check (len, saddr, daddr, csum); -if (uh->check ==0 ) - uh->check =CSUM_MANGLED_0 ; - - skb->ip_summed =CHECKSUM_UNNECESSARY ; } }EXPORT_SYMBOL (udp6_set_csum);
RFC PATCH 2/2 net: Add support for UDP local checksum offload as a part of tunnel segmentation
In reply to Alexander DuyckThis change makes it so that we can use the local checksum offload as a part of UDP tunnel segmentation offload. The advantage to this is significant as we can get both inner and outer checksum offloads on hardware that supports inner checksum offloads. This allows us to make use of the UDP Rx checksum offload available on most hardware to validate the outer and inner headers via the code that converts CHECKSUM_UNNECESSARY into CHECKSUM_COMPLETE for UDP tunnels.
--- net/ipv4/udp_offload.c | 38 +++++++++++++++++++++++--------------- 1 file changed, 23 insertions(+), 15 deletions(-)
net/ipv4/udp_offload.c +23 -15
@@ -42,28 +42,28 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,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); +bool offload_csum; oldlen = (u16 )~skb->len;if (unlikely (!pskb_may_pull (skb, tnl_hlen))) goto out; +/* 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->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 = new_protocol; - skb->encap_hdr_csum = need_csum; + skb->encap_hdr_csum = need_csum && !offload_csum; 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 )))); -/* segment inner packet. */ enc_features = skb->dev->hw_enc_features & features; segs =gso_inner_segment (skb, enc_features);
@@ -81,13 +81,10 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb,int len;__be32 delta; -if (dont_encap) { - skb->encapsulation =0 ; - skb->ip_summed =CHECKSUM_NONE ; - }else { -/* Only set up inner headers if we might be offloading -* inner checksum. -*/ +/* Only set up inner headers if we might be offloading +* inner checksum. +*/ +if (!remcsum) {skb_reset_inner_headers (skb); skb->encapsulation =1 ; }
@@ -111,6 +108,17 @@ 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 (skb->ip_summed ==CHECKSUM_PARTIAL ) { + uh->check =csum_fold (lco_csum (skb)); +if (uh->check ==0 ) + uh->check =CSUM_MANGLED_0 ; +continue ; + } + + skb->encapsulation =0 ; + skb->ip_summed =CHECKSUM_NONE ; +if (offload_csum) { skb->ip_summed =CHECKSUM_PARTIAL ; skb->csum_start =skb_transport_header (skb) - skb->head;