[ovs-dev] [PATCH 1/3] Classifier: Staged sub-table matching.

[Ben Pfaff]

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 <jrajahalme at 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.