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 2c7c3b3b0862401f5573a60f54dc70c835df3420..6d6ea9ec04aaa4f6ba0959684f80258524d414d3 100644 (file)
--- 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 5b4a31999c341c7346e674068554c021359d5994..ae56e931a695b87faaa50cd5369657be2dd72e39 100644 (file)
--- 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 7570aa0f0c9f50157bce83719c1cbe6eab3eac10..fc86d38b9fb3a2c11d3562d78a2ed27e326fd5ed 100644 (file)
--- 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 *);