Skip to content

Commit

Permalink
lib/dpif-netdev: Fix EMC lookup.
Browse files Browse the repository at this point in the history
Patch 0de8783 (lib/dpif-netdev: Integrate megaflow classifier.)
broke exact match cache lookup, but it went undetected since there are
no separate stats for EMC.

This patch fixes the problem by changing the struct netdev_flow_key
'len' member to cover only the 'mf' member, not the whole
netdev_flow_key, and ignoring the 'len' field in
netdev_flow_key_equal.  Comparison is still accurate, as the miniflow
'map' field encodes the length in the number of 1-bits, and the map is
included in the comparison.

Reported-by: Alex Wang <alexw@nicira.com>
Signed-off-by: Jarno Rajahalme <jrajahalme@nicira.com>
Acked-by: Daniele Di Proietto <ddiproietto@vmware.com>
  • Loading branch information
Jarno Rajahalme committed Oct 18, 2014
1 parent 10173ce commit caeb490
Showing 1 changed file with 16 additions and 15 deletions.
31 changes: 16 additions & 15 deletions lib/dpif-netdev.c
Expand Up @@ -89,8 +89,8 @@ static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
/* Stores a miniflow with inline values */

struct netdev_flow_key {
uint32_t hash; /* hash function differs for different users. */
uint32_t len;
uint32_t hash; /* Hash function differs for different users. */
uint32_t len; /* Length of the following miniflow (incl. map). */
struct miniflow mf;
uint32_t buf[FLOW_MAX_PACKET_U32S - MINI_N_INLINE];
};
Expand Down Expand Up @@ -433,10 +433,13 @@ emc_cache_init(struct emc_cache *flow_cache)
{
int i;

BUILD_ASSERT(offsetof(struct miniflow, inline_values) == sizeof(uint64_t));

for (i = 0; i < ARRAY_SIZE(flow_cache->entries); i++) {
flow_cache->entries[i].flow = NULL;
flow_cache->entries[i].key.hash = 0;
flow_cache->entries[i].key.len = 0;
flow_cache->entries[i].key.len
= offsetof(struct miniflow, inline_values);
miniflow_initialize(&flow_cache->entries[i].key.mf,
flow_cache->entries[i].key.buf);
}
Expand Down Expand Up @@ -1269,20 +1272,20 @@ BUILD_ASSERT_DECL(offsetof(struct miniflow, inline_values)
== sizeof(uint64_t));

/* Given the number of bits set in the miniflow map, returns the size of the
* netdev_flow key */
* 'netdev_flow_key.mf' */
static inline uint32_t
netdev_flow_key_size(uint32_t flow_u32s)
{
return offsetof(struct netdev_flow_key, mf.inline_values) +
return offsetof(struct miniflow, inline_values) +
MINIFLOW_VALUES_SIZE(flow_u32s);
}

static inline bool
netdev_flow_key_equal(const struct netdev_flow_key *a,
const struct netdev_flow_key *b)
{
/* 'b's size and hash may be not set yet. */
return !memcmp(a, b, a->len);
/* 'b->len' may be not set yet. */
return a->hash == b->hash && !memcmp(&a->mf, &b->mf, a->len);
}

/* Used to compare 'netdev_flow_key' in the exact match cache to a miniflow.
Expand All @@ -1292,15 +1295,15 @@ static inline bool
netdev_flow_key_equal_mf(const struct netdev_flow_key *key,
const struct miniflow *mf)
{
return !memcmp(&key->mf, mf,
key->len - offsetof(struct netdev_flow_key, mf));
return !memcmp(&key->mf, mf, key->len);
}

static inline void
netdev_flow_key_clone(struct netdev_flow_key *dst,
const struct netdev_flow_key *src)
{
memcpy(dst, src, src->len);
memcpy(dst, src,
offsetof(struct netdev_flow_key, mf) + src->len);
}

/* Slow. */
Expand Down Expand Up @@ -1685,7 +1688,7 @@ dp_netdev_flow_add(struct dp_netdev *dp, struct match *match,
ovs_assert(!(mask.mf.map & (MINIFLOW_MAP(metadata) | MINIFLOW_MAP(regs))));

/* Do not allocate extra space. */
flow = xmalloc(sizeof *flow - sizeof flow->cr.flow + mask.len);
flow = xmalloc(sizeof *flow - sizeof flow->cr.flow.mf + mask.len);
flow->dead = false;
*CONST_CAST(struct flow *, &flow->flow) = match->flow;
ovs_refcount_init(&flow->ref_cnt);
Expand Down Expand Up @@ -2840,8 +2843,6 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,
}
ovs_mutex_unlock(&dp->flow_mutex);

/* EMC uses different hash. */
keys[i].hash = dpif_packet_get_dp_hash(packets[i]);
emc_insert(flow_cache, &keys[i], netdev_flow);
}
}
Expand Down Expand Up @@ -2873,7 +2874,6 @@ fast_path_processing(struct dp_netdev_pmd_thread *pmd,

flow = dp_netdev_flow_cast(rules[i]);

keys[i].hash = dpif_packet_get_dp_hash(packet);
emc_insert(flow_cache, &keys[i], flow);
dp_netdev_queue_batches(packet, flow, &keys[i].mf, batches,
&n_batches, ARRAY_SIZE(batches));
Expand Down Expand Up @@ -3277,7 +3277,8 @@ dpcls_create_subtable(struct dpcls *cls, const struct netdev_flow_key *mask)
struct dpcls_subtable *subtable;

/* Need to add one. */
subtable = xmalloc(sizeof *subtable - sizeof subtable->mask + mask->len);
subtable = xmalloc(sizeof *subtable
- sizeof subtable->mask.mf + mask->len);
cmap_init(&subtable->rules);
netdev_flow_key_clone(&subtable->mask, mask);
cmap_insert(&cls->subtables_map, &subtable->cmap_node, mask->hash);
Expand Down

0 comments on commit caeb490

Please sign in to comment.