On Tue, Apr 29, 2014 at 9:43 AM, Ethan Jackson <et...@nicira.com> wrote: > Acked-by: Ethan Jackson <et...@nicira.com> > > I'm not sure I totally follow Kmindg's comment, perhaps he could > explain further? At any rate, once addressed I"m happy with this. >
After a second look into this, I found that I misunderstood these code before. Sorry for the noise. > Ethan > > On Sat, Apr 19, 2014 at 10:09 PM, Kmindg G <kmi...@gmail.com> wrote: >> On Sat, Apr 19, 2014 at 3:42 AM, Jarno Rajahalme <jrajaha...@nicira.com> >> wrote: >>> This allows use of miniflows that have all of their values inline. >>> >>> Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> >>> --- >>> lib/classifier.c | 36 +++++++++++---------- >>> lib/dpif-netdev.c | 32 ++++++++++--------- >>> lib/flow.c | 91 >>> ++++++++++++++++++++++++++--------------------------- >>> lib/flow.h | 70 +++++++++++++++++++++++++---------------- >>> lib/match.c | 4 +-- >>> 5 files changed, 127 insertions(+), 106 deletions(-) >>> >>> diff --git a/lib/classifier.c b/lib/classifier.c >>> index 0517aa7..75befad 100644 >>> --- a/lib/classifier.c >>> +++ b/lib/classifier.c >>> @@ -40,7 +40,7 @@ struct cls_trie { >>> >>> struct cls_subtable_entry { >>> struct cls_subtable *subtable; >>> - uint32_t *mask_values; >>> + const uint32_t *mask_values; >>> tag_type tag; >>> unsigned int max_priority; >>> }; >>> @@ -279,8 +279,9 @@ static inline uint32_t >>> flow_hash_in_minimask(const struct flow *flow, const struct minimask *mask, >>> uint32_t basis) >>> { >>> + const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks); >>> const uint32_t *flow_u32 = (const uint32_t *)flow; >>> - const uint32_t *p = mask->masks.values; >>> + const uint32_t *p = mask_values; >>> uint32_t hash; >>> uint64_t map; >>> >>> @@ -289,7 +290,7 @@ flow_hash_in_minimask(const struct flow *flow, const >>> struct minimask *mask, >>> hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p++); >>> } >>> >>> - return mhash_finish(hash, (p - mask->masks.values) * 4); >>> + return mhash_finish(hash, (p - mask_values) * 4); >>> } >>> >>> /* Returns a hash value for the bits of 'flow' where there are 1-bits in >>> @@ -301,7 +302,8 @@ static inline uint32_t >>> miniflow_hash_in_minimask(const struct miniflow *flow, >>> const struct minimask *mask, uint32_t basis) >>> { >>> - const uint32_t *p = mask->masks.values; >>> + const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks); >>> + const uint32_t *p = mask_values; >>> uint32_t hash = basis; >>> uint32_t flow_u32; >>> >>> @@ -309,7 +311,7 @@ miniflow_hash_in_minimask(const struct miniflow *flow, >>> hash = mhash_add(hash, flow_u32 & *p++); >>> } >>> >>> - return mhash_finish(hash, (p - mask->masks.values) * 4); >>> + return mhash_finish(hash, (p - mask_values) * 4); >>> } >>> >>> /* Returns a hash value for the bits of range [start, end) in 'flow', >>> @@ -322,11 +324,12 @@ flow_hash_in_minimask_range(const struct flow *flow, >>> const struct minimask *mask, >>> uint8_t start, uint8_t end, uint32_t *basis) >>> { >>> + const uint32_t *mask_values = miniflow_get_u32_values(&mask->masks); >>> const uint32_t *flow_u32 = (const uint32_t *)flow; >>> unsigned int offset; >>> uint64_t map = miniflow_get_map_in_range(&mask->masks, start, end, >>> &offset); >>> - const uint32_t *p = mask->masks.values + offset; >>> + const uint32_t *p = mask_values + offset; >>> uint32_t hash = *basis; >>> >>> for (; map; map = zero_rightmost_1bit(map)) { >>> @@ -334,7 +337,7 @@ flow_hash_in_minimask_range(const struct flow *flow, >>> } >>> >>> *basis = hash; /* Allow continuation from the unfinished value. */ >>> - return mhash_finish(hash, (p - mask->masks.values) * 4); >>> + return mhash_finish(hash, (p - mask_values) * 4); >>> } >>> >>> /* Fold minimask 'mask''s wildcard mask into 'wc's wildcard mask. */ >>> @@ -356,7 +359,7 @@ flow_wildcards_fold_minimask_range(struct >>> flow_wildcards *wc, >>> unsigned int offset; >>> uint64_t map = miniflow_get_map_in_range(&mask->masks, start, end, >>> &offset); >>> - const uint32_t *p = mask->masks.values + offset; >>> + const uint32_t *p = miniflow_get_u32_values(&mask->masks) + offset; >>> >>> for (; map; map = zero_rightmost_1bit(map)) { >>> dst_u32[raw_ctz(map)] |= *p++; >>> @@ -367,7 +370,8 @@ flow_wildcards_fold_minimask_range(struct >>> flow_wildcards *wc, >>> static inline uint32_t >>> miniflow_hash(const struct miniflow *flow, uint32_t basis) >>> { >>> - const uint32_t *p = flow->values; >>> + const uint32_t *values = miniflow_get_u32_values(flow); >>> + const uint32_t *p = values; >>> uint32_t hash = basis; >>> uint64_t hash_map = 0; >>> uint64_t map; >>> @@ -382,7 +386,7 @@ miniflow_hash(const struct miniflow *flow, uint32_t >>> basis) >>> hash = mhash_add(hash, hash_map); >>> hash = mhash_add(hash, hash_map >> 32); >>> >>> - return mhash_finish(hash, p - flow->values); >>> + return mhash_finish(hash, p - values); >>> } >>> >>> /* Returns a hash value for 'mask', given 'basis'. */ >>> @@ -415,8 +419,8 @@ minimatch_hash_range(const struct minimatch *match, >>> uint8_t start, uint8_t end, >>> >>> 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 = miniflow_get_u32_values(&match->mask.masks) + offset; >>> + p = miniflow_get_u32_values(&match->flow) + offset; >>> >>> for (i = 0; i < n; i++) { >>> hash = mhash_add(hash, p[i] & q[i]); >>> @@ -970,8 +974,8 @@ static bool >>> minimatch_matches_miniflow(const struct minimatch *match, >>> const struct miniflow *target) >>> { >>> - const uint32_t *flowp = (const uint32_t *)match->flow.values; >>> - const uint32_t *maskp = (const uint32_t *)match->mask.masks.values; >>> + const uint32_t *flowp = miniflow_get_u32_values(&match->flow); >>> + const uint32_t *maskp = miniflow_get_u32_values(&match->mask.masks); >>> uint32_t target_u32; >>> >>> MINIFLOW_FOR_EACH_IN_MAP(target_u32, target, match->mask.masks.map) { >>> @@ -1325,7 +1329,7 @@ insert_subtable(struct cls_classifier *cls, const >>> struct minimask *mask) >>> >>> hmap_insert(&cls->subtables, &subtable->hmap_node, hash); >>> elem.subtable = subtable; >>> - elem.mask_values = subtable->mask.masks.values; >>> + elem.mask_values = miniflow_get_values(&subtable->mask.masks); >>> elem.tag = subtable->tag; >>> elem.max_priority = subtable->max_priority; >>> cls_subtable_cache_push_back(&cls->subtables_priority, elem); >>> @@ -1988,7 +1992,7 @@ minimask_get_prefix_len(const struct minimask >>> *minimask, >>> static const ovs_be32 * >>> minimatch_get_prefix(const struct minimatch *match, const struct mf_field >>> *mf) >>> { >>> - return match->flow.values + >>> + return miniflow_get_be32_values(&match->flow) + >>> count_1bits(match->flow.map & ((UINT64_C(1) << mf->flow_be32ofs) - >>> 1)); >>> } >>> >>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c >>> index 790fe23..cdb3422 100644 >>> --- a/lib/dpif-netdev.c >>> +++ b/lib/dpif-netdev.c >>> @@ -1527,8 +1527,10 @@ dpif_netdev_execute(struct dpif *dpif, struct >>> dpif_execute *execute) >>> { >>> struct dp_netdev *dp = get_dp_netdev(dpif); >>> struct pkt_metadata *md = &execute->md; >>> - struct miniflow key; >>> - uint32_t buf[FLOW_U32S]; >>> + struct { >>> + struct miniflow flow; >>> + uint32_t buf[FLOW_U32S]; >>> + } key; >>> >>> if (ofpbuf_size(execute->packet) < ETH_HEADER_LEN || >>> ofpbuf_size(execute->packet) > UINT16_MAX) { >>> @@ -1536,11 +1538,11 @@ dpif_netdev_execute(struct dpif *dpif, struct >>> dpif_execute *execute) >>> } >>> >>> /* Extract flow key. */ >>> - miniflow_initialize(&key, buf); >>> - miniflow_extract(execute->packet, md, &key); >>> + miniflow_initialize(&key.flow, key.buf); >>> + miniflow_extract(execute->packet, md, &key.flow); >>> >>> ovs_rwlock_rdlock(&dp->port_rwlock); >>> - dp_netdev_execute_actions(dp, &key, execute->packet, false, md, >>> + dp_netdev_execute_actions(dp, &key.flow, execute->packet, false, md, >>> execute->actions, execute->actions_len); >>> ovs_rwlock_unlock(&dp->port_rwlock); >>> >>> @@ -2012,32 +2014,34 @@ dp_netdev_input(struct dp_netdev *dp, struct ofpbuf >>> *packet, >>> OVS_REQ_RDLOCK(dp->port_rwlock) >>> { >>> struct dp_netdev_flow *netdev_flow; >>> - struct miniflow key; >>> - uint32_t buf[FLOW_U32S]; >>> + struct { >>> + struct miniflow flow; >>> + uint32_t buf[FLOW_U32S]; >>> + } key; >>> >>> if (ofpbuf_size(packet) < ETH_HEADER_LEN) { >>> ofpbuf_delete(packet); >>> return; >>> } >>> - miniflow_initialize(&key, buf); >>> - miniflow_extract(packet, md, &key); >>> + miniflow_initialize(&key.flow, key.buf); >>> + miniflow_extract(packet, md, &key.flow); >>> >>> - netdev_flow = dp_netdev_lookup_flow(dp, &key); >>> + netdev_flow = dp_netdev_lookup_flow(dp, &key.flow); >>> if (netdev_flow) { >>> struct dp_netdev_actions *actions; >>> >>> - dp_netdev_flow_used(netdev_flow, packet, &key); >>> + dp_netdev_flow_used(netdev_flow, packet, &key.flow); >>> >>> actions = dp_netdev_flow_get_actions(netdev_flow); >>> - dp_netdev_execute_actions(dp, &key, packet, true, md, >>> + dp_netdev_execute_actions(dp, &key.flow, packet, true, md, >>> actions->actions, actions->size); >>> dp_netdev_count_packet(dp, DP_STAT_HIT); >>> } else if (dp->handler_queues) { >>> dp_netdev_count_packet(dp, DP_STAT_MISS); >>> dp_netdev_output_userspace(dp, packet, >>> - miniflow_hash_5tuple(&key, 0) >>> + miniflow_hash_5tuple(&key.flow, 0) >>> % dp->n_handlers, >>> - DPIF_UC_MISS, &key, NULL); >>> + DPIF_UC_MISS, &key.flow, NULL); >>> ofpbuf_delete(packet); >>> } >>> } >>> diff --git a/lib/flow.c b/lib/flow.c >>> index 22e95d2..8e8e67e 100644 >>> --- a/lib/flow.c >>> +++ b/lib/flow.c >>> @@ -344,26 +344,29 @@ void >>> flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, >>> struct flow *flow) >>> { >>> - uint32_t buf[FLOW_U32S]; >>> - struct miniflow mf; >>> + struct { >>> + struct miniflow mf; >>> + uint32_t buf[FLOW_U32S]; >>> + } m; >>> >>> COVERAGE_INC(flow_extract); >>> >>> - miniflow_initialize(&mf, buf); >>> - miniflow_extract(packet, md, &mf); >>> - miniflow_expand(&mf, flow); >>> + miniflow_initialize(&m.mf, m.buf); >>> + miniflow_extract(packet, md, &m.mf); >>> + miniflow_expand(&m.mf, flow); >>> } >>> >>> -/* Caller is responsible for initializing 'dst->values' with enough storage >>> - * for FLOW_U32S * 4 bytes. */ >>> +/* Caller is responsible for initializing 'dst' with enough storage for >>> + * FLOW_U32S * 4 bytes. */ >>> void >>> miniflow_extract(struct ofpbuf *packet, const struct pkt_metadata *md, >>> struct miniflow *dst) >>> { >>> void *data = ofpbuf_data(packet); >>> size_t size = ofpbuf_size(packet); >>> + uint32_t *values = miniflow_values(dst); >>> + struct mf_ctx mf = { 0, values, values + FLOW_U32S }; >>> char *l2; >>> - struct mf_ctx mf = { 0, dst->values, dst->values + FLOW_U32S }; >>> ovs_be16 dl_type; >>> uint8_t nw_frag, nw_tos, nw_ttl, nw_proto; >>> >>> @@ -1578,11 +1581,14 @@ miniflow_n_values(const struct miniflow *flow) >>> static uint32_t * >>> miniflow_alloc_values(struct miniflow *flow, int n) >>> { >>> - if (n <= MINI_N_INLINE) { >>> + if (n <= sizeof flow->inline_values / sizeof(uint32_t)) { >>> + flow->values_inline = true; >>> return flow->inline_values; >>> } else { >>> COVERAGE_INC(miniflow_malloc); >>> - return xmalloc(n * sizeof *flow->values); >>> + flow->values_inline = false; >>> + flow->offline_values = xmalloc(n * sizeof(uint32_t)); >>> + return flow->offline_values; >>> } >>> } >>> >>> @@ -1595,25 +1601,24 @@ miniflow_alloc_values(struct miniflow *flow, int n) >>> * when a miniflow is initialized from a (mini)mask, the values can be >>> zeroes, >>> * so that the flow and mask always have the same maps. >>> * >>> - * This function initializes 'dst->values' (either inline if possible or >>> with >>> + * This function initializes values (either inline if possible or with >>> * malloc() otherwise) and copies the uint32_t elements of 'src' indicated >>> by >>> * 'dst->map' into it. */ >>> static void >>> miniflow_init__(struct miniflow *dst, const struct flow *src, int n) >>> { >>> const uint32_t *src_u32 = (const uint32_t *) src; >>> - unsigned int ofs; >>> + uint32_t *dst_u32 = miniflow_alloc_values(dst, n); >>> uint64_t map; >>> >>> - dst->values = miniflow_alloc_values(dst, n); >>> - ofs = 0; >>> for (map = dst->map; map; map = zero_rightmost_1bit(map)) { >>> - dst->values[ofs++] = src_u32[raw_ctz(map)]; >>> + *dst_u32++ = src_u32[raw_ctz(map)]; >>> } >>> } >>> >>> /* Initializes 'dst' as a copy of 'src'. The caller must eventually free >>> 'dst' >>> - * with miniflow_destroy(). */ >>> + * with miniflow_destroy(). >>> + * Always allocates offline storage. */ >>> void >>> miniflow_init(struct miniflow *dst, const struct flow *src) >>> { >>> @@ -1652,8 +1657,8 @@ miniflow_clone(struct miniflow *dst, const struct >>> miniflow *src) >>> { >>> int n = miniflow_n_values(src); >>> dst->map = src->map; >>> - dst->values = miniflow_alloc_values(dst, n); >>> - memcpy(dst->values, src->values, n * sizeof *dst->values); >>> + memcpy(miniflow_alloc_values(dst, n), miniflow_get_values(src), >>> + n * sizeof(uint32_t)); >>> } >>> >>> /* Initializes 'dst' with the data in 'src', destroying 'src'. >>> @@ -1661,14 +1666,7 @@ miniflow_clone(struct miniflow *dst, const struct >>> miniflow *src) >>> void >>> miniflow_move(struct miniflow *dst, struct miniflow *src) >>> { >>> - if (src->values == src->inline_values) { >>> - dst->values = dst->inline_values; >>> - memcpy(dst->values, src->values, >>> - miniflow_n_values(src) * sizeof *dst->values); >>> - } else { >>> - dst->values = src->values; >>> - } >>> - dst->map = src->map; >>> + *dst = *src; >>> } >>> >>> /* Frees any memory owned by 'flow'. Does not free the storage in which >>> 'flow' >>> @@ -1676,8 +1674,8 @@ miniflow_move(struct miniflow *dst, struct miniflow >>> *src) >>> void >>> miniflow_destroy(struct miniflow *flow) >>> { >>> - if (flow->values != flow->inline_values) { >>> - free(flow->values); >>> + if (!flow->values_inline) { >>> + free(flow->offline_values); >>> } >>> } >>> >>> @@ -1695,7 +1693,7 @@ static uint32_t >>> miniflow_get(const struct miniflow *flow, unsigned int u32_ofs) >>> { >>> return (flow->map & UINT64_C(1) << u32_ofs) >>> - ? *(flow->values + >>> + ? *(miniflow_get_u32_values(flow) + >>> count_1bits(flow->map & ((UINT64_C(1) << u32_ofs) - 1))) >>> : 0; >>> } >>> @@ -1704,19 +1702,22 @@ miniflow_get(const struct miniflow *flow, unsigned >>> int u32_ofs) >>> bool >>> miniflow_equal(const struct miniflow *a, const struct miniflow *b) >>> { >>> - const uint32_t *ap = a->values; >>> - const uint32_t *bp = b->values; >>> + 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; >>> - uint64_t map; >>> >>> - if (a_map == b_map) { >>> - for (map = a_map; map; map = zero_rightmost_1bit(map)) { >>> + if (OVS_LIKELY(a_map == b_map)) { >>> + int count = miniflow_n_values(a); >>> + >>> + while (count--) { >>> if (*ap++ != *bp++) { >>> return false; >>> } >>> } >>> } 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; >>> @@ -1737,18 +1738,15 @@ bool >>> miniflow_equal_in_minimask(const struct miniflow *a, const struct miniflow >>> *b, >>> const struct minimask *mask) >>> { >>> - const uint32_t *p; >>> + const uint32_t *p = miniflow_get_u32_values(&mask->masks); >>> uint64_t map; >>> >>> - p = mask->masks.values; >>> - >>> for (map = mask->masks.map; map; map = zero_rightmost_1bit(map)) { >>> int ofs = raw_ctz(map); >>> >>> - if ((miniflow_get(a, ofs) ^ miniflow_get(b, ofs)) & *p) { >>> + if ((miniflow_get(a, ofs) ^ miniflow_get(b, ofs)) & *p++) { >>> return false; >>> } >>> - p++; >>> } >>> >>> return true; >>> @@ -1761,18 +1759,15 @@ miniflow_equal_flow_in_minimask(const struct >>> miniflow *a, const struct flow *b, >>> const struct minimask *mask) >>> { >>> const uint32_t *b_u32 = (const uint32_t *) b; >>> - const uint32_t *p; >>> + const uint32_t *p = miniflow_get_u32_values(&mask->masks); >>> uint64_t map; >>> >>> - p = mask->masks.values; >>> - >>> for (map = mask->masks.map; map; map = zero_rightmost_1bit(map)) { >>> int ofs = raw_ctz(map); >>> >>> - if ((miniflow_get(a, ofs) ^ b_u32[ofs]) & *p) { >>> + if ((miniflow_get(a, ofs) ^ b_u32[ofs]) & *p++) { >>> return false; >>> } >>> - p++; >>> } >>> >>> return true; >>> @@ -1813,12 +1808,14 @@ minimask_combine(struct minimask *dst_, >>> uint32_t storage[FLOW_U32S]) >>> { >>> struct miniflow *dst = &dst_->masks; >>> + uint32_t *dst_values = storage; >>> const struct miniflow *a = &a_->masks; >>> const struct miniflow *b = &b_->masks; >>> uint64_t map; >>> int n = 0; >>> >>> - dst->values = storage; >>> + dst->values_inline = false; >>> + dst->offline_values = storage; >>> >>> dst->map = 0; >>> for (map = a->map & b->map; map; map = zero_rightmost_1bit(map)) { >>> @@ -1827,7 +1824,7 @@ minimask_combine(struct minimask *dst_, >>> >>> if (mask) { >>> dst->map |= rightmost_1bit(map); >>> - dst->values[n++] = mask; >>> + dst_values[n++] = mask; >>> } >>> } >>> } >>> @@ -1867,7 +1864,7 @@ minimask_equal(const struct minimask *a, const struct >>> minimask *b) >>> bool >>> minimask_has_extra(const struct minimask *a, const struct minimask *b) >>> { >>> - const uint32_t *p = b->masks.values; >>> + const uint32_t *p = miniflow_get_u32_values(&b->masks); >>> uint64_t map; >>> >>> for (map = b->masks.map; map; map = zero_rightmost_1bit(map)) { >>> diff --git a/lib/flow.h b/lib/flow.h >>> index 47ccccb..c508e1e 100644 >>> --- a/lib/flow.h >>> +++ b/lib/flow.h >>> @@ -323,7 +323,7 @@ bool flow_equal_except(const struct flow *a, const >>> struct flow *b, >>> /* Compressed flow. */ >>> >>> #define MINI_N_INLINE (sizeof(void *) == 4 ? 7 : 8) >>> -BUILD_ASSERT_DECL(FLOW_U32S <= 64); >>> +BUILD_ASSERT_DECL(FLOW_U32S <= 63); >>> >>> /* A sparse representation of a "struct flow". >>> * >>> @@ -334,42 +334,59 @@ BUILD_ASSERT_DECL(FLOW_U32S <= 64); >>> * >>> * The 'map' member holds one bit for each uint32_t in a "struct flow". >>> Each >>> * 0-bit indicates that the corresponding uint32_t is zero, each 1-bit >>> that it >>> - * *may* be nonzero. >>> + * *may* be nonzero (see below how this applies to minimasks). >>> * >>> - * 'values' points to the start of an array that has one element for each >>> 1-bit >>> - * in 'map'. The least-numbered 1-bit is in values[0], the next 1-bit is >>> in >>> - * values[1], and so on. >>> + * The 'values_inline' boolean member indicates that the values are at >>> + * 'inline_values'. If 'values_inline' is zero, then the values are >>> + * offline at 'offline_values'. In either case, values is an array that >>> has >>> + * one element for each 1-bit in 'map'. The least-numbered 1-bit is in >>> + * the first element of the values array, the next 1-bit is in the next >>> array >>> + * element, and so on. >>> * >>> - * 'values' may point to a few different locations: >>> - * >>> - * - If 'map' has MINI_N_INLINE or fewer 1-bits, it may point to >>> - * 'inline_values'. One hopes that this is the common case. >>> - * >>> - * - If 'map' has more than MINI_N_INLINE 1-bits, it may point to >>> memory >>> - * allocated with malloc(). >>> - * >>> - * - The caller could provide storage on the stack for situations where >>> - * that makes sense. So far that's only proved useful for >>> - * minimask_combine(), but the principle works elsewhere. >>> - * >>> - * Elements in 'values' are allowed to be zero. This is useful for "struct >>> + * 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'. >>> */ >>> struct miniflow { >>> - uint64_t map; >>> - uint32_t *values; >>> - uint32_t inline_values[MINI_N_INLINE]; >>> + uint64_t map:63; >>> + uint64_t values_inline:1; >>> + union { >>> + uint32_t *offline_values; >>> + uint32_t inline_values[MINI_N_INLINE]; >>> + }; >>> }; >>> >>> +static inline uint32_t *miniflow_values(struct miniflow *mf) >>> +{ >>> + return mf->values_inline ? mf->inline_values : mf->offline_values; >>> +} >>> + >>> +static inline const uint32_t *miniflow_get_values(const struct miniflow >>> *mf) >>> +{ >>> + return mf->values_inline ? mf->inline_values : mf->offline_values; >>> +} >>> + >>> +static inline const uint32_t *miniflow_get_u32_values(const struct >>> miniflow *mf) >>> +{ >>> + return miniflow_get_values(mf); >>> +} >>> + >>> +static inline const ovs_be32 *miniflow_get_be32_values(const struct >>> miniflow *mf) >>> +{ >>> + return (OVS_FORCE const ovs_be32 *)miniflow_get_values(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]) >>> { >>> mf->map = 0; >>> - mf->values = buf; >>> + mf->values_inline = (buf == (uint32_t *)(mf + 1)); >>> + if (!mf->values_inline) { >>> + mf->offline_values = buf; >>> + } >> >> If mf->values_inline is true, the real data is stored in the buf >> followed the mf instead of the mf->inline_values. >> But, miniflow_values tries to get real data in mf->values_inline later. >> >>> } >>> >>> struct pkt_metadata; >>> @@ -415,7 +432,7 @@ mf_get_next_in_map(uint64_t *fmap, uint64_t rm1bit, >>> const uint32_t **fp, >>> /* Iterate through all miniflow u32 values specified by the 'MAP'. >>> * This works as the first statement in a block.*/ >>> #define MINIFLOW_FOR_EACH_IN_MAP(VALUE, FLOW, MAP) \ >>> - const uint32_t *fp_ = (FLOW)->values; \ >>> + const uint32_t *fp_ = miniflow_get_u32_values(FLOW); \ >>> uint64_t rm1bit_, fmap_, map_; \ >>> for (fmap_ = (FLOW)->map, map_ = (MAP), rm1bit_ = >>> rightmost_1bit(map_); \ >>> mf_get_next_in_map(&fmap_, rm1bit_, &fp_, &(VALUE)); \ >>> @@ -426,7 +443,7 @@ mf_get_next_in_map(uint64_t *fmap, uint64_t rm1bit, >>> const uint32_t **fp, >>> #define MINIFLOW_GET_TYPE(MF, TYPE, OFS) \ >>> (((MF)->map & (UINT64_C(1) << (OFS) / 4)) \ >>> ? ((OVS_FORCE const TYPE *) \ >>> - ((MF)->values \ >>> + (miniflow_get_u32_values(MF) \ >>> + count_1bits((MF)->map & ((UINT64_C(1) << (OFS) / 4) - 1)))) \ >>> [(OFS) % 4 / sizeof(TYPE)] \ >>> : 0) \ >>> @@ -485,6 +502,7 @@ static inline ovs_be64 minimask_get_metadata_mask(const >>> struct minimask *); >>> bool minimask_equal(const struct minimask *a, const struct minimask *b); >>> bool minimask_has_extra(const struct minimask *, const struct minimask *); >>> >>> + >>> /* Returns true if 'mask' matches every packet, false if 'mask' fixes any >>> bits >>> * or fields. */ >>> static inline bool >>> @@ -496,8 +514,6 @@ minimask_is_catchall(const struct minimask *mask) >>> return mask->masks.map == 0; >>> } >>> >>> - >>> - >>> /* Returns the VID within the vlan_tci member of the "struct flow" >>> represented >>> * by 'flow'. */ >>> static inline uint16_t >>> @@ -560,7 +576,7 @@ static inline void >>> flow_union_with_miniflow(struct flow *dst, const struct miniflow *src) >>> { >>> uint32_t *dst_u32 = (uint32_t *) dst; >>> - const uint32_t *p = (uint32_t *)src->values; >>> + const uint32_t *p = miniflow_get_u32_values(src); >>> uint64_t map; >>> >>> for (map = src->map; map; map = zero_rightmost_1bit(map)) { >>> diff --git a/lib/match.c b/lib/match.c >>> index 5bbd2f0..308f906 100644 >>> --- a/lib/match.c >>> +++ b/lib/match.c >>> @@ -1255,8 +1255,8 @@ minimatch_matches_flow(const struct minimatch *match, >>> const struct flow *target) >>> { >>> const uint32_t *target_u32 = (const uint32_t *) target; >>> - const uint32_t *flowp = match->flow.values; >>> - const uint32_t *maskp = match->mask.masks.values; >>> + const uint32_t *flowp = miniflow_get_u32_values(&match->flow); >>> + const uint32_t *maskp = miniflow_get_u32_values(&match->mask.masks); >>> uint64_t map; >>> >>> for (map = match->flow.map; map; map = zero_rightmost_1bit(map)) { >>> -- >>> 1.7.10.4 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> http://openvswitch.org/mailman/listinfo/dev >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev