On Wed, Aug 27, 2014 at 12:11:34AM +0000, Saurabh Shah wrote: > > -----Original Message----- > > From: Ben Pfaff [mailto:b...@nicira.com] > > Sent: Tuesday, August 26, 2014 4:56 PM > > To: Saurabh Shah > > Cc: Jarno Rajahalme; dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH] lib/flow.h: Improve struct miniflow comment > > and definition. > > > > On Tue, Aug 26, 2014 at 11:32:55PM +0000, Saurabh Shah wrote: > > > > > > > > > >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 <b...@nicira.com> > > > >Signed-off-by: Jarno Rajahalme <jrajaha...@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 > > > > Does it fail because of the uint64_t -> uint8_t change or because of the > > change to MINI_N_INLINE? (If it's the former let's just drop that > > change.) > > I reverted uint8_t -> uint64_t and was able to move past compile flow.h.
OK, so much for that idea. Jarno, let's stick with uint64_t then? _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
