[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] Make checksum handling in the virtual network drivers more robust.



# 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.