From 09b0fa9c559e448936feafa3fe68b4f955832afa Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Thu, 16 Jul 2015 17:42:24 -0700 Subject: [PATCH] flow: Make compile with MSVC. MSVC does not like zero sized arrays in structs. Hence, remove the 'values' member from struct miniflow and add back the getters miniflow_values() and miniflow_get_values(). Signed-off-by: Jarno Rajahalme Acked-by: Ben Pfaff --- lib/classifier-private.h | 19 ++++++++++--------- lib/classifier.c | 10 +++++----- lib/dpif-netdev.c | 36 ++++++++++++++++++------------------ lib/flow.c | 23 ++++++++++++----------- lib/flow.h | 28 +++++++++++++++++++++------- lib/match.c | 10 ++++++---- 6 files changed, 72 insertions(+), 54 deletions(-) diff --git a/lib/classifier-private.h b/lib/classifier-private.h index 8096a36da..2dfaf9339 100644 --- a/lib/classifier-private.h +++ b/lib/classifier-private.h @@ -254,7 +254,7 @@ static inline uint32_t flow_hash_in_minimask(const struct flow *flow, const struct minimask *mask, uint32_t basis) { - const uint64_t *mask_values = mask->masks.values; + const uint64_t *mask_values = miniflow_get_values(&mask->masks); const uint64_t *flow_u64 = (const uint64_t *)flow; const uint64_t *p = mask_values; uint32_t hash; @@ -277,7 +277,7 @@ static inline uint32_t miniflow_hash_in_minimask(const struct miniflow *flow, const struct minimask *mask, uint32_t basis) { - const uint64_t *mask_values = mask->masks.values; + const uint64_t *mask_values = miniflow_get_values(&mask->masks); const uint64_t *p = mask_values; uint32_t hash = basis; uint64_t flow_u64; @@ -299,7 +299,7 @@ flow_hash_in_minimask_range(const struct flow *flow, const struct minimask *mask, uint8_t start, uint8_t end, uint32_t *basis) { - const uint64_t *mask_values = mask->masks.values; + const uint64_t *mask_values = miniflow_get_values(&mask->masks); const uint64_t *flow_u64 = (const uint64_t *)flow; unsigned int offset; uint64_t map; @@ -332,14 +332,14 @@ flow_wildcards_fold_minimask_range(struct flow_wildcards *wc, const struct minimask *mask, uint8_t start, uint8_t end) { + const uint64_t *p = miniflow_get_values(&mask->masks); uint64_t *dst_u64 = (uint64_t *)&wc->masks; unsigned int offset; uint64_t map; - const uint64_t *p; int idx; map = miniflow_get_map_in_range(&mask->masks, start, end, &offset); - p = mask->masks.values + offset; + p += offset; MAP_FOR_EACH_INDEX(idx, map) { dst_u64[idx] |= *p++; } @@ -349,7 +349,7 @@ flow_wildcards_fold_minimask_range(struct flow_wildcards *wc, static inline uint32_t miniflow_hash(const struct miniflow *flow, uint32_t basis) { - const uint64_t *values = flow->values; + const uint64_t *values = miniflow_get_values(flow); const uint64_t *p = values; uint32_t hash = basis; uint64_t hash_map = 0; @@ -390,15 +390,16 @@ static inline uint32_t minimatch_hash_range(const struct minimatch *match, uint8_t start, uint8_t end, uint32_t *basis) { + const uint64_t *p = miniflow_get_values(match->flow); + const uint64_t *q = miniflow_get_values(&match->mask->masks); unsigned int offset; - const uint64_t *p, *q; uint32_t hash = *basis; int n, i; n = count_1bits(miniflow_get_map_in_range(&match->mask->masks, start, end, &offset)); - q = match->mask->masks.values + offset; - p = match->flow->values + offset; + q += offset; + p += offset; for (i = 0; i < n; i++) { hash = hash_add64(hash, p[i] & q[i]); diff --git a/lib/classifier.c b/lib/classifier.c index 2ed86971b..c75e9b4a5 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -1698,8 +1698,8 @@ miniflow_and_mask_matches_flow(const struct miniflow *flow, const struct minimask *mask, const struct flow *target) { - const uint64_t *flowp = flow->values; - const uint64_t *maskp = mask->masks.values; + const uint64_t *flowp = miniflow_get_values(flow); + const uint64_t *maskp = miniflow_get_values(&mask->masks); int idx; MAP_FOR_EACH_INDEX(idx, mask->masks.map) { @@ -1747,8 +1747,8 @@ miniflow_and_mask_matches_flow_wc(const struct miniflow *flow, const struct flow *target, struct flow_wildcards *wc) { - const uint64_t *flowp = flow->values; - const uint64_t *maskp = mask->masks.values; + const uint64_t *flowp = miniflow_get_values(flow); + const uint64_t *maskp = miniflow_get_values(&mask->masks); int idx; MAP_FOR_EACH_INDEX(idx, mask->masks.map) { @@ -2191,7 +2191,7 @@ static const ovs_be32 * minimatch_get_prefix(const struct minimatch *match, const struct mf_field *mf) { return (OVS_FORCE const ovs_be32 *) - (match->flow->values + (miniflow_get_values(match->flow) + count_1bits(match->flow->map & ((UINT64_C(1) << mf->flow_be32ofs / 2) - 1))) + (mf->flow_be32ofs & 1); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 9e7343b91..a2bd1b966 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -488,14 +488,13 @@ emc_cache_init(struct emc_cache *flow_cache) { int i; - BUILD_ASSERT(offsetof(struct miniflow, values) == sizeof(uint64_t)); + BUILD_ASSERT(sizeof(struct miniflow) == sizeof(uint64_t)); flow_cache->sweep_idx = 0; 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 - = offsetof(struct miniflow, values); + flow_cache->entries[i].key.len = sizeof(struct miniflow); flow_cache->entries[i].key.mf.map = 0; } } @@ -1525,14 +1524,14 @@ static bool dp_netdev_flow_ref(struct dp_netdev_flow *flow) * The following assertions make sure that what we're doing with miniflow is * safe */ -BUILD_ASSERT_DECL(offsetof(struct miniflow, values) == sizeof(uint64_t)); +BUILD_ASSERT_DECL(sizeof(struct miniflow) == sizeof(uint64_t)); /* Given the number of bits set in the miniflow map, returns the size of the * 'netdev_flow_key.mf' */ static inline uint32_t netdev_flow_key_size(uint32_t flow_u32s) { - return offsetof(struct miniflow, values) + MINIFLOW_VALUES_SIZE(flow_u32s); + return sizeof(struct miniflow) + MINIFLOW_VALUES_SIZE(flow_u32s); } static inline bool @@ -1585,7 +1584,7 @@ netdev_flow_mask_init(struct netdev_flow_key *mask, const struct match *match) { const uint64_t *mask_u64 = (const uint64_t *) &match->wc.masks; - uint64_t *dst = mask->mf.values; + uint64_t *dst = miniflow_values(&mask->mf); uint64_t map, mask_map = 0; uint32_t hash = 0; int n; @@ -1609,7 +1608,7 @@ netdev_flow_mask_init(struct netdev_flow_key *mask, hash = hash_add64(hash, mask_map); - n = dst - mask->mf.values; + n = dst - miniflow_get_values(&mask->mf); mask->hash = hash_finish(hash, n * 8); mask->len = netdev_flow_key_size(n); @@ -1621,8 +1620,8 @@ netdev_flow_key_init_masked(struct netdev_flow_key *dst, const struct flow *flow, const struct netdev_flow_key *mask) { - uint64_t *dst_u64 = dst->mf.values; - const uint64_t *mask_u64 = mask->mf.values; + uint64_t *dst_u64 = miniflow_values(&dst->mf); + const uint64_t *mask_u64 = miniflow_get_values(&mask->mf); uint32_t hash = 0; uint64_t value; @@ -1633,14 +1632,15 @@ netdev_flow_key_init_masked(struct netdev_flow_key *dst, *dst_u64 = value & *mask_u64++; hash = hash_add64(hash, *dst_u64++); } - dst->hash = hash_finish(hash, (dst_u64 - dst->mf.values) * 8); + dst->hash = hash_finish(hash, + (dst_u64 - miniflow_get_values(&dst->mf)) * 8); } /* Iterate through all netdev_flow_key u64 values specified by 'MAP' */ -#define NETDEV_FLOW_KEY_FOR_EACH_IN_MAP(VALUE, KEY, MAP) \ - for (struct mf_for_each_in_map_aux aux__ \ - = { (KEY)->mf.values, (KEY)->mf.map, MAP }; \ - mf_get_next_in_map(&aux__, &(VALUE)); \ +#define NETDEV_FLOW_KEY_FOR_EACH_IN_MAP(VALUE, KEY, MAP) \ + for (struct mf_for_each_in_map_aux aux__ \ + = { miniflow_get_values(&(KEY)->mf), (KEY)->mf.map, MAP }; \ + mf_get_next_in_map(&aux__, &(VALUE)); \ ) /* Returns a hash value for the bits of 'key' where there are 1-bits in @@ -1649,7 +1649,7 @@ static inline uint32_t netdev_flow_key_hash_in_mask(const struct netdev_flow_key *key, const struct netdev_flow_key *mask) { - const uint64_t *p = mask->mf.values; + const uint64_t *p = miniflow_get_values(&mask->mf); uint32_t hash = 0; uint64_t key_u64; @@ -1657,7 +1657,7 @@ netdev_flow_key_hash_in_mask(const struct netdev_flow_key *key, hash = hash_add64(hash, key_u64 & *p++); } - return hash_finish(hash, (p - mask->mf.values) * 8); + return hash_finish(hash, (p - miniflow_get_values(&mask->mf)) * 8); } static inline bool @@ -3855,8 +3855,8 @@ static inline bool dpcls_rule_matches_key(const struct dpcls_rule *rule, const struct netdev_flow_key *target) { - const uint64_t *keyp = rule->flow.mf.values; - const uint64_t *maskp = rule->mask->mf.values; + const uint64_t *keyp = miniflow_get_values(&rule->flow.mf); + const uint64_t *maskp = miniflow_get_values(&rule->mask->mf); uint64_t target_u64; NETDEV_FLOW_KEY_FOR_EACH_IN_MAP(target_u64, target, rule->flow.mf.map) { diff --git a/lib/flow.c b/lib/flow.c index e9ae81328..1fcb67120 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -428,7 +428,7 @@ miniflow_extract(struct dp_packet *packet, struct miniflow *dst) const struct pkt_metadata *md = &packet->md; const void *data = dp_packet_data(packet); size_t size = dp_packet_size(packet); - uint64_t *values = dst->values; + uint64_t *values = miniflow_values(dst); struct mf_ctx mf = { 0, values, values + FLOW_U64S }; const char *l2; ovs_be16 dl_type; @@ -2191,7 +2191,7 @@ void miniflow_init(struct miniflow *dst, const struct flow *src) { const uint64_t *src_u64 = (const uint64_t *) src; - uint64_t *dst_u64 = dst->values; + uint64_t *dst_u64 = miniflow_values(dst); int idx; MAP_FOR_EACH_INDEX(idx, dst->map) { @@ -2258,7 +2258,8 @@ miniflow_clone(struct miniflow *dst, const struct miniflow *src, size_t n_values) { dst->map = src->map; - memcpy(dst->values, src->values, MINIFLOW_VALUES_SIZE(n_values)); + memcpy(miniflow_values(dst), miniflow_get_values(src), + MINIFLOW_VALUES_SIZE(n_values)); } /* Initializes 'dst' as a copy of 'src'. */ @@ -2273,8 +2274,8 @@ miniflow_expand(const struct miniflow *src, struct flow *dst) bool miniflow_equal(const struct miniflow *a, const struct miniflow *b) { - const uint64_t *ap = a->values; - const uint64_t *bp = b->values; + const uint64_t *ap = miniflow_get_values(a); + const uint64_t *bp = miniflow_get_values(b); if (OVS_LIKELY(a->map == b->map)) { int count = miniflow_n_values(a); @@ -2301,7 +2302,7 @@ bool miniflow_equal_in_minimask(const struct miniflow *a, const struct miniflow *b, const struct minimask *mask) { - const uint64_t *p = mask->masks.values; + const uint64_t *p = miniflow_get_values(&mask->masks); int idx; MAP_FOR_EACH_INDEX(idx, mask->masks.map) { @@ -2320,7 +2321,7 @@ miniflow_equal_flow_in_minimask(const struct miniflow *a, const struct flow *b, const struct minimask *mask) { const uint64_t *b_u64 = (const uint64_t *) b; - const uint64_t *p = mask->masks.values; + const uint64_t *p = miniflow_get_values(&mask->masks); int idx; MAP_FOR_EACH_INDEX(idx, mask->masks.map) { @@ -2389,8 +2390,8 @@ bool minimask_equal(const struct minimask *a, const struct minimask *b) { return a->masks.map == b->masks.map && - !memcmp(a->masks.values, b->masks.values, - count_1bits(a->masks.map) * sizeof *a->masks.values); + !memcmp(miniflow_get_values(&a->masks), miniflow_get_values(&b->masks), + MINIFLOW_VALUES_SIZE(count_1bits(a->masks.map))); } /* Returns true if at least one bit matched by 'b' is wildcarded by 'a', @@ -2398,8 +2399,8 @@ minimask_equal(const struct minimask *a, const struct minimask *b) bool minimask_has_extra(const struct minimask *a, const struct minimask *b) { - const uint64_t *ap = a->masks.values; - const uint64_t *bp = b->masks.values; + const uint64_t *ap = miniflow_get_values(&a->masks); + const uint64_t *bp = miniflow_get_values(&b->masks); int idx; MAP_FOR_EACH_INDEX(idx, b->masks.map) { diff --git a/lib/flow.h b/lib/flow.h index 392cb59c7..8a63af105 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -388,22 +388,36 @@ BUILD_ASSERT_DECL(FLOW_U64S <= 64); * 0-bit indicates that the corresponding uint64_t is zero, each 1-bit that it * *may* be nonzero (see below how this applies to minimasks). * + * The values indicated by 'map' always follow the 'map' in memory. The user + * of the miniflow is responsible for always having enough storage after 'map' + * corresponding to the number of 1-bits in 'map'. + * Elements in values array are allowed to be zero. This is useful for "struct * minimatch", for which ensuring that the miniflow and minimask members have * same 'map' allows optimization. This allowance applies only to a miniflow - * that is not a mask. That is, a minimask may NOT have zero elements in - * its 'values'. + * that is not a mask. That is, a minimask may NOT have zero elements in its + * 'values'. * * A miniflow is always dynamically allocated so that the 'values' array has as * many elements as there are 1-bits in 'map'. */ struct miniflow { uint64_t map; - uint64_t values[0]; + /* uint64_t values[]; Storage follows 'map' in memory. */ }; BUILD_ASSERT_DECL(sizeof(struct miniflow) == sizeof(uint64_t)); #define MINIFLOW_VALUES_SIZE(COUNT) ((COUNT) * sizeof(uint64_t)) +static inline uint64_t *miniflow_values(struct miniflow *mf) +{ + return (uint64_t *)(mf + 1); +} + +static inline const uint64_t *miniflow_get_values(const struct miniflow *mf) +{ + return (const uint64_t *)(mf + 1); +} + struct pkt_metadata; /* The 'dst' must follow with buffer space for FLOW_U64S 64-bit units. @@ -495,7 +509,7 @@ mf_get_next_in_map(struct mf_for_each_in_map_aux *aux, uint64_t *value) /* Iterate through all miniflow u64 values specified by 'MAP'. */ #define MINIFLOW_FOR_EACH_IN_MAP(VALUE, FLOW, MAP) \ for (struct mf_for_each_in_map_aux aux__ \ - = { (FLOW)->values, (FLOW)->map, MAP }; \ + = { miniflow_get_values(FLOW), (FLOW)->map, MAP }; \ mf_get_next_in_map(&aux__, &(VALUE)); \ ) @@ -511,7 +525,7 @@ miniflow_values_get__(const uint64_t *values, uint64_t map, int u64_idx) static inline uint64_t miniflow_get__(const struct miniflow *mf, int u64_idx) { - return miniflow_values_get__(mf->values, mf->map, u64_idx); + return miniflow_values_get__(miniflow_get_values(mf), mf->map, u64_idx); } /* Get the value of 'FIELD' of an up to 8 byte wide integer type 'TYPE' of @@ -519,7 +533,7 @@ miniflow_get__(const struct miniflow *mf, int u64_idx) #define MINIFLOW_GET_TYPE(MF, TYPE, OFS) \ (((MF)->map & (UINT64_C(1) << (OFS) / sizeof(uint64_t))) \ ? ((OVS_FORCE const TYPE *) \ - ((MF)->values \ + (miniflow_get_values(MF) \ + count_1bits((MF)->map & \ ((UINT64_C(1) << (OFS) / sizeof(uint64_t)) - 1)))) \ [(OFS) % sizeof(uint64_t) / sizeof(TYPE)] \ @@ -692,7 +706,7 @@ static inline void flow_union_with_miniflow(struct flow *dst, const struct miniflow *src) { uint64_t *dst_u64 = (uint64_t *) dst; - const uint64_t *p = src->values; + const uint64_t *p = miniflow_get_values(src); int idx; MAP_FOR_EACH_INDEX(idx, src->map) { diff --git a/lib/match.c b/lib/match.c index a638dfd96..1585a1f11 100644 --- a/lib/match.c +++ b/lib/match.c @@ -1206,8 +1206,10 @@ minimatch_clone(struct minimatch *dst, const struct minimatch *src) /* Allocate two consecutive miniflows. */ size_t data_size = miniflow_alloc(dst->flows, 2, &src->mask->masks); - memcpy(dst->flow->values, src->flow->values, data_size); - memcpy(dst->mask->masks.values, src->mask->masks.values, data_size); + memcpy(miniflow_values(dst->flow), + miniflow_get_values(src->flow), data_size); + memcpy(miniflow_values(&dst->mask->masks), + miniflow_get_values(&src->mask->masks), data_size); } /* Initializes 'dst' with the data in 'src', destroying 'src'. The caller must @@ -1255,8 +1257,8 @@ minimatch_matches_flow(const struct minimatch *match, const struct flow *target) { const uint64_t *target_u64 = (const uint64_t *) target; - const uint64_t *flowp = match->flow->values; - const uint64_t *maskp = match->mask->masks.values; + const uint64_t *flowp = miniflow_get_values(match->flow); + const uint64_t *maskp = miniflow_get_values(&match->mask->masks); int idx; MAP_FOR_EACH_INDEX(idx, match->flow->map) { -- 2.39.5