[ovs-dev] [wdp] classifier: Fix classifier bugs.

Jesse Gross jesse at nicira.com
Mon Jul 12 23:57:34 UTC 2010


On Mon, Jul 12, 2010 at 1:26 PM, Ben Pfaff <blp at nicira.com> wrote:

> The classifier tests have been failing on the 'wdp' branch for a long time
> now, ever since commit 8980c78 "ofproto: Start work to enable datapaths
> with built-in wildcard support."  I finally got around to finding the
> cause, which is that the classifier was hashing and comparing the
> 'priority' member of exact-match flows.  That member is basically
> meaningless for such flows, so this commit fixes the problem by changing
> the functions that hash and compare flows to ignore it.  They also now
> ignore the 'wildcards' field.  These functions' callers already assumed
> these semantics, so there is no change to the classifier itself.  The
> classifier tests, on the other hand, were not treating the priority of
> exact-match flows in the same way as the classifier itself, so this commit
> changes the tests' behavior to match.


Looks good.

All of the current callers either already know that priority and wildcards
are fixed values or they explicitly do the comparison themselves, so this
works fine.  However, generally speaking, I would probably assume that a
function called flow_equal() either does a full comparison of all fields or
perhaps it takes into account wildcards when deciding what fields to
compare.  In fact, I see in test-classifier.c:tcls_insert():

/* XXX flow_equal should ignore wildcarded fields */

So while I think the change is overall fine, it might be better to put
something in the name of the function to make it blinding obvious what it
actually does.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20100712/a6e4dffa/attachment-0003.html>


More information about the dev mailing list