[ovs-dev] [PATCH 2/2] lib/flow: Maintain miniflow values as 64-bit aligned.
Jarno Rajahalme
jrajahalme at nicira.com
Tue Sep 9 23:19:30 UTC 2014
Make miniflow values 64-bit aligned, both in the starting address and
in length. This allows for 64-bit operations in some functions, such
as miniflow_hash().
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/classifier.c | 2 +-
lib/dpif-netdev.c | 7 ++++---
lib/flow.c | 51 ++++++++++++++++++++++++-----------------------
lib/flow.h | 40 +++++++++++++++++++++++++++----------
tests/test-classifier.c | 10 +++++++---
5 files changed, 67 insertions(+), 43 deletions(-)
diff --git a/lib/classifier.c b/lib/classifier.c
index 84381ed..cb2c394 100644
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -1110,7 +1110,7 @@ classifier_rule_overlaps(const struct classifier *cls,
/* Iterate subtables in the descending max priority order. */
PVECTOR_FOR_EACH_PRIORITY (subtable, stop_at_priority, 2,
sizeof(struct cls_subtable), &cls->subtables) {
- uint32_t storage[FLOW_U32S];
+ uint64_t storage[FLOW_U64S];
struct minimask mask;
struct cls_match *head;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 26ed052..6c02a94 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -89,11 +89,12 @@ static struct vlog_rate_limit upcall_rl = VLOG_RATE_LIMIT_INIT(600, 600);
/* Stores a miniflow */
-/* There are fields in the flow structure that we never use. Therefore we can
- * save a few words of memory. */
+/* There are a few words in struct miniflow already, and there are fields in
+ * the flow structure that we never use. Therefore we can save a few words of
+ * memory. */
struct netdev_flow_key {
struct miniflow flow;
- uint32_t buf[FLOW_MAX_PACKET_U32S - MINI_N_INLINE];
+ uint64_t buf[DIV_ROUND_UP(FLOW_MAX_PACKET_U32S - MINI_N_INLINE, 2)];
};
/* Exact match cache for frequently used flows
diff --git a/lib/flow.c b/lib/flow.c
index 79dd832..576977c 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -349,7 +349,7 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
{
struct {
struct miniflow mf;
- uint32_t buf[FLOW_U32S];
+ uint64_t buf[FLOW_U64S];
} m;
COVERAGE_INC(flow_extract);
@@ -367,7 +367,7 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
{
void *data = ofpbuf_data(packet);
size_t size = ofpbuf_size(packet);
- uint32_t *values = miniflow_values(dst);
+ uint32_t *values = (uint32_t *)miniflow_values(dst);
struct mf_ctx mf = { 0, values, values + FLOW_U32S };
char *l2;
ovs_be16 dl_type;
@@ -637,6 +637,9 @@ miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
out:
dst->count = mf.data - values;
dst->map = mf.map;
+ if (dst->count & 1) {
+ *mf.data = 0; /* Clear out the remaining part of the last U64. */
+ }
}
/* For every bit of a field that is wildcarded in 'wildcards', sets the
@@ -1603,7 +1606,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
/* Compressed flow. */
-static uint32_t *
+static uint64_t *
miniflow_alloc_values(struct miniflow *flow)
{
int size = MINIFLOW_VALUES_SIZE(flow->count);
@@ -1635,12 +1638,16 @@ static void
miniflow_init__(struct miniflow *dst, const struct flow *src)
{
const uint32_t *src_u32 = (const uint32_t *) src;
- uint32_t *dst_u32 = miniflow_alloc_values(dst);
+ uint32_t *dst_u32 = (uint32_t *)miniflow_alloc_values(dst);
uint64_t map;
for (map = dst->map; map; map = zero_rightmost_1bit(map)) {
*dst_u32++ = src_u32[raw_ctz(map)];
}
+ /* Pad the last u64? */
+ if ((uintptr_t)dst_u32 & sizeof(uint32_t)) {
+ *dst_u32 = 0;
+ }
}
/* Initializes 'dst' as a copy of 'src'. The caller must eventually free 'dst'
@@ -1685,7 +1692,7 @@ void
miniflow_clone(struct miniflow *dst, const struct miniflow *src)
{
int size = MINIFLOW_VALUES_SIZE(src->count);
- uint32_t *values;
+ uint64_t *values;
dst->count = src->count;
dst->map = src->map;
@@ -1769,30 +1776,24 @@ miniflow_get(const struct miniflow *flow, unsigned int u32_ofs)
: 0;
}
-/* Returns true if 'a' and 'b' are the same flow, false otherwise. */
+/* Returns true if 'a' and 'b' are the same flow, false otherwise.
+ * This is called by the inline version only if needed. */
bool
-miniflow_equal(const struct miniflow *a, const struct miniflow *b)
+miniflow_equal__(const struct miniflow *a, const struct miniflow *b)
{
- const uint32_t *ap = miniflow_get_u32_values(a);
- const uint32_t *bp = miniflow_get_u32_values(b);
const uint64_t a_map = a->map;
const uint64_t b_map = b->map;
+ const uint32_t *ap = miniflow_get_u32_values(a);
+ const uint32_t *bp = miniflow_get_u32_values(b);
+ uint64_t map;
- if (OVS_LIKELY(a_map == b_map)) {
- int count = a->count;
-
- return !memcmp(ap, bp, count * sizeof *ap);
- } else {
- uint64_t map;
-
- for (map = a_map | b_map; map; map = zero_rightmost_1bit(map)) {
- uint64_t bit = rightmost_1bit(map);
- uint64_t a_value = a_map & bit ? *ap++ : 0;
- uint64_t b_value = b_map & bit ? *bp++ : 0;
+ for (map = a_map | b_map; map; map = zero_rightmost_1bit(map)) {
+ uint64_t bit = rightmost_1bit(map);
+ uint32_t a_value = a_map & bit ? *ap++ : 0;
+ uint32_t b_value = b_map & bit ? *bp++ : 0;
- if (a_value != b_value) {
- return false;
- }
+ if (a_value != b_value) {
+ return false;
}
}
@@ -1872,10 +1873,10 @@ minimask_move(struct minimask *dst, struct minimask *src)
void
minimask_combine(struct minimask *dst_,
const struct minimask *a_, const struct minimask *b_,
- uint32_t storage[FLOW_U32S])
+ uint64_t storage[FLOW_U64S])
{
struct miniflow *dst = &dst_->masks;
- uint32_t *dst_values = storage;
+ uint32_t *dst_values = (uint32_t *)storage;
const struct miniflow *a = &a_->masks;
const struct miniflow *b = &b_->masks;
uint64_t map;
diff --git a/lib/flow.h b/lib/flow.h
index 0e01a6e..0237687 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -137,6 +137,7 @@ struct flow {
BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
#define FLOW_U32S (sizeof(struct flow) / 4)
+#define FLOW_U64S DIV_ROUND_UP(FLOW_U32S, 2)
#define FLOW_U32_SIZE(FIELD) \
DIV_ROUND_UP(sizeof(((struct flow *)0)->FIELD), sizeof(uint32_t))
@@ -365,7 +366,7 @@ bool flow_equal_except(const struct flow *a, const struct flow *b,
/* Number of 32-bit words present in struct miniflow. */
#define MINI_N_INLINE 8
-
+BUILD_ASSERT_DECL((MINI_N_INLINE & 1) == 0); /* Even. */
/* Maximum number of 32-bit words supported. */
BUILD_ASSERT_DECL(FLOW_U32S <= 55);
@@ -403,22 +404,23 @@ struct miniflow {
uint64_t values_inline:1;
uint64_t count:8;
union {
- uint32_t *offline_values;
- uint32_t inline_values[MINI_N_INLINE]; /* Minimum inline size. */
+ uint64_t *offline_values;
+ uint64_t inline_values[MINI_N_INLINE / 2]; /* Minimum inline size. */
};
};
BUILD_ASSERT_DECL(sizeof(struct miniflow)
== sizeof(uint64_t) + MINI_N_INLINE * sizeof(uint32_t));
-#define MINIFLOW_VALUES_SIZE(COUNT) ((COUNT) * sizeof(uint32_t))
+#define MINIFLOW_VALUES_SIZE(COUNT) \
+ (DIV_ROUND_UP((COUNT), 2) * sizeof(uint64_t))
-static inline uint32_t *miniflow_values(struct miniflow *mf)
+static inline uint64_t *miniflow_values(struct miniflow *mf)
{
return OVS_LIKELY(mf->values_inline)
? mf->inline_values : mf->offline_values;
}
-static inline const uint32_t *miniflow_get_values(const struct miniflow *mf)
+static inline const uint64_t *miniflow_get_values(const struct miniflow *mf)
{
return OVS_LIKELY(mf->values_inline)
? mf->inline_values : mf->offline_values;
@@ -426,7 +428,7 @@ static inline const uint32_t *miniflow_get_values(const struct miniflow *mf)
static inline const uint32_t *miniflow_get_u32_values(const struct miniflow *mf)
{
- return miniflow_get_values(mf);
+ return (OVS_FORCE const uint32_t *)miniflow_get_values(mf);
}
static inline const ovs_be32 *miniflow_get_be32_values(const struct miniflow *mf)
@@ -436,10 +438,10 @@ static inline const ovs_be32 *miniflow_get_be32_values(const struct miniflow *mf
/* This is useful for initializing a miniflow for a miniflow_extract() call. */
static inline void miniflow_initialize(struct miniflow *mf,
- uint32_t buf[FLOW_U32S])
+ uint64_t buf[FLOW_U64S])
{
mf->count = 0;
- mf->values_inline = (buf == (uint32_t *)(mf + 1));
+ mf->values_inline = (buf == (uint64_t *)(mf + 1));
mf->map = 0;
if (!mf->values_inline) {
mf->offline_values = buf;
@@ -550,7 +552,10 @@ static inline uint16_t miniflow_get_vid(const struct miniflow *);
static inline uint16_t miniflow_get_tcp_flags(const struct miniflow *);
static inline ovs_be64 miniflow_get_metadata(const struct miniflow *);
-bool miniflow_equal(const struct miniflow *a, const struct miniflow *b);
+static inline bool miniflow_equal(const struct miniflow *a,
+ const struct miniflow *b);
+bool miniflow_equal__(const struct miniflow *a, const struct miniflow *b);
+
bool miniflow_equal_in_minimask(const struct miniflow *a,
const struct miniflow *b,
const struct minimask *);
@@ -577,7 +582,7 @@ void minimask_clone(struct minimask *, const struct minimask *);
void minimask_move(struct minimask *dst, struct minimask *src);
void minimask_combine(struct minimask *dst,
const struct minimask *a, const struct minimask *b,
- uint32_t storage[FLOW_U32S]);
+ uint64_t storage[FLOW_U64S]);
void minimask_destroy(struct minimask *);
void minimask_expand(const struct minimask *, struct flow_wildcards *);
@@ -601,6 +606,19 @@ minimask_is_catchall(const struct minimask *mask)
return mask->masks.map == 0;
}
+static inline bool
+miniflow_equal(const struct miniflow *a, const struct miniflow *b)
+{
+ if (OVS_LIKELY(a->map == b->map)) {
+ const uint64_t *ap = miniflow_get_values(a);
+ const uint64_t *bp = miniflow_get_values(b);
+
+ return !memcmp(ap, bp, DIV_ROUND_UP(a->count, 2) * sizeof *ap);
+ }
+ return miniflow_equal__(a,b);
+}
+
+
/* Returns the VID within the vlan_tci member of the "struct flow" represented
* by 'flow'. */
static inline uint16_t
diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 0dfa910..777d106 100644
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -768,6 +768,7 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
struct classifier cls;
struct test_rule *rule1;
struct test_rule *rule2;
+ struct test_rule *old_rule;
struct tcls tcls;
rule1 = make_rule(wc_fields, OFP_DEFAULT_PRIORITY, UINT_MAX);
@@ -787,8 +788,11 @@ test_rule_replacement(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
tcls_init(&tcls);
tcls_insert(&tcls, rule2);
- assert(test_rule_from_cls_rule(
- classifier_replace(&cls, &rule2->cls_rule)) == rule1);
+ old_rule = test_rule_from_cls_rule(
+ classifier_replace(&cls, &rule2->cls_rule));
+ assert(old_rule != NULL);
+ assert(old_rule == rule1);
+
free_rule(rule1);
compare_classifiers(&cls, &tcls);
check_tables(&cls, 1, 1, 0);
@@ -1376,7 +1380,7 @@ test_minimask_combine(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)
for (idx = 0; next_random_flow(&flow, idx); idx++) {
struct minimask minimask, minimask2, minicombined;
struct flow_wildcards mask, mask2, combined, combined2;
- uint32_t storage[FLOW_U32S];
+ uint64_t storage[FLOW_U64S];
struct flow flow2;
mask.masks = flow;
--
1.7.10.4
More information about the dev
mailing list