WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

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

To: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] Fix checksum errors when firewalling in domU
From: James Dykman <dykman@xxxxxxxxxx>
Date: Wed, 10 May 2006 10:51:34 -0400
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 10 May 2006 07:52:09 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <59e0d0fc156c212b642ad159500c628e@xxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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