[ovs-dev] [PATCH 2/2] ofproto-dpif: Store relevant fields for wildcarding in facet.

[Ben Pfaff]

On Tue, Jun 11, 2013 at 01:20:32AM -0700, Justin Pettit wrote:
> Dynamically determines the flow fields that were relevant in
> processing flows based on the OpenFlow flow table and switch
> configuration.  The immediate use for this functionality is to
> cache action translations for similar flows in facets.  This yields
> a roughly 80% improvement in flow set up rates for a complicated
> flow table.
> 
> More importantly, these wildcards will be used to determine what to
> wildcard for the forthcoming kernel wildcard (megaflow) patches
> that will allow wildcarding in the kernel, which will provide
> significant flow set up improvements.
> 
> The approach to tracking fields and caching action translations in
> facets was based on an impressive prototype by Ethan Jackson.
> 
> Co-authored-by: Ethan Jackson <ethan at nicira.com>
> Signed-off-by: Ethan Jackson <ethan at nicira.com>
> Signed-off-by: Justin Pettit <jpettit at nicira.com>

struct facet needs a comment update.

In handle_flow_miss_without_facet(), the declaration of 'op' can be
moved inward.

Is struct facet's new 'match' member good for anything?  The only use I
see, in facet_create(), could be replaced by a local variable in that
function.

facet_lookup_valid()'s comment needs an update.

The first loop in facet_revalidate() looks wrong to me.  I don't think
that that the subfacets would usually have flows that are bitwise
exactly equal (with memcmp()) to the facet's flow when wildcards are
involved:

    /* Check that child subfacets still correspond to this facet.  Tunnel
     * configuration changes could cause a subfacet's OpenFlow in_port to
     * change. */
    LIST_FOR_EACH (subfacet, list_node, &facet->subfacets) {
        struct ofproto_dpif *recv_ofproto;
        struct flow recv_flow;
        int error;

        error = ofproto_receive(ofproto->backer, NULL, subfacet->key,
                                subfacet->key_len, &recv_flow, NULL,
                                &recv_ofproto, NULL, NULL);
        if (error
            || recv_ofproto != ofproto
            || memcmp(&recv_flow, &facet->flow, sizeof recv_flow)) {
            facet_remove(facet);
            return false;
        }
    }