[ovs-dev] [PATCH] datapath: Handle packets with precomputed checksums.

[Jesse Gross]

Some devices are capable of computing checksums of received packets
in hardware (the GRE module also falls into this category).
Unfortunately before CHECKSUM_HW was split into two different values
it was impossible to tell the checksum status of a packet during
bridging.  It turns out that a workaround that we already had in the
GRE module is very similar to that in mainline kernel, so this
generalizes that workaround.
---
 datapath/actions.c                     |    1 +
 datapath/datapath.c                    |   21 +++++++++++++++++++++
 datapath/datapath.h                    |    1 +
 datapath/linux-2.6/compat-2.6/ip_gre.c |    7 -------
 4 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/datapath/actions.c b/datapath/actions.c
index cadab05..a24abca 100644
--- a/datapath/actions.c
+++ b/datapath/actions.c
@@ -302,6 +302,7 @@ int dp_xmit_skb(struct sk_buff *skb)
 		return -E2BIG;
 	}
 
+	forward_ip_summed(skb);
 	dev_queue_xmit(skb);
 
 	return len;
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 2a8fb50..5b5cc2c 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -625,6 +625,25 @@ out:
 int vswitch_skb_checksum_setup(struct sk_buff *skb) { return 0; }
 #endif /* CONFIG_XEN && linux == 2.6.18 */
 
+/* This function serves the same purpose as skb_forward_csum, which exists
+ * in later kernels.  However, it is not exactly the same because it only
+ * has an effect on kernels before CHECKSUM_HW was split.  Since we are
+ * only concerned with bridging and not other types of forwarding we don't
+ * have to worry about updating checksums which have been already been
+ * computed.  The issue that we have is that given an skb with CHECKSUM_HW
+ * we can't tell whether the packet actually has a correct checksum (since
+ * it depends on the whether it is on the transmit or receive path).  If we
+ * guess wrong it causes panics, so we just throw away the checksum to be
+ * sure (it isn't common to have a checksum precomputed anyways). */
+void
+forward_ip_summed(struct sk_buff *skb)
+{
+#ifdef CHECKSUM_HW
+	if (skb->ip_summed == CHECKSUM_HW)
+		skb->ip_summed = CHECKSUM_NONE;
+#endif
+}
+
 /* Append each packet in 'skb' list to 'queue'.  There will be only one packet
  * unless we broke up a GSO packet. */
 static int
@@ -722,6 +741,8 @@ dp_output_control(struct datapath *dp, struct sk_buff *skb, int queue_no,
 	if (skb_queue_len(queue) >= DP_MAX_QUEUE_LEN)
 		goto err_kfree_skb;
 
+	forward_ip_summed(skb);
+
 	/* Break apart GSO packets into their component pieces.  Otherwise
 	 * userspace may try to stuff a 64kB packet into a 1500-byte MTU. */
 	if (skb_is_gso(skb)) {
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 9b4c438..7548ba2 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -169,5 +169,6 @@ static inline int skb_checksum_setup(struct sk_buff *skb)
 #endif
 
 int vswitch_skb_checksum_setup(struct sk_buff *skb);
+void forward_ip_summed(struct sk_buff *skb);
 
 #endif /* datapath.h */
diff --git a/datapath/linux-2.6/compat-2.6/ip_gre.c b/datapath/linux-2.6/compat-2.6/ip_gre.c
index 48d7d6c..05c6055 100644
--- a/datapath/linux-2.6/compat-2.6/ip_gre.c
+++ b/datapath/linux-2.6/compat-2.6/ip_gre.c
@@ -679,13 +679,6 @@ static int ipgre_rcv(struct sk_buff *skb)
 		skb_reset_network_header(skb);
 		ipgre_ecn_decapsulate(iph, skb);
 
-#ifdef CHECKSUM_HW
-		/* XXX: Temporary workaround to avoid a panic when doing
-		 * bridging due to multiple meanings of CHECKSUM_HW. */
-		if (skb->ip_summed == CHECKSUM_HW)
-			skb->ip_summed = CHECKSUM_NONE;
-#endif
-
 		netif_rx(skb);
 		read_unlock(&ipgre_lock);
 		return(0);
-- 
1.6.3.3