We allow zero 'values' in a miniflow for it to have the same map
as the corresponding minimask.  Minimasks themselves never have
zero data values, though.  Document this and optimize the code
accordingly.

Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
---
 lib/flow.c  |   16 +++-------------
 lib/flow.h  |    9 +++++++--
 lib/match.c |    6 ++----
 3 files changed, 12 insertions(+), 19 deletions(-)

diff --git a/lib/flow.c b/lib/flow.c
index 8f80f88..4592023 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1516,11 +1516,7 @@ miniflow_hash_in_minimask(const struct miniflow *flow,
     hash = basis;
 
     for (map = mask->masks.map; map; map = zero_rightmost_1bit(map)) {
-        if (*p) {
-            int ofs = raw_ctz(map);
-            hash = mhash_add(hash, miniflow_get(flow, ofs) & *p);
-        }
-        p++;
+       hash = mhash_add(hash, miniflow_get(flow, raw_ctz(map)) & *p++);
     }
 
     return mhash_finish(hash, (p - mask->masks.values) * 4);
@@ -1542,10 +1538,7 @@ flow_hash_in_minimask(const struct flow *flow, const 
struct minimask *mask,
 
     hash = basis;
     for (map = mask->masks.map; map; map = zero_rightmost_1bit(map)) {
-        if (*p) {
-            hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p);
-        }
-        p++;
+       hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p++);
     }
 
     return mhash_finish(hash, (p - mask->masks.values) * 4);
@@ -1567,10 +1560,7 @@ flow_hash_in_minimask_range(const struct flow *flow,
     uint32_t hash = *basis;
 
     for (; map; map = zero_rightmost_1bit(map)) {
-        if (*p) {
-            hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p);
-        }
-        p++;
+       hash = mhash_add(hash, flow_u32[raw_ctz(map)] & *p++);
     }
 
     *basis = hash; /* Allow continuation from the unfinished value. */
diff --git a/lib/flow.h b/lib/flow.h
index a828b66..0b0f593 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -362,7 +362,9 @@ BUILD_ASSERT_DECL(FLOW_U32S <= 64);
  *
  * Elements in 'values' 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 .
+ * 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;
@@ -400,7 +402,10 @@ uint64_t miniflow_get_map_in_range(const struct miniflow 
*, uint8_t start,
 
 /* A sparse representation of a "struct flow_wildcards".
  *
- * See the large comment on struct miniflow for details. */
+ * See the large comment on struct miniflow for details.
+ *
+ * Note: While miniflow can have zero data for a 1-bit in the map,
+ * a minimask may not!  We rely on this in the implementation. */
 struct minimask {
     struct miniflow masks;
 };
diff --git a/lib/match.c b/lib/match.c
index 9e5da13..cb72843 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -1191,10 +1191,8 @@ minimatch_hash_range(const struct minimatch *match, 
uint8_t start, uint8_t end,
     uint32_t hash = *basis;
 
     for (; map; map = zero_rightmost_1bit(map)) {
-        if (*p) {
-            hash = mhash_add(hash, *(p - df) & *p);
-        }
-        p++;
+       hash = mhash_add(hash, *(p - df) & *p);
+       p++;
     }
     *basis = hash; /* Allow continuation from the unfinished value. */
     return mhash_finish(hash, (p - match->mask.masks.values) * 4);
-- 
1.7.9.5

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to