From: Ethan Jackson Date: Wed, 27 Feb 2013 04:10:46 +0000 (-0800) Subject: ofproto-dpif: Always maintain subfacet key. X-Git-Tag: sliver-openvswitch-1.10.90-1~10^2~126 X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=9566abf914f3f7f1ef15898049d818b63d60af68;p=sliver-openvswitch.git ofproto-dpif: Always maintain subfacet key. Due to flow based tunneling, we can no longer assume that it's possible to reconstruct a subfacet's key from its facet's flow. The flow's in_port may be stale due to tunnel configuration changes. Signed-off-by: Ethan Jackson --- diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index a4d7e59ef..a572f1875 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -341,11 +341,6 @@ struct subfacet { struct list list_node; /* In struct facet's 'facets' list. */ struct facet *facet; /* Owning facet. */ - /* Key. - * - * To save memory in the common case, 'key' is NULL if 'key_fitness' is - * ODP_FIT_PERFECT, that is, odp_flow_key_from_flow() can accurately - * regenerate the ODP flow key from ->facet->flow. */ enum odp_key_fitness key_fitness; struct nlattr *key; int key_len; @@ -384,14 +379,11 @@ static struct subfacet *subfacet_create(struct facet *, struct flow_miss *miss, long long int now); static struct subfacet *subfacet_find(struct ofproto_dpif *, const struct nlattr *key, size_t key_len, - uint32_t key_hash, - const struct flow *flow); + uint32_t key_hash); static void subfacet_destroy(struct subfacet *); static void subfacet_destroy__(struct subfacet *); static void subfacet_destroy_batch(struct ofproto_dpif *, struct subfacet **, int n); -static void subfacet_get_key(struct subfacet *, struct odputil_keybuf *, - struct ofpbuf *key); static void subfacet_reset_dp_stats(struct subfacet *, struct dpif_flow_stats *); static void subfacet_update_time(struct subfacet *, long long int used); @@ -4153,7 +4145,7 @@ update_stats(struct dpif_backer *backer) } key_hash = odp_flow_key_hash(key, key_len); - subfacet = subfacet_find(ofproto, key, key_len, key_hash, &flow); + subfacet = subfacet_find(ofproto, key, key_len, key_hash); switch (subfacet ? subfacet->path : SF_NOT_INSTALLED) { case SF_FAST_PATH: update_subfacet_stats(subfacet, stats); @@ -4693,9 +4685,7 @@ facet_check_consistency(struct facet *facet) ofpbuf_use_stub(&odp_actions, odp_actions_stub, sizeof odp_actions_stub); LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) { enum subfacet_path want_path; - struct odputil_keybuf keybuf; struct action_xlate_ctx ctx; - struct ofpbuf key; struct ds s; action_xlate_ctx_init(&ctx, ofproto, &facet->flow, @@ -4731,8 +4721,7 @@ facet_check_consistency(struct facet *facet) } ds_init(&s); - subfacet_get_key(subfacet, &keybuf, &key); - odp_flow_key_format(key.data, key.size, &s); + odp_flow_key_format(subfacet->key, subfacet->key_len, &s); ds_put_cstr(&s, ": inconsistency in subfacet"); if (want_path != subfacet->path) { @@ -4948,17 +4937,14 @@ flow_push_stats(struct rule_dpif *rule, static struct subfacet * subfacet_find(struct ofproto_dpif *ofproto, - const struct nlattr *key, size_t key_len, uint32_t key_hash, - const struct flow *flow) + const struct nlattr *key, size_t key_len, uint32_t key_hash) { struct subfacet *subfacet; HMAP_FOR_EACH_WITH_HASH (subfacet, hmap_node, key_hash, &ofproto->subfacets) { - if (subfacet->key - ? (subfacet->key_len == key_len - && !memcmp(key, subfacet->key, key_len)) - : flow_equal(flow, &subfacet->facet->flow)) { + if (subfacet->key_len == key_len + && !memcmp(key, subfacet->key, key_len)) { return subfacet; } } @@ -4990,8 +4976,7 @@ subfacet_create(struct facet *facet, struct flow_miss *miss, if (list_is_empty(&facet->subfacets)) { subfacet = &facet->one_subfacet; } else { - subfacet = subfacet_find(ofproto, key, key_len, key_hash, - &facet->flow); + subfacet = subfacet_find(ofproto, key, key_len, key_hash); if (subfacet) { if (subfacet->facet == facet) { return subfacet; @@ -5009,13 +4994,8 @@ subfacet_create(struct facet *facet, struct flow_miss *miss, list_push_back(&facet->subfacets, &subfacet->list_node); subfacet->facet = facet; subfacet->key_fitness = key_fitness; - if (key_fitness != ODP_FIT_PERFECT) { - subfacet->key = xmemdup(key, key_len); - subfacet->key_len = key_len; - } else { - subfacet->key = NULL; - subfacet->key_len = 0; - } + subfacet->key = xmemdup(key, key_len); + subfacet->key_len = key_len; subfacet->used = now; subfacet->dp_packet_count = 0; subfacet->dp_byte_count = 0; @@ -5068,18 +5048,15 @@ static void subfacet_destroy_batch(struct ofproto_dpif *ofproto, struct subfacet **subfacets, int n) { - struct odputil_keybuf keybufs[SUBFACET_DESTROY_MAX_BATCH]; struct dpif_op ops[SUBFACET_DESTROY_MAX_BATCH]; struct dpif_op *opsp[SUBFACET_DESTROY_MAX_BATCH]; - struct ofpbuf keys[SUBFACET_DESTROY_MAX_BATCH]; struct dpif_flow_stats stats[SUBFACET_DESTROY_MAX_BATCH]; int i; for (i = 0; i < n; i++) { ops[i].type = DPIF_OP_FLOW_DEL; - subfacet_get_key(subfacets[i], &keybufs[i], &keys[i]); - ops[i].u.flow_del.key = keys[i].data; - ops[i].u.flow_del.key_len = keys[i].size; + ops[i].u.flow_del.key = subfacets[i]->key; + ops[i].u.flow_del.key_len = subfacets[i]->key_len; ops[i].u.flow_del.stats = &stats[i]; opsp[i] = &ops[i]; } @@ -5092,24 +5069,6 @@ subfacet_destroy_batch(struct ofproto_dpif *ofproto, } } -/* Initializes 'key' with the sequence of OVS_KEY_ATTR_* Netlink attributes - * that can be used to refer to 'subfacet'. The caller must provide 'keybuf' - * for use as temporary storage. */ -static void -subfacet_get_key(struct subfacet *subfacet, struct odputil_keybuf *keybuf, - struct ofpbuf *key) -{ - - if (!subfacet->key) { - struct flow *flow = &subfacet->facet->flow; - - ofpbuf_use_stack(key, keybuf, sizeof *keybuf); - odp_flow_key_from_flow(key, flow, subfacet->odp_in_port); - } else { - ofpbuf_use_const(key, subfacet->key, subfacet->key_len); - } -} - /* Composes the datapath actions for 'subfacet' based on its rule's actions. * Translates the actions into 'odp_actions', which the caller must have * initialized and is responsible for uninitializing. */ @@ -5158,9 +5117,7 @@ subfacet_install(struct subfacet *subfacet, struct ofproto_dpif *ofproto = ofproto_dpif_cast(facet->rule->up.ofproto); enum subfacet_path path = subfacet_want_path(slow); uint64_t slow_path_stub[128 / 8]; - struct odputil_keybuf keybuf; enum dpif_flow_put_flags flags; - struct ofpbuf key; int ret; flags = DPIF_FP_CREATE | DPIF_FP_MODIFY; @@ -5174,9 +5131,8 @@ subfacet_install(struct subfacet *subfacet, &actions, &actions_len); } - subfacet_get_key(subfacet, &keybuf, &key); - ret = dpif_flow_put(ofproto->backer->dpif, flags, key.data, key.size, - actions, actions_len, stats); + ret = dpif_flow_put(ofproto->backer->dpif, flags, subfacet->key, + subfacet->key_len, actions, actions_len, stats); if (stats) { subfacet_reset_dp_stats(subfacet, stats); @@ -5202,14 +5158,11 @@ subfacet_uninstall(struct subfacet *subfacet) if (subfacet->path != SF_NOT_INSTALLED) { struct rule_dpif *rule = subfacet->facet->rule; struct ofproto_dpif *ofproto = ofproto_dpif_cast(rule->up.ofproto); - struct odputil_keybuf keybuf; struct dpif_flow_stats stats; - struct ofpbuf key; int error; - subfacet_get_key(subfacet, &keybuf, &key); - error = dpif_flow_del(ofproto->backer->dpif, - key.data, key.size, &stats); + error = dpif_flow_del(ofproto->backer->dpif, subfacet->key, + subfacet->key_len, &stats); subfacet_reset_dp_stats(subfacet, &stats); if (!error) { subfacet_update_stats(subfacet, &stats); @@ -8064,11 +8017,7 @@ ofproto_unixctl_dpif_dump_flows(struct unixctl_conn *conn, update_stats(ofproto->backer); HMAP_FOR_EACH (subfacet, hmap_node, &ofproto->subfacets) { - struct odputil_keybuf keybuf; - struct ofpbuf key; - - subfacet_get_key(subfacet, &keybuf, &key); - odp_flow_key_format(key.data, key.size, &ds); + odp_flow_key_format(subfacet->key, subfacet->key_len, &ds); ds_put_format(&ds, ", packets:%"PRIu64", bytes:%"PRIu64", used:", subfacet->dp_packet_count, subfacet->dp_byte_count);