From fa8c8a7c975c91edae26a561365a3bf1b31e6d67 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 22 Jul 2010 11:28:42 -0700 Subject: [PATCH] ofproto: Add Nicira extension to OpenFlow multiple flow tables support. Some switching hardware has multiple flow tables with varying capabilities. For example, one flow table may be able to match only on L2 fields and another only on L3 fields, and the tables may support different sets of actions. Tables may also have overlapping capabilities. Until now, there has been no way for OpenFlow controllers to specify a particular table to put a flow into, which is necessary in the case of overlapping capabilities. OpenFlow 1.1 will standardize a set of features to handle this, but for use until then this commit adds a Nicira extension with simple support for explicitly manipulating specific tables. In short, this commit uses the upper 8 bits of the 'command' field in ofp_flow_mod to specify a table. The details are described in include/openflow/nicira-ext.h. I've confirmed that this doesn't break anything in my simple test cases when the extension is not used. I haven't yet tested the extension itself. --- include/openflow/nicira-ext.h | 54 +++++++++++++++++++++++- include/openflow/openflow.h | 3 +- lib/ofp-print.c | 8 +++- ofproto/ofproto.c | 77 ++++++++++++++++++++++++++++------- ofproto/wdp-provider.h | 57 +++++++++++++++++++------- ofproto/wdp-xflow.c | 17 ++++++-- ofproto/wdp.c | 10 ++++- ofproto/wdp.h | 7 +++- 8 files changed, 195 insertions(+), 38 deletions(-) diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index e6f34baab..620fecbeb 100644 --- a/include/openflow/nicira-ext.h +++ b/include/openflow/nicira-ext.h @@ -53,7 +53,12 @@ enum nicira_type { /* Controller role support. The request body is struct nx_role_request. * The reply echos the request. */ NXT_ROLE_REQUEST, - NXT_ROLE_REPLY + NXT_ROLE_REPLY, + + /* Use the upper 8 bits of the 'command' member in struct ofp_flow_mod to + * designate the table to which a flow is to be added? See the big comment + * on struct nxt_flow_mod_table_id for more information. */ + NXT_FLOW_MOD_TABLE_ID }; struct nicira_header { @@ -72,6 +77,53 @@ struct nxt_tun_id_cookie { }; OFP_ASSERT(sizeof(struct nxt_tun_id_cookie) == 24); +/* This command enables or disables an Open vSwitch extension that allows a + * controller to specify the OpenFlow table to which a flow should be added, + * instead of having the switch decide which table is most appropriate as + * required by OpenFlow 1.0. By default, the extension is disabled. + * + * When this feature is enabled, Open vSwitch treats struct ofp_flow_mod's + * 16-bit 'command' member as two separate fields. The upper 8 bits are used + * as the table ID, the lower 8 bits specify the command as usual. A table ID + * of 0xff is treated like a wildcarded table ID. + * + * The specific treatment of the table ID depends on the type of flow mod: + * + * - OFPFC_ADD: Given a specific table ID, the flow is always placed in that + * table. If an identical flow already exists in that table only, then it + * is replaced. If the flow cannot be placed in the specified table, + * either because the table is full or because the table cannot support + * flows of the given type, the switch replies with an + * OFPFMFC_ALL_TABLES_FULL error. (A controller can distinguish these + * cases by comparing the current and maximum number of entries reported + * in ofp_table_stats.) + * + * If the table ID is wildcarded, the switch picks an appropriate table + * itself. If an identical flow or flows already exist in some flow + * table, then one of them is replaced. The choice of table might depend + * on the flows that are already in the switch; for example, if one table + * fills up then the switch might fall back to another one. + * + * - OFPFC_MODIFY, OFPFC_DELETE: Given a specific table ID, only flows + * within that table are matched and modified or deleted. If the table ID + * is wildcarded, flows within any table may be matched and modified or + * deleted. + * + * - OFPFC_MODIFY_STRICT, OFPFC_DELETE_STRICT: Given a specific table ID, + * only a flow within that table may be matched and modified or deleted. + * If the table ID is wildcarded and exactly one flow within any table + * matches, then it is modified or deleted; if flows in more than one + * table match, then none is modified or deleted. + */ +struct nxt_flow_mod_table_id { + struct ofp_header header; + uint32_t vendor; /* NX_VENDOR_ID. */ + uint32_t subtype; /* NXT_FLOW_MOD_TABLE_ID. */ + uint8_t set; /* Nonzero to enable, zero to disable. */ + uint8_t pad[7]; +}; +OFP_ASSERT(sizeof(struct nxt_flow_mod_table_id) == 24); + /* Configures the "role" of the sending controller. The default role is: * * - Other (NX_ROLE_OTHER), which allows the controller access to all diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h index b31a12dc4..9a02669d4 100644 --- a/include/openflow/openflow.h +++ b/include/openflow/openflow.h @@ -567,7 +567,8 @@ struct ofp_flow_mod { uint64_t cookie; /* Opaque controller-issued identifier. */ /* Flow actions. */ - uint16_t command; /* One of OFPFC_*. */ + uint16_t command; /* One of OFPFC_* (NXT_FLOW_MOD_TABLE_ID + * affects interpretation of high 8 bits). */ uint16_t idle_timeout; /* Idle time before discarding (seconds). */ uint16_t hard_timeout; /* Max time before discarding (seconds). */ uint16_t priority; /* Priority level of flow entry. */ diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 804b5aa5c..912bfe687 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -732,9 +732,10 @@ ofp_print_flow_mod(struct ds *string, const void *oh, size_t len, int verbosity) { const struct ofp_flow_mod *ofm = oh; + unsigned int command = ntohs(ofm->command); ofp_print_match(string, &ofm->match, verbosity); - switch (ntohs(ofm->command)) { + switch (command & 0xff) { case OFPFC_ADD: ds_put_cstr(string, " ADD: "); break; @@ -751,7 +752,10 @@ ofp_print_flow_mod(struct ds *string, const void *oh, size_t len, ds_put_cstr(string, " DEL_STRICT: "); break; default: - ds_put_format(string, " cmd:%d ", ntohs(ofm->command)); + ds_put_format(string, " cmd:%u ", command); + } + if (command & 0xff00) { + ds_put_format(string, "table_id:%u ", command >> 8); } ds_put_format(string, "cookie:0x%"PRIx64" idle:%d hard:%d pri:%d " "buf:%#x flags:%"PRIx16" ", ntohll(ofm->cookie), diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 8ab2bec28..2fe73de5d 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -120,6 +120,7 @@ struct ofconn { struct list node; /* In struct ofproto's "all_conns" list. */ struct rconn *rconn; /* OpenFlow connection. */ enum ofconn_type type; /* Type. */ + bool flow_mod_table_id; /* NXT_FLOW_MOD_TABLE_ID enabled? */ /* OFPT_PACKET_IN related data. */ struct rconn_packet_counter *packet_in_counter; /* # queued on 'rconn'. */ @@ -1053,6 +1054,8 @@ ofproto_send_packet(struct ofproto *p, const flow_t *flow, return 0; } +/* Intended for used by ofproto clients and ofproto submodules to add flows to + * the flow table. */ void ofproto_add_flow(struct ofproto *p, const flow_t *flow, const union ofp_action *actions, size_t n_actions, @@ -1067,16 +1070,19 @@ ofproto_add_flow(struct ofproto *p, const flow_t *flow, put.n_actions = n_actions; put.idle_timeout = idle_timeout; put.hard_timeout = 0; + put.ofp_table_id = 0xff; if (!wdp_flow_put(p->wdp, &put, NULL, &rule)) { ofproto_rule_init(rule); } } +/* Intended for used by ofproto clients and ofproto submodules to delete flows + * that they earlier added to the flow table. */ void ofproto_delete_flow(struct ofproto *ofproto, const flow_t *flow) { - struct wdp_rule *rule = wdp_flow_get(ofproto->wdp, flow); + struct wdp_rule *rule = wdp_flow_get(ofproto->wdp, flow, UINT_MAX); if (rule) { delete_flow(ofproto, rule, OFPRR_DELETE); } @@ -1693,6 +1699,12 @@ table_id_to_include(uint8_t table_id) : 0); } +static unsigned int +flow_mod_table_id(const struct ofconn *ofconn, const struct ofp_flow_mod *ofm) +{ + return ofconn->flow_mod_table_id ? ntohs(ofm->command) >> 8 : 0xff; +} + static int handle_flow_stats_request(struct ofproto *p, struct ofconn *ofconn, const struct ofp_stats_request *osr, @@ -2003,6 +2015,7 @@ add_flow(struct ofproto *p, struct ofconn *ofconn, put.n_actions = n_actions; put.idle_timeout = ntohs(ofm->idle_timeout); put.hard_timeout = ntohs(ofm->hard_timeout); + put.ofp_table_id = flow_mod_table_id(ofconn, ofm); error = wdp_flow_put(p->wdp, &put, NULL, &rule); if (error) { /* XXX wdp_flow_put should return OpenFlow error code. */ @@ -2023,13 +2036,16 @@ add_flow(struct ofproto *p, struct ofconn *ofconn, } static struct wdp_rule * -find_flow_strict(struct ofproto *p, const struct ofp_flow_mod *ofm) +find_flow_strict(struct ofproto *p, const struct ofconn *ofconn, + const struct ofp_flow_mod *ofm) { + uint8_t table_id; flow_t flow; flow_from_match(&ofm->match, ntohs(ofm->priority), p->tun_id_from_cookie, ofm->cookie, &flow); - return wdp_flow_get(p->wdp, &flow); + table_id = flow_mod_table_id(ofconn, ofm); + return wdp_flow_get(p->wdp, &flow, table_id_to_include(table_id)); } static int @@ -2069,8 +2085,8 @@ static int modify_flow(struct ofproto *, const struct ofp_flow_mod *, size_t n_actions, struct wdp_rule *); static void modify_flows_cb(struct wdp_rule *, void *cbdata_); -/* Implements OFPFC_MODIFY. Returns 0 on success or an OpenFlow error code as - * encoded by ofp_mkerr() on failure. +/* Implements OFPFC_ADD and OFPFC_MODIFY. Returns 0 on success or an OpenFlow + * error code as encoded by ofp_mkerr() on failure. * * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id, * if any. */ @@ -2079,6 +2095,7 @@ modify_flows_loose(struct ofproto *p, struct ofconn *ofconn, const struct ofp_flow_mod *ofm, size_t n_actions) { struct modify_flows_cbdata cbdata; + uint8_t table_id; flow_t target; cbdata.ofproto = p; @@ -2089,7 +2106,8 @@ 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, UINT_MAX, + table_id = flow_mod_table_id(ofconn, ofm); + wdp_flow_for_each_match(p->wdp, &target, table_id_to_include(table_id), modify_flows_cb, &cbdata); if (cbdata.match) { /* This credits the packet to whichever flow happened to match last. @@ -2111,7 +2129,7 @@ static int modify_flow_strict(struct ofproto *p, struct ofconn *ofconn, struct ofp_flow_mod *ofm, size_t n_actions) { - struct wdp_rule *rule = find_flow_strict(p, ofm); + struct wdp_rule *rule = find_flow_strict(p, ofconn, ofm); if (rule && !rule_is_hidden(rule)) { modify_flow(p, ofm, n_actions, rule); return send_buffered_packet(p, ofconn, rule, ofm); @@ -2158,6 +2176,7 @@ modify_flow(struct ofproto *p, const struct ofp_flow_mod *ofm, put.actions = (const union ofp_action *) actions; put.n_actions = n_actions; put.idle_timeout = put.hard_timeout = 0; + put.ofp_table_id = rule->ofp_table_id; return wdp_flow_put(p->wdp, &put, NULL, NULL); } @@ -2174,9 +2193,11 @@ static void delete_flow_core(struct ofproto *, struct wdp_rule *, /* Implements OFPFC_DELETE. */ static void -delete_flows_loose(struct ofproto *p, const struct ofp_flow_mod *ofm) +delete_flows_loose(struct ofproto *p, const struct ofconn *ofconn, + const struct ofp_flow_mod *ofm) { struct delete_flows_cbdata cbdata; + uint8_t table_id; flow_t target; cbdata.ofproto = p; @@ -2184,16 +2205,18 @@ 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); + table_id = flow_mod_table_id(ofconn, ofm); - wdp_flow_for_each_match(p->wdp, &target, UINT_MAX, + wdp_flow_for_each_match(p->wdp, &target, table_id_to_include(table_id), delete_flows_cb, &cbdata); } /* Implements OFPFC_DELETE_STRICT. */ static void -delete_flow_strict(struct ofproto *p, struct ofp_flow_mod *ofm) +delete_flow_strict(struct ofproto *p, const struct ofconn *ofconn, + struct ofp_flow_mod *ofm) { - struct wdp_rule *rule = find_flow_strict(p, ofm); + struct wdp_rule *rule = find_flow_strict(p, ofconn, ofm); if (rule) { delete_flow_core(p, rule, ofm->out_port); } @@ -2266,7 +2289,15 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn, return error; } - switch (ntohs(ofm->command)) { + if (!ofconn->flow_mod_table_id && ofm->command & htons(0xff00)) { + static struct vlog_rate_limit table_id_rl = VLOG_RATE_LIMIT_INIT(1, 1); + VLOG_WARN_RL(&table_id_rl, "%s: flow_mod table_id feature must be " + "enabled with NXT_FLOW_MOD_TABLE_ID", + rconn_get_name(ofconn->rconn)); + return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_COMMAND); + } + + switch (ntohs(ofm->command) & 0xff) { case OFPFC_ADD: return modify_flows_loose(p, ofconn, ofm, n_actions); @@ -2277,11 +2308,11 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn, return modify_flow_strict(p, ofconn, ofm, n_actions); case OFPFC_DELETE: - delete_flows_loose(p, ofm); + delete_flows_loose(p, ofconn, ofm); return 0; case OFPFC_DELETE_STRICT: - delete_flow_strict(p, ofm); + delete_flow_strict(p, ofconn, ofm); return 0; default: @@ -2303,6 +2334,21 @@ handle_tun_id_from_cookie(struct ofproto *p, struct nxt_tun_id_cookie *msg) return 0; } +static int +handle_flow_mod_table_id(struct ofconn *ofconn, + struct nxt_flow_mod_table_id *msg) +{ + int error; + + error = check_ofp_message(&msg->header, OFPT_VENDOR, sizeof *msg); + if (error) { + return error; + } + + ofconn->flow_mod_table_id = !!msg->set; + return 0; +} + static int handle_role_request(struct ofproto *ofproto, struct ofconn *ofconn, struct nicira_header *msg) @@ -2387,6 +2433,9 @@ handle_vendor(struct ofproto *p, struct ofconn *ofconn, void *msg) case NXT_TUN_ID_FROM_COOKIE: return handle_tun_id_from_cookie(p, msg); + case NXT_FLOW_MOD_TABLE_ID: + return handle_flow_mod_table_id(ofconn, msg); + case NXT_ROLE_REQUEST: return handle_role_request(p, ofconn, msg); } diff --git a/ofproto/wdp-provider.h b/ofproto/wdp-provider.h index 414ce6d8b..80110be72 100644 --- a/ofproto/wdp-provider.h +++ b/ofproto/wdp-provider.h @@ -251,10 +251,14 @@ struct wdp_class { * callback. */ int (*port_poll_wait)(const struct wdp *wdp); - /* If 'wdp' contains a flow exactly equal to 'flow', returns that flow. - * Otherwise returns null. */ + /* If 'wdp' contains exactly one flow exactly equal to 'flow' in one of the + * tables in the bit-mask in 'include', returns that flow. Otherwise (if + * there is no match or more than one match), returns null. + * + * A flow in table 'table_id' is a candidate for matching if 'include & (1u + * << table_id)' is nonzero. */ struct wdp_rule *(*flow_get)(const struct wdp *wdp, - const flow_t *flow); + const flow_t *flow, unsigned int include); /* If 'wdp' contains one or more flows that match 'flow', returns the * highest-priority matching flow. If there is more than one @@ -303,22 +307,47 @@ struct wdp_class { * feature. */ bool (*flow_overlaps)(const struct wdp *wdp, const flow_t *flow); - /* Adds or modifies a flow in 'wdp' as specified in 'put': + /* Adds or modifies a flow in 'wdp' as specified in 'put'. + * + * As the first step, this function determines the table of 'wdp' in which + * the flow should be inserted or modified: + * + * - If 'put->ofp_table_id' is 0xff, then 'wdp' chooses the flow table + * itself. It may do so based on, for example, the flow's wildcards + * and how full the various tables in 'wdp' already are. + * + * If a flow identical to 'put->flow' already exists in one of 'wdp''s + * tables, this does not necessarily mean that that table must be + * chosen. That is, 'wdp' may allow duplicate flows, as long as they + * are in different tables. Conversely, 'wdp' may refuse to add + * duplicate flows. (Whether to allow duplicate flows ordinarily + * depends on the structure of the hardware.) + * + * - If 'put->ofp_table_id' is not 0xff, then that table must be used. + * If 'wdp' cannot support 'put->flow' in the specified table, or if no + * such table exists, then the function must return an error. Reasons + * that 'wdp' might not support a flow in a given table include: the + * table is full, the flow has wildcards not supported by that table or + * otherwise is not in a form supported by the table, or (if 'wdp' does + * not support duplicate flows) putting the flow in that table would + * cause a duplicate flow. + * + * Afterward, it performs the action: * - * - If a rule with the same priority, wildcards, and values for fields - * that are not wildcarded specified in 'put->flow' does not already - * exist in 'wdp', then behavior depends on whether WDP_PUT_CREATE is - * specified in 'put->flags': if it is, the flow will be added, - * otherwise the operation will fail with ENOENT. + * - If a rule with the same priority, wildcards, values specified in + * 'put->flow' (for fields that are not wildcarded) does not already + * exist in 'wdp' in the selected table, then behavior depends on + * whether WDP_PUT_CREATE is specified in 'put->flags': if it is, the + * flow will be added, otherwise the operation will fail with ENOENT. * * The new flow's actions and timeouts are set from the values in * 'put'. * - * - Otherwise, the flow specified in 'put->flow' does exist in 'wdp'. - * Behavior in this case depends on whether WDP_PUT_MODIFY is specified - * in 'put->flags': if it is, the flow will be updated, otherwise the - * operation will fail with EEXIST. The exact updates depend on the - * remaining flags in 'put->flags': + * - Otherwise, the flow specified in 'put->flow' does exist in 'wdp' in + * the selected table. Behavior in this case depends on whether + * WDP_PUT_MODIFY is specified in 'put->flags': if it is, the flow will + * be updated, otherwise the operation will fail with EEXIST. The + * exact updates depend on the remaining flags in 'put->flags': * * . If WDP_PUT_COUNTERS is set, packet counters, byte counters, TCP * flags, and IP TOS values are set to 0. diff --git a/ofproto/wdp-xflow.c b/ofproto/wdp-xflow.c index 35d6ed215..42655f239 100644 --- a/ofproto/wdp-xflow.c +++ b/ofproto/wdp-xflow.c @@ -1544,10 +1544,16 @@ wx_port_poll_wait(const struct wdp *wdp) } static struct wdp_rule * -wx_flow_get(const struct wdp *wdp, const flow_t *flow) +wx_flow_get(const struct wdp *wdp, const flow_t *flow, unsigned int include) { struct wx *wx = wx_cast(wdp); struct wx_rule *rule; + int table_id; + + table_id = flow->wildcards ? TABLEID_CLASSIFIER : TABLEID_HASH; + if (!(include & (1u << table_id))) { + return NULL; + } rule = wx_rule_cast(classifier_find_rule_exactly(&wx->cls, flow)); return rule && !wx_rule_is_hidden(rule) ? &rule->wr : NULL; @@ -1699,6 +1705,12 @@ wx_flow_put(struct wdp *wdp, const struct wdp_flow_put *put, { struct wx *wx = wx_cast(wdp); struct wx_rule *rule; + uint8_t ofp_table_id; + + ofp_table_id = put->flow->wildcards ? TABLEID_CLASSIFIER : TABLEID_HASH; + if (put->ofp_table_id != 0xff && put->ofp_table_id != ofp_table_id) { + return EINVAL; + } rule = wx_rule_cast(classifier_find_rule_exactly(&wx->cls, put->flow)); if (rule && wx_rule_is_hidden(rule)) { @@ -1724,8 +1736,7 @@ 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); + rule->wr.ofp_table_id = ofp_table_id; wx_rule_insert(wx, rule, NULL, 0); if (old_stats) { diff --git a/ofproto/wdp.c b/ofproto/wdp.c index 93129182e..f48f42dbf 100644 --- a/ofproto/wdp.c +++ b/ofproto/wdp.c @@ -782,10 +782,16 @@ wdp_flow_flush(struct wdp *wdp) return error; } +/* If 'wdp' contains exactly one flow exactly equal to 'flow' in one of the + * tables in the bit-mask in 'include', returns that flow. Otherwise (if there + * is no match or more than one match), returns null. + * + * A flow in table 'table_id' is a candidate for matching if 'include & (1u << + * table_id)' is nonzero. */ struct wdp_rule * -wdp_flow_get(struct wdp *wdp, const flow_t *flow) +wdp_flow_get(struct wdp *wdp, const flow_t *flow, unsigned int include) { - return wdp->wdp_class->flow_get(wdp, flow); + return wdp->wdp_class->flow_get(wdp, flow, include); } struct wdp_rule * diff --git a/ofproto/wdp.h b/ofproto/wdp.h index 4272119e8..0342d7734 100644 --- a/ofproto/wdp.h +++ b/ofproto/wdp.h @@ -141,7 +141,8 @@ struct wdp_flow_stats { }; /* Finding and inspecting flows. */ -struct wdp_rule *wdp_flow_get(struct wdp *, const flow_t *); +struct wdp_rule *wdp_flow_get(struct wdp *, const flow_t *, + unsigned int include); struct wdp_rule *wdp_flow_match(struct wdp *, const flow_t *); typedef void wdp_flow_cb_func(struct wdp_rule *, void *aux); @@ -178,6 +179,10 @@ struct wdp_flow_put { unsigned short int idle_timeout; unsigned short int hard_timeout; + + /* OpenFlow 'table_id' to which a new flow is to be added. Value 0xff + * means that the WDP implementation should select a table. */ + uint8_t ofp_table_id; }; int wdp_flow_put(struct wdp *, struct wdp_flow_put *, -- 2.47.0