From: Justin Pettit <jpettit@nicira.com>
Date: Mon, 26 Jan 2009 09:05:39 +0000 (-0800)
Subject: For SNAT, don't store the pre-fragment L2 header before actions are applied.
X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bf4091f1d27834b353c59f004a4cfd9789ab2668;p=openvswitch

For SNAT, don't store the pre-fragment L2 header before actions are applied.

The IP fragment code doesn't always write the L2 header when generating
new fragments.  This problem was fixed in an earlier commit.
Unfortunately, we stored the pre-fragment L2 header when the packet
first arrived--before other packet modifications were applied.  This
meant that the results of any OpenFlow L2 modification actions were lost.
This patch pushes the storage of the L2 header until right before the
packet is transmitted (and possibly refragmented).

Thanks to Dan for catching this behavior.
(cherry picked from commit b4cd6fb07e0751832a22759e27c6ba63e3538c8b)
---

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 2c7c3b3b..6d6ea9ec 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -657,10 +657,8 @@ void dp_set_origin(struct datapath *dp, uint16_t in_port,
 static int 
 dp_xmit_skb_finish(struct sk_buff *skb)
 {
-	/* The ip_fragment function does not copy the Ethernet header into
-	 * the newly generated frames, so put back the values stowed
-	 * earlier. */
-	if (snat_copy_header(skb)) {
+	/* Copy back the Ethernet header that was stowed earlier. */
+	if (skb->protocol == htons(ETH_P_IP) && snat_copy_header(skb)) {
 		kfree_skb(skb);
 		return -EINVAL;
 	}
@@ -687,6 +685,11 @@ dp_xmit_skb(struct sk_buff *skb)
 
 	skb_pull(skb, ETH_HLEN);
 
+	/* The ip_fragment function does not copy the Ethernet header into
+	 * the newly generated frames, so stow the original. */
+	if (skb->protocol == htons(ETH_P_IP))
+		snat_save_header(skb);
+
 	if (skb->protocol == htons(ETH_P_IP) &&
 			skb->len > skb->dev->mtu &&
 			!skb_is_gso(skb)) {
diff --git a/datapath/nx_act_snat.c b/datapath/nx_act_snat.c
index 5b4a3199..ae56e931 100644
--- a/datapath/nx_act_snat.c
+++ b/datapath/nx_act_snat.c
@@ -70,10 +70,13 @@ static inline struct nf_bridge_info *nf_bridge_alloc(struct sk_buff *skb)
 }
 
 /* Save a copy of the original Ethernet header. */
-static inline void snat_save_header(struct sk_buff *skb)
+void snat_save_header(struct sk_buff *skb)
 {
 	int header_size = ETH_HLEN + nf_bridge_encap_header_len(skb);
 
+	if (!skb->nf_bridge)
+		return;
+
 	skb_copy_from_linear_data_offset(skb, -header_size, 
 			skb->nf_bridge->data, header_size);
 }
@@ -239,7 +242,6 @@ snat_pre_route_finish(struct sk_buff *skb)
 
 	/* Pass the translated packet as input to the OpenFlow stack, which
 	 * consumes it. */
-	snat_save_header(skb);
 	skb_push(skb, ETH_HLEN);
 	skb_reset_mac_header(skb);
 	fwd_port_input(p->dp->chain, skb, p);
diff --git a/datapath/nx_act_snat.h b/datapath/nx_act_snat.h
index 7570aa0f..fc86d38b 100644
--- a/datapath/nx_act_snat.h
+++ b/datapath/nx_act_snat.h
@@ -34,6 +34,7 @@ struct snat_conf {
 void snat_local_in(struct sk_buff *skb);
 int snat_pre_route(struct sk_buff *skb);
 void snat_skb(struct datapath *dp, const struct sk_buff *skb, int out_port);
+void snat_save_header(struct sk_buff *skb);
 int snat_copy_header(struct sk_buff *skb);
 void snat_maint(struct net_bridge_port *p);
 int snat_mod_config(struct datapath *, const struct nx_act_config *);