[ovs-dev] [classifier-opt 25/28] classifier: Break cls_rule 'flow' and 'wc' members into new "struct match".

Ben Pfaff blp at nicira.com
Tue Aug 7 21:42:33 UTC 2012


On Mon, Aug 06, 2012 at 02:29:30PM -0700, Ethan Jackson wrote:
> ==== lib/match.c:
> s/clr_match/match

Thanks, fixed.

> match_set_metadata_masked()'s prototype is too long by a character.

Oops, I've broken the line now.

> ==== lib/ofp-util.c:
> It's strange to me that ofputil_match_from_ofp10_match() has to take a
>      priority at all.  Only a couple of callers use the returned
> value.  I would be inclined to remove the priority argument and have
> it return nothing.  The affected callers could either do the
> calculation by hand, or we could give them a separate helper that
> takes an ofp10_match and a priority and returns a priority.  If you
> decide to leave it as is, please documente the return value.

You're right, this is a wart.

I dropped the parameter and return value from
ofputil_match_from_ofp10_match().  After some thought, it seemed that
I only needed the priority calculation in one place (in
ofputil_decode_flow_mod()) so I inlined it there.

> The indentation is incorrect in ofputil_match_from_ofp11_match().

OK, fixed.

> Everything else looks good, thanks.

Thank you.



More information about the dev mailing list