On Apr 24, 2014, at 5:15 PM, Ethan Jackson <et...@nicira.com> wrote:
> In the commit message s/suppoort/support/
>
> Out of curiosity why replace the miniflow_get inline functions with macros?
I got tired of writing (or rather copy&pasting) the “offset of(struct flow,
<FIELD>)”. The macro can take the field name as an argument instead.
>
>> + /* Separate loops for better optimization. */
>
> Why do we need separate loops? You think it somehow improves the
> branch predictor or something? Does this actually help? It smells
> like premature optimization.
>
There is the qualitative difference that forcing the map to be a variable and
sharing the loop results in much more code for examining the bits one at a
time. Keeping it separate makes the map a compile-time constant in each loop,
so that most of the bit arithmetic can be done at the compile time instead.
>> + hash = mhash_add(hash, saddr[0]);
>> + hash = mhash_add(hash, saddr[1]);
>> + hash = mhash_add(hash, saddr[2]);
>> + hash = mhash_add(hash, saddr[3]);
>> + hash = mhash_add(hash, daddr[0]);
>> + hash = mhash_add(hash, daddr[1]);
>> + hash = mhash_add(hash, daddr[2]);
>> + hash = mhash_add(hash, daddr[3]);
>
> Maybe use a loop?
>
OK, done, like this:
+ int ofs = offsetof(struct flow, ipv6_src) / 4;
+ int end = ofs + 2 * sizeof flow->ipv6_src / 4;
+
+ while (ofs < end) {
+ hash = mhash_add(hash, flow_u32[ofs++]);
+ }
with a preceding
+BUILD_ASSERT_DECL(offsetof(struct flow, ipv6_src) + 16
+ == offsetof(struct flow, ipv6_dst));
> Acked-by: Ethan Jackson <et...@nicira.com>
Thanks for the review, will push in a minute.
Jarno
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
