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

[Xen-devel] [PATCH] xen: netfront: handle incoming GSO SKBs which are no

To: "netdev@xxxxxxxxxxxxxxx" <netdev@xxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] xen: netfront: handle incoming GSO SKBs which are not CHECKSUM_PARTIAL
From: Ian Campbell <ian.campbell@xxxxxxxxxx>
Date: Thu, 27 Jan 2011 14:14:03 +0000
Cc: netdev@xxxxxxxxxxxxxxx, Jeremy Fitzhardinge <jeremy@xxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, Ian Campbell <ian.campbell@xxxxxxxxxx>, David Miller <davem@xxxxxxxxxxxxx>
Delivery-date: Thu, 27 Jan 2011 06:15:03 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <1296042981.14780.6813.camel@xxxxxxxxxxxxxxxxxxxxxx>
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/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <1296042981.14780.6813.camel@xxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
The Linux network stack expects all GSO SKBs to have ip_summed ==
CHECKSUM_PARTIAL (which implies that the frame contains a partial
checksum) and the Xen network ring protocol similarly expects an SKB
which has GSO set to also have NETRX_csum_blank (which also implies a
partial checksum).

However there have been cases of buggy guests which mark a frame as
GSO but do not set csum_blank. If we detect that we a receiving such a
frame (which manifests as ip_summed != PARTIAL && skb_is_gso) then
force the SKB to partial and recalculate the checksum, since we cannot
rely on the peer having done so if they have not set csum_blank.

Add an ethtool stat to track occurances of this event.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Cc: David Miller <davem@xxxxxxxxxxxxx>
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Cc: netdev@xxxxxxxxxxxxxxx
---
 drivers/net/xen-netfront.c |   96 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 88 insertions(+), 8 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 546de57..da1f121 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -120,6 +120,9 @@ struct netfront_info {
        unsigned long rx_pfn_array[NET_RX_RING_SIZE];
        struct multicall_entry rx_mcl[NET_RX_RING_SIZE+1];
        struct mmu_update rx_mmu[NET_RX_RING_SIZE];
+
+       /* Statistics */
+       int rx_gso_checksum_fixup;
 };
 
 struct netfront_rx_info {
@@ -770,11 +773,29 @@ static RING_IDX xennet_fill_frags(struct netfront_info 
*np,
        return cons;
 }
 
-static int skb_checksum_setup(struct sk_buff *skb)
+static int checksum_setup(struct net_device *dev, struct sk_buff *skb)
 {
        struct iphdr *iph;
        unsigned char *th;
        int err = -EPROTO;
+       int recalculate_partial_csum = 0;
+
+       /*
+        * A GSO SKB must be CHECKSUM_PARTIAL. However some buggy
+        * peers can fail to set NETRXF_csum_blank when sending a GSO
+        * frame. In this case force the SKB to CHECKSUM_PARTIAL and
+        * recalculate the partial checksum.
+        */
+       if (skb->ip_summed != CHECKSUM_PARTIAL && skb_is_gso(skb)) {
+               struct netfront_info *np = netdev_priv(dev);
+               np->rx_gso_checksum_fixup++;
+               skb->ip_summed = CHECKSUM_PARTIAL;
+               recalculate_partial_csum = 1;
+       }
+
+       /* A non-CHECKSUM_PARTIAL SKB does not require setup. */
+       if (skb->ip_summed != CHECKSUM_PARTIAL)
+               return 0;
 
        if (skb->protocol != htons(ETH_P_IP))
                goto out;
@@ -788,9 +809,23 @@ static int skb_checksum_setup(struct sk_buff *skb)
        switch (iph->protocol) {
        case IPPROTO_TCP:
                skb->csum_offset = offsetof(struct tcphdr, check);
+
+               if (recalculate_partial_csum) {
+                       struct tcphdr *tcph = (struct tcphdr *)th;
+                       tcph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
+                                                        skb->len - iph->ihl*4,
+                                                        IPPROTO_TCP, 0);
+               }
                break;
        case IPPROTO_UDP:
                skb->csum_offset = offsetof(struct udphdr, check);
+
+               if (recalculate_partial_csum) {
+                       struct udphdr *udph = (struct udphdr *)th;
+                       udph->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
+                                                        skb->len - iph->ihl*4,
+                                                        IPPROTO_UDP, 0);
+               }
                break;
        default:
                if (net_ratelimit())
@@ -829,13 +864,11 @@ static int handle_incoming_queue(struct net_device *dev,
                /* Ethernet work: Delayed to here as it peeks the header. */
                skb->protocol = eth_type_trans(skb, dev);
 
-               if (skb->ip_summed == CHECKSUM_PARTIAL) {
-                       if (skb_checksum_setup(skb)) {
-                               kfree_skb(skb);
-                               packets_dropped++;
-                               dev->stats.rx_errors++;
-                               continue;
-                       }
+               if (checksum_setup(dev, skb)) {
+                       kfree_skb(skb);
+                       packets_dropped++;
+                       dev->stats.rx_errors++;
+                       continue;
                }
 
                dev->stats.rx_packets++;
@@ -1632,12 +1665,59 @@ static void netback_changed(struct xenbus_device *dev,
        }
 }
 
+static const struct xennet_stat {
+       char name[ETH_GSTRING_LEN];
+       u16 offset;
+} xennet_stats[] = {
+       {
+               "rx_gso_checksum_fixup",
+               offsetof(struct netfront_info, rx_gso_checksum_fixup)
+       },
+};
+
+static int xennet_get_sset_count(struct net_device *dev, int string_set)
+{
+       switch (string_set) {
+       case ETH_SS_STATS:
+               return ARRAY_SIZE(xennet_stats);
+       default:
+               return -EINVAL;
+       }
+}
+
+static void xennet_get_ethtool_stats(struct net_device *dev,
+                                    struct ethtool_stats *stats, u64 * data)
+{
+       void *np = netdev_priv(dev);
+       int i;
+
+       for (i = 0; i < ARRAY_SIZE(xennet_stats); i++)
+               data[i] = *(int *)(np + xennet_stats[i].offset);
+}
+
+static void xennet_get_strings(struct net_device *dev, u32 stringset, u8 * 
data)
+{
+       int i;
+
+       switch (stringset) {
+       case ETH_SS_STATS:
+               for (i = 0; i < ARRAY_SIZE(xennet_stats); i++)
+                       memcpy(data + i * ETH_GSTRING_LEN,
+                              xennet_stats[i].name, ETH_GSTRING_LEN);
+               break;
+       }
+}
+
 static const struct ethtool_ops xennet_ethtool_ops =
 {
        .set_tx_csum = ethtool_op_set_tx_csum,
        .set_sg = xennet_set_sg,
        .set_tso = xennet_set_tso,
        .get_link = ethtool_op_get_link,
+
+       .get_sset_count = xennet_get_sset_count,
+       .get_ethtool_stats = xennet_get_ethtool_stats,
+       .get_strings = xennet_get_strings,
 };
 
 #ifdef CONFIG_SYSFS
-- 
1.5.6.5


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