[ovs-dev] [PATCH v4 11/12] ofproto: Use minimatch for making bundles smaller.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Jun 10 00:24:18 UTC 2015
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 <jrajahalme at 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
More information about the dev
mailing list