From aebdcb93e0f81d6e6a910ffdfa56ba10f2918afe Mon Sep 17 00:00:00 2001 From: Jesse Gross Date: Wed, 14 Jul 2010 20:15:53 -0700 Subject: [PATCH] datapath: Don't update flow key when applying actions. Currently the flow key is updated to match an action that is applied to a packet but these field are never looked at again. Not only is this a waste of time it also makes optimizations involving caching the flow key more difficult. --- datapath/actions.c | 51 ++++++++++++++++------------------------------ datapath/actions.h | 2 +- lib/dpif-netdev.c | 45 +++++++++++----------------------------- 3 files changed, 30 insertions(+), 68 deletions(-) diff --git a/datapath/actions.c b/datapath/actions.c index d0b74d4b..1a6cc357 100644 --- a/datapath/actions.c +++ b/datapath/actions.c @@ -70,19 +70,18 @@ static struct sk_buff *vlan_pull_tag(struct sk_buff *skb) } static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, - struct odp_flow_key *key, const union odp_action *a, - int n_actions, gfp_t gfp) + const struct odp_flow_key *key, + const union odp_action *a, int n_actions, + gfp_t gfp) { u16 tci, mask; if (a->type == ODPAT_SET_VLAN_VID) { tci = ntohs(a->vlan_vid.vlan_vid); mask = VLAN_VID_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); @@ -153,9 +152,8 @@ static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, segs = __vlan_put_tag(segs, tci); err = -ENOMEM; if (segs) { - struct odp_flow_key segkey = *key; err = execute_actions(dp, segs, - &segkey, a + 1, + key, a + 1, n_actions - 1, gfp); } @@ -195,32 +193,26 @@ static struct sk_buff *modify_vlan_tci(struct datapath *dp, struct sk_buff *skb, return skb; } -static struct sk_buff *strip_vlan(struct sk_buff *skb, - struct odp_flow_key *key, gfp_t gfp) +static struct sk_buff *strip_vlan(struct sk_buff *skb, gfp_t gfp) { skb = make_writable(skb, 0, gfp); - if (skb) { + if (skb) vlan_pull_tag(skb); - key->dl_vlan = htons(ODP_VLAN_NONE); - } + return 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); - if (a->type == ODPAT_SET_DL_SRC) { + 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 { + else memcpy(eh->h_dest, a->dl_addr, ETH_ALEN); - memcpy(key->dl_dst, a->dl_addr, ETH_ALEN); - } } return skb; } @@ -246,7 +238,7 @@ static void update_csum(__sum16 *sum, struct sk_buff *skb, } static struct sk_buff *set_nw_addr(struct sk_buff *skb, - struct odp_flow_key *key, + const struct odp_flow_key *key, const struct odp_action_nw_addr *a, gfp_t gfp) { @@ -269,17 +261,12 @@ 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; } static struct sk_buff *set_nw_tos(struct sk_buff *skb, - struct odp_flow_key *key, + const struct odp_flow_key *key, const struct odp_action_nw_tos *a, gfp_t gfp) { @@ -298,12 +285,12 @@ 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; } -static struct sk_buff *set_tp_port(struct sk_buff *skb, struct odp_flow_key *key, +static struct sk_buff *set_tp_port(struct sk_buff *skb, + const struct odp_flow_key *key, const struct odp_action_tp_port *a, gfp_t gfp) { int check_ofs; @@ -327,10 +314,6 @@ static struct sk_buff *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; } @@ -412,7 +395,7 @@ static void sflow_sample(struct datapath *dp, struct sk_buff *skb, /* Execute a list of actions against 'skb'. */ int execute_actions(struct datapath *dp, struct sk_buff *skb, - struct odp_flow_key *key, + const struct odp_flow_key *key, const union odp_action *a, int n_actions, gfp_t gfp) { @@ -462,7 +445,7 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb, break; case ODPAT_SET_TUNNEL: - OVS_CB(skb)->tun_id = key->tun_id = a->tunnel.tun_id; + OVS_CB(skb)->tun_id = a->tunnel.tun_id; break; case ODPAT_SET_VLAN_VID: @@ -473,12 +456,12 @@ int execute_actions(struct datapath *dp, struct sk_buff *skb, break; case ODPAT_STRIP_VLAN: - skb = strip_vlan(skb, key, gfp); + skb = strip_vlan(skb, gfp); break; case ODPAT_SET_DL_SRC: case ODPAT_SET_DL_DST: - skb = set_dl_addr(skb, key, &a->dl_addr, gfp); + skb = set_dl_addr(skb, &a->dl_addr, gfp); break; case ODPAT_SET_NW_SRC: diff --git a/datapath/actions.h b/datapath/actions.h index 09e35bd7..e4fc3974 100644 --- a/datapath/actions.h +++ b/datapath/actions.h @@ -18,7 +18,7 @@ struct odp_flow_key; union odp_action; int execute_actions(struct datapath *dp, struct sk_buff *skb, - struct odp_flow_key *key, + const struct odp_flow_key *key, const union odp_action *, int n_actions, gfp_t gfp); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 5021874e..fa47d39e 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -138,7 +138,7 @@ static int do_del_port(struct dp_netdev *, uint16_t port_no); static int dp_netdev_output_control(struct dp_netdev *, const struct ofpbuf *, int queue_no, int port_no, uint32_t arg); static int dp_netdev_execute_actions(struct dp_netdev *, - struct ofpbuf *, flow_t *, + struct ofpbuf *, const flow_t *, const union odp_action *, int n); static struct dpif_netdev * @@ -1094,7 +1094,7 @@ dp_netdev_wait(void) * bits outside of 'mask'. */ static void -dp_netdev_modify_vlan_tci(struct ofpbuf *packet, flow_t *key, +dp_netdev_modify_vlan_tci(struct ofpbuf *packet, const flow_t *key, uint16_t tci, uint16_t mask) { struct vlan_eth_header *veh; @@ -1118,12 +1118,10 @@ dp_netdev_modify_vlan_tci(struct ofpbuf *packet, flow_t *key, memcpy(veh, &tmp, sizeof tmp); packet->l2 = (char*)packet->l2 - VLAN_HEADER_LEN; } - - key->dl_vlan = veh->veth_tci & htons(VLAN_VID_MASK); } static void -dp_netdev_strip_vlan(struct ofpbuf *packet, flow_t *key) +dp_netdev_strip_vlan(struct ofpbuf *packet) { struct vlan_eth_header *veh = packet->l2; if (veh->veth_type == htons(ETH_TYPE_VLAN)) { @@ -1137,31 +1135,25 @@ dp_netdev_strip_vlan(struct ofpbuf *packet, flow_t *key) packet->data = (char*)packet->data + VLAN_HEADER_LEN; packet->l2 = (char*)packet->l2 + VLAN_HEADER_LEN; memcpy(packet->data, &tmp, sizeof tmp); - - key->dl_vlan = htons(ODP_VLAN_NONE); } } static void -dp_netdev_set_dl_src(struct ofpbuf *packet, flow_t *key, - const uint8_t dl_addr[ETH_ADDR_LEN]) +dp_netdev_set_dl_src(struct ofpbuf *packet, 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, flow_t *key, - const uint8_t dl_addr[ETH_ADDR_LEN]) +dp_netdev_set_dl_dst(struct ofpbuf *packet, 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 -dp_netdev_set_nw_addr(struct ofpbuf *packet, flow_t *key, +dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key, const struct odp_action_nw_addr *a) { if (key->dl_type == htons(ETH_TYPE_IP)) { @@ -1183,17 +1175,11 @@ 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; - } } } static void -dp_netdev_set_nw_tos(struct ofpbuf *packet, flow_t *key, +dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key, const struct odp_action_nw_tos *a) { if (key->dl_type == htons(ETH_TYPE_IP)) { @@ -1206,12 +1192,11 @@ 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; } } static void -dp_netdev_set_tp_port(struct ofpbuf *packet, flow_t *key, +dp_netdev_set_tp_port(struct ofpbuf *packet, const flow_t *key, const struct odp_action_tp_port *a) { if (key->dl_type == htons(ETH_TYPE_IP)) { @@ -1229,12 +1214,6 @@ dp_netdev_set_tp_port(struct ofpbuf *packet, flow_t *key, } else { return; } - - if (a->type == ODPAT_SET_TP_SRC) { - key->tp_src = a->tp_port; - } else { - key->tp_dst = a->tp_port; - } } } @@ -1293,7 +1272,7 @@ dp_netdev_output_control(struct dp_netdev *dp, const struct ofpbuf *packet, static int dp_netdev_execute_actions(struct dp_netdev *dp, - struct ofpbuf *packet, flow_t *key, + struct ofpbuf *packet, const flow_t *key, const union odp_action *actions, int n_actions) { int i; @@ -1327,15 +1306,15 @@ dp_netdev_execute_actions(struct dp_netdev *dp, break; case ODPAT_STRIP_VLAN: - dp_netdev_strip_vlan(packet, key); + dp_netdev_strip_vlan(packet); break; case ODPAT_SET_DL_SRC: - dp_netdev_set_dl_src(packet, key, a->dl_addr.dl_addr); + dp_netdev_set_dl_src(packet, a->dl_addr.dl_addr); break; case ODPAT_SET_DL_DST: - dp_netdev_set_dl_dst(packet, key, a->dl_addr.dl_addr); + dp_netdev_set_dl_dst(packet, a->dl_addr.dl_addr); break; case ODPAT_SET_NW_SRC: -- 2.30.2