> In update_stats(), I think that we could save some time by only
> calling drop_key_lookup() if ofproto_receive() returns ENODEV, since
> drop keys should not be the common case.
Actually, upon reflection I've realized that we don't need the
drop_key_lookup() in update_stats() at all. ofproto_receive() will simply
return ENODEV and we'll skip it naturally.
An incremental follows.
Ethan
---
lib/ofpbuf.c | 1 -
lib/ofpbuf.h | 2 --
ofproto/ofproto-dpif.c | 52 +++++++++++++++++++++++++++++-------------------
3 files changed, 31 insertions(+), 24 deletions(-)
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 1c2e378..6484ab3 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -31,7 +31,6 @@ ofpbuf_use__(struct ofpbuf *b, void *base, size_t allocated,
b->size = 0;
b->l2 = b->l3 = b->l4 = b->l7 = NULL;
list_poison(&b->list_node);
- memset(&b->hmap_node, 0, sizeof b->hmap_node);
b->private_p = NULL;
}
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index c562721..520455d 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -19,7 +19,6 @@
#include <stddef.h>
#include <stdint.h>
-#include "hmap.h"
#include "list.h"
#include "util.h"
@@ -49,7 +48,6 @@ struct ofpbuf {
void *l7; /* Application data. */
struct list list_node; /* Private list element for use by owner. */
- struct hmap_node hmap_node; /* Private hmap node for use by owner. */
void *private_p; /* Private pointer for use by owner. */
};
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2d7db24..7a7b253 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -615,6 +615,15 @@ COVERAGE_DEFINE(rev_port_toggled);
COVERAGE_DEFINE(rev_flow_table);
COVERAGE_DEFINE(rev_inconsistency);
+/* Drop keys are odp flow keys which have drop flows installed in the kernel.
+ * These are datapath flows which have no associated ofproto, if they did we
+ * would use facets. */
+struct drop_key {
+ struct hmap_node hmap_node;
+ struct nlattr *key;
+ size_t key_len;
+};
+
/* All datapaths of a given type share a single dpif backer instance. */
struct dpif_backer {
char *type;
@@ -627,10 +636,7 @@ struct dpif_backer {
enum revalidate_reason need_revalidate; /* Revalidate every facet. */
struct tag_set revalidate_set; /* Revalidate only matching facets. */
- /* Set of odp flow keys which have drop flows installed in the kernel.
- * These are datapath flows which have no associated ofproto, if they did
- * we would use facets. */
- struct hmap drop_keys;
+ struct hmap drop_keys; /* Set of dropped odp keys. */
};
/* All existing ofproto_backer instances, indexed by ofproto->up.type. */
@@ -852,7 +858,7 @@ type_run(const char *type)
if (backer->need_revalidate) {
/* Clear the drop_keys in case we should now be accepting some
- * formally dropped flows. */
+ * formerly dropped flows. */
drop_key_clear(backer);
}
@@ -1008,6 +1014,9 @@ close_dpif_backer(struct dpif_backer *backer)
return;
}
+ drop_key_clear(backer);
+ hmap_destroy(&backer->drop_keys);
+
hmap_destroy(&backer->odp_to_ofport_map);
node = shash_find(&all_dpif_backers, backer->type);
free(backer->type);
@@ -3486,16 +3495,16 @@ handle_flow_miss(struct flow_miss *miss, struct
flow_miss_op *ops,
handle_flow_miss_with_facet(miss, facet, now, ops, n_ops);
}
-static struct ofpbuf *
+static struct drop_key *
drop_key_lookup(const struct dpif_backer *backer, const struct nlattr *key,
size_t key_len)
{
- struct ofpbuf *drop_key;
+ struct drop_key *drop_key;
HMAP_FOR_EACH_WITH_HASH (drop_key, hmap_node, hash_bytes(key, key_len, 0),
&backer->drop_keys) {
- if (drop_key->size == key_len
- && !memcmp(drop_key->data, key, key_len)) {
+ if (drop_key->key_len == key_len
+ && !memcmp(drop_key->key, key, key_len)) {
return drop_key;
}
}
@@ -3506,23 +3515,24 @@ static void
drop_key_clear(struct dpif_backer *backer)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 15);
- struct ofpbuf *drop_key, *next;
+ struct drop_key *drop_key, *next;
HMAP_FOR_EACH_SAFE (drop_key, next, hmap_node, &backer->drop_keys) {
int error;
- error = dpif_flow_del(backer->dpif, drop_key->data, drop_key->size,
+ error = dpif_flow_del(backer->dpif, drop_key->key, drop_key->key_len,
NULL);
if (error && !VLOG_DROP_WARN(&rl)) {
struct ds ds = DS_EMPTY_INITIALIZER;
- odp_flow_key_format(drop_key->data, drop_key->size, &ds);
+ odp_flow_key_format(drop_key->key, drop_key->key_len, &ds);
VLOG_WARN("Failed to delete drop key (%s) (%s)", strerror(error),
ds_cstr(&ds));
ds_destroy(&ds);
}
hmap_remove(&backer->drop_keys, &drop_key->hmap_node);
- ofpbuf_delete(drop_key);
+ free(drop_key->key);
+ free(drop_key);
}
}
@@ -3649,7 +3659,6 @@ handle_miss_upcalls(struct dpif_backer *backer, struct
dpif_upcall *upcalls,
struct flow_miss *miss = &misses[n_misses];
struct flow_miss *existing_miss;
struct ofproto_dpif *ofproto;
- struct ofpbuf *drop_key;
uint32_t odp_in_port;
struct flow flow;
uint32_t hash;
@@ -3659,6 +3668,8 @@ handle_miss_upcalls(struct dpif_backer *backer, struct
dpif_upcall *upcalls,
upcall->key_len, &flow, &miss->key_fitness,
&ofproto, &odp_in_port, &miss->initial_tci);
if (error == ENODEV) {
+ struct drop_key *drop_key;
+
/* Received packet on port for which we couldn't associate
* an ofproto. This can happen if a port is removed while
* traffic is being received. Print a rate-limited message
@@ -3670,11 +3681,14 @@ handle_miss_upcalls(struct dpif_backer *backer, struct
dpif_upcall *upcalls,
drop_key = drop_key_lookup(backer, upcall->key, upcall->key_len);
if (!drop_key) {
- drop_key = ofpbuf_clone_data(upcall->key, upcall->key_len);
+ drop_key = xmalloc(sizeof *drop_key);
+ drop_key->key = xmemdup(upcall->key, upcall->key_len);
+ drop_key->key_len = upcall->key_len;
+
hmap_insert(&backer->drop_keys, &drop_key->hmap_node,
- hash_bytes(drop_key->data, drop_key->size, 0));
+ hash_bytes(drop_key->key, drop_key->key_len, 0));
dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
- drop_key->data, drop_key->size, NULL, 0, NULL);
+ drop_key->key, drop_key->key_len, NULL, 0, NULL);
}
continue;
}
@@ -3990,10 +4004,6 @@ update_stats(struct dpif_backer *backer)
struct ofproto_dpif *ofproto;
uint32_t key_hash;
- if (drop_key_lookup(backer, key, key_len)) {
- continue;
- }
-
if (ofproto_receive(backer, NULL, key, key_len, &flow, NULL, &ofproto,
NULL, NULL)) {
continue;
--
1.7.9.5
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
