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@xensource.com>

diff --git a/linux-2.6-xen-sparse/drivers/xen/netback/interface.c b/linux-2.6-xen-sparse/drivers/xen/netback/interface.c
index be96a6d3ac72b1b20bf6194760233505018399bd..5c3cbc75d971f7673373c86a1b566853aaa5c730 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/netback/interface.c
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/interface.c
@@ -31,6 +31,7 @@
  */
 
 #include "common.h"
+#include <linux/ethtool.h>
 #include <linux/rtnetlink.h>
 
 static void __netif_up(netif_t *netif)
@@ -71,6 +72,12 @@ static int net_close(struct net_device *dev)
        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])
 {
        int err = 0, i;
@@ -101,7 +108,9 @@ netif_t *alloc_netif(domid_t domid, unsigned int handle, u8 be_mac[ETH_ALEN])
        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 --git a/linux-2.6-xen-sparse/drivers/xen/netback/loopback.c b/linux-2.6-xen-sparse/drivers/xen/netback/loopback.c
index e9f354a837a3e23967b71e6212d8c5efaacfeee0..870b4aa2a6d1b0ef5da05193bca8154b9f7bfccb 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/netback/loopback.c
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/loopback.c
@@ -100,10 +100,10 @@ static int loopback_start_xmit(struct sk_buff *skb, struct net_device *dev)
                /* 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 @@ static struct net_device_stats *loopback_get_stats(struct net_device *dev)
        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 @@ static void loopback_construct(struct net_device *dev, struct net_device *lo)
 
        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 @@ static void loopback_construct(struct net_device *dev, struct net_device *lo)
        /*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;
@@ -172,11 +176,6 @@ static int __init make_loopback(int i)
        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
         * choose the numerically largest non-broadcast address to prevent the

diff --git a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c
index 41d4bf90d224b825c0976c8ed5b99ea3b654597e..d9f259ce26f02d8e7d22b938aa42b39bf907c352 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/netback/netback.c
+++ b/linux-2.6-xen-sparse/drivers/xen/netback/netback.c
@@ -171,7 +171,7 @@ int netif_be_start_xmit(struct sk_buff *skb, struct net_device *dev)
                                     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 @@ static void net_rx_action(unsigned long unused)
 {
        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 @@ static void net_rx_action(unsigned long unused)
                }
                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 @@ static void net_tx_action(unsigned long unused)
                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 --git a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
index 6c7b82f789ddc8065ebc96dbc8664f168133c06a..1f2ee99e5818c63777eaa39a5cd170ab54df7718 100644 (file)
--- a/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
+++ b/linux-2.6-xen-sparse/drivers/xen/netfront/netfront.c
@@ -696,7 +696,12 @@ static int network_start_xmit(struct sk_buff *skb, struct net_device *dev)
        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 @@ static int netif_poll(struct net_device *dev, int *pbudget)
                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 --git a/linux-2.6-xen-sparse/include/linux/skbuff.h b/linux-2.6-xen-sparse/include/linux/skbuff.h
index 0d2052865f32b6e9a43a7ef24f68542ace88b54a..911eb6d045316fb638ef3d3ca7bdec9a9f6d0f34 100644 (file)
--- a/linux-2.6-xen-sparse/include/linux/skbuff.h
+++ b/linux-2.6-xen-sparse/include/linux/skbuff.h
@@ -189,7 +189,7 @@ enum {
  *     @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 @@ struct sk_buff {
                                ipvs_property:1;
 #else
                                ipvs_property:1,
-                               proto_csum_valid:1,
+                               proto_data_valid:1,
                                proto_csum_blank:1;
 #endif
        __be16                  protocol;

diff --git a/linux-2.6-xen-sparse/net/core/dev.c b/linux-2.6-xen-sparse/net/core/dev.c
index 30baa344b7067f440f8c1d64a0ebdff947d505b8..7a98ceb6585454c3e8755dd462568f3cc10cfd0d 100644 (file)
--- a/linux-2.6-xen-sparse/net/core/dev.c
+++ b/linux-2.6-xen-sparse/net/core/dev.c
@@ -1649,12 +1649,12 @@ int netif_receive_skb(struct sk_buff *skb)
 #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 --git a/linux-2.6-xen-sparse/net/core/skbuff.c b/linux-2.6-xen-sparse/net/core/skbuff.c
index dadc9cfa93a31334761be0dbe8a6bd142c7eccc2..d00e58e216fefa63f940bf1f7e07fd9fc15ef3a3 100644 (file)
--- a/linux-2.6-xen-sparse/net/core/skbuff.c
+++ b/linux-2.6-xen-sparse/net/core/skbuff.c
@@ -428,7 +428,7 @@ struct sk_buff *skb_clone(struct sk_buff *skb, gfp_t gfp_mask)
        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);