From: Ben Pfaff Date: Thu, 19 May 2011 23:18:57 +0000 (-0700) Subject: ofproto: Properly initialize table_id when creating rules. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0ab6decf2ce8bae6290967b6f0a3252dc86c4c55;p=openvswitch ofproto: Properly initialize table_id when creating rules. Nothing was initializing table_id, and there was no "hook" to choose an appropriate table. This fixes both problems. Found by inspection. --- diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 63bc3799..fd3f2127 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -3926,6 +3926,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 ac646e50..4c370d3b 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, @@ -871,7 +872,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 @@ -1222,7 +1223,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, @@ -1231,6 +1233,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; @@ -1239,6 +1255,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; @@ -2209,7 +2226,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 db0edef1..7a41a10a 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: