[ovs-dev] [PATCH v2 12/19] ofproto: Split add_flow().
Jarno Rajahalme
jrajahalme at nicira.com
Mon May 18 23:10:21 UTC 2015
Split add_flow() to add_flow_begin() which does all the error
checking, and add_flow_finish() which can not fail.
Since we still want to send an error response for an unknown
'buffer_id', send_buffered_packet() now send the error response
itself.
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
ofproto/ofproto.c | 213 +++++++++++++++++++++++++++++++----------------------
1 file changed, 123 insertions(+), 90 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5c8b1a5..d3fce08 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -249,7 +249,7 @@ static void ofproto_rule_remove__(struct ofproto *, struct rule *)
* meaningful and thus supplied as NULL. */
struct flow_mod_requester {
struct ofconn *ofconn; /* Connection on which flow_mod arrived. */
- ovs_be32 xid; /* OpenFlow xid of flow_mod request. */
+ const struct ofp_header *request;
};
/* OpenFlow. */
@@ -269,8 +269,8 @@ static void delete_flows__(const struct rule_collection *,
const struct flow_mod_requester *)
OVS_REQUIRES(ofproto_mutex);
-static enum ofperr send_buffered_packet(struct ofconn *, uint32_t buffer_id,
- struct rule *)
+static void send_buffered_packet(const struct flow_mod_requester *,
+ uint32_t buffer_id, struct rule *)
OVS_REQUIRES(ofproto_mutex);
static bool ofproto_group_exists__(const struct ofproto *ofproto,
@@ -4336,21 +4336,9 @@ set_conjunctions(struct rule *rule, const struct cls_conjunction *conjs,
cls_rule_set_conjunctions(cr, conjs, n_conjs);
}
-/* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
- * in which no matching flow already exists in the flow table.
- *
- * Adds the flow specified by 'ofm', which is followed by 'n_actions'
- * ofp_actions, to the ofproto's flow table. Returns 0 on success, an OpenFlow
- * error code on failure, or OFPROTO_POSTPONE if the operation cannot be
- * initiated now but may be retried later.
- *
- * The caller retains ownership of 'fm->ofpacts'.
- *
- * 'ofconn' is used to retrieve the packet buffer specified in ofm->buffer_id,
- * if any. */
static enum ofperr
-add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
- const struct flow_mod_requester *req)
+add_flow_begin(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ struct rule **rulep, bool *modify)
OVS_REQUIRES(ofproto_mutex)
{
struct oftable *table;
@@ -4389,93 +4377,140 @@ add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
return OFPERR_OFPBRC_EPERM;
}
- if (!(fm->flags & OFPUTIL_FF_HIDDEN_FIELDS)) {
- if (!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 (!(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);
- /* Transform "add" into "modify" if there's an existing identical flow. */
+ /* Check for the existence of an identical rule. */
rule = rule_from_cls_rule(classifier_find_rule_exactly(&table->cls, &cr));
if (rule) {
+ /* Transform "add" into "modify" of an existing identical flow. */
cls_rule_destroy(&cr);
fm->modify_cookie = true;
error = modify_flow_check__(ofproto, fm, rule);
- if (!error) {
- struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
+ if (error) {
+ return error;
+ }
- modify_flow__(ofproto, fm, rule, req, &dead_cookies);
- learned_cookies_flush(ofproto, &dead_cookies);
+ *modify = true;
+ } else { /* New rule. */
+ struct cls_conjunction *conjs;
+ size_t n_conjs;
- goto send_packet;
+ /* Check for overlap, if requested. */
+ if (fm->flags & OFPUTIL_FF_CHECK_OVERLAP
+ && classifier_rule_overlaps(&table->cls, &cr)) {
+ cls_rule_destroy(&cr);
+ return OFPERR_OFPFMFC_OVERLAP;
}
- return error;
- }
- /* Check for overlap, if requested. */
- if (fm->flags & OFPUTIL_FF_CHECK_OVERLAP) {
- if (classifier_rule_overlaps(&table->cls, &cr)) {
+ /* If necessary, evict an existing rule to clear out space. */
+ error = evict_rules_from_table(table, 1);
+ if (error) {
cls_rule_destroy(&cr);
- return OFPERR_OFPFMFC_OVERLAP;
+ return error;
}
- }
- /* If necessary, evict an existing rule to clear out space. */
- error = evict_rules_from_table(table, 1);
- if (error) {
- cls_rule_destroy(&cr);
- return error;
- }
+ /* Allocate new rule. */
+ error = ofproto_rule_create(ofproto, fm, &cr, table - ofproto->tables,
+ &rule);
+ if (error) {
+ return error;
+ }
- /* Allocate new rule. */
- error = ofproto_rule_create(ofproto, fm, &cr, table - ofproto->tables,
- &rule);
- if (error) {
- return error;
+ /* Insert flow to the classifier, so that later flow_mods may relate
+ * to it. This is reversible, in case later errors require this to
+ * be reverted. */
+ ofproto_rule_insert__(ofproto, rule);
+ /* Make the new rule invisible for classifier lookups. */
+ classifier_defer(&table->cls);
+ get_conjunctions(fm, &conjs, &n_conjs);
+ classifier_insert(&table->cls, &rule->cr, conjs, n_conjs);
+ free(conjs);
+
+ error = ofproto->ofproto_class->rule_insert(rule);
+ if (error) {
+ oftable_remove_rule(rule);
+ ofproto_rule_unref(rule);
+ return error;
+ }
+
+ *modify = false;
}
- ofproto_rule_insert__(ofproto, rule);
+ *rulep = rule;
+ return 0;
+}
- classifier_defer(&table->cls);
+static void
+add_flow_finish(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct flow_mod_requester *req,
+ struct rule *rule, bool modify)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ if (modify) {
+ struct ovs_list dead_cookies = OVS_LIST_INITIALIZER(&dead_cookies);
- struct cls_conjunction *conjs;
- size_t n_conjs;
- get_conjunctions(fm, &conjs, &n_conjs);
- classifier_insert(&table->cls, &rule->cr, conjs, n_conjs);
- free(conjs);
+ modify_flow__(ofproto, fm, rule, req, &dead_cookies);
+ learned_cookies_flush(ofproto, &dead_cookies);
+ } else {
+ struct oftable *table = &ofproto->tables[rule->table_id];
- error = ofproto->ofproto_class->rule_insert(rule);
- if (error) {
- oftable_remove_rule(rule);
- ofproto_rule_unref(rule);
- return error;
- }
- cls_rule_make_visible(&rule->cr);
- classifier_publish(&table->cls);
+ cls_rule_make_visible(&rule->cr);
+ classifier_publish(&table->cls);
- learned_cookies_inc(ofproto, rule_get_actions(rule));
+ learned_cookies_inc(ofproto, rule_get_actions(rule));
- if (minimask_get_vid_mask(&rule->cr.match.mask) == VLAN_VID_MASK) {
- if (ofproto->vlan_bitmap) {
- uint16_t vid = miniflow_get_vid(&rule->cr.match.flow);
- if (!bitmap_is_set(ofproto->vlan_bitmap, vid)) {
- bitmap_set1(ofproto->vlan_bitmap, vid);
+ if (minimask_get_vid_mask(&rule->cr.match.mask) == VLAN_VID_MASK) {
+ if (ofproto->vlan_bitmap) {
+ uint16_t vid = miniflow_get_vid(&rule->cr.match.flow);
+
+ if (!bitmap_is_set(ofproto->vlan_bitmap, vid)) {
+ bitmap_set1(ofproto->vlan_bitmap, vid);
+ ofproto->vlans_changed = true;
+ }
+ } else {
ofproto->vlans_changed = true;
}
- } else {
- ofproto->vlans_changed = true;
}
+
+ ofmonitor_report(ofproto->connmgr, rule, NXFME_ADDED, 0,
+ req ? req->ofconn : NULL,
+ req ? req->request->xid : 0, NULL);
+ }
+
+ send_buffered_packet(req, fm->buffer_id, rule);
+}
+
+/* Implements OFPFC_ADD and the cases for OFPFC_MODIFY and OFPFC_MODIFY_STRICT
+ * in which no matching flow already exists in the flow table.
+ *
+ * Adds the flow specified by 'fm', to the ofproto's flow table. Returns 0 on
+ * success, an OpenFlow error code on failure, or OFPROTO_POSTPONE if the
+ * operation cannot be initiated now but may be retried later.
+ *
+ * The caller retains ownership of 'fm->ofpacts'. */
+static enum ofperr
+add_flow(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
+ const struct flow_mod_requester *req)
+ OVS_REQUIRES(ofproto_mutex)
+{
+ struct rule *rule;
+ bool modify;
+ enum ofperr error;
+
+ error = add_flow_begin(ofproto, fm, &rule, &modify);
+ if (!error) {
+ add_flow_finish(ofproto, fm, req, rule, modify);
}
- ofmonitor_report(ofproto->connmgr, rule, NXFME_ADDED, 0,
- req ? req->ofconn : NULL, req ? req->xid : 0, NULL);
-send_packet:
- return req ? send_buffered_packet(req->ofconn, fm->buffer_id, rule) : 0;
+ return error;
}
/* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */
@@ -4626,7 +4661,7 @@ modify_flow__(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
if (event != NXFME_MODIFIED || change_actions || change_cookie) {
ofmonitor_report(ofproto->connmgr, rule, event, 0,
- req ? req->ofconn : NULL, req ? req->xid : 0,
+ req ? req->ofconn : NULL, req ? req->request->xid : 0,
change_actions ? actions : NULL);
}
@@ -4680,11 +4715,7 @@ modify_flows(struct ofproto *ofproto, struct ofputil_flow_mod *fm,
error = modify_flows_check__(ofproto, fm, rules);
if (!error) {
modify_flows__(ofproto, fm, rules, req);
-
- if (fm->buffer_id != UINT32_MAX && req) {
- error = send_buffered_packet(req->ofconn, fm->buffer_id,
- rules->rules[0]);
- }
+ send_buffered_packet(req, fm->buffer_id, rules->rules[0]);
}
} else {
error = modify_flows_add__(ofproto, fm, req);
@@ -4773,8 +4804,8 @@ delete_flows__(const struct rule_collection *rules,
ofproto_rule_send_removed(rule, reason);
ofmonitor_report(ofproto->connmgr, rule, NXFME_DELETED, reason,
- req ? req->ofconn : NULL, req ? req->xid : 0,
- NULL);
+ req ? req->ofconn : NULL,
+ req ? req->request->xid : 0, NULL);
if (next_table == rule->table_id) {
classifier_defer(cls);
@@ -4960,7 +4991,7 @@ handle_flow_mod(struct ofconn *ofconn, const struct ofp_header *oh)
struct flow_mod_requester req;
req.ofconn = ofconn;
- req.xid = oh->xid;
+ req.request = oh;
error = handle_flow_mod__(ofproto, &fm, &req);
}
if (error) {
@@ -6670,18 +6701,19 @@ handle_openflow(struct ofconn *ofconn, const struct ofpbuf *ofp_msg)
/* Asynchronous operations. */
-static enum ofperr
-send_buffered_packet(struct ofconn *ofconn, uint32_t buffer_id,
+static void
+send_buffered_packet(const struct flow_mod_requester *req, uint32_t buffer_id,
struct rule *rule)
OVS_REQUIRES(ofproto_mutex)
{
- enum ofperr error = 0;
- if (ofconn && buffer_id != UINT32_MAX) {
- struct ofproto *ofproto = ofconn_get_ofproto(ofconn);
+ if (req && req->ofconn && buffer_id != UINT32_MAX) {
+ struct ofproto *ofproto = ofconn_get_ofproto(req->ofconn);
struct dp_packet *packet;
ofp_port_t in_port;
+ enum ofperr error;
- error = ofconn_pktbuf_retrieve(ofconn, buffer_id, &packet, &in_port);
+ error = ofconn_pktbuf_retrieve(req->ofconn, buffer_id, &packet,
+ &in_port);
if (packet) {
struct rule_execute *re;
@@ -6698,9 +6730,10 @@ send_buffered_packet(struct ofconn *ofconn, uint32_t buffer_id,
dp_packet_delete(re->packet);
free(re);
}
+ } else {
+ ofconn_send_error(req->ofconn, req->request, error);
}
}
- return error;
}
static uint64_t
--
1.7.10.4
More information about the dev
mailing list