struct match in ofputil_flow_mod uses a lot of space, when used for a stored bundle message. This patch adds a new struct ofputil_miniflow_mod, that uses a minimatch instead of match in hopes of using less memory when handling large bundles.
Signed-off-by: Jarno Rajahalme <jrajaha...@nicira.com> --- lib/match.c | 9 ++ lib/match.h | 1 + lib/ofp-util.c | 14 ++++ lib/ofp-util.h | 100 +++++++++++++--------- ofproto/bundles.h | 7 +- ofproto/ofproto-provider.h | 2 +- ofproto/ofproto.c | 197 +++++++++++++++++++++++++++----------------- 7 files changed, 212 insertions(+), 118 deletions(-) diff --git a/lib/match.c b/lib/match.c index 7d0b409..a56a306 100644 --- a/lib/match.c +++ b/lib/match.c @@ -1193,6 +1193,15 @@ minimatch_init(struct minimatch *dst, const struct match *src) miniflow_init_with_minimask(&dst->flow, &src->flow, &dst->mask); } +/* Initializes 'match' as a "catch-all" match that matches every packet. */ +void +minimatch_init_catchall(struct minimatch *match) +{ + match->mask.masks.map = match->flow.map = 0; + match->mask.masks.values_inline = true; + match->flow.values_inline = true; +} + /* Initializes 'dst' as a copy of 'src'. The caller must eventually free 'dst' * with minimatch_destroy(). */ void diff --git a/lib/match.h b/lib/match.h index 6633304..822ab3f 100644 --- a/lib/match.h +++ b/lib/match.h @@ -170,6 +170,7 @@ struct minimatch { }; void minimatch_init(struct minimatch *, const struct match *); +void minimatch_init_catchall(struct minimatch *); void minimatch_clone(struct minimatch *, const struct minimatch *); void minimatch_move(struct minimatch *dst, struct minimatch *src); void minimatch_destroy(struct minimatch *); diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 89359c1..645fc4a 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -1839,6 +1839,20 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm, fm->table_id, max_table, protocol); } +void +ofputil_miniflow_mod_init(struct ofputil_miniflow_mod *mfm, + const struct ofputil_flow_mod *fm) +{ + memcpy(mfm, fm, offsetof(struct ofputil_miniflow_mod, match)); + minimatch_init(&mfm->match, &fm->match); +} + +void +ofputil_miniflow_mod_uninit(struct ofputil_miniflow_mod *mfm) +{ + minimatch_destroy(&mfm->match); +} + static enum ofperr ofputil_pull_bands(struct ofpbuf *msg, size_t len, uint16_t *n_bands, struct ofpbuf *bands) diff --git a/lib/ofp-util.h b/lib/ofp-util.h index 596c2e2..0f8930d 100644 --- a/lib/ofp-util.h +++ b/lib/ofp-util.h @@ -270,51 +270,59 @@ enum ofputil_flow_mod_flags { * * The handling of cookies across multiple versions of OpenFlow is a bit * confusing. See DESIGN for the details. */ -struct ofputil_flow_mod { - struct ovs_list list_node; /* For queuing flow_mods. */ - struct match match; +/* We need this same layout in two places. Ideally this would be a struct, but + * then all access to these would become uglier. This is one of the rare + * instances where I hope we allowed some elemeentary C++ in the code base. + * This define is somewhat ugly, but avoids repeating these in multiple places + * and the related maintenance problems. */ +#define OFPUTIL_FLOW_MOD_DATA \ + /* Cookie matching. The flow_mod affects only flows that have \ + * cookies that bitwise match 'cookie' bits in positions where \ + * 'cookie_mask' has 1-bits. \ + * 'cookie_mask' should be zero for OFPFC_ADD flow_mods. */ \ + ovs_be64 cookie; /* Cookie bits to match. */ \ + ovs_be64 cookie_mask; /* 1-bit in each 'cookie' bit to match. */ \ + \ + /* Cookie changes. \ + * \ + * OFPFC_ADD uses 'new_cookie' as the new flow's cookie. \ + * 'new_cookie' should not be UINT64_MAX. \ + * \ + * OFPFC_MODIFY and OFPFC_MODIFY_STRICT have two cases: \ + * \ + * - If one or more matching flows exist and 'modify_cookie' is \ + * true, then the flow_mod changes the existing flows' cookies \ + * to 'new_cookie'. 'new_cookie' should not be UINT64_MAX. \ + * \ + * - If no matching flow exists, 'new_cookie' is not UINT64_MAX, \ + * and 'cookie_mask' is 0, then the flow_mod adds a new flow \ + * with 'new_cookie' as its cookie. \ + */ \ + ovs_be64 new_cookie; /* New cookie to install or UINT64_MAX. */ \ + bool modify_cookie; /* Set cookie of existing flow to 'new_cookie'? */ \ + \ + uint8_t table_id; \ + uint16_t command; \ + uint16_t idle_timeout; \ + uint16_t hard_timeout; \ + uint32_t buffer_id; \ + ofp_port_t out_port; \ + uint32_t out_group; \ + enum ofputil_flow_mod_flags flags; \ + uint16_t importance; /* Eviction precedence. */ \ + struct ofpact *ofpacts; /* Series of "struct ofpact"s. */ \ + size_t ofpacts_len; /* Length of ofpacts, in bytes. */ \ + \ + /* Reason for delete; ignored for non-delete commands */ \ + enum ofp_flow_removed_reason delete_reason; \ int priority; - /* Cookie matching. The flow_mod affects only flows that have cookies that - * bitwise match 'cookie' bits in positions where 'cookie_mask has 1-bits. - * - * 'cookie_mask' should be zero for OFPFC_ADD flow_mods. */ - ovs_be64 cookie; /* Cookie bits to match. */ - ovs_be64 cookie_mask; /* 1-bit in each 'cookie' bit to match. */ - - /* Cookie changes. - * - * OFPFC_ADD uses 'new_cookie' as the new flow's cookie. 'new_cookie' - * should not be UINT64_MAX. - * - * OFPFC_MODIFY and OFPFC_MODIFY_STRICT have two cases: - * - * - If one or more matching flows exist and 'modify_cookie' is true, - * then the flow_mod changes the existing flows' cookies to - * 'new_cookie'. 'new_cookie' should not be UINT64_MAX. - * - * - If no matching flow exists, 'new_cookie' is not UINT64_MAX, and - * 'cookie_mask' is 0, then the flow_mod adds a new flow with - * 'new_cookie' as its cookie. - */ - ovs_be64 new_cookie; /* New cookie to install or UINT64_MAX. */ - bool modify_cookie; /* Set cookie of existing flow to 'new_cookie'? */ - - uint8_t table_id; - uint16_t command; - uint16_t idle_timeout; - uint16_t hard_timeout; - uint32_t buffer_id; - ofp_port_t out_port; - uint32_t out_group; - enum ofputil_flow_mod_flags flags; - uint16_t importance; /* Eviction precedence. */ - struct ofpact *ofpacts; /* Series of "struct ofpact"s. */ - size_t ofpacts_len; /* Length of ofpacts, in bytes. */ +struct ofputil_flow_mod { + OFPUTIL_FLOW_MOD_DATA /* Keep first to get consistent layout. */ - /* Reason for delete; ignored for non-delete commands */ - enum ofp_flow_removed_reason delete_reason; + struct match match; /* First field after OFPUTIL_FLOW_MOD_DATA! */ + struct ovs_list list_node; /* For queuing flow_mods. */ }; enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod *, @@ -326,6 +334,16 @@ enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod *, struct ofpbuf *ofputil_encode_flow_mod(const struct ofputil_flow_mod *, enum ofputil_protocol); +struct ofputil_miniflow_mod { + OFPUTIL_FLOW_MOD_DATA /* Keep first to get consistent layout. */ + + struct minimatch match; /* First field after OFPUTIL_FLOW_MOD_DATA! */ +}; + +void ofputil_miniflow_mod_init(struct ofputil_miniflow_mod *, + const struct ofputil_flow_mod *); +void ofputil_miniflow_mod_uninit(struct ofputil_miniflow_mod *); + /* Flow stats or aggregate stats request, independent of protocol. */ struct ofputil_flow_stats_request { bool aggregate; /* Aggregate results? */ diff --git a/ofproto/bundles.h b/ofproto/bundles.h index 0c7daf2..98cb2ba 100644 --- a/ofproto/bundles.h +++ b/ofproto/bundles.h @@ -35,7 +35,7 @@ struct ofp_bundle_entry { struct ovs_list node; enum ofptype type; /* OFPTYPE_FLOW_MOD or OFPTYPE_PORT_MOD. */ union { - struct ofputil_flow_mod fm; /* 'fm.ofpacts' must be malloced. */ + struct ofputil_miniflow_mod fm; /* 'fm.ofpacts' must be malloced. */ struct ofputil_port_mod pm; }; @@ -80,7 +80,9 @@ ofp_bundle_entry_alloc(enum ofptype type, const struct ofp_header *oh) struct ofp_bundle_entry *entry = xmalloc(sizeof *entry); entry->type = type; - + if (type == OFPTYPE_FLOW_MOD) { + minimatch_init_catchall(&entry->fm.match); + } /* Max 64 bytes for error reporting. */ memcpy(entry->ofp_msg, oh, MIN(ntohs(oh->length), sizeof entry->ofp_msg)); @@ -93,6 +95,7 @@ ofp_bundle_entry_free(struct ofp_bundle_entry *entry) if (entry) { if (entry->type == OFPTYPE_FLOW_MOD) { free(entry->fm.ofpacts); + ofputil_miniflow_mod_uninit(&entry->fm); } free(entry); } diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h index 57c6d7b..335c843 100644 --- a/ofproto/ofproto-provider.h +++ b/ofproto/ofproto-provider.h @@ -1083,7 +1083,7 @@ struct ofproto_class { * * If this function is NULL then table 0 is always chosen. */ enum ofperr (*rule_choose_table)(const struct ofproto *ofproto, - const struct match *match, + const struct minimatch *match, uint8_t *table_idp); /* Life-cycle functions for a "struct rule". diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 644b201..ca7be4f 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -153,6 +153,14 @@ static void rule_criteria_init(struct rule_criteria *, uint8_t table_id, long long version, ovs_be64 cookie, ovs_be64 cookie_mask, ofp_port_t out_port, uint32_t out_group); +static void rule_criteria_init_from_minimatch(struct rule_criteria *, + uint8_t table_id, + const struct minimatch *, + int priority, long long version, + ovs_be64 cookie, + ovs_be64 cookie_mask, + ofp_port_t out_port, + uint32_t out_group); static void rule_criteria_require_rw(struct rule_criteria *, bool can_write_readonly); static void rule_criteria_destroy(struct rule_criteria *); @@ -253,7 +261,7 @@ struct flow_mod_requester { /* OpenFlow. */ static enum ofperr replace_rule_create(struct ofproto *, - struct ofputil_flow_mod *, + struct ofputil_miniflow_mod *, struct cls_rule *cr, uint8_t table_id, struct rule *old_rule, struct rule **new_rule) @@ -269,7 +277,8 @@ static void replace_rule_revert(struct ofproto *, struct rule *old_rule, struct rule *new_rule) OVS_REQUIRES(ofproto_mutex); -static void replace_rule_finish(struct ofproto *, struct ofputil_flow_mod *, +static void replace_rule_finish(struct ofproto *, + struct ofputil_miniflow_mod *, const struct flow_mod_requester *, struct rule *old_rule, struct rule *new_rule, struct ovs_list *dead_cookies) @@ -292,11 +301,11 @@ static bool ofproto_group_exists(const struct ofproto *ofproto, static enum ofperr add_group(struct ofproto *, struct ofputil_group_mod *); static void handle_openflow(struct ofconn *, const struct ofpbuf *); static enum ofperr do_bundle_flow_mod_start(struct ofproto *, - struct ofputil_flow_mod *, + struct ofputil_miniflow_mod *, struct ofp_bundle_entry *) OVS_REQUIRES(ofproto_mutex); static void do_bundle_flow_mod_finish(struct ofproto *, - struct ofputil_flow_mod *, + struct ofputil_miniflow_mod *, const struct flow_mod_requester *, struct ofp_bundle_entry *) OVS_REQUIRES(ofproto_mutex); @@ -3734,13 +3743,11 @@ next_matching_table(const struct ofproto *ofproto, * For "loose" matching, the 'priority' parameter is unimportant and may be * supplied as 0. */ static void -rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id, - const struct match *match, int priority, long long version, - ovs_be64 cookie, ovs_be64 cookie_mask, - ofp_port_t out_port, uint32_t out_group) +rule_criteria_init__(struct rule_criteria *criteria, uint8_t table_id, + int priority, ovs_be64 cookie, ovs_be64 cookie_mask, + ofp_port_t out_port, uint32_t out_group) { criteria->table_id = table_id; - cls_rule_init(&criteria->cr, match, priority, version); criteria->cookie = cookie; criteria->cookie_mask = cookie_mask; criteria->out_port = out_port; @@ -3759,6 +3766,30 @@ rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id, criteria->include_readonly = true; } +static void +rule_criteria_init(struct rule_criteria *criteria, uint8_t table_id, + const struct match *match, int priority, long long version, + ovs_be64 cookie, ovs_be64 cookie_mask, + ofp_port_t out_port, uint32_t out_group) +{ + cls_rule_init(&criteria->cr, match, priority, version); + rule_criteria_init__(criteria, table_id, priority, cookie, cookie_mask, + out_port, out_group); +} + +static void +rule_criteria_init_from_minimatch(struct rule_criteria *criteria, + uint8_t table_id, + const struct minimatch *match, int priority, + long long version, ovs_be64 cookie, + ovs_be64 cookie_mask, ofp_port_t out_port, + uint32_t out_group) +{ + cls_rule_init_from_minimatch(&criteria->cr, match, priority, version); + rule_criteria_init__(criteria, table_id, priority, cookie, cookie_mask, + out_port, out_group); +} + /* By default, criteria initialized by rule_criteria_init() will match flows * that are read-only, on the assumption that the collected flows won't be * modified. Call this function to match only flows that are be modifiable. @@ -4401,7 +4432,7 @@ is_conjunction(const struct ofpact *ofpacts, size_t ofpacts_len) } static void -get_conjunctions(const struct ofputil_flow_mod *fm, +get_conjunctions(const struct ofputil_miniflow_mod *fm, struct cls_conjunction **conjsp, size_t *n_conjsp) OVS_REQUIRES(ofproto_mutex) { @@ -4444,7 +4475,7 @@ get_conjunctions(const struct ofputil_flow_mod *fm, * * The caller retains ownership of 'fm->ofpacts'. */ static enum ofperr -add_flow_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +add_flow_start(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, struct rule **old_rule, struct rule **new_rule) OVS_REQUIRES(ofproto_mutex) { @@ -4486,14 +4517,8 @@ add_flow_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm, return OFPERR_OFPBRC_EPERM; } - if (!(fm->flags & OFPUTIL_FF_HIDDEN_FIELDS) - && !match_has_default_hidden_fields(&fm->match)) { - VLOG_WARN_RL(&rl, "%s: (add_flow) only internal flows can set " - "non-default values to hidden fields", ofproto->name); - return OFPERR_OFPBRC_EPERM; - } - - cls_rule_init(&cr, &fm->match, fm->priority, ofproto->tables_version + 1); + cls_rule_init_from_minimatch(&cr, &fm->match, fm->priority, + ofproto->tables_version + 1); /* Check for the existence of an identical rule. * This will not return rules earlier marked for removal. */ @@ -4540,7 +4565,7 @@ add_flow_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm, /* Revert the effects of add_flow_start(). */ static void -add_flow_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +add_flow_revert(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, struct rule *old_rule, struct rule *new_rule) OVS_REQUIRES(ofproto_mutex) { @@ -4554,7 +4579,7 @@ add_flow_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm, /* To be called after version bump. */ static void -add_flow_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +add_flow_finish(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, const struct flow_mod_requester *req, struct rule *old_rule, struct rule *new_rule) OVS_REQUIRES(ofproto_mutex) @@ -4594,7 +4619,7 @@ add_flow_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, * and 'old_rule'. Note that the rule is NOT inserted into a any data * structures yet. Takes ownership of 'cr'. */ static enum ofperr -replace_rule_create(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +replace_rule_create(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, struct cls_rule *cr, uint8_t table_id, struct rule *old_rule, struct rule **new_rule) { @@ -4719,7 +4744,7 @@ static void replace_rule_revert(struct ofproto *ofproto, /* Adds the 'new_rule', replacing the 'old_rule'. */ static void -replace_rule_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +replace_rule_finish(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, const struct flow_mod_requester *req, struct rule *old_rule, struct rule *new_rule, struct ovs_list *dead_cookies) @@ -4781,7 +4806,7 @@ replace_rule_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, } static enum ofperr -modify_flows_start__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +modify_flows_start__(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, struct rule_collection *old_rules, struct rule_collection *new_rules) OVS_REQUIRES(ofproto_mutex) @@ -4844,7 +4869,8 @@ modify_flows_start__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, * 'ofconn' is used to retrieve the packet buffer specified in fm->buffer_id, * if any. */ static enum ofperr -modify_flows_start_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +modify_flows_start_loose(struct ofproto *ofproto, + struct ofputil_miniflow_mod *fm, struct rule_collection *old_rules, struct rule_collection *new_rules) OVS_REQUIRES(ofproto_mutex) @@ -4852,9 +4878,9 @@ modify_flows_start_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm, struct rule_criteria criteria; enum ofperr error; - rule_criteria_init(&criteria, fm->table_id, &fm->match, 0, - CLS_MAX_VERSION, fm->cookie, fm->cookie_mask, - OFPP_ANY, OFPG11_ANY); + rule_criteria_init_from_minimatch(&criteria, fm->table_id, &fm->match, 0, + CLS_MAX_VERSION, fm->cookie, + fm->cookie_mask, OFPP_ANY, OFPG11_ANY); rule_criteria_require_rw(&criteria, (fm->flags & OFPUTIL_FF_NO_READONLY) != 0); error = collect_rules_loose(ofproto, &criteria, old_rules); @@ -4871,7 +4897,7 @@ modify_flows_start_loose(struct ofproto *ofproto, struct ofputil_flow_mod *fm, } static void -modify_flows_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +modify_flows_revert(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, struct rule_collection *old_rules, struct rule_collection *new_rules) OVS_REQUIRES(ofproto_mutex) @@ -4890,7 +4916,7 @@ modify_flows_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm, } static void -modify_flows_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +modify_flows_finish(struct ofproto *ofproto, struct ofputil_miniflow_mod *fm, const struct flow_mod_requester *req, struct rule_collection *old_rules, struct rule_collection *new_rules) @@ -4919,7 +4945,8 @@ modify_flows_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, /* Implements OFPFC_MODIFY_STRICT. Returns 0 on success or an OpenFlow error * code on failure. */ static enum ofperr -modify_flow_start_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +modify_flow_start_strict(struct ofproto *ofproto, + struct ofputil_miniflow_mod *fm, struct rule_collection *old_rules, struct rule_collection *new_rules) OVS_REQUIRES(ofproto_mutex) @@ -4927,9 +4954,9 @@ modify_flow_start_strict(struct ofproto *ofproto, struct ofputil_flow_mod *fm, struct rule_criteria criteria; enum ofperr error; - rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority, - CLS_MAX_VERSION, fm->cookie, fm->cookie_mask, - OFPP_ANY, OFPG11_ANY); + rule_criteria_init_from_minimatch(&criteria, fm->table_id, &fm->match, + fm->priority, CLS_MAX_VERSION,fm->cookie, + fm->cookie_mask, OFPP_ANY, OFPG11_ANY); rule_criteria_require_rw(&criteria, (fm->flags & OFPUTIL_FF_NO_READONLY) != 0); error = collect_rules_strict(ofproto, &criteria, old_rules); @@ -5016,16 +5043,17 @@ delete_flows__(struct rule_collection *rules, /* Implements OFPFC_DELETE. */ static enum ofperr delete_flows_start_loose(struct ofproto *ofproto, - const struct ofputil_flow_mod *fm, + const struct ofputil_miniflow_mod *fm, struct rule_collection *rules) OVS_REQUIRES(ofproto_mutex) { struct rule_criteria criteria; enum ofperr error; - rule_criteria_init(&criteria, fm->table_id, &fm->match, 0, CLS_MAX_VERSION, - fm->cookie, fm->cookie_mask, - fm->out_port, fm->out_group); + rule_criteria_init_from_minimatch(&criteria, fm->table_id, &fm->match, 0, + CLS_MAX_VERSION, fm->cookie, + fm->cookie_mask, fm->out_port, + fm->out_group); rule_criteria_require_rw(&criteria, (fm->flags & OFPUTIL_FF_NO_READONLY) != 0); error = collect_rules_loose(ofproto, &criteria, rules); @@ -5058,7 +5086,7 @@ delete_flows_revert(struct ofproto *ofproto, static void delete_flows_finish(struct ofproto *ofproto, - const struct ofputil_flow_mod *fm, + const struct ofputil_miniflow_mod *fm, const struct flow_mod_requester *req, struct rule_collection *rules) OVS_REQUIRES(ofproto_mutex) @@ -5069,16 +5097,17 @@ delete_flows_finish(struct ofproto *ofproto, /* Implements OFPFC_DELETE_STRICT. */ static enum ofperr delete_flow_start_strict(struct ofproto *ofproto, - const struct ofputil_flow_mod *fm, + const struct ofputil_miniflow_mod *fm, struct rule_collection *rules) OVS_REQUIRES(ofproto_mutex) { struct rule_criteria criteria; enum ofperr error; - rule_criteria_init(&criteria, fm->table_id, &fm->match, fm->priority, - CLS_MAX_VERSION, fm->cookie, fm->cookie_mask, - fm->out_port, fm->out_group); + rule_criteria_init_from_minimatch(&criteria, fm->table_id, &fm->match, + fm->priority, CLS_MAX_VERSION, + fm->cookie, fm->cookie_mask, + fm->out_port, fm->out_group); rule_criteria_require_rw(&criteria, (fm->flags & OFPUTIL_FF_NO_READONLY) != 0); error = collect_rules_strict(ofproto, &criteria, rules); @@ -5173,6 +5202,32 @@ ofproto_rule_reduce_timeouts(struct rule *rule, } static enum ofperr +ofproto_decode_flow_mod(struct ofputil_flow_mod *fm, + const struct ofp_header *oh, + struct ofconn *ofconn, struct ofpbuf *ofpacts) +{ + struct ofproto *ofproto = ofconn_get_ofproto(ofconn); + enum ofperr error; + + error = ofputil_decode_flow_mod(fm, oh, ofconn_get_protocol(ofconn), + ofpacts, u16_to_ofp(ofproto->max_ports), + ofproto->n_tables); + if (!error) { + if (!(fm->flags & OFPUTIL_FF_HIDDEN_FIELDS) + && !match_has_default_hidden_fields(&fm->match)) { + VLOG_WARN_RL(&rl, "%s: (add_flow) only internal flows can set " + "non-default values to hidden fields", ofproto->name); + return OFPERR_OFPBRC_EPERM; + } + } + if (!error && fm->ofpacts_len) { + error = ofproto_check_ofpacts(ofproto, fm->ofpacts, fm->ofpacts_len); + } + + return error; +} + +static enum ofperr handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh) OVS_EXCLUDED(ofproto_mutex) { @@ -5188,13 +5243,7 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh) } ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); - error = ofputil_decode_flow_mod(&fm, oh, ofconn_get_protocol(ofconn), - &ofpacts, - u16_to_ofp(ofproto->max_ports), - ofproto->n_tables); - if (!error) { - error = ofproto_check_ofpacts(ofproto, fm.ofpacts, fm.ofpacts_len); - } + error = ofproto_decode_flow_mod(&fm, oh, ofconn, &ofpacts); if (!error) { struct flow_mod_requester req; @@ -5222,15 +5271,19 @@ handle_flow_mod__(struct ofproto *ofproto, struct ofputil_flow_mod *fm, struct ofp_bundle_entry be; enum ofperr error; + ofputil_miniflow_mod_init(&be.fm, fm); + ovs_mutex_lock(&ofproto_mutex); - error = do_bundle_flow_mod_start(ofproto, fm, &be); + error = do_bundle_flow_mod_start(ofproto, &be.fm, &be); if (!error) { ofproto_bump_tables_version(ofproto); - do_bundle_flow_mod_finish(ofproto, fm, req, &be); + do_bundle_flow_mod_finish(ofproto, &be.fm, req, &be); } ofmonitor_flush(ofproto->connmgr); ovs_mutex_unlock(&ofproto_mutex); + ofputil_miniflow_mod_uninit(&be.fm); + run_rule_executes(ofproto); return error; } @@ -6589,7 +6642,8 @@ handle_table_mod(struct ofconn *ofconn, const struct ofp_header *oh) } static enum ofperr -do_bundle_flow_mod_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +do_bundle_flow_mod_start(struct ofproto *ofproto, + struct ofputil_miniflow_mod *fm, struct ofp_bundle_entry *be) OVS_REQUIRES(ofproto_mutex) { @@ -6614,7 +6668,8 @@ do_bundle_flow_mod_start(struct ofproto *ofproto, struct ofputil_flow_mod *fm, } static void -do_bundle_flow_mod_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +do_bundle_flow_mod_revert(struct ofproto *ofproto, + struct ofputil_miniflow_mod *fm, struct ofp_bundle_entry *be) OVS_REQUIRES(ofproto_mutex) { @@ -6640,7 +6695,8 @@ do_bundle_flow_mod_revert(struct ofproto *ofproto, struct ofputil_flow_mod *fm, } static void -do_bundle_flow_mod_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm, +do_bundle_flow_mod_finish(struct ofproto *ofproto, + struct ofputil_miniflow_mod *fm, const struct flow_mod_requester *req, struct ofp_bundle_entry *be) OVS_REQUIRES(ofproto_mutex) @@ -6815,11 +6871,10 @@ handle_bundle_control(struct ofconn *ofconn, const struct ofp_header *oh) static enum ofperr handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh) { - struct ofproto *ofproto = ofconn_get_ofproto(ofconn); - enum ofperr error; struct ofputil_bundle_add_msg badd; - struct ofp_bundle_entry *bmsg; + struct ofp_bundle_entry *be; enum ofptype type; + enum ofperr error; error = reject_slave_controller(ofconn); if (error) { @@ -6831,38 +6886,32 @@ handle_bundle_add(struct ofconn *ofconn, const struct ofp_header *oh) return error; } - bmsg = ofp_bundle_entry_alloc(type, badd.msg); + be = ofp_bundle_entry_alloc(type, badd.msg); if (type == OFPTYPE_PORT_MOD) { - error = ofputil_decode_port_mod(badd.msg, &bmsg->pm, false); + error = ofputil_decode_port_mod(badd.msg, &be->pm, false); } else if (type == OFPTYPE_FLOW_MOD) { + struct ofputil_flow_mod fm; struct ofpbuf ofpacts; uint64_t ofpacts_stub[1024 / 8]; ofpbuf_use_stub(&ofpacts, ofpacts_stub, sizeof ofpacts_stub); - error = ofputil_decode_flow_mod(&bmsg->fm, badd.msg, - ofconn_get_protocol(ofconn), - &ofpacts, - u16_to_ofp(ofproto->max_ports), - ofproto->n_tables); - /* Move actions to heap. */ - bmsg->fm.ofpacts = ofpbuf_steal_data(&ofpacts); - - if (!error && bmsg->fm.ofpacts_len) { - error = ofproto_check_ofpacts(ofproto, bmsg->fm.ofpacts, - bmsg->fm.ofpacts_len); + error = ofproto_decode_flow_mod(&fm, badd.msg, ofconn, &ofpacts); + if (!error) { + ofputil_miniflow_mod_init(&be->fm, &fm); } + /* Move actions to heap. */ + be->fm.ofpacts = ofpbuf_steal_data(&ofpacts); } else { OVS_NOT_REACHED(); } if (!error) { - error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags, - bmsg); + error = ofp_bundle_add_message(ofconn, badd.bundle_id, badd.flags, be); } if (error) { - ofp_bundle_entry_free(bmsg); + ofp_bundle_entry_free(be); } return error; -- 1.7.10.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev