[ovs-dev] [PATCH] lib/flow.h: Improve struct miniflow comment and definition.

[Saurabh Shah]

>Miniflows can nowadays be dynamically allocated to different inline
>sizes, as done by lib/classifier.c, but this had not been documented
>at the struct miniflow definition.
>
>Also, MINI_N_INLINE had a different value for 32-bit and 64-bit builds
>due to a historical reason.  Now we use 8 for both.
>
>Finally, use change the storage type of 'values_inline' to uint8_t, as
>uint64_t looks kind of wide for a boolean, even though we intend the
>bit be carved out from the uint64_t where 'map' resides.
>
>Suggested-by: Ben Pfaff <blp at nicira.com>
>Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
>---
> lib/flow.c |    3 ++-
> lib/flow.h |   16 +++++++++++++---
> 2 files changed, 15 insertions(+), 4 deletions(-)
>
>diff --git a/lib/flow.c b/lib/flow.c
>index bef2f27..d322ac2 100644
>--- a/lib/flow.c
>+++ b/lib/flow.c
>@@ -1715,7 +1715,8 @@ miniflow_clone_inline(struct miniflow *dst, const
>struct miniflow *src,
> /* Initializes 'dst' with the data in 'src', destroying 'src'.
>  * The caller must eventually free 'dst' with miniflow_destroy().
>  * 'dst' must be regularly sized miniflow, but 'src' can have
>- * larger than default inline values. */
>+ * storage for more than the default MINI_N_INLINE inline
>+ * values. */
> void
> miniflow_move(struct miniflow *dst, struct miniflow *src)
> {
>diff --git a/lib/flow.h b/lib/flow.h
>index 3b8d24d..2891d60 100644
>--- a/lib/flow.h
>+++ b/lib/flow.h
>@@ -346,7 +346,10 @@ bool flow_equal_except(const struct flow *a, const
>struct flow *b,
> ?
> /* Compressed flow. */
> 
>-#define MINI_N_INLINE (sizeof(void *) == 4 ? 7 : 8)
>+/* Number of 32-bit words present in struct miniflow. */
>+#define MINI_N_INLINE 8
>+
>+/* Maximum number of 32-bit words supported. */
> BUILD_ASSERT_DECL(FLOW_U32S <= 63);
> 
> /* A sparse representation of a "struct flow".
>@@ -367,6 +370,11 @@ BUILD_ASSERT_DECL(FLOW_U32S <= 63);
>  * the first element of the values array, the next 1-bit is in the next
>array
>  * element, and so on.
>  *
>+ * MINI_N_INLINE is the default number of inline words.  When a miniflow
>is
>+ * dynamically allocated the actual amount of inline storage may be
>different.
>+ * In that case 'inline_values' contains storage at least for the number
>+ * of words indicated by 'map' (one uint32_t for each 1-bit in the 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
>@@ -375,12 +383,14 @@ BUILD_ASSERT_DECL(FLOW_U32S <= 63);
>  */
> struct miniflow {
>     uint64_t map:63;
>-    uint64_t values_inline:1;
>+    uint8_t values_inline:1;
>     union {
>         uint32_t *offline_values;
>-        uint32_t inline_values[MINI_N_INLINE];
>+        uint32_t inline_values[MINI_N_INLINE]; /* Minimum inline size. */
>     };
> };
>+BUILD_ASSERT_DECL(sizeof(struct miniflow)
>+                  == sizeof(uint64_t) + MINI_N_INLINE * sizeof(uint32_t))

This fails on windows:

c:\mingw\msys\1.0\home\administrator\work\ro\ovs\lib\hash.h(148) : warning
C4028
: formal parameter 2 different from declaration
c:\mingw\msys\1.0\home\administrator\work\ro\ovs\lib\flow.h(393) : error
C2034:
'build_assert_failed' : type of bit field too small for number of bits



>;
> 
> #define MINIFLOW_VALUES_SIZE(COUNT) ((COUNT) * sizeof(uint32_t))
> 
>-- 
>1.7.10.4
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://urldefense.proofpoint.com/v1/url?u=http://openvswitch.org/mailman/
>listinfo/dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=pEkjsHfytvHEWufeZPpgqSO
>JMdMjuZPbesVsNhCUc0E%3D%0A&m=Kjocqp3bw37mqe%2F0YlklDszxdR%2Fv4%2FTksPmp5lB
>wRz4%3D%0A&s=524ceb158df47fe0fda5ae235b222b03309a6c306570ea713b6ef8a5072a4
>de0