datapath: Consistently maintain flow key.
authorJesse Gross <jesse@nicira.com>
Fri, 12 Mar 2010 21:36:58 +0000 (16:36 -0500)
committerJesse Gross <jesse@nicira.com>
Mon, 15 Mar 2010 19:44:41 +0000 (15:44 -0400)
After executing an action that changes a packet sometimes we update
the flow key and sometimes we don't.  This is potentially problematic
because we sometimes use the key for checks later on.  This consistently
maintains the key.

datapath/actions.c
lib/dpif-netdev.c

index 476786283e3d96065813abd765eacebfd46086bf..bef7d108c3e7b1ef61f53824bbd426bbea925a22 100644 (file)
@@ -94,10 +94,11 @@ modify_vlan_tci(struct datapath *dp, struct sk_buff *skb,
        if (a->type == ODPAT_SET_VLAN_VID) {
                tci = ntohs(a->vlan_vid.vlan_vid);
                mask = VLAN_VID_MASK;
-               key->dl_vlan = htons(tci & mask);
+               key->dl_vlan = a->vlan_vid.vlan_vid;
        } else {
                tci = a->vlan_pcp.vlan_pcp << VLAN_PCP_SHIFT;
                mask = VLAN_PCP_MASK;
+               key->dl_vlan_pcp = a->vlan_pcp.vlan_pcp;
        }
 
        skb = make_writable(skb, VLAN_HLEN, gfp);
@@ -216,14 +217,20 @@ static struct sk_buff *strip_vlan(struct sk_buff *skb,
 }
 
 static struct sk_buff *set_dl_addr(struct sk_buff *skb,
+                                  struct odp_flow_key *key,
                                   const struct odp_action_dl_addr *a,
                                   gfp_t gfp)
 {
        skb = make_writable(skb, 0, gfp);
        if (skb) {
                struct ethhdr *eh = eth_hdr(skb);
-               memcpy(a->type == ODPAT_SET_DL_SRC ? eh->h_source : eh->h_dest,
-                      a->dl_addr, ETH_ALEN);
+               if (a->type == ODPAT_SET_DL_SRC) {
+                       memcpy(eh->h_source, a->dl_addr, ETH_ALEN);
+                       memcpy(key->dl_src, a->dl_addr, ETH_ALEN);
+               } else {
+                       memcpy(eh->h_dest, a->dl_addr, ETH_ALEN);
+                       memcpy(key->dl_dst, a->dl_addr, ETH_ALEN);
+               }
        }
        return skb;
 }
@@ -272,6 +279,11 @@ static struct sk_buff *set_nw_addr(struct sk_buff *skb,
                }
                update_csum(&nh->check, skb, old, new, 0);
                *f = new;
+
+               if (a->type == ODPAT_SET_NW_SRC)
+                       key->nw_src = a->nw_addr;
+               else
+                       key->nw_dst = a->nw_addr;
        }
        return skb;
 }
@@ -296,6 +308,7 @@ static struct sk_buff *set_nw_tos(struct sk_buff *skb,
                update_csum(&nh->check, skb, htons((uint16_t)old),
                                htons((uint16_t)new), 0);
                *f = new;
+               key->nw_tos = a->nw_tos;
        }
        return skb;
 }
@@ -326,6 +339,10 @@ set_tp_port(struct sk_buff *skb, struct odp_flow_key *key,
                update_csum((u16*)(skb_transport_header(skb) + check_ofs), 
                                skb, old, new, 0);
                *f = new;
+               if (a->type == ODPAT_SET_TP_SRC)
+                       key->tp_src = a->tp_port;
+               else
+                       key->tp_dst = a->tp_port;
        }
        return skb;
 }
@@ -498,7 +515,7 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb,
 
                case ODPAT_SET_DL_SRC:
                case ODPAT_SET_DL_DST:
-                       skb = set_dl_addr(skb, &a->dl_addr, gfp);
+                       skb = set_dl_addr(skb, key, &a->dl_addr, gfp);
                        break;
 
                case ODPAT_SET_NW_SRC:
index 365b78a545361d48cf7889d10ac09abc51d63ee9..1cc4ed46d3b510e9b2930dff7fb5d30155c8f109 100644 (file)
@@ -1145,19 +1145,21 @@ dp_netdev_strip_vlan(struct ofpbuf *packet, flow_t *key)
 }
 
 static void
-dp_netdev_set_dl_src(struct ofpbuf *packet,
+dp_netdev_set_dl_src(struct ofpbuf *packet, flow_t *key,
                      const uint8_t dl_addr[ETH_ADDR_LEN])
 {
     struct eth_header *eh = packet->l2;
     memcpy(eh->eth_src, dl_addr, sizeof eh->eth_src);
+    memcpy(key->dl_src, dl_addr, sizeof key->dl_src);
 }
 
 static void
-dp_netdev_set_dl_dst(struct ofpbuf *packet,
+dp_netdev_set_dl_dst(struct ofpbuf *packet, flow_t *key,
                      const uint8_t dl_addr[ETH_ADDR_LEN])
 {
     struct eth_header *eh = packet->l2;
     memcpy(eh->eth_dst, dl_addr, sizeof eh->eth_dst);
+    memcpy(key->dl_dst, dl_addr, sizeof key->dl_dst);
 }
 
 static void
@@ -1183,6 +1185,12 @@ dp_netdev_set_nw_addr(struct ofpbuf *packet, flow_t *key,
         }
         nh->ip_csum = recalc_csum32(nh->ip_csum, *field, a->nw_addr);
         *field = a->nw_addr;
+
+        if (a->type == ODPAT_SET_NW_SRC) {
+            key->nw_src = a->type;
+        } else {
+            key->nw_dst = a->type;
+        }
     }
 }
 
@@ -1200,6 +1208,7 @@ dp_netdev_set_nw_tos(struct ofpbuf *packet, flow_t *key,
         nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t)*field),
                 htons((uint16_t)a->nw_tos));
         *field = new;
+        key->nw_tos = a->nw_tos;
     }
 }
 
@@ -1219,6 +1228,14 @@ dp_netdev_set_tp_port(struct ofpbuf *packet, flow_t *key,
             field = a->type == ODPAT_SET_TP_SRC ? &uh->udp_src : &uh->udp_dst;
             uh->udp_csum = recalc_csum16(uh->udp_csum, *field, a->tp_port);
             *field = a->tp_port;
+        } else {
+            return;
+        }
+
+        if (a->type == ODPAT_SET_TP_SRC) {
+            key->tp_src = a->tp_port;
+        } else {
+            key->tp_dst = a->tp_port;
         }
     }
 }
@@ -1315,11 +1332,11 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
                        break;
 
                case ODPAT_SET_DL_SRC:
-            dp_netdev_set_dl_src(packet, a->dl_addr.dl_addr);
+            dp_netdev_set_dl_src(packet, key, a->dl_addr.dl_addr);
                        break;
 
                case ODPAT_SET_DL_DST:
-            dp_netdev_set_dl_dst(packet, a->dl_addr.dl_addr);
+            dp_netdev_set_dl_dst(packet, key, a->dl_addr.dl_addr);
                        break;
 
                case ODPAT_SET_NW_SRC: