[Xen-devel] [PATCH] [BUG 143] [2nd try] Fix checksum errors over IPSec ESP tunnels

[James Dykman <dykman@xxxxxxxxxx>]

This patch moves the prep for checksum calculations from dev_queue_xmit() 
into netback. The pointers in the skb headerare
set up for all IPv4 TCP/UDP traffic passing through netback, but the 
checksums are not actually calculated until they absolutely
have to be (currently in dev_queue_xmit() and xfrm4_output()).

This removes some xen-specific code from the net/core/dev.c, at the 
expense of some overhead for domU-domU traffic. Also
note that xfrm4_output() has to recalculate skb->h.raw, which gets 
stomped. I have not bothered to track down where this happens;
I think it's a sign that setting up these pointers so far from where they 
will be used is a bit fragile. But, it does work/fix the problem.

Tested on:
changeset:   9533:806d04252761
tag:         tip
user:        kaf24@xxxxxxxxxxxxxxxxxxxx
date:        Wed Apr  5 05:37:37 2006 +0100
summary:     Fix checksum-offload problems introduced in c/s 9514, due to

With xm-test w/network-route/vif-route, and also with the failing config 
from bug 143.

For reference, the first try: 
http://lists.xensource.com/archives/html/xen-devel/2006-02/msg00162.html
Comments were: 
http://lists.xensource.com/archives/html/xen-devel/2006-03/msg00521.html

Signed-off-by: Jim Dykman <dykman@xxxxxxxxxx>

Jim


diff -r a8b1d4fad72d linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon Mar 20 
11:01:32 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Fri Mar 24 
16:45:17 2006
@@ -13,6 +13,9 @@
 #include "common.h"
 #include <xen/balloon.h> 
 #include <xen/interface/memory.h>
+#include <net/ip.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
 
 /*#define NETBE_DEBUG_INTERRUPT*/
 
@@ -451,6 +454,33 @@
        }
 }

+inline static int checksum_setup(struct sk_buff *skb)
+{
+       if (skb->protocol != htons(ETH_P_IP))
+               return -1;
+       skb->nh.raw = skb->data;
+       skb->h.raw = (unsigned char *)skb->nh.iph + 4*skb->nh.iph->ihl;
+       if (skb->h.raw >= skb->tail)
+               return -1;
+       switch (skb->nh.iph->protocol) {
+       case IPPROTO_TCP:
+               skb->csum = offsetof(struct tcphdr, check);
+               break;
+       case IPPROTO_UDP:
+               skb->csum = offsetof(struct udphdr, check); 
+               break;
+       default:
+               if (net_ratelimit())
+                       printk(KERN_ERR "Attempting to checksum a non-"
+                              "TCP/UDP packet, dropping a protocol"
diff -r a8b1d4fad72d linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon Mar 20 
11:01:32 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Fri Mar 24 
16:45:17 2006
@@ -13,6 +13,9 @@
 #include "common.h"
 #include <xen/balloon.h> 
 #include <xen/interface/memory.h>
+#include <net/ip.h>
+#include <linux/tcp.h>
+#include <linux/udp.h>
 
 /*#define NETBE_DEBUG_INTERRUPT*/
 
@@ -451,6 +454,33 @@
        }
 }

+inline static int checksum_setup(struct sk_buff *skb)
+{
+       if (skb->protocol != htons(ETH_P_IP))
+               return -1;
+       skb->nh.raw = skb->data;
+       skb->h.raw = (unsigned char *)skb->nh.iph + 4*skb->nh.iph->ihl;
+       if (skb->h.raw >= skb->tail)
+               return -1;
+       switch (skb->nh.iph->protocol) {
+       case IPPROTO_TCP:
+               skb->csum = offsetof(struct tcphdr, check);
+               break;
+       case IPPROTO_UDP:
+               skb->csum = offsetof(struct udphdr, check); 
+               break;
+       default:
+               if (net_ratelimit())
+                       printk(KERN_ERR "Attempting to checksum a non-"
+                              "TCP/UDP packet, dropping a protocol"
+                                     " %d packet", 
skb->nh.iph->protocol);
+               return -1;
+       }
+       if ((skb->h.raw + skb->csum + 2) > skb->tail)
+               return -1;
+       return 0;
+}
+
 /* Called after netfront has transmitted */
 static void net_tx_action(unsigned long unused)
 {
@@ -637,6 +667,14 @@
                skb->ip_summed        = CHECKSUM_UNNECESSARY;
                skb->proto_csum_valid = 1;
                skb->proto_csum_blank = !!(txreq.flags & 
NETTXF_csum_blank);
+               if (skb->proto_csum_blank) {
+                       ret = checksum_setup(skb);
+                       if (ret < 0) {
+                               kfree_skb(skb);
+                               mop++;
+                               continue;
+                       }
+               }

                netif->stats.rx_bytes += txreq.size;
                netif->stats.rx_packets++;
diff -r a8b1d4fad72d linux-2.6-xen-sparse/net/core/dev.c
--- a/linux-2.6-xen-sparse/net/core/dev.c       Mon Mar 20 11:01:32 2006
+++ b/linux-2.6-xen-sparse/net/core/dev.c       Fri Mar 24 16:45:17 2006
@@ -116,12 +116,6 @@
 #endif /* CONFIG_NET_RADIO */
 #include <asm/current.h>

-#ifdef CONFIG_XEN
-#include <net/ip.h>
-#include <linux/tcp.h>
-#include <linux/udp.h>
-#endif
-
 /*
  *     The list of packet types we will receive (as opposed to discard)
  *     and the routines to invoke.
@@ -1270,31 +1264,8 @@
        /* If a checksum-deferred packet is forwarded to a device that 
needs a
         * checksum, correct the pointers and force checksumming.
         */
-       if (skb->proto_csum_blank) {
-               if (skb->protocol != htons(ETH_P_IP))
-                       goto out_kfree_skb;
-               skb->h.raw = (unsigned char *)skb->nh.iph + 
4*skb->nh.iph->ihl;
-               if (skb->h.raw >= skb->tail)
-                       goto out_kfree_skb;
-               switch (skb->nh.iph->protocol) {
-               case IPPROTO_TCP:
-                       skb->csum = offsetof(struct tcphdr, check);
-                       break;
-               case IPPROTO_UDP:
-                       skb->csum = offsetof(struct udphdr, check);
-                       break;
-               default:
-                       if (net_ratelimit())
-                               printk(KERN_ERR "Attempting to checksum a 
non-"
-                                      "TCP/UDP packet, dropping a 
protocol"
-                                      " %d packet", 
skb->nh.iph->protocol);
-                       rc = -EPROTO;
-                       goto out_kfree_skb;
-               }
-               if ((skb->h.raw + skb->csum + 2) > skb->tail)
-                       goto out_kfree_skb;
+       if (skb->proto_csum_blank)
                skb->ip_summed = CHECKSUM_HW;
-       }
 #endif

        /* If packet is not checksummed and device does not support
diff -r a8b1d4fad72d patches/linux-2.6.16-rc6/net-csum.patch
--- a/patches/linux-2.6.16/net-csum.patch       Mon Mar 20 11:01:32 2006
+++ b/patches/linux-2.6.16/net-csum.patch       Fri Mar 24 16:45:17 2006
@@ -39,3 +39,18 @@
        *portptr = newport;
        return 1;
  }
+--- ../pristine-linux-2.6.16/net/ipv4/xfrm4_output.c   2006-03-24 
16:17:38.000000000 -0500
++++ ./net/ipv4/xfrm4_output.c  2006-03-24 16:13:24.000000000 -0500
+@@ -103,6 +103,12 @@ static int xfrm4_output_one(struct sk_bu
+       struct xfrm_state *x = dst->xfrm;
+       int err;
+
++      if (skb->proto_csum_blank) {
++              skb->ip_summed = CHECKSUM_HW;
++              skb->proto_csum_blank = 0;
++              skb->h.raw = (unsigned char *)skb->nh.iph + 
4*skb->nh.iph->ihl;
++      }
++
+       if (skb->ip_summed == CHECKSUM_HW) {
+               err = skb_checksum_help(skb, 0);
+               if (err)

Attachment: bug143.patch.v2
Description: Binary data

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel