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-changelog

[Xen-changelog] Make checksum handling in the virtual network drivers mo

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] Make checksum handling in the virtual network drivers more robust.
From: Xen patchbot -unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Sun, 02 Apr 2006 16:36:07 +0000
Delivery-date: Sun, 02 Apr 2006 16:38:07 +0000
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User kaf24@xxxxxxxxxxxxxxxxxxxx
# Node ID 4ad317429111d43cc99810af25b3faf6ffed4209
# Parent  99b2e765d643cc3008ac95b5596487b082d8ad56
Make checksum handling in the virtual network drivers more robust.
Largely this involves making the logic symmetrical: for example,
not only should netfront be able to tell netback that a packet has
an empty protocol checksum field, but the reverse must also be true.

Another change is that the drivers only advertise IP checksum
offload functionality. There is currently no information
propagated across the device channel about the offset of the
protocol-specific checksum field. Therefore it is not safe to
defer checksum calculation for protocols the remote end may not
understand -- it will end up dropping having to drop the packet.

Yet another change is to allow netback to disable tx checksum
offload, just as we already could for netfront. Currently there is
no support for disabling rx checksum offload -- that would seem
to require some way of propagating the checksum-offload advertisement
(or lack of it) across the device channel, as it really ought to be
the transmitter that acts on it.

Thanks to Ian Jackson for pointing out some of the problems with
our checksum-offload handling. Several of the changes here are
due to his comments.

Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>

diff -r 99b2e765d643 -r 4ad317429111 
linux-2.6-xen-sparse/drivers/xen/netback/interface.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/interface.c      Sun Apr  2 
08:49:17 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/interface.c      Sun Apr  2 
15:16:53 2006
@@ -31,6 +31,7 @@
  */
 
 #include "common.h"
+#include <linux/ethtool.h>
 #include <linux/rtnetlink.h>
 
 static void __netif_up(netif_t *netif)
@@ -70,6 +71,12 @@
                __netif_down(netif);
        return 0;
 }
+
+static struct ethtool_ops network_ethtool_ops =
+{
+       .get_tx_csum = ethtool_op_get_tx_csum,
+       .set_tx_csum = ethtool_op_set_tx_csum,
+};
 
 netif_t *alloc_netif(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN])
 {
@@ -101,7 +108,9 @@
        dev->get_stats       = netif_be_get_stats;
        dev->open            = net_open;
        dev->stop            = net_close;
-       dev->features        = NETIF_F_NO_CSUM;
+       dev->features        = NETIF_F_IP_CSUM;
+
+       SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
        /* Disable queuing. */
        dev->tx_queue_len = 0;
diff -r 99b2e765d643 -r 4ad317429111 
linux-2.6-xen-sparse/drivers/xen/netback/loopback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/loopback.c       Sun Apr  2 
08:49:17 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/loopback.c       Sun Apr  2 
15:16:53 2006
@@ -100,10 +100,10 @@
                /* Defer checksum calculation. */
                skb->proto_csum_blank = 1;
                /* Must be a local packet: assert its integrity. */
-               skb->proto_csum_valid = 1;
+               skb->proto_data_valid = 1;
        }
 
-       skb->ip_summed = skb->proto_csum_valid ?
+       skb->ip_summed = skb->proto_data_valid ?
                CHECKSUM_UNNECESSARY : CHECKSUM_NONE;
 
        skb->pkt_type = PACKET_HOST; /* overridden by eth_type_trans() */
@@ -121,6 +121,12 @@
        return &np->stats;
 }
 
+static struct ethtool_ops network_ethtool_ops =
+{
+       .get_tx_csum = ethtool_op_get_tx_csum,
+       .set_tx_csum = ethtool_op_set_tx_csum,
+};
+
 static void loopback_construct(struct net_device *dev, struct net_device *lo)
 {
        struct net_private *np = netdev_priv(dev);
@@ -134,7 +140,11 @@
 
        dev->tx_queue_len    = 0;
 
-       dev->features        = NETIF_F_HIGHDMA | NETIF_F_LLTX;
+       dev->features        = (NETIF_F_HIGHDMA |
+                               NETIF_F_LLTX |
+                               NETIF_F_IP_CSUM);
+
+       SET_ETHTOOL_OPS(dev, &network_ethtool_ops);
 
        /*
         * We do not set a jumbo MTU on the interface. Otherwise the network
@@ -147,12 +157,6 @@
        /*dev->mtu             = 16*1024;*/
 }
 
-static struct ethtool_ops network_ethtool_ops =
-{
-       .get_tx_csum = ethtool_op_get_tx_csum,
-       .set_tx_csum = ethtool_op_set_tx_csum,
-};
-
 static int __init make_loopback(int i)
 {
        struct net_device *dev1, *dev2;
@@ -171,11 +175,6 @@
 
        loopback_construct(dev1, dev2);
        loopback_construct(dev2, dev1);
-
-       dev1->features |= NETIF_F_NO_CSUM;
-       dev2->features |= NETIF_F_IP_CSUM;
-
-       SET_ETHTOOL_OPS(dev2, &network_ethtool_ops);
 
        /*
         * Initialise a dummy MAC address for the 'dummy backend' interface. We
diff -r 99b2e765d643 -r 4ad317429111 
linux-2.6-xen-sparse/drivers/xen/netback/netback.c
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Sun Apr  2 
08:49:17 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c        Sun Apr  2 
15:16:53 2006
@@ -171,7 +171,7 @@
                                     skb->len + hlen);
                BUG_ON(ret);
                nskb->dev = skb->dev;
-               nskb->proto_csum_valid = skb->proto_csum_valid;
+               nskb->proto_data_valid = skb->proto_data_valid;
                dev_kfree_skb(skb);
                skb = nskb;
        }
@@ -213,7 +213,7 @@
 {
        netif_t *netif = NULL; 
        s8 status;
-       u16 size, id, irq;
+       u16 size, id, irq, flags;
        multicall_entry_t *mcl;
        mmu_update_t *mmu;
        gnttab_transfer_t *gop;
@@ -328,10 +328,14 @@
                }
                irq = netif->irq;
                id = RING_GET_REQUEST(&netif->rx, netif->rx.rsp_prod_pvt)->id;
+               flags = 0;
+               if (skb->ip_summed == CHECKSUM_HW)
+                       flags |= NETRXF_csum_blank;
+               if (skb->proto_data_valid)
+                       flags |= NETRXF_data_validated;
                if (make_rx_response(netif, id, status,
                                     (unsigned long)skb->data & ~PAGE_MASK,
-                                    size, skb->proto_csum_valid ?
-                                    NETRXF_data_validated : 0) &&
+                                    size, flags) &&
                    (rx_notify[irq] == 0)) {
                        rx_notify[irq] = 1;
                        notify_list[notify_nr++] = irq;
@@ -654,12 +658,13 @@
                skb->dev      = netif->dev;
                skb->protocol = eth_type_trans(skb, skb->dev);
 
-               /*
-                 * No checking needed on localhost, but remember the field is
-                 * blank. 
-                 */
-               skb->ip_summed        = CHECKSUM_UNNECESSARY;
-               skb->proto_csum_valid = 1;
+               if (txreq.flags & NETTXF_data_validated) {
+                       skb->ip_summed = CHECKSUM_UNNECESSARY;
+                       skb->proto_data_valid = 1;
+               } else {
+                       skb->ip_summed = CHECKSUM_NONE;
+                       skb->proto_data_valid = 0;
+               }
                skb->proto_csum_blank = !!(txreq.flags & NETTXF_csum_blank);
 
                netif->stats.rx_bytes += txreq.size;
diff -r 99b2e765d643 -r 4ad317429111 
linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
--- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Sun Apr  2 
08:49:17 2006
+++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c      Sun Apr  2 
15:16:53 2006
@@ -696,7 +696,12 @@
        tx->gref = np->grant_tx_ref[id] = ref;
        tx->offset = (unsigned long)skb->data & ~PAGE_MASK;
        tx->size = skb->len;
-       tx->flags = (skb->ip_summed == CHECKSUM_HW) ? NETTXF_csum_blank : 0;
+
+       tx->flags = 0;
+       if (skb->ip_summed == CHECKSUM_HW)
+               tx->flags |= NETTXF_csum_blank;
+       if (skb->proto_data_valid)
+               tx->flags |= NETTXF_data_validated;
 
        np->tx.req_prod_pvt = i + 1;
        RING_PUSH_REQUESTS_AND_CHECK_NOTIFY(&np->tx, notify);
@@ -811,8 +816,14 @@
                skb->len  = rx->status;
                skb->tail = skb->data + skb->len;
 
-               if (rx->flags & NETRXF_data_validated)
+               if (rx->flags & NETRXF_data_validated) {
                        skb->ip_summed = CHECKSUM_UNNECESSARY;
+                       skb->proto_data_valid = 1;
+               } else {
+                       skb->ip_summed = CHECKSUM_NONE;
+                       skb->proto_data_valid = 0;
+               }
+               skb->proto_csum_blank = !!(rx->flags & NETRXF_csum_blank);
 
                np->stats.rx_packets++;
                np->stats.rx_bytes += rx->status;
diff -r 99b2e765d643 -r 4ad317429111 linux-2.6-xen-sparse/include/linux/skbuff.h
--- a/linux-2.6-xen-sparse/include/linux/skbuff.h       Sun Apr  2 08:49:17 2006
+++ b/linux-2.6-xen-sparse/include/linux/skbuff.h       Sun Apr  2 15:16:53 2006
@@ -189,7 +189,7 @@
  *     @local_df: allow local fragmentation
  *     @cloned: Head may be cloned (check refcnt to be sure)
  *     @nohdr: Payload reference only, must not modify header
- *     @proto_csum_valid: Protocol csum validated since arriving at localhost
+ *     @proto_data_valid: Protocol data validated since arriving at localhost
  *     @proto_csum_blank: Protocol csum must be added before leaving localhost
  *     @pkt_type: Packet class
  *     @fclone: skbuff clone status
@@ -271,7 +271,7 @@
                                ipvs_property:1;
 #else
                                ipvs_property:1,
-                               proto_csum_valid:1,
+                               proto_data_valid:1,
                                proto_csum_blank:1;
 #endif
        __be16                  protocol;
diff -r 99b2e765d643 -r 4ad317429111 linux-2.6-xen-sparse/net/core/dev.c
--- a/linux-2.6-xen-sparse/net/core/dev.c       Sun Apr  2 08:49:17 2006
+++ b/linux-2.6-xen-sparse/net/core/dev.c       Sun Apr  2 15:16:53 2006
@@ -1649,12 +1649,12 @@
 #ifdef CONFIG_XEN
        switch (skb->ip_summed) {
        case CHECKSUM_UNNECESSARY:
-               skb->proto_csum_valid = 1;
+               skb->proto_data_valid = 1;
                break;
        case CHECKSUM_HW:
                /* XXX Implement me. */
        default:
-               skb->proto_csum_valid = 0;
+               skb->proto_data_valid = 0;
                break;
        }
 #endif
diff -r 99b2e765d643 -r 4ad317429111 linux-2.6-xen-sparse/net/core/skbuff.c
--- a/linux-2.6-xen-sparse/net/core/skbuff.c    Sun Apr  2 08:49:17 2006
+++ b/linux-2.6-xen-sparse/net/core/skbuff.c    Sun Apr  2 15:16:53 2006
@@ -428,7 +428,7 @@
        n->cloned = 1;
        n->nohdr = 0;
 #ifdef CONFIG_XEN
-       C(proto_csum_valid);
+       C(proto_data_valid);
        C(proto_csum_blank);
 #endif
        C(pkt_type);

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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] Make checksum handling in the virtual network drivers more robust., Xen patchbot -unstable <=