[ovs-dev] [PATCH] ofproto: Properly initialize table_id when creating rules.

Ethan Jackson ethan at nicira.com
Mon May 23 21:21:09 UTC 2011


Looks (GOOD).

Ethan

On Thu, May 19, 2011 at 16:21, Ben Pfaff <blp at nicira.com> wrote:
> Nothing was initializing table_id, and there was no "hook" to choose an
> appropriate table.  This fixes both problems.
>
> Found by inspection.
> ---
>  ofproto/ofproto-dpif.c |    1 +
>  ofproto/ofproto.c      |   25 +++++++++++++++++++++----
>  ofproto/private.h      |   19 +++++++++++++++++++
>  3 files changed, 41 insertions(+), 4 deletions(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index d3cb92b..bf2f6a4 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3933,6 +3933,7 @@ const struct ofproto_class ofproto_dpif_class = {
>     port_poll,
>     port_poll_wait,
>     port_is_lacp_current,
> +    NULL,                       /* rule_choose_table */
>     rule_alloc,
>     rule_construct,
>     rule_destruct,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index beab99c..26d3b87 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -64,7 +64,8 @@ COVERAGE_DEFINE(ofproto_update_port);
>  static void ofport_destroy__(struct ofport *);
>  static void ofport_destroy(struct ofport *);
>
> -static int rule_create(struct ofproto *, const struct cls_rule *,
> +static int rule_create(struct ofproto *,
> +                       const struct cls_rule *, uint8_t table_id,
>                        const union ofp_action *, size_t n_actions,
>                        uint16_t idle_timeout, uint16_t hard_timeout,
>                        ovs_be64 flow_cookie, bool send_flow_removed,
> @@ -886,7 +887,7 @@ ofproto_add_flow(struct ofproto *p, const struct cls_rule *cls_rule,
>                  const union ofp_action *actions, size_t n_actions)
>  {
>     struct rule *rule;
> -    rule_create(p, cls_rule, actions, n_actions, 0, 0, 0, false, &rule);
> +    rule_create(p, cls_rule, 0, actions, n_actions, 0, 0, 0, false, &rule);
>  }
>
>  /* Searches for a rule with matching criteria exactly equal to 'target' in
> @@ -1237,7 +1238,8 @@ init_ports(struct ofproto *p)
>  * flow table, and stores the new rule into '*rulep'.  Returns 0 on success,
>  * otherwise a positive errno value or OpenFlow error code. */
>  static int
> -rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
> +rule_create(struct ofproto *ofproto,
> +            const struct cls_rule *cls_rule, uint8_t table_id,
>             const union ofp_action *actions, size_t n_actions,
>             uint16_t idle_timeout, uint16_t hard_timeout,
>             ovs_be64 flow_cookie, bool send_flow_removed,
> @@ -1246,6 +1248,20 @@ rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
>     struct rule *rule;
>     int error;
>
> +    if (table_id == 0xff) {
> +        if (ofproto->n_tables > 1) {
> +            error = ofproto->ofproto_class->rule_choose_table(ofproto,
> +                                                              cls_rule,
> +                                                              &table_id);
> +            if (error) {
> +                return error;
> +            }
> +            assert(table_id < ofproto->n_tables);
> +        } else {
> +            table_id = 0;
> +        }
> +    }
> +
>     rule = ofproto->ofproto_class->rule_alloc();
>     if (!rule) {
>         error = ENOMEM;
> @@ -1254,6 +1270,7 @@ rule_create(struct ofproto *ofproto, const struct cls_rule *cls_rule,
>
>     rule->ofproto = ofproto;
>     rule->cr = *cls_rule;
> +    rule->table_id = table_id;
>     rule->flow_cookie = flow_cookie;
>     rule->created = time_msec();
>     rule->idle_timeout = idle_timeout;
> @@ -2212,7 +2229,7 @@ add_flow(struct ofconn *ofconn, struct flow_mod *fm)
>     }
>
>     buf_err = ofconn_pktbuf_retrieve(ofconn, fm->buffer_id, &packet, &in_port);
> -    error = rule_create(p, &fm->cr, fm->actions, fm->n_actions,
> +    error = rule_create(p, &fm->cr, fm->table_id, fm->actions, fm->n_actions,
>                         fm->idle_timeout, fm->hard_timeout, fm->cookie,
>                         fm->flags & OFPFF_SEND_FLOW_REM, &rule);
>     if (error) {
> diff --git a/ofproto/private.h b/ofproto/private.h
> index 16d30d5..5cbcef8 100644
> --- a/ofproto/private.h
> +++ b/ofproto/private.h
> @@ -506,6 +506,25 @@ struct ofproto_class {
>  /* ## OpenFlow Rule Functions ## */
>  /* ## ----------------------- ## */
>
> +    /* Chooses an appropriate table for 'cls_rule' within 'ofproto'.  On
> +     * success, stores the table ID into '*table_idp' and returns 0.  On
> +     * failure, returns an OpenFlow error code (as returned by ofp_mkerr()).
> +     *
> +     * The choice of table should be a function of 'cls_rule' and 'ofproto''s
> +     * datapath capabilities.  It should not depend on the flows already in
> +     * 'ofproto''s flow tables.  Failure implies that an OpenFlow rule with
> +     * 'cls_rule' as its matching condition can never be inserted into
> +     * 'ofproto', even starting from an empty flow table.
> +     *
> +     * If multiple tables are candidates for inserting the flow, the function
> +     * should choose one arbitrarily (but deterministically).
> +     *
> +     * This function will never be called for an ofproto that has only one
> +     * table, so it may be NULL in that case. */
> +    int (*rule_choose_table)(const struct ofproto *ofproto,
> +                             const struct cls_rule *cls_rule,
> +                             uint8_t *table_idp);
> +
>     /* Life-cycle functions for a "struct rule" (see "Life Cycle" above).
>      *
>      * ->rule_construct() should first check whether the rule is acceptable:
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list