[ovs-dev] [recirc datapath V4 1/5] ofproto-dpif: Rule lookup starts from table zero for non-recirc datapath

Jarno Rajahalme jrajahalme at nicira.com
Fri Apr 18 19:59:49 UTC 2014


On Apr 18, 2014, at 2:50 AM, Andy Zhou <azhou at nicira.com> wrote:

> Currently, all packet lookup starts from internal table for possible
> matching of post recirculation rules. This is not necessary for
> datapath that does not support recirculation.
> 
> This patch adds the ability to steering rule lookup starting table
> based on whether datapath supports recirculation.
> 
> Signed-off-by: Andy Zhou <azhou at nicira.com> ---
> ofproto/ofproto-dpif.c | 83 +++++++++++++++++++++++++-------------------------
> 1 file changed, 41 insertions(+), 42 deletions(-)
> 
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index 3648dd7..1e51ff2 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3171,41 +3171,57 @@ rule_dpif_get_actions(const struct rule_dpif *rule)
>     return rule_get_actions(&rule->up);
> }
> 
> -static uint8_t
> -rule_dpif_lookup__ (struct ofproto_dpif *ofproto, const struct flow *flow,
> -                    struct flow_wildcards *wc, struct rule_dpif **rule)
> +/* Lookup 'flow' in table 0 of 'ofproto''s classifier.
> + * If 'wc' is non-null, sets the fields that were relevant as part of
> + * the lookup. Returns the table_id where a match or miss occurred.
> + *
> + * The return value will be zero unless there was a miss and
> + * OFPTC11_TABLE_MISS_CONTINUE is in effect for the sequence of tables
> + * where misses occur. */
> +uint8_t
> +rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
> +                 struct flow_wildcards *wc, struct rule_dpif **rule)
> {
>     enum rule_dpif_lookup_verdict verdict;
>     enum ofputil_port_config config = 0;
> -    uint8_t table_id = TBL_INTERNAL;
> +    uint8_t table_id;
> +
> +    if (ofproto_dpif_get_enable_recirc(ofproto)) {

Would this test be faster as “if (flow->recirc_id)”?

> +        /* Set metadata to the value of recirc_id to speed up internal
> +         * rule lookup.  */
> +        flow->metadata = htonll(flow->recirc_id);

Since we currently have only one kind of a match pattern in the internal table, this will actually make the lookup slightly slower.

> +        table_id = TBL_INTERNAL;
> +    } else {
> +        table_id = 0;
> +    }
> 
>     verdict = rule_dpif_lookup_from_table(ofproto, flow, wc, true,
>                                           &table_id, rule);
> 
>     switch (verdict) {
> -    case RULE_DPIF_LOOKUP_VERDICT_MATCH:
> -        return table_id;
> -    case RULE_DPIF_LOOKUP_VERDICT_CONTROLLER: {
> -        struct ofport_dpif *port;
> -
> -        port = get_ofp_port(ofproto, flow->in_port.ofp_port);
> -        if (!port) {
> -            VLOG_WARN_RL(&rl, "packet-in on unknown OpenFlow port %"PRIu16,
> -                         flow->in_port.ofp_port);
> +        case RULE_DPIF_LOOKUP_VERDICT_MATCH:
> +            return table_id;
> +        case RULE_DPIF_LOOKUP_VERDICT_CONTROLLER: {
> +            struct ofport_dpif *port;
> +
> +            port = get_ofp_port(ofproto, flow->in_port.ofp_port);
> +            if (!port) {
> +                VLOG_WARN_RL(&rl, "packet-in on unknown OpenFlow port %"PRIu16,
> +                             flow->in_port.ofp_port);
> +            }
> +            config = port ? port->up.pp.config : 0;
> +            break;
>         }
> -        config = port ? port->up.pp.config : 0;
> -        break;
> -    }
> -    case RULE_DPIF_LOOKUP_VERDICT_DROP:
> -        config = OFPUTIL_PC_NO_PACKET_IN;
> -        break;
> -    case RULE_DPIF_LOOKUP_VERDICT_DEFAULT:
> -        if (!connmgr_wants_packet_in_on_miss(ofproto->up.connmgr)) {
> +        case RULE_DPIF_LOOKUP_VERDICT_DROP:
>             config = OFPUTIL_PC_NO_PACKET_IN;
> -        }
> -        break;
> -    default:
> -        OVS_NOT_REACHED();
> +            break;
> +        case RULE_DPIF_LOOKUP_VERDICT_DEFAULT:
> +            if (!connmgr_wants_packet_in_on_miss(ofproto->up.connmgr)) {
> +                config = OFPUTIL_PC_NO_PACKET_IN;
> +            }
> +            break;
> +        default:
> +            OVS_NOT_REACHED();
>     }
> 

The previous indentation was according to the CodingStyle. It seems to me nothing changed here?

>     choose_miss_rule(config, ofproto->miss_rule,
> @@ -3213,23 +3229,6 @@ rule_dpif_lookup__ (struct ofproto_dpif *ofproto, const struct flow *flow,
>     return table_id;
> }
> 
> -/* Lookup 'flow' in table 0 of 'ofproto''s classifier.
> - * If 'wc' is non-null, sets the fields that were relevant as part of
> - * the lookup. Returns the table_id where a match or miss occurred.
> - *
> - * The return value will be zero unless there was a miss and
> - * OFPTC11_TABLE_MISS_CONTINUE is in effect for the sequence of tables
> - * where misses occur. */
> -uint8_t
> -rule_dpif_lookup(struct ofproto_dpif *ofproto, struct flow *flow,
> -                 struct flow_wildcards *wc, struct rule_dpif **rule)
> -{
> -    /* Set metadata to the value of recirc_id to speed up internal
> -     * rule lookup. */
> -    flow->metadata = htonll(flow->recirc_id);
> -    return rule_dpif_lookup__(ofproto, flow, wc, rule);
> -}
> -
> static struct rule_dpif *
> rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto, uint8_t table_id,
>                           const struct flow *flow, struct flow_wildcards *wc)
> -- 
> 1.9.1
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev


With the caveats above,

Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>




More information about the dev mailing list