[ovs-dev] [PATCH 4/5] ofproto: Define ofproto_flow_lookup()

[Ben Pfaff]

On Thu, Mar 27, 2014 at 09:44:35AM -0700, Pravin wrote:
> Define ofproto flow lookup function that can be used by dpif
> implementation.
> 
> Signed-off-by: Pravin B Shelar <pshelar at nicira.com>

The ofproto_flow_lookup() return value convention seems inconsistent.
If xlate_lookup_port() returns an error, it returns that value (a
positive errno).  If fail_open is in effect, it returns a negative
return value.  Positive error values are more consistent with the rest
of userspace, so I'd prefer that convention.

I don't think that ofproto_flow_lookup() modifies its 'flow' argument,
but dp_handle_miss() makes a writable copy and passes in that copy.  I
think that we could drop making that copy and change
ofproto_flow_lookup() to make the 'flow' parameter const.

ofproto_flow_lookup() duplicates a lot of logic elsewhere, especially
in handle_upcalls().  Is there a way to avoid duplicating so much?
Some of it seems like it could be broken out very easily into helper
functions, e.g. the fail open logic.

Speaking of the fail open logic, I think that ofproto_flow_lookup()
gets it a little wrong.  The packet-ins are supposed to be sent in
addition to the flow table behavior, not instead of it.