From 49bdc010dfc9f396eec608148ca0f4ea71a0c4dd Mon Sep 17 00:00:00 2001 From: Justin Pettit Date: Thu, 12 Nov 2009 15:40:33 -0800 Subject: [PATCH] ofproto: Check overlap, emerg flow cache, and error code sync (OpenFlow 0.9) This commit adds (some) support for a couple new OpenFlow 0.9 features: - The OFPFF_CHECK_OVERLAP flag in Flow Mod messages allows the controller to prevent flows that would conflict at the same priority. - An emergency flow cache that contains a small flow table that is used if the switch loses connectivity with the controller. I believe the design has fundamental flaws and looks likely to be retired. If a controller attempts to add a flow to the emergency flow cache, OVS always responds that the tables are full. The OpenFlow 0.9 error codes are also sync'd in the commit. NOTE: OVS at this point is not wire-compatible with OpenFlow 0.9 until the final commit in this OpenFlow 0.9 set. --- include/openflow/openflow.h | 23 ++++++++++++--- lib/classifier.c | 56 +++++++++++++++++++++++++++++++++++++ lib/classifier.h | 2 ++ lib/ofp-print.c | 10 +++++-- lib/vconn.c | 10 +++---- ofproto/ofproto.c | 27 +++++++++++++++--- ofproto/pktbuf.c | 6 ++-- 7 files changed, 116 insertions(+), 18 deletions(-) diff --git a/include/openflow/openflow.h b/include/openflow/openflow.h index c45dbeb5..a9199c71 100644 --- a/include/openflow/openflow.h +++ b/include/openflow/openflow.h @@ -529,6 +529,13 @@ OFP_ASSERT(sizeof(struct ofp_match) == 40); /* By default, choose a priority in the middle. */ #define OFP_DEFAULT_PRIORITY 0x8000 +enum ofp_flow_mod_flags { + OFPFF_SEND_FLOW_REM = 1 << 0, /* Send flow removed message when flow + * expires or is deleted. */ + OFPFF_CHECK_OVERLAP = 1 << 1, /* Check for overlapping entries first. */ + OFPFF_EMERG = 1 << 2 /* Ramark this is for emergency. */ +}; + /* Flow setup and teardown (controller -> datapath). */ struct ofp_flow_mod { struct ofp_header header; @@ -545,7 +552,7 @@ struct ofp_flow_mod { matching entries to include this as an output port. A value of OFPP_NONE indicates no restriction. */ - uint8_t pad[2]; /* Align to 32-bits. */ + uint16_t flags; /* One of OFPFF_*. */ uint32_t reserved; /* Reserved for future use. */ struct ofp_action_header actions[0]; /* The action length is inferred from the length field in the @@ -589,7 +596,8 @@ enum ofp_error_type { /* ofp_error_msg 'code' values for OFPET_HELLO_FAILED. 'data' contains an * ASCII text string that may give failure details. */ enum ofp_hello_failed_code { - OFPHFC_INCOMPATIBLE /* No compatible version. */ + OFPHFC_INCOMPATIBLE, /* No compatible version. */ + OFPHFC_EPERM /* Permissions error. */ }; /* ofp_error_msg 'code' values for OFPET_BAD_REQUEST. 'data' contains at least @@ -601,9 +609,10 @@ enum ofp_bad_request_code { OFPBRC_BAD_VENDOR, /* Vendor not supported (in ofp_vendor_header * or ofp_stats_request or ofp_stats_reply). */ OFPBRC_BAD_SUBTYPE, /* Vendor subtype not supported. */ - OFPBRC_BAD_LENGTH, /* Wrong request length for type. */ + OFPBRC_EPERM, /* Permissions error. */ + OFPBRC_BAD_LEN, /* Wrong request length for type. */ OFPBRC_BUFFER_EMPTY, /* Specified buffer has already been used. */ - OFPBRC_BAD_COOKIE /* Specified buffer does not exist. */ + OFPBRC_BUFFER_UNKNOWN /* Specified buffer does not exist. */ }; /* ofp_error_msg 'code' values for OFPET_BAD_ACTION. 'data' contains at least @@ -615,6 +624,7 @@ enum ofp_bad_action_code { OFPBAC_BAD_VENDOR_TYPE, /* Unknown action type for vendor id. */ OFPBAC_BAD_OUT_PORT, /* Problem validating output action. */ OFPBAC_BAD_ARGUMENT, /* Bad action argument. */ + OFPBAC_EPERM, /* Permissions error. */ OFPBAC_TOO_MANY /* Can't handle this many actions. */ }; @@ -622,6 +632,11 @@ enum ofp_bad_action_code { * at least the first 64 bytes of the failed request. */ enum ofp_flow_mod_failed_code { OFPFMFC_ALL_TABLES_FULL, /* Flow not added because of full tables. */ + OFPFMFC_OVERLAP, /* Attempted to add overlapping flow with + * CHECK_OVERLAP flag set. */ + OFPFMFC_EPERM, /* Permissions error. */ + OFPFMFC_BAD_EMERG_TIMEOUT, /* Flow not added because of non-zero idle/hard + * timeout. */ OFPFMFC_BAD_COMMAND /* Unknown command. */ }; diff --git a/lib/classifier.c b/lib/classifier.c index 6ddfa82d..02cb0230 100644 --- a/lib/classifier.c +++ b/lib/classifier.c @@ -48,6 +48,8 @@ static struct cls_rule *search_exact_table(const struct classifier *, size_t hash, const flow_t *); static bool rules_match_1wild(const struct cls_rule *fixed, const struct cls_rule *wild, int field_idx); +static bool rules_match_2wild(const struct cls_rule *wild1, + const struct cls_rule *wild2, int field_idx); /* Converts the flow in 'flow' into a cls_rule in 'rule', with the given * 'wildcards' and 'priority'.*/ @@ -335,6 +337,43 @@ classifier_find_rule_exactly(const struct classifier *cls, return NULL; } +/* Checks if the flow defined by 'target' with 'wildcards' at 'priority' + * overlaps with any other rule at the same priority in the classifier. + * Two rules are considered overlapping if a packet could match both. */ +bool +classifier_rule_overlaps(const struct classifier *cls, + const flow_t *target, uint32_t wildcards, + unsigned int priority) +{ + struct cls_rule target_rule; + const struct hmap *tbl; + + if (!wildcards) { + return search_exact_table(cls, flow_hash(target, 0), target) ? + true : false; + } + + cls_rule_from_flow(&target_rule, target, wildcards, priority); + + for (tbl = &cls->tables[0]; tbl < &cls->tables[CLS_N_FIELDS]; tbl++) { + struct cls_bucket *bucket; + + HMAP_FOR_EACH (bucket, struct cls_bucket, hmap_node, tbl) { + struct cls_rule *rule; + + LIST_FOR_EACH (rule, struct cls_rule, node.list, + &bucket->rules) { + if (rule->priority == priority + && rules_match_2wild(rule, &target_rule, 0)) { + return true; + } + } + } + } + + return false; +} + /* Ignores target->priority. * * 'callback' is allowed to delete the rule that is passed as its argument, but @@ -763,6 +802,23 @@ rules_match_1wild(const struct cls_rule *fixed, const struct cls_rule *wild, wild->wc.nw_dst_mask, field_idx); } +/* Returns true if 'wild1' and 'wild2' match, that is, if their fields + * are equal modulo wildcards in 'wild1' or 'wild2'. + * + * 'field_idx' is the index of the first field to be compared; fields before + * 'field_idx' are assumed to match. Always returns true if 'field_idx' is + * CLS_N_FIELDS. */ +static bool +rules_match_2wild(const struct cls_rule *wild1, const struct cls_rule *wild2, + int field_idx) +{ + return rules_match(wild1, wild2, + wild1->wc.wildcards | wild2->wc.wildcards, + wild1->wc.nw_src_mask & wild2->wc.nw_src_mask, + wild1->wc.nw_dst_mask & wild2->wc.nw_dst_mask, + field_idx); +} + /* Searches 'bucket' for a rule that matches 'target'. Returns the * highest-priority match, if one is found, or a null pointer if there is no * match. diff --git a/lib/classifier.h b/lib/classifier.h index 84a3461e..45cb9572 100644 --- a/lib/classifier.h +++ b/lib/classifier.h @@ -143,6 +143,8 @@ struct cls_rule *classifier_lookup_wild(const struct classifier *, const flow_t *); struct cls_rule *classifier_lookup_exact(const struct classifier *, const flow_t *); +bool classifier_rule_overlaps(const struct classifier *, const flow_t *, + uint32_t wildcards, unsigned int priority); typedef void cls_cb_func(struct cls_rule *, void *aux); diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 64116f77..ced57c4f 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -804,6 +804,7 @@ static const struct error_type error_types[] = { #define ERROR_CODE(TYPE, CODE) {TYPE, CODE, #CODE} ERROR_TYPE(OFPET_HELLO_FAILED), ERROR_CODE(OFPET_HELLO_FAILED, OFPHFC_INCOMPATIBLE), + ERROR_CODE(OFPET_HELLO_FAILED, OFPHFC_EPERM), ERROR_TYPE(OFPET_BAD_REQUEST), ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_VERSION), @@ -811,9 +812,10 @@ static const struct error_type error_types[] = { ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_STAT), ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR), ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_SUBTYPE), - ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH), + ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_EPERM), + ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN), ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BUFFER_EMPTY), - ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE), + ERROR_CODE(OFPET_BAD_REQUEST, OFPBRC_BUFFER_UNKNOWN), ERROR_TYPE(OFPET_BAD_ACTION), ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_TYPE), @@ -822,10 +824,14 @@ static const struct error_type error_types[] = { ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_VENDOR_TYPE), ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_OUT_PORT), ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_BAD_ARGUMENT), + ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_EPERM), ERROR_CODE(OFPET_BAD_ACTION, OFPBAC_TOO_MANY), ERROR_TYPE(OFPET_FLOW_MOD_FAILED), ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL), + ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_OVERLAP), + ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_EPERM), + ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_EMERG_TIMEOUT), ERROR_CODE(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_COMMAND), ERROR_TYPE(OFPET_PORT_MOD_FAILED), diff --git a/lib/vconn.c b/lib/vconn.c index e381c3d8..19a19786 100644 --- a/lib/vconn.c +++ b/lib/vconn.c @@ -1073,7 +1073,7 @@ check_ofp_message(const struct ofp_header *msg, uint8_t type, size_t size) "received %s message of length %zu (expected %zu)", type_name, got_size, size); free(type_name); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } return 0; @@ -1109,7 +1109,7 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type, "(expected at least %zu)", type_name, got_size, min_size); free(type_name); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } if ((got_size - min_size) % array_elt_size) { char *type_name = ofp_message_type_to_string(type); @@ -1120,7 +1120,7 @@ check_ofp_message_array(const struct ofp_header *msg, uint8_t type, type_name, got_size, min_size, got_size - min_size, array_elt_size, (got_size - min_size) % array_elt_size); free(type_name); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } if (n_array_elts) { *n_array_elts = (got_size - min_size) / array_elt_size; @@ -1150,13 +1150,13 @@ check_ofp_packet_out(const struct ofp_header *oh, struct ofpbuf *data, VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out claims %u bytes of actions " "but message has room for only %zu bytes", actions_len, extra); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } if (actions_len % sizeof(union ofp_action)) { VLOG_WARN_RL(&bad_ofmsg_rl, "packet-out claims %u bytes of actions, " "which is not a multiple of %zu", actions_len, sizeof(union ofp_action)); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } n_actions = actions_len / sizeof(union ofp_action); diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 447d7f09..6bdbc73f 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -2584,7 +2584,7 @@ handle_flow_stats_request(struct ofproto *p, struct ofconn *ofconn, struct cls_rule target; if (arg_size != sizeof *fsr) { - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } fsr = (struct ofp_flow_stats_request *) osr->body; @@ -2692,7 +2692,7 @@ handle_aggregate_stats_request(struct ofproto *p, struct ofconn *ofconn, struct ofpbuf *msg; if (arg_size != sizeof *asr) { - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } asr = (struct ofp_aggregate_stats_request *) osr->body; @@ -2797,6 +2797,17 @@ add_flow(struct ofproto *p, struct ofconn *ofconn, uint16_t in_port; int error; + if (ofm->flags & htons(OFPFF_CHECK_OVERLAP)) { + flow_t flow; + uint32_t wildcards; + + flow_from_match(&flow, &wildcards, &ofm->match); + if (classifier_rule_overlaps(&p->cls, &flow, wildcards, + ntohs(ofm->priority))) { + return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_OVERLAP); + } + } + rule = rule_create(p, NULL, (const union ofp_action *) ofm->actions, n_actions, ntohs(ofm->idle_timeout), ntohs(ofm->hard_timeout)); @@ -2932,6 +2943,14 @@ handle_flow_mod(struct ofproto *p, struct ofconn *ofconn, return error; } + /* We do not support the emergency flow cache. It will hopefully + * get dropped from OpenFlow in the near future. */ + if (ofm->flags & htons(OFPFF_EMERG)) { + /* There isn't a good fit for an error code, so just state that the + * flow table is full. */ + return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_ALL_TABLES_FULL); + } + normalize_match(&ofm->match); if (!ofm->match.wildcards) { ofm->priority = htons(UINT16_MAX); @@ -2971,13 +2990,13 @@ handle_vendor(struct ofproto *p, struct ofconn *ofconn, void *msg) struct nicira_header *nh; if (ntohs(ovh->header.length) < sizeof(struct ofp_vendor_header)) { - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } if (ovh->vendor != htonl(NX_VENDOR_ID)) { return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_VENDOR); } if (ntohs(ovh->header.length) < sizeof(struct nicira_header)) { - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LENGTH); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } nh = msg; diff --git a/ofproto/pktbuf.c b/ofproto/pktbuf.c index 3701aebd..495a1ee3 100644 --- a/ofproto/pktbuf.c +++ b/ofproto/pktbuf.c @@ -165,7 +165,7 @@ pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct ofpbuf **bufferp, if (!pb) { VLOG_WARN_RL(&rl, "attempt to send buffered packet via connection " "without buffers"); - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE); + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BUFFER_UNKNOWN); } p = &pb->packets[id & PKTBUF_MASK]; @@ -183,10 +183,10 @@ pktbuf_retrieve(struct pktbuf *pb, uint32_t id, struct ofpbuf **bufferp, error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BUFFER_EMPTY); } } else if (id >> PKTBUF_BITS != COOKIE_MAX) { - COVERAGE_INC(pktbuf_bad_cookie); + COVERAGE_INC(pktbuf_buffer_unknown); VLOG_WARN_RL(&rl, "cookie mismatch: %08"PRIx32" != %08"PRIx32, id, (id & PKTBUF_MASK) | (p->cookie << PKTBUF_BITS)); - error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_COOKIE); + error = ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BUFFER_UNKNOWN); } else { COVERAGE_INC(pktbuf_null_cookie); VLOG_INFO_RL(&rl, "Received null cookie %08"PRIx32" (this is normal " -- 2.30.2