* [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets @ 2017-04-10 18:42 Ansis Atteka 2017-04-11 7:07 ` Steffen Klassert 0 siblings, 1 reply; 8+ messages in thread From: Ansis Atteka @ 2017-04-10 18:42 UTC (permalink / raw) To: netdev; +Cc: Ansis Atteka Otherwise, if L4 checksum calculation is done after encryption, then all ESP packets end up being corrupted at the location where pre-encryption L4 checksum field resides. One of the ways to reproduce this bug is to have a VM with virtio_net driver (UFO set to ON in the guest VM); and then encapsulate all guest's Ethernet frames in GENEVE; and then further encrypt GENEVE with IPsec. In this case following symptoms are observed: 1. If using ixgbe NIC, then the driver will also emit following warning message: ixgbe 0000:01:00.1: partial checksum but l4 proto=32! 2. Receiving VM will drop all the corrupted ESP packets, hence UDP iperf test with large packets will fail completely or TCP iperf will get ridiculously low performance because TCP window will never grow above MTU. Signed-off-by: Ansis Atteka <aatteka@ovn.org> --- net/xfrm/xfrm_output.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index 8ba29fe..7ad7e5f 100644 --- a/net/xfrm/xfrm_output.c +++ b/net/xfrm/xfrm_output.c @@ -168,7 +168,8 @@ static int xfrm_output2(struct net *net, struct sock *sk, struct sk_buff *skb) static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb) { - struct sk_buff *segs; + struct sk_buff *segs, *nskb; + int err; BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); BUILD_BUG_ON(sizeof(*IP6CB(skb)) > SKB_SGO_CB_OFFSET); @@ -180,21 +181,27 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb return -EINVAL; do { - struct sk_buff *nskb = segs->next; - int err; + nskb = segs->next; segs->next = NULL; - err = xfrm_output2(net, sk, segs); + err = skb_checksum_help(segs); + if (unlikely(err)) { + XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR); + goto error; + } + err = xfrm_output2(net, sk, segs); if (unlikely(err)) { - kfree_skb_list(nskb); - return err; + goto error; } segs = nskb; } while (segs); return 0; +error: + kfree_skb_list(nskb); + return err; } int xfrm_output(struct sock *sk, struct sk_buff *skb) -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets 2017-04-10 18:42 [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets Ansis Atteka @ 2017-04-11 7:07 ` Steffen Klassert [not found] ` <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com> 0 siblings, 1 reply; 8+ messages in thread From: Steffen Klassert @ 2017-04-11 7:07 UTC (permalink / raw) To: Ansis Atteka; +Cc: netdev On Mon, Apr 10, 2017 at 11:42:07AM -0700, Ansis Atteka wrote: > Otherwise, if L4 checksum calculation is done after encryption, > then all ESP packets end up being corrupted at the location > where pre-encryption L4 checksum field resides. > > One of the ways to reproduce this bug is to have a VM with virtio_net > driver (UFO set to ON in the guest VM); and then encapsulate all guest's > Ethernet frames in GENEVE; and then further encrypt GENEVE with IPsec. > In this case following symptoms are observed: > 1. If using ixgbe NIC, then the driver will also emit following > warning message: > ixgbe 0000:01:00.1: partial checksum but l4 proto=32! > 2. Receiving VM will drop all the corrupted ESP packets, hence UDP iperf test > with large packets will fail completely or TCP iperf will get ridiculously > low performance because TCP window will never grow above MTU. > > Signed-off-by: Ansis Atteka <aatteka@ovn.org> > --- > net/xfrm/xfrm_output.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c > index 8ba29fe..7ad7e5f 100644 > --- a/net/xfrm/xfrm_output.c > +++ b/net/xfrm/xfrm_output.c > @@ -168,7 +168,8 @@ static int xfrm_output2(struct net *net, struct sock *sk, struct sk_buff *skb) > > static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb) > { > - struct sk_buff *segs; > + struct sk_buff *segs, *nskb; > + int err; > > BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); > BUILD_BUG_ON(sizeof(*IP6CB(skb)) > SKB_SGO_CB_OFFSET); > @@ -180,21 +181,27 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb > return -EINVAL; > > do { > - struct sk_buff *nskb = segs->next; > - int err; > + nskb = segs->next; > > segs->next = NULL; > - err = xfrm_output2(net, sk, segs); > + err = skb_checksum_help(segs); What's wrong with the checksum provided by the GSO layer and why we have to do this unconditionally here? We don't announce any checksum capabilities, so the GSO layer should provide the checksum. If this is not the case, something along the path is taking wrong assumptions. Btw. all GSO packets on a standard IPv4 xfrm tunnel are getting dropped with your patch applied. ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com>]
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets [not found] ` <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com> @ 2017-04-14 2:54 ` Ansis Atteka 2017-04-18 9:09 ` Steffen Klassert 1 sibling, 0 replies; 8+ messages in thread From: Ansis Atteka @ 2017-04-14 2:54 UTC (permalink / raw) To: Steffen Klassert, netdev, Ansis Atteka On 13 April 2017 at 19:45, Ansis Atteka <ansisatteka@gmail.com> wrote: > > > > On 11 April 2017 at 00:07, Steffen Klassert <steffen.klassert@secunet.com> wrote: >> >> On Mon, Apr 10, 2017 at 11:42:07AM -0700, Ansis Atteka wrote: >> > Otherwise, if L4 checksum calculation is done after encryption, >> > then all ESP packets end up being corrupted at the location >> > where pre-encryption L4 checksum field resides. >> > >> > One of the ways to reproduce this bug is to have a VM with virtio_net >> > driver (UFO set to ON in the guest VM); and then encapsulate all guest's >> > Ethernet frames in GENEVE; and then further encrypt GENEVE with IPsec. >> > In this case following symptoms are observed: >> > 1. If using ixgbe NIC, then the driver will also emit following >> > warning message: >> > ixgbe 0000:01:00.1: partial checksum but l4 proto=32! >> > 2. Receiving VM will drop all the corrupted ESP packets, hence UDP iperf test >> > with large packets will fail completely or TCP iperf will get ridiculously >> > low performance because TCP window will never grow above MTU. >> > >> > Signed-off-by: Ansis Atteka <aatteka@ovn.org> >> > --- >> > net/xfrm/xfrm_output.c | 19 +++++++++++++------ >> > 1 file changed, 13 insertions(+), 6 deletions(-) >> > >> > diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c >> > index 8ba29fe..7ad7e5f 100644 >> > --- a/net/xfrm/xfrm_output.c >> > +++ b/net/xfrm/xfrm_output.c >> > @@ -168,7 +168,8 @@ static int xfrm_output2(struct net *net, struct sock *sk, struct sk_buff *skb) >> > >> > static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb) >> > { >> > - struct sk_buff *segs; >> > + struct sk_buff *segs, *nskb; >> > + int err; >> > >> > BUILD_BUG_ON(sizeof(*IPCB(skb)) > SKB_SGO_CB_OFFSET); >> > BUILD_BUG_ON(sizeof(*IP6CB(skb)) > SKB_SGO_CB_OFFSET); >> > @@ -180,21 +181,27 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb >> > return -EINVAL; >> > >> > do { >> > - struct sk_buff *nskb = segs->next; >> > - int err; >> > + nskb = segs->next; >> > >> > segs->next = NULL; >> > - err = xfrm_output2(net, sk, segs); >> > + err = skb_checksum_help(segs); >> >> What's wrong with the checksum provided by the GSO layer and >> why we have to do this unconditionally here? I believe with "GSO layer" you meant the skb_gso_segment() function invocation from xfrm_output_gso()? If so, then the problem with that is that the list of the skb's returned by that function could be in CHECKSUM_PARTIAL state, if skbs came from a UDP tunnel such as Geneve: xfrm_output() { __skb_gso_segment() { skb_mac_gso_segment() { skb_network_protocol(); inet_gso_segment() { udp4_ufo_fragment() { skb_udp_tunnel_segment() { skb_mac_gso_segment() { skb_network_protocol(); inet_gso_segment() { udp4_ufo_fragment() { skb_checksum() { __skb_checksum() { csum_partial() { do_csum(); } csum_partial() { do_csum(); } } Since those skbs could remain in CHECKSUM_PARTIAL state even after IPsec encryption, then ixgbe tries to calculate L4 checksums on already encrypted skb where L4 layer is already protected through IPsec integrity checks. Hence, ESP packets end up being corrupted and dropped on receive side by XFRM. I clearly see this ESP packet corruption happening by observing: 1. in wireshark that the same ESP packet differs at the offset where UDP checksum field should reside; AND 2. in dmesg that ixgbe driver complains on send side with "partial checksum but L4 proto is 0x32 (ESP)". AND 3. in /proc/net/xfrm_stat where XfrmInStateProtoErrorcounter is incremented on receive side each time it receives corrupted packet. >> >> >> We don't announce any checksum capabilities, so the GSO >> layer should provide the checksum. If this is not the case, >> something along the path is taking wrong assumptions. > > The same explicit checksum calculation is done from xfrm_output() for non-GSO case, so it was tempting for me to simply put a similar skb_checksum_help() for GSO case as well. > >> Btw. all GSO packets on a standard IPv4 xfrm tunnel are getting >> dropped with your patch applied. >> I think I just noticed possible issue with my patch that I sent out. In your setup were packets getting dropped on receive side due to UDP checksum failure (and not IPsec integrity check failure)? If so then I wonder if after my patch applied skb_checksum_help() was called twice under conditions that you tested for. Hence the skbs ended up with wrong checksums. So, would you mind to give another spin for my patch after applying this small amendment that calls skb_checkum_help() only in CHECKSUM_PARTIAL case: diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c index 7ad7e5f..a870164 100644 --- a/net/xfrm/xfrm_output.c +++ b/net/xfrm/xfrm_output.c @@ -184,10 +184,12 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb nskb = segs->next; segs->next = NULL; - err = skb_checksum_help(segs); - if (unlikely(err)) { - XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR); - goto error; + if (skb->ip_summed == CHECKSUM_PARTIAL) { + err = skb_checksum_help(segs); + if (unlikely(err)) { + XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR); + goto error; + } } err = xfrm_output2(net, sk, segs); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets [not found] ` <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com> 2017-04-14 2:54 ` Ansis Atteka @ 2017-04-18 9:09 ` Steffen Klassert 2017-04-19 2:10 ` Ansis Atteka 1 sibling, 1 reply; 8+ messages in thread From: Steffen Klassert @ 2017-04-18 9:09 UTC (permalink / raw) To: Ansis Atteka; +Cc: Ansis Atteka, netdev On Thu, Apr 13, 2017 at 07:45:08PM -0700, Ansis Atteka wrote: > On 11 April 2017 at 00:07, Steffen Klassert <steffen.klassert@secunet.com> > wrote: > > > > What's wrong with the checksum provided by the GSO layer and > > why we have to do this unconditionally here? > > > > I believe with "GSO layer" you meant the skb_gso_segment() function > invocation from xfrm_output_gso()? > > If so, then the problem with that is that the list of the skb's returned by > that function could be in CHECKSUM_PARTIAL state if skb came from a UDP > tunnel such as Geneve: This should not happen. We don't announce checksum capabilities, so the GSO layer should generate the full checksum. __skb_udp_tunnel_segment() and udp4_ufo_fragment() add the NETIF_F_HW_CSUM flag to features. This is certainly wrong if the packet undergoes an IPsec transformation. I don't have a testcase for this, so not sure if this is your problem. Could you try the (untested) patch below? diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index b2be1d9..cc0c89c 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -74,7 +74,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb, * outer one so strip the existing checksum feature flags and * instead set the flag based on our outer checksum offload value. */ - if (remcsum || ufo) { + if ((remcsum || ufo) && !(skb_dst(skb) && dst_xfrm(skb_dst(skb)))) { features &= ~NETIF_F_CSUM_MASK; if (!need_csum || offload_csum) features |= NETIF_F_HW_CSUM; @@ -238,7 +238,7 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb, * due to the fact that we have already done the checksum in * software prior to segmenting the frame. */ - if (!skb->encap_hdr_csum) + if (!skb->encap_hdr_csum && !(skb_dst(skb) && dst_xfrm(skb_dst(skb)))) features |= NETIF_F_HW_CSUM; /* Fragment the skb. IP headers of the fragments are updated in ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets 2017-04-18 9:09 ` Steffen Klassert @ 2017-04-19 2:10 ` Ansis Atteka 2017-04-20 9:47 ` Steffen Klassert 0 siblings, 1 reply; 8+ messages in thread From: Ansis Atteka @ 2017-04-19 2:10 UTC (permalink / raw) To: Steffen Klassert; +Cc: Ansis Atteka, netdev On 18 April 2017 at 02:09, Steffen Klassert <steffen.klassert@secunet.com> wrote: > On Thu, Apr 13, 2017 at 07:45:08PM -0700, Ansis Atteka wrote: >> On 11 April 2017 at 00:07, Steffen Klassert <steffen.klassert@secunet.com> >> wrote: >> > >> > What's wrong with the checksum provided by the GSO layer and >> > why we have to do this unconditionally here? >> > >> >> I believe with "GSO layer" you meant the skb_gso_segment() function >> invocation from xfrm_output_gso()? >> >> If so, then the problem with that is that the list of the skb's returned by >> that function could be in CHECKSUM_PARTIAL state if skb came from a UDP >> tunnel such as Geneve: > > This should not happen. We don't announce checksum capabilities, > so the GSO layer should generate the full checksum. > > __skb_udp_tunnel_segment() and udp4_ufo_fragment() add the > NETIF_F_HW_CSUM flag to features. This is certainly wrong > if the packet undergoes an IPsec transformation. > > I don't have a testcase for this, so not sure if this is > your problem. Could you try the (untested) patch below? I am still seeing the same ESP packet corruption issue with the patch you attached. However, after taking pointers from your patch I came up with this one that may solve this problem once and for all (note, that I was seeing this bug only with ixgbe NIC that supports tx csum offloads). I hope it does not break any other IPsec tests that you have. diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index b2be1d9..7812501 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -29,6 +29,7 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb, u16 mac_len = skb->mac_len; int udp_offset, outer_hlen; __wsum partial; + bool need_ipsec; if (unlikely(!pskb_may_pull(skb, tnl_hlen))) goto out; @@ -62,8 +63,10 @@ static struct sk_buff *__skb_udp_tunnel_segment(struct sk_buff *skb, ufo = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP); + need_ipsec = skb_dst(skb) && dst_xfrm(skb_dst(skb)); /* Try to offload checksum if possible */ offload_csum = !!(need_csum && + !need_ipsec && (skb->dev->features & (is_ipv6 ? (NETIF_F_HW_CSUM | NETIF_F_IPV6_CSUM) : (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM)))); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets 2017-04-19 2:10 ` Ansis Atteka @ 2017-04-20 9:47 ` Steffen Klassert 2017-04-21 21:45 ` Ansis Atteka 0 siblings, 1 reply; 8+ messages in thread From: Steffen Klassert @ 2017-04-20 9:47 UTC (permalink / raw) To: Ansis Atteka; +Cc: Ansis Atteka, netdev On Tue, Apr 18, 2017 at 07:10:03PM -0700, Ansis Atteka wrote: > > However, after taking pointers from your patch I came up with this one > that may solve this problem once and for all (note, that I was seeing > this bug only with ixgbe NIC that supports tx csum offloads). I hope > it does not break any other IPsec tests that you have. > > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c > index b2be1d9..7812501 100644 > --- a/net/ipv4/udp_offload.c > +++ b/net/ipv4/udp_offload.c > @@ -29,6 +29,7 @@ static struct sk_buff > *__skb_udp_tunnel_segment(struct sk_buff *skb, > u16 mac_len = skb->mac_len; > int udp_offset, outer_hlen; > __wsum partial; > + bool need_ipsec; > > if (unlikely(!pskb_may_pull(skb, tnl_hlen))) > goto out; > @@ -62,8 +63,10 @@ static struct sk_buff > *__skb_udp_tunnel_segment(struct sk_buff *skb, > > ufo = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP); > > + need_ipsec = skb_dst(skb) && dst_xfrm(skb_dst(skb)); > /* Try to offload checksum if possible */ > offload_csum = !!(need_csum && > + !need_ipsec && > (skb->dev->features & > (is_ipv6 ? (NETIF_F_HW_CSUM | NETIF_F_IPV6_CSUM) : > (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM)))); This looks good, but we should fix udp4_ufo_fragment() too. Thanks! ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets 2017-04-20 9:47 ` Steffen Klassert @ 2017-04-21 21:45 ` Ansis Atteka 2017-04-27 9:04 ` Steffen Klassert 0 siblings, 1 reply; 8+ messages in thread From: Ansis Atteka @ 2017-04-21 21:45 UTC (permalink / raw) To: Steffen Klassert; +Cc: Ansis Atteka, netdev On 20 April 2017 at 02:47, Steffen Klassert <steffen.klassert@secunet.com> wrote: > On Tue, Apr 18, 2017 at 07:10:03PM -0700, Ansis Atteka wrote: >> >> However, after taking pointers from your patch I came up with this one >> that may solve this problem once and for all (note, that I was seeing >> this bug only with ixgbe NIC that supports tx csum offloads). I hope >> it does not break any other IPsec tests that you have. >> >> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c >> index b2be1d9..7812501 100644 >> --- a/net/ipv4/udp_offload.c >> +++ b/net/ipv4/udp_offload.c >> @@ -29,6 +29,7 @@ static struct sk_buff >> *__skb_udp_tunnel_segment(struct sk_buff *skb, >> u16 mac_len = skb->mac_len; >> int udp_offset, outer_hlen; >> __wsum partial; >> + bool need_ipsec; >> >> if (unlikely(!pskb_may_pull(skb, tnl_hlen))) >> goto out; >> @@ -62,8 +63,10 @@ static struct sk_buff >> *__skb_udp_tunnel_segment(struct sk_buff *skb, >> >> ufo = !!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP); >> >> + need_ipsec = skb_dst(skb) && dst_xfrm(skb_dst(skb)); >> /* Try to offload checksum if possible */ >> offload_csum = !!(need_csum && >> + !need_ipsec && >> (skb->dev->features & >> (is_ipv6 ? (NETIF_F_HW_CSUM | NETIF_F_IPV6_CSUM) : >> (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM)))); > > This looks good, but we should fix udp4_ufo_fragment() too. > > Thanks! I removed Geneve tunneling from equation and tried to run a simple iperf underlay UDP test while IPsec was still enabled to observe issues with the udp4_ufo_fragment() case. Unfortunately, as can be seen from kernel tracer output below, I was unable to come up with a test case where udp4_ufo_fragment function would ever be invoked while IPsec is enabled: admin1@ubuntu1:~/xfrm_test/net$ ifconfig em2.4001 | grep "inet addr" inet addr:192.168.1.1 Bcast:192.168.1.255 Mask:255.255.255.0 admin1@ubuntu1:~/xfrm_test/net$ ethtool -k em2.4001 | grep udp-fragmentation-offload udp-fragmentation-offload: on admin1@ubuntu1:~/xfrm_test/net$ sudo trace-cmd record -p function_graph -c -F iperf -c 192.168.1.2 -u -l20000 admin1@ubuntu1:~/xfrm_test/net$ trace-cmd report | grep udp4 admin1@ubuntu1:~/xfrm_test/net$ Nevertheless, after disabling IPsec and leaving everything else the same, I start to see that udp4_ufo_fragment() gets invoked: admin1@ubuntu1:~/xfrm_test/net$ trace-cmd report | grep udp4 iperf-25466 [004] 242431.203307: funcgraph_entry: 0.113 us | udp4_hwcsum(); iperf-25466 [004] 242431.203360: funcgraph_entry: | udp4_ufo_fragment() { iperf-25466 [004] 242431.508436: funcgraph_entry: 0.080 us | udp4_hwcsum(); iperf-25466 [004] 242431.508542: funcgraph_entry: | udp4_ufo_fragment() { However, non-IPsec case really does not have this ESP packet corruption problem, because then the packets are in plain and can utilize checksum offloads. Do we really have a problem there for IPsec? I did not have time yet to look into the code to understand why exactly udp4_ufo_fragment() is not called for IPsec case, but since I can't come up with a real life test case, then for now I will simply resubmit the previous patch as-is to netdev mailinglist to solve the problem I encountered previously for Geneve tunneling case. If you could drop more hints on how to come up with an IPsec test case where udp4_ufo_fragment() is still invoked and packets get corrupted, then I can later send another patch for that. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets 2017-04-21 21:45 ` Ansis Atteka @ 2017-04-27 9:04 ` Steffen Klassert 0 siblings, 0 replies; 8+ messages in thread From: Steffen Klassert @ 2017-04-27 9:04 UTC (permalink / raw) To: Ansis Atteka; +Cc: Ansis Atteka, netdev On Fri, Apr 21, 2017 at 02:45:17PM -0700, Ansis Atteka wrote: > > I removed Geneve tunneling from equation and tried to run a simple > iperf underlay UDP test while IPsec was still enabled to observe > issues with the udp4_ufo_fragment() case. > > Unfortunately, as can be seen from kernel tracer output below, I was > unable to come up with a test case where udp4_ufo_fragment function > would ever be invoked while IPsec is enabled: > > admin1@ubuntu1:~/xfrm_test/net$ ifconfig em2.4001 | grep "inet addr" > inet addr:192.168.1.1 Bcast:192.168.1.255 Mask:255.255.255.0 > admin1@ubuntu1:~/xfrm_test/net$ ethtool -k em2.4001 | grep > udp-fragmentation-offload > udp-fragmentation-offload: on > admin1@ubuntu1:~/xfrm_test/net$ sudo trace-cmd record -p > function_graph -c -F iperf -c 192.168.1.2 -u -l20000 > admin1@ubuntu1:~/xfrm_test/net$ trace-cmd report | grep udp4 > admin1@ubuntu1:~/xfrm_test/net$ > > > Nevertheless, after disabling IPsec and leaving everything else the > same, I start to see that udp4_ufo_fragment() gets invoked: > > admin1@ubuntu1:~/xfrm_test/net$ trace-cmd report | grep udp4 > iperf-25466 [004] 242431.203307: funcgraph_entry: > 0.113 us | udp4_hwcsum(); > iperf-25466 [004] 242431.203360: funcgraph_entry: > | > udp4_ufo_fragment() { > iperf-25466 [004] 242431.508436: funcgraph_entry: > 0.080 us | udp4_hwcsum(); > iperf-25466 [004] 242431.508542: funcgraph_entry: > | > udp4_ufo_fragment() { > > > However, non-IPsec case really does not have this ESP packet > corruption problem, because then the packets are in plain and can > utilize checksum offloads. Do we really have a problem there for > IPsec? Probably not, at least locally generated packets don't do ufo if they have an IPsec route. So it seems to be ok to leave udp4_ufo_fragment as it is. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-04-27 9:23 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-04-10 18:42 [PATCH net] xfrm: calculate L4 checksums also for GSO case before encrypting packets Ansis Atteka 2017-04-11 7:07 ` Steffen Klassert [not found] ` <CAMQa7Biajree5Kc1fOWQN42R1UDDGp7ZevZZRtUMZOKDWTk-Vw@mail.gmail.com> 2017-04-14 2:54 ` Ansis Atteka 2017-04-18 9:09 ` Steffen Klassert 2017-04-19 2:10 ` Ansis Atteka 2017-04-20 9:47 ` Steffen Klassert 2017-04-21 21:45 ` Ansis Atteka 2017-04-27 9:04 ` Steffen Klassert
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.