Re: [Xen-devel] [PATCH] Fix checksum errors when firewalling in domU

[James Dykman <dykman@xxxxxxxxxx>]

xen-devel-bounces@xxxxxxxxxxxxxxxxxxx wrote on 05/09/2006 04:05:30 PM:

> 
> On 9 May 2006, at 20:22, James Dykman wrote:
> 
> > @@ -819,7 +819,10 @@
> >                  * can infer it from csum_blank so test both flags.
> >                  */
> >                 if (rx->flags & 
> > (NETRXF_data_validated|NETRXF_csum_blank))
> > {
> > -                       skb->ip_summed = CHECKSUM_UNNECESSARY;
> > +                       if (rx->flags & NETRXF_csum_blank)
> > +                               skb->ip_summed = CHECKSUM_HW;
> > +                       else
> > +                               skb->ip_summed = CHECKSUM_UNNECESSARY;
> >                         skb->proto_data_valid = 1;
> >                 } else {
> >                         skb->ip_summed = CHECKSUM_NONE;
> 
> This hunk seems dodgy to me. According to the comment in linux/skbuff.h 
> we shouldn't be passing up CHECKSUM_HW unless we have set skb->csum to 
> the 1s-complement sum of the packet contents. You added code to do this 
> in both netfront and netback, and it doesn't seem right in either case.
> 
>   -- Keir
> 

Right, that was hiding more brokenness. Fixed below.

Signed-off-by: Jim Dykman <dykman@xxxxxxxxxx>
Fix checksum errors when firewalling in domU

diff -r 1e3977e029fd linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Mon May  8 
18:21:41 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Wed May 10 
09:57:42 2006
@@ -172,6 +172,7 @@
                BUG_ON(ret);
                nskb->dev = skb->dev;
                nskb->proto_data_valid = skb->proto_data_valid;
+               nskb->proto_csum_blank = skb->proto_csum_blank;
                dev_kfree_skb(skb);
                skb = nskb;
        }
@@ -338,8 +339,10 @@
                flags = 0;
                if (skb->ip_summed == CHECKSUM_HW) /* local packet? */
                        flags |= NETRXF_csum_blank | 
NETRXF_data_validated;
-               else if (skb->proto_data_valid) /* remote but checksummed? 
*/
+               if (skb->proto_data_valid) /* remote but checksummed? */
                        flags |= NETRXF_data_validated;
+               if (skb->proto_csum_blank) /* forwarded, !checksummed */
+                       flags |= NETRXF_csum_blank;
                if (make_rx_response(netif, id, status,
                                     (unsigned long)skb->data & 
~PAGE_MASK,
                                     size, flags) &&
diff -r 1e3977e029fd linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
--- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Mon May  8 
18:21:41 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Wed May 10 
09:57:42 2006
@@ -699,6 +699,8 @@
                tx->flags |= NETTXF_csum_blank | NETTXF_data_validated;
        if (skb->proto_data_valid) /* remote but checksummed? */
                tx->flags |= NETTXF_data_validated;
+       if (skb->proto_csum_blank) /* remote, not checksummed */
+               tx->flags |= NETTXF_csum_blank;

        np->tx.req_prod_pvt = i + 1;
        RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->tx, notify);
@@ -1024,6 +1026,8 @@
                        tx->flags |= NETTXF_csum_blank | 
NETTXF_data_validated;
                if (skb->proto_data_valid) /* remote but checksummed? */
                        tx->flags |= NETTXF_data_validated;
+               if (skb->proto_csum_blank) /* remote, not checksummed */
+                       tx->flags |= NETTXF_csum_blank;

                np->stats.tx_bytes += skb->len;
                np->stats.tx_packets++;
diff -r 1e3977e029fd linux-2.6-xen-sparse/include/linux/skbuff.h
--- a/linux-2.6-xen-sparse/include/linux/skbuff.h       Mon May  8 
18:21:41 2006
+++ b/linux-2.6-xen-sparse/include/linux/skbuff.h       Wed May 10 
09:57:42 2006
@@ -1281,6 +1281,14 @@
 extern void skb_init(void);
 extern void skb_add_mtu(int mtu);

+#ifdef CONFIG_XEN
+static inline void reset_proto_csum_blank(struct sk_buff *skb)
+{
+       skb->proto_csum_blank=0;
+}
+#else
+static inline void reset_proto_csum_blank(const struct sk_buff *skb) { }
+#endif
 /**
  *     skb_get_timestamp - get timestamp from a skb
  *     @skb: skb to get stamp from
diff -r 1e3977e029fd linux-2.6-xen-sparse/net/core/dev.c
--- a/linux-2.6-xen-sparse/net/core/dev.c       Mon May  8 18:21:41 2006
+++ b/linux-2.6-xen-sparse/net/core/dev.c       Wed May 10 09:57:42 2006
@@ -1246,7 +1246,6 @@
                if ((skb->h.raw + skb->csum + 2) > skb->tail)
                        goto out;
                skb->ip_summed = CHECKSUM_HW;
-               skb->proto_csum_blank = 0;
        }
        return 0;
 out:
@@ -1315,9 +1314,11 @@
        if (skb->ip_summed == CHECKSUM_HW &&
            (!(dev->features & (NETIF_F_HW_CSUM | NETIF_F_NO_CSUM)) &&
             (!(dev->features & NETIF_F_IP_CSUM) ||
-             skb->protocol != htons(ETH_P_IP))))
+             skb->protocol != htons(ETH_P_IP)))) {
                if (skb_checksum_help(skb, 0))
                        goto out_kfree_skb;
+               reset_proto_csum_blank(skb);
+       }

        spin_lock_prefetch(&dev->queue_lock);

diff -r 1e3977e029fd patches/linux-2.6.16.13/net-csum.patch
--- a/patches/linux-2.6.16.13/net-csum.patch    Mon May  8 18:21:41 2006
+++ b/patches/linux-2.6.16.13/net-csum.patch    Wed May 10 09:57:42 2006
@@ -40,8 +40,8 @@
        return 1;
  }
 diff -pruN ../pristine-linux-2.6.16.13/net/ipv4/xfrm4_output.c 
./net/ipv4/xfrm4_output.c
---- ../pristine-linux-2.6.16.13/net/ipv4/xfrm4_output.c        2006-05-02 
22:38:44.000000000 +0100
-+++ ./net/ipv4/xfrm4_output.c  2006-05-04 17:41:37.000000000 +0100
+--- ../pristine-linux-2.6.16.13/net/ipv4/xfrm4_output.c        2006-05-02 
17:38:44.000000000 -0400
++++ ./net/ipv4/xfrm4_output.c  2006-05-09 13:08:15.000000000 -0400
 @@ -17,6 +17,8 @@
  #include <net/xfrm.h>
  #include <net/icmp.h>
@@ -51,7 +51,7 @@
  /* Add encapsulation header.
   *
   * In transport mode, the IP header will be moved forward to make space
-@@ -103,6 +105,10 @@ static int xfrm4_output_one(struct sk_bu
+@@ -103,10 +105,15 @@ static int xfrm4_output_one(struct sk_bu
        struct xfrm_state *x = dst->xfrm;
        int err;

@@ -62,3 +62,8 @@
        if (skb->ip_summed == CHECKSUM_HW) {
                err = skb_checksum_help(skb, 0);
                if (err)
+                       goto error_nolock;
++              reset_proto_csum_blank(skb);
+       }
+
+       if (x->props.mode) {

Attachment: vlanfw2.patch
Description: Binary data

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