[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