[ovs-dev] [wdp error reporting 3/5] ofproto: Propagate OpenFlow OFPT_FLOW_MOD errors to controller.
Ben Pfaff
blp at nicira.com
Mon Jul 26 22:44:07 UTC 2010
Errors can occur during flow table modifications requested via OpenFlow
OFPT_FLOW_MOD commands, but ofproto was in most cases failing to report
these errors back to the controller using OpenFlow error codes.
Reported-by: Tom Everman <teverman at google.com>
---
ofproto/ofproto.c | 71 ++++++++++++++++++++++++++++++----------------------
ofproto/wdp.h | 6 +++-
2 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 3c13273..e5bc8d2 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2033,12 +2033,12 @@ add_flow(struct ofproto *p, struct ofconn *ofconn,
error = pktbuf_retrieve(ofconn->pktbuf, ntohl(ofm->buffer_id),
&packet, &in_port);
if (!error) {
- wdp_flow_inject(p->wdp, rule, in_port, packet);
+ error = wdp_flow_inject(p->wdp, rule, in_port, packet);
ofpbuf_delete(packet);
}
}
- return 0;
+ return error;
}
static struct wdp_rule *
@@ -2072,10 +2072,10 @@ send_buffered_packet(struct ofproto *ofproto, struct ofconn *ofconn,
return error;
}
- wdp_flow_inject(ofproto->wdp, rule, in_port, packet);
+ error = wdp_flow_inject(ofproto->wdp, rule, in_port, packet);
ofpbuf_delete(packet);
- return 0;
+ return error;
}
/* OFPFC_MODIFY and OFPFC_MODIFY_STRICT. */
@@ -2088,7 +2088,8 @@ struct modify_flows_cbdata {
};
static int modify_flow(struct ofproto *, const struct ofp_flow_mod *,
- size_t n_actions, struct wdp_rule *);
+ size_t n_actions, struct wdp_rule *)
+ WARN_UNUSED_RESULT;
static int modify_flows_cb(struct wdp_rule *, void *cbdata_);
/* Implements OFPFC_ADD and OFPFC_MODIFY. Returns 0 on success or an OpenFlow
@@ -2103,6 +2104,7 @@ modify_flows_loose(struct ofproto *p, struct ofconn *ofconn,
struct modify_flows_cbdata cbdata;
uint8_t table_id;
flow_t target;
+ int error;
cbdata.ofproto = p;
cbdata.ofm = ofm;
@@ -2113,14 +2115,18 @@ modify_flows_loose(struct ofproto *p, struct ofconn *ofconn,
&target);
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);
+ error = wdp_flow_for_each_match(p->wdp, &target,
+ table_id_to_include(table_id),
+ modify_flows_cb, &cbdata);
+ if (error) {
+ return error;
+ }
+
if (cbdata.match) {
/* This credits the packet to whichever flow happened to match last.
* That's weird. Maybe we should do a lookup for the flow that
* actually matches the packet? Who knows. */
- send_buffered_packet(p, ofconn, cbdata.match, ofm);
- return 0;
+ return send_buffered_packet(p, ofconn, cbdata.match, ofm);
} else {
return add_flow(p, ofconn, ofm, n_actions);
}
@@ -2137,8 +2143,8 @@ modify_flow_strict(struct ofproto *p, struct ofconn *ofconn,
{
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);
+ int error = modify_flow(p, ofm, n_actions, rule);
+ return error ? error : send_buffered_packet(p, ofconn, rule, ofm);
} else {
return add_flow(p, ofconn, ofm, n_actions);
}
@@ -2152,7 +2158,8 @@ modify_flows_cb(struct wdp_rule *rule, void *cbdata_)
if (!rule_is_hidden(rule)) {
cbdata->match = rule;
- modify_flow(cbdata->ofproto, cbdata->ofm, cbdata->n_actions, rule);
+ return modify_flow(cbdata->ofproto, cbdata->ofm, cbdata->n_actions,
+ rule);
}
return 0;
}
@@ -2195,11 +2202,11 @@ struct delete_flows_cbdata {
};
static int delete_flows_cb(struct wdp_rule *, void *cbdata_);
-static void delete_flow_core(struct ofproto *, struct wdp_rule *,
+static int delete_flow_core(struct ofproto *, struct wdp_rule *,
uint16_t out_port);
/* Implements OFPFC_DELETE. */
-static void
+static int
delete_flows_loose(struct ofproto *p, const struct ofconn *ofconn,
const struct ofp_flow_mod *ofm)
{
@@ -2214,19 +2221,21 @@ delete_flows_loose(struct ofproto *p, const struct ofconn *ofconn,
&target);
table_id = flow_mod_table_id(ofconn, ofm);
- wdp_flow_for_each_match(p->wdp, &target, table_id_to_include(table_id),
- delete_flows_cb, &cbdata);
+ return wdp_flow_for_each_match(p->wdp, &target,
+ table_id_to_include(table_id),
+ delete_flows_cb, &cbdata);
}
/* Implements OFPFC_DELETE_STRICT. */
-static void
+static int
delete_flow_strict(struct ofproto *p, const struct ofconn *ofconn,
struct ofp_flow_mod *ofm)
{
struct wdp_rule *rule = find_flow_strict(p, ofconn, ofm);
if (rule) {
- delete_flow_core(p, rule, ofm->out_port);
+ return delete_flow_core(p, rule, ofm->out_port);
}
+ return 0;
}
/* Callback for delete_flows_loose(). */
@@ -2235,7 +2244,7 @@ delete_flows_cb(struct wdp_rule *rule, void *cbdata_)
{
struct delete_flows_cbdata *cbdata = cbdata_;
- delete_flow_core(cbdata->ofproto, rule, cbdata->out_port);
+ return delete_flow_core(cbdata->ofproto, rule, cbdata->out_port);
}
/* Implements core of OFPFC_DELETE and OFPFC_DELETE_STRICT where 'rule' has
@@ -2246,18 +2255,18 @@ delete_flows_cb(struct wdp_rule *rule, void *cbdata_)
* Will not delete 'rule' if it is hidden. Will delete 'rule' only if
* 'out_port' is htons(OFPP_NONE) or if 'rule' actually outputs to the
* specified 'out_port'. */
-static void
+static int
delete_flow_core(struct ofproto *p, struct wdp_rule *rule, uint16_t out_port)
{
if (rule_is_hidden(rule)) {
- return;
+ return 0;
}
if (out_port != htons(OFPP_NONE) && !rule_has_out_port(rule, out_port)) {
- return;
+ return 0;
}
- delete_flow(p, rule, OFPRR_DELETE);
+ return delete_flow(p, rule, OFPRR_DELETE);
}
static int
@@ -2334,12 +2343,10 @@ 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, ofconn, ofm);
- return 0;
+ return delete_flows_loose(p, ofconn, ofm);
case OFPFC_DELETE_STRICT:
- delete_flow_strict(p, ofconn, ofm);
- return 0;
+ return delete_flow_strict(p, ofconn, ofm);
default:
return ofp_mkerr(OFPET_FLOW_MOD_FAILED, OFPFMFC_BAD_COMMAND);
@@ -2648,7 +2655,7 @@ compose_flow_removed(struct ofproto *p, const struct wdp_rule *rule,
return buf;
}
-static void
+static int
delete_flow(struct ofproto *p, struct wdp_rule *rule, uint8_t reason)
{
/* We limit the maximum number of queued flow expirations it by accounting
@@ -2660,6 +2667,7 @@ delete_flow(struct ofproto *p, struct wdp_rule *rule, uint8_t reason)
struct ofproto_rule *ofproto_rule = ofproto_rule_cast(rule);
struct wdp_flow_stats stats;
struct ofpbuf *buf;
+ int error;
if (ofproto_rule->send_flow_removed) {
/* Compose most of the ofp_flow_removed before 'rule' is destroyed. */
@@ -2668,8 +2676,9 @@ delete_flow(struct ofproto *p, struct wdp_rule *rule, uint8_t reason)
buf = NULL;
}
- if (wdp_flow_delete(p->wdp, rule, &stats)) {
- return;
+ error = wdp_flow_delete(p->wdp, rule, &stats);
+ if (error) {
+ return error;
}
if (buf) {
@@ -2697,6 +2706,8 @@ delete_flow(struct ofproto *p, struct wdp_rule *rule, uint8_t reason)
}
}
free(ofproto_rule);
+
+ return 0;
}
/* pinsched callback for sending 'packet' on 'ofconn'. */
diff --git a/ofproto/wdp.h b/ofproto/wdp.h
index 11ab028..1a89fed 100644
--- a/ofproto/wdp.h
+++ b/ofproto/wdp.h
@@ -187,9 +187,11 @@ struct wdp_flow_put {
int wdp_flow_put(struct wdp *, struct wdp_flow_put *,
struct wdp_flow_stats *old_stats,
- struct wdp_rule **rulep);
+ struct wdp_rule **rulep)
+ WARN_UNUSED_RESULT;
int wdp_flow_delete(struct wdp *, struct wdp_rule *,
- struct wdp_flow_stats *final_stats);
+ struct wdp_flow_stats *final_stats)
+ WARN_UNUSED_RESULT;
/* Sending packets in flows. */
int wdp_flow_inject(struct wdp *, struct wdp_rule *,
--
1.7.1
More information about the dev
mailing list