On Fri, Nov 01, 2013 at 03:20:10PM -0700, Jarno Rajahalme wrote: > v2: Properly finish hashes, more flexible configuration. > > Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com>
Here are my comments for a first pass. In general, I'm surprised and very happy that the code appears simple and straightforward. classifier.h should explain what's going on in comments, at least the basics. In gitk it looks like the change mis-indents some code in insert_subtable(): list_push_back(&cls->subtables_priority, &subtable->list_node); subtable->tag = (minimask_get_metadata_mask(mask) == OVS_BE64_MAX - ? tag_create_deterministic(hash) - : TAG_ALL); + ? tag_create_deterministic(hash) + : TAG_ALL); Here, this check is always true if there is a single rule, but it does not catch every case where there is a single rule, because of hash collisions. That requires a full collision of all 32 bits of the hash, so it is probably rare and not worth concerning ourselves about, but I thought I'd point it out in case you didn't know. (It might be worth noting in the comment?) + /* Check if we have narrowed down to a single rule already. This + * check shows a measurable benefit with non-trivial flow tables. */ + if (!inode->s) { I didn't look at the flow.[ch] code yet. It looks tricky. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev