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.

diff --git a/datapath/datapath.c b/datapath/datapath.c
index 28f6c4f..242af24 100644 (file)
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -570,10 +570,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;
        }
@@ -600,6 +598,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 127714f..77fd39a 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);
 }
@@ -242,7 +245,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 7570aa0..fc86d38 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 *);