From 11cdf5e612949b61ee0f531d355a911e0c59d6de Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Fri, 12 Mar 2010 16:36:58 -0500 Subject: [PATCH] datapath: Consistently maintain flow key. 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 | 25 +++++++++++++++++++++---- lib/dpif-netdev.c | 25 +++++++++++++++++++++---- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index 47678628..bef7d108 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -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: diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 365b78a5..1cc4ed46 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -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: -- 2.30.2