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 <jrajaha...@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 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev