From: Ben Pfaff Date: Mon, 12 Jul 2010 17:43:53 +0000 (-0700) Subject: ofproto: Fix abstraction of OpenFlow multiple table support. X-Git-Url: http://git.onelab.eu/?a=commitdiff_plain;h=8de77246db6f7a33036a1696dd0c47c9864777fb;p=sliver-openvswitch.git ofproto: Fix abstraction of OpenFlow multiple table support. The ofproto code until now has assumed to some extent that the switch has exactly two tables, one that handles exact-match flow and the other that handles all other flows. Commit 840ee2faf "wdp: Add new wdp_class member function 'get_table_stats'" did part of the work on getting rid of that assumption. This commit does the rest of the work. Now the only assumption left is that a switch has no more than 32 tables. This seems reasonable enough to me; OpenFlow allows for no more 255. --- diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index f7ce9dbc7..7150d9602 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -1669,7 +1669,7 @@ flow_stats_cb(struct wdp_rule *rule, void *cbdata_) ofs = append_stats_reply(len, cbdata->ofconn, &cbdata->msg); ofs->length = htons(len); - ofs->table_id = rule->cr.flow.wildcards ? TABLEID_CLASSIFIER : TABLEID_HASH; + ofs->table_id = rule->ofp_table_id; ofs->pad = 0; flow_to_match(&rule->cr.flow, cbdata->ofproto->tun_id_from_cookie, &ofs->match); @@ -1685,12 +1685,11 @@ flow_stats_cb(struct wdp_rule *rule, void *cbdata_) memcpy(ofs->actions, rule->actions, act_len); } -static int +static unsigned int table_id_to_include(uint8_t table_id) { - return (table_id == TABLEID_HASH ? CLS_INC_EXACT - : table_id == TABLEID_CLASSIFIER ? CLS_INC_WILD - : table_id == 0xff ? CLS_INC_ALL + return (table_id == 0xff ? UINT_MAX + : table_id < 32 ? 1u << table_id : 0); } @@ -1714,7 +1713,7 @@ handle_flow_stats_request(struct ofproto *p, struct ofconn *ofconn, cbdata.out_port = fsr->out_port; cbdata.msg = start_stats_reply(osr, 1024); flow_from_match(&fsr->match, 0, false, 0, &target); - wdp_flow_for_each_match(p->wdp, &target, + wdp_flow_for_each_match(p->wdp, &target, table_id_to_include(fsr->table_id), flow_stats_cb, &cbdata); queue_tx(cbdata.msg, ofconn, ofconn->reply_counter); @@ -1765,7 +1764,7 @@ ofproto_get_all_flows(struct ofproto *p, struct ds *results) cbdata.results = results; flow_from_match(&match, 0, false, 0, &target); - wdp_flow_for_each_match(p->wdp, &target, CLS_INC_ALL, + wdp_flow_for_each_match(p->wdp, &target, UINT_MAX, flow_stats_ds_cb, &cbdata); } @@ -2090,7 +2089,7 @@ modify_flows_loose(struct ofproto *p, struct ofconn *ofconn, flow_from_match(&ofm->match, 0, p->tun_id_from_cookie, ofm->cookie, &target); - wdp_flow_for_each_match(p->wdp, &target, CLS_INC_ALL, + wdp_flow_for_each_match(p->wdp, &target, UINT_MAX, modify_flows_cb, &cbdata); if (cbdata.match) { /* This credits the packet to whichever flow happened to happened to @@ -2186,7 +2185,7 @@ delete_flows_loose(struct ofproto *p, const struct ofp_flow_mod *ofm) flow_from_match(&ofm->match, 0, p->tun_id_from_cookie, ofm->cookie, &target); - wdp_flow_for_each_match(p->wdp, &target, CLS_INC_ALL, + wdp_flow_for_each_match(p->wdp, &target, UINT_MAX, delete_flows_cb, &cbdata); } diff --git a/ofproto/wdp-provider.h b/ofproto/wdp-provider.h index 6414a8094..414ce6d8b 100644 --- a/ofproto/wdp-provider.h +++ b/ofproto/wdp-provider.h @@ -269,9 +269,9 @@ struct wdp_class { * flow that matches the specified search criteria to 'callback' along with * 'aux'. * - * Flows are filtered out in two ways. First, based on 'include': - * Exact-match flows are excluded unless CLS_INC_EXACT is in 'include'. - * Wildcarded flows are excluded unless CLS_INC_WILD is in 'include'. + * Flows are filtered out in two ways. First, based on the bit-mask in + * 'include': wdp_rule 'wr' is included only if 'include & (1u << + * wr->ofp_table_id)' is nonzero. * * Flows are also filtered out based on 'target': on a field-by-field * basis, a flow is included if 'target' wildcards that field or if the @@ -285,7 +285,7 @@ struct wdp_class { * 'callback' must not delete flows from 'wdp' other than its argument * flow, nor may it insert new flows into 'wdp'. */ void (*flow_for_each_match)(const struct wdp *wdp, const flow_t *flow, - int include, + unsigned int include, wdp_flow_cb_func *callback, void *aux); /* Retrieves flow statistics for 'rule', which must be in 'wdp''s flow diff --git a/ofproto/wdp-xflow.c b/ofproto/wdp-xflow.c index 71f04556d..35d6ed215 100644 --- a/ofproto/wdp-xflow.c +++ b/ofproto/wdp-xflow.c @@ -51,6 +51,11 @@ #define THIS_MODULE VLM_wdp_xflow #include "vlog.h" +enum { + TABLEID_HASH = 0, + TABLEID_CLASSIFIER = 1 +}; + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); /* Maximum numbers of rules. */ @@ -355,8 +360,8 @@ wx_rule_has_out_port(const struct wx_rule *rule, uint16_t out_port) } #endif -/* Caller is responsible for initializing the 'cr' member of the returned - * rule. */ +/* Caller is responsible for initializing the 'cr' and ofp_table_id members of + * the returned rule. */ static struct wx_rule * wx_rule_create(struct wx_rule *super, const union ofp_action *actions, size_t n_actions, @@ -478,6 +483,8 @@ wx_rule_create_subrule(struct wx *wx, struct wx_rule *rule, const flow_t *flow) subrule = wx_rule_create(rule, NULL, 0, rule->wr.idle_timeout, rule->wr.hard_timeout); + /* Subrules aren't really in any OpenFlow table, so don't bother with + * subrule->wr.ofp_table_id. */ COVERAGE_INC(wx_subrule_create); cls_rule_from_flow(flow, &subrule->wr.cr); classifier_insert_exact(&wx->cls, &subrule->wr.cr); @@ -1581,15 +1588,24 @@ wx_for_each_thunk(struct cls_rule *cls_rule, void *aux_) static void wx_flow_for_each_match(const struct wdp *wdp, const flow_t *target, - int include, + unsigned int include, wdp_flow_cb_func *client_callback, void *client_aux) { struct wx *wx = wx_cast(wdp); struct wx_for_each_thunk_aux aux; + int cls_include; + + cls_include = 0; + if (include & (1u << TABLEID_HASH)) { + cls_include |= CLS_INC_EXACT; + } + if (include & (1u << TABLEID_CLASSIFIER)) { + cls_include |= CLS_INC_WILD; + } aux.client_callback = client_callback; aux.client_aux = client_aux; - classifier_for_each_match(&wx->cls, target, include, + classifier_for_each_match(&wx->cls, target, cls_include, wx_for_each_thunk, &aux); } @@ -1708,6 +1724,8 @@ wx_flow_put(struct wdp *wdp, const struct wdp_flow_put *put, rule = wx_rule_create(NULL, put->actions, put->n_actions, put->idle_timeout, put->hard_timeout); cls_rule_from_flow(put->flow, &rule->wr.cr); + rule->wr.ofp_table_id = (put->flow->wildcards + ? TABLEID_CLASSIFIER : TABLEID_HASH); wx_rule_insert(wx, rule, NULL, 0); if (old_stats) { diff --git a/ofproto/wdp.c b/ofproto/wdp.c index 72519eeb8..93129182e 100644 --- a/ofproto/wdp.c +++ b/ofproto/wdp.c @@ -48,7 +48,8 @@ /* Initializes a new 'struct wdp_rule', copying in the 'n_actions' elements of * 'actions'. * - * The caller is responsible for initializing 'rule->cr'. */ + * The caller is responsible for initializing 'rule->cr'. The caller must also + * fill in 'rule->ofp_table_id', if the wdp has more than one table. */ void wdp_rule_init(struct wdp_rule *rule, const union ofp_action *actions, size_t n_actions) @@ -58,6 +59,7 @@ wdp_rule_init(struct wdp_rule *rule, const union ofp_action *actions, rule->created = time_msec(); rule->idle_timeout = 0; rule->hard_timeout = 0; + rule->ofp_table_id = 0; rule->client_data = NULL; } @@ -792,12 +794,30 @@ wdp_flow_match(struct wdp *wdp, const flow_t *flow) return wdp->wdp_class->flow_match(wdp, flow); } +/* Iterates through all of the flows in 'wdp''s flow table, passing each flow + * that matches the specified search criteria to 'callback' along with 'aux'. + * + * Flows are filtered out in two ways. First, based on the bit-mask in + * 'include': wdp_rule 'wr' is included only if 'include & (1u << + * wr->ofp_table_id)' is nonzero. + * + * Flows are also filtered out based on 'target': on a field-by-field basis, a + * flow is included if 'target' wildcards that field or if the flow and + * 'target' both have the same exact value for the field. A flow is excluded + * if any field does not match based on these criteria. + * + * Ignores 'target->priority'. + * + * 'callback' is allowed to delete the rule that is passed as its argument. It + * may modify any flow in 'wdp', e.g. changing their actions. 'callback' must + * not delete flows from 'wdp' other than its argument flow, nor may it insert + * new flows into 'wdp'. */ void wdp_flow_for_each_match(const struct wdp *wdp, const flow_t *target, - int include, wdp_flow_cb_func *callback, void *aux) + unsigned int include, + wdp_flow_cb_func *callback, void *aux) { - wdp->wdp_class->flow_for_each_match(wdp, target, include, - callback, aux); + wdp->wdp_class->flow_for_each_match(wdp, target, include, callback, aux); } int diff --git a/ofproto/wdp.h b/ofproto/wdp.h index b20079a97..4272119e8 100644 --- a/ofproto/wdp.h +++ b/ofproto/wdp.h @@ -25,11 +25,6 @@ extern "C" { #endif -enum { - TABLEID_HASH = 0, - TABLEID_CLASSIFIER = 1 -}; - struct ofhooks; struct ofpbuf; struct svec; @@ -59,6 +54,8 @@ struct wdp_rule { long long int created; /* Time created, in ms since the epoch. */ uint16_t idle_timeout; /* In seconds from time of last use. */ uint16_t hard_timeout; /* In seconds from time of creation. */ + uint8_t ofp_table_id; /* OpenFlow table_id in e.g. ofp_flow_stats. + * Supported range is at most 0...31. */ /* OpenFlow actions. * @@ -149,7 +146,8 @@ struct wdp_rule *wdp_flow_match(struct wdp *, const flow_t *); typedef void wdp_flow_cb_func(struct wdp_rule *, void *aux); void wdp_flow_for_each_match(const struct wdp *, const flow_t *, - int include, wdp_flow_cb_func *, void *aux); + unsigned int include, wdp_flow_cb_func *, + void *aux); int wdp_flow_get_stats(const struct wdp *, const struct wdp_rule *, struct wdp_flow_stats *);