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 4767862..bef7d10 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 365b78a..1cc4ed4 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: