When OF11+ only actions is introduced, ofpacts_put_openflow10() will be
unable to handle all ofp-actions. So allow it return error in that case.
Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
---
v4
- newly introduced
---
lib/learning-switch.c | 35 +++++++++++++++++++++++++----
lib/ofp-actions.c | 11 ++++++--
lib/ofp-actions.h | 4 +-
lib/ofp-util.c | 59 ++++++++++++++++++++++++++++++++++++++----------
lib/ofp-util.h | 15 ++++++-----
ofproto/connmgr.c | 16 ++++++++++--
ofproto/ofproto.c | 13 +++++++++-
utilities/ovs-ofctl.c | 35 +++++++++++++++++++++++++----
8 files changed, 148 insertions(+), 40 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index 887a365..c7b5a96 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -171,8 +171,15 @@ lswitch_create(struct rconn *rconn, const struct
lswitch_config *cfg)
}
for (i = 0; !error && i < cfg->n_default_flows; i++) {
- msg = ofputil_encode_flow_mod(&cfg->default_flows[i], protocol);
- error = rconn_send(rconn, msg, NULL);
+ enum ofperr err;
+ err = ofputil_encode_flow_mod(&cfg->default_flows[i], protocol,
+ &msg);
+ if (err) {
+ VLOG_INFO_RL(&rl, "ofputil_encode_flow_mod returned error: %s",
+ ofperr_get_name(error));
+ } else {
+ error = rconn_send(rconn, msg, NULL);
+ }
}
if (error) {
@@ -453,6 +460,7 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
struct ofpbuf pkt;
struct flow flow;
+ struct ofpbuf *packet_out;
error = ofputil_decode_packet_in(&pi, oh);
if (error) {
@@ -518,19 +526,36 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn,
fm.out_port = OFPP_NONE;
fm.ofpacts = ofpacts.data;
fm.ofpacts_len = ofpacts.size;
- buffer = ofputil_encode_flow_mod(&fm, sw->protocol);
+ error = ofputil_encode_flow_mod(&fm, sw->protocol, &buffer);
+ if (error) {
+ VLOG_ERR("ofputil_encode_flow_mod returned error: %s",
+ ofperr_get_name(error));
+ return;
+ }
queue_tx(sw, rconn, buffer);
/* If the switch didn't buffer the packet, we need to send a copy. */
if (pi.buffer_id == UINT32_MAX && out_port != OFPP_NONE) {
- queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
+ error = ofputil_encode_packet_out(&po, &packet_out);
+ if (error) {
+ VLOG_ERR("ofputil_encode_packet_out returns error %s",
+ ofperr_get_name(error));
+ } else {
+ queue_tx(sw, rconn, packet_out);
+ }
}
} else {
/* We don't know that MAC, or we don't set up flows. Send along the
* packet without setting up a flow. */
if (pi.buffer_id != UINT32_MAX || out_port != OFPP_NONE) {
- queue_tx(sw, rconn, ofputil_encode_packet_out(&po));
+ error = ofputil_encode_packet_out(&po, &packet_out);
+ if (error) {
+ VLOG_ERR("ofputil_encode_packet_out returns error %s",
+ ofperr_get_name(error));
+ } else {
+ queue_tx(sw, rconn, packet_out);
+ }
}
}
}
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 0874cc4..8d9f0ba 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1191,7 +1191,7 @@ ofpact_enqueue_to_openflow10(const struct ofpact_enqueue
*enqueue,
oae->queue_id = htonl(enqueue->queue);
}
-static void
+static enum ofperr
ofpact_to_openflow10(const struct ofpact *a, struct ofpbuf *out)
{
switch (a->type) {
@@ -1271,20 +1271,25 @@ ofpact_to_openflow10(const struct ofpact *a, struct
ofpbuf *out)
ofpact_to_nxast(a, out);
break;
}
+ return 0;
}
/* Converts the 'ofpacts_len' bytes of ofpacts in 'ofpacts' into OpenFlow 1.0
* actions in 'openflow', appending the actions to any existing data in
* 'openflow'. */
-void
+enum ofperr
ofpacts_put_openflow10(const struct ofpact ofpacts[], size_t ofpacts_len,
struct ofpbuf *openflow)
{
const struct ofpact *a;
OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
- ofpact_to_openflow10(a, openflow);
+ enum ofperr error = ofpact_to_openflow10(a, openflow);
+ if (error) {
+ return error;
+ }
}
+ return 0;
}
/* Converting ofpacts to OpenFlow 1.1. */
diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
index 7c9cb05..af1792a 100644
--- a/lib/ofp-actions.h
+++ b/lib/ofp-actions.h
@@ -394,8 +394,8 @@ enum ofperr ofpacts_check(const struct ofpact[], size_t
ofpacts_len,
const struct flow *, int max_ports);
/* Converting ofpacts to OpenFlow. */
-void ofpacts_put_openflow10(const struct ofpact[], size_t ofpacts_len,
- struct ofpbuf *openflow);
+enum ofperr ofpacts_put_openflow10(const struct ofpact[], size_t ofpacts_len,
+ struct ofpbuf *openflow);
void ofpacts_put_openflow11_actions(const struct ofpact[], size_t ofpacts_len,
struct ofpbuf *openflow);
void ofpacts_put_openflow11_instructions(const struct ofpact[],
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index ac12b1b..c6e0e74 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1262,9 +1262,9 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
/* Converts 'fm' into an OFPT_FLOW_MOD or NXT_FLOW_MOD message according to
* 'protocol' and returns the message. */
-struct ofpbuf *
+enum ofperr
ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,
- enum ofputil_protocol protocol)
+ enum ofputil_protocol protocol, struct ofpbuf **msgp)
{
struct ofpbuf *msg;
uint16_t command;
@@ -1322,10 +1322,17 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod
*fm,
}
if (fm->ofpacts) {
- ofpacts_put_openflow10(fm->ofpacts, fm->ofpacts_len, msg);
+ enum ofperr error = ofpacts_put_openflow10(fm->ofpacts,
+ fm->ofpacts_len, msg);
+ if (error) {
+ *msgp = NULL;
+ ofpbuf_delete(msg);
+ return error;
+ }
}
ofpmsg_update_length(msg);
- return msg;
+ *msgp = msg;
+ return 0;
}
/* Returns a bitmask with a 1-bit for each protocol that could be used to
@@ -1640,7 +1647,7 @@ unknown_to_zero(uint64_t count)
/* Appends an OFPST_FLOW or NXST_FLOW reply that contains the data in 'fs' to
* those already present in the list of ofpbufs in 'replies'. 'replies' should
* have been initialized with ofputil_start_stats_reply(). */
-void
+enum ofperr
ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *fs,
struct list *replies)
{
@@ -1651,9 +1658,14 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
ofpraw_decode_partial(&raw, reply->data, reply->size);
if (raw == OFPRAW_OFPST_FLOW_REPLY) {
struct ofp10_flow_stats *ofs;
+ enum ofperr error;
ofpbuf_put_uninit(reply, sizeof *ofs);
- ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply);
+ error = ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply);
+ if (error) {
+ reply->size = start_ofs;
+ return error;
+ }
ofs = ofpbuf_at_assert(reply, start_ofs, sizeof *ofs);
ofs->length = htons(reply->size - start_ofs);
@@ -1674,10 +1686,15 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
} else if (raw == OFPRAW_NXST_FLOW_REPLY) {
struct nx_flow_stats *nfs;
int match_len;
+ enum ofperr error;
ofpbuf_put_uninit(reply, sizeof *nfs);
match_len = nx_put_match(reply, false, &fs->rule, 0, 0);
- ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply);
+ error = ofpacts_put_openflow10(fs->ofpacts, fs->ofpacts_len, reply);
+ if (error) {
+ reply->size = start_ofs;
+ return error;
+ }
nfs = ofpbuf_at_assert(reply, start_ofs, sizeof *nfs);
nfs->length = htons(reply->size - start_ofs);
@@ -1703,6 +1720,7 @@ ofputil_append_flow_stats_reply(const struct
ofputil_flow_stats *fs,
}
ofpmp_postappend(replies, start_ofs);
+ return 0;
}
/* Converts abstract ofputil_aggregate_stats 'stats' into an OFPST_AGGREGATE or
@@ -2958,7 +2976,7 @@ ofputil_start_flow_update(struct list *replies)
list_push_back(replies, &msg->list_node);
}
-void
+enum ofperr
ofputil_append_flow_update(const struct ofputil_flow_update *update,
struct list *replies)
{
@@ -2977,11 +2995,17 @@ ofputil_append_flow_update(const struct
ofputil_flow_update *update,
} else {
struct nx_flow_update_full *nfuf;
int match_len;
+ enum ofperr error;
ofpbuf_put_zeros(msg, sizeof *nfuf);
match_len = nx_put_match(msg, false, update->match,
htonll(0), htonll(0));
- ofpacts_put_openflow10(update->ofpacts, update->ofpacts_len, msg);
+ error = ofpacts_put_openflow10(update->ofpacts, update->ofpacts_len,
+ msg);
+ if (error) {
+ msg->size = start_ofs;
+ return error;
+ }
nfuf = ofpbuf_at_assert(msg, start_ofs, sizeof *nfuf);
nfuf->reason = htons(update->reason);
@@ -2998,14 +3022,17 @@ ofputil_append_flow_update(const struct
ofputil_flow_update *update,
nfuh->event = htons(update->event);
ofpmp_postappend(replies, start_ofs);
+ return 0;
}
-struct ofpbuf *
-ofputil_encode_packet_out(const struct ofputil_packet_out *po)
+enum ofperr
+ofputil_encode_packet_out(const struct ofputil_packet_out *po,
+ struct ofpbuf **msgp)
{
struct ofp_packet_out *opo;
size_t actions_ofs;
struct ofpbuf *msg;
+ enum ofperr error;
size_t size;
size = po->ofpacts_len;
@@ -3016,7 +3043,12 @@ ofputil_encode_packet_out(const struct
ofputil_packet_out *po)
msg = ofpraw_alloc(OFPRAW_OFPT10_PACKET_OUT, OFP10_VERSION, size);
ofpbuf_put_zeros(msg, sizeof *opo);
actions_ofs = msg->size;
- ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg);
+ error = ofpacts_put_openflow10(po->ofpacts, po->ofpacts_len, msg);
+ if (error) {
+ ofpbuf_delete(msg);
+ *msgp = NULL;
+ return error;
+ }
opo = msg->l3;
opo->buffer_id = htonl(po->buffer_id);
@@ -3029,7 +3061,8 @@ ofputil_encode_packet_out(const struct ofputil_packet_out
*po)
ofpmsg_update_length(msg);
- return msg;
+ *msgp = msg;
+ return 0;
}
/* Creates and returns an OFPT_ECHO_REQUEST message with an empty payload. */
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 032b156..b9d71f0 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -177,8 +177,8 @@ enum ofperr ofputil_decode_flow_mod(struct ofputil_flow_mod
*,
const struct ofp_header *,
enum ofputil_protocol,
struct ofpbuf *ofpacts);
-struct ofpbuf *ofputil_encode_flow_mod(const struct ofputil_flow_mod *,
- enum ofputil_protocol);
+enum ofperr ofputil_encode_flow_mod(const struct ofputil_flow_mod *,
+ enum ofputil_protocol, struct ofpbuf **);
enum ofputil_protocol ofputil_flow_mod_usable_protocols(
const struct ofputil_flow_mod *fms, size_t n_fms);
@@ -221,8 +221,8 @@ int ofputil_decode_flow_stats_reply(struct
ofputil_flow_stats *,
struct ofpbuf *msg,
bool flow_age_extension,
struct ofpbuf *ofpacts);
-void ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *,
- struct list *replies);
+enum ofperr ofputil_append_flow_stats_reply(const struct ofputil_flow_stats *,
+ struct list *replies);
/* Aggregate stats reply, independent of protocol. */
struct ofputil_aggregate_stats {
@@ -297,7 +297,8 @@ struct ofputil_packet_out {
enum ofperr ofputil_decode_packet_out(struct ofputil_packet_out *,
const struct ofp_header *,
struct ofpbuf *ofpacts);
-struct ofpbuf *ofputil_encode_packet_out(const struct ofputil_packet_out *);
+enum ofperr ofputil_encode_packet_out(const struct ofputil_packet_out *,
+ struct ofpbuf **);
enum ofputil_port_config {
/* OpenFlow 1.0 and 1.1 share these values for these port config bits. */
@@ -481,8 +482,8 @@ struct ofputil_flow_update {
int ofputil_decode_flow_update(struct ofputil_flow_update *,
struct ofpbuf *msg, struct ofpbuf *ofpacts);
void ofputil_start_flow_update(struct list *replies);
-void ofputil_append_flow_update(const struct ofputil_flow_update *,
- struct list *replies);
+enum ofperr ofputil_append_flow_update(const struct ofputil_flow_update *,
+ struct list *replies);
/* Abstract nx_flow_monitor_cancel. */
uint32_t ofputil_decode_flow_monitor_cancel(const struct ofp_header *);
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 5ce77c0..54404b4 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1777,6 +1777,7 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
enum nx_flow_monitor_flags flags = 0;
struct ofmonitor *m;
+ enum ofperr error;
if (ofconn->monitor_paused) {
/* Only send NXFME_DELETED notifications for flows that were added
@@ -1819,15 +1820,24 @@ ofmonitor_report(struct connmgr *mgr, struct rule *rule,
fu.ofpacts = NULL;
fu.ofpacts_len = 0;
}
- ofputil_append_flow_update(&fu, &ofconn->updates);
+ error = ofputil_append_flow_update(&fu, &ofconn->updates);
+ if (error) {
+ VLOG_ERR("ofputil_append_flow_update: %s",
+ ofperr_get_name(error));
+ }
} else if (!ofconn->sent_abbrev_update) {
struct ofputil_flow_update fu;
fu.event = NXFME_ABBREV;
fu.xid = abbrev_xid;
- ofputil_append_flow_update(&fu, &ofconn->updates);
+ error = ofputil_append_flow_update(&fu, &ofconn->updates);
- ofconn->sent_abbrev_update = true;
+ if (error) {
+ VLOG_ERR("ofputil_append_flow_update: %s",
+ ofperr_get_name(error));
+ } else {
+ ofconn->sent_abbrev_update = true;
+ }
}
}
}
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 0f24fd0..af1ef16 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2531,7 +2531,11 @@ handle_flow_stats_request(struct ofconn *ofconn,
&fs.byte_count);
fs.ofpacts = rule->ofpacts;
fs.ofpacts_len = rule->ofpacts_len;
- ofputil_append_flow_stats_reply(&fs, &replies);
+ error = ofputil_append_flow_stats_reply(&fs, &replies);
+ if (error) {
+ ofpbuf_list_delete(&replies);
+ return error;
+ }
}
ofconn_send_replies(ofconn, &replies);
@@ -3441,6 +3445,7 @@ ofproto_compose_flow_refresh_update(const struct rule
*rule,
{
struct ofoperation *op = rule->pending;
struct ofputil_flow_update fu;
+ enum ofperr error;
if (op && op->type == OFOPERATION_ADD && !op->victim) {
/* We'll report the final flow when the operation completes. Reporting
@@ -3495,7 +3500,11 @@ ofproto_compose_flow_refresh_update(const struct rule
*rule,
if (list_is_empty(msgs)) {
ofputil_start_flow_update(msgs);
}
- ofputil_append_flow_update(&fu, msgs);
+ error = ofputil_append_flow_update(&fu, msgs);
+ if (error) {
+ VLOG_ERR("ofputil_append_flow_update returned error: %s",
+ ofperr_get_name(error));
+ }
}
void
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 00ba06b..08dd312 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1029,8 +1029,15 @@ ofctl_flow_mod__(const char *remote, struct
ofputil_flow_mod *fms,
for (i = 0; i < n_fms; i++) {
struct ofputil_flow_mod *fm = &fms[i];
+ enum ofperr error;
+ struct ofpbuf *flow_mod;
- transact_noreply(vconn, ofputil_encode_flow_mod(fm, protocol));
+ error = ofputil_encode_flow_mod(fm, protocol, &flow_mod);
+ if (error) {
+ ovs_fatal(0, "ofputil_encode_flow_mod returned error: %s",
+ ofperr_get_name(error));
+ }
+ transact_noreply(vconn, flow_mod);
free(fm->ofpacts);
}
vconn_close(vconn);
@@ -1472,6 +1479,7 @@ ofctl_packet_out(int argc, char *argv[])
for (i = 4; i < argc; i++) {
struct ofpbuf *packet, *opo;
const char *error_msg;
+ enum ofperr error;
error_msg = eth_from_hex(argv[i], &packet);
if (error_msg) {
@@ -1480,7 +1488,11 @@ ofctl_packet_out(int argc, char *argv[])
po.packet = packet->data;
po.packet_len = packet->size;
- opo = ofputil_encode_packet_out(&po);
+ error = ofputil_encode_packet_out(&po, &opo);
+ if (error) {
+ ovs_fatal(0, "ofputil_encode_packet_out: %s",
+ ofperr_get_name(error));
+ }
transact_noreply(vconn, opo);
ofpbuf_delete(packet);
}
@@ -1962,6 +1974,7 @@ fte_make_flow_mod(const struct fte *fte, int index,
uint16_t command,
const struct fte_version *version = fte->versions[index];
struct ofputil_flow_mod fm;
struct ofpbuf *ofm;
+ enum ofperr error;
fm.cr = fte->rule;
fm.cookie = htonll(0);
@@ -1983,7 +1996,11 @@ fte_make_flow_mod(const struct fte *fte, int index,
uint16_t command,
fm.ofpacts_len = 0;
}
- ofm = ofputil_encode_flow_mod(&fm, protocol);
+ error = ofputil_encode_flow_mod(&fm, protocol, &ofm);
+ if (error) {
+ ovs_fatal(0, "ofputil_encode_flow_mod returned error: %s",
+ ofperr_get_name(error));
+ }
list_push_back(packets, &ofm->list_node);
}
@@ -2134,8 +2151,13 @@ ofctl_parse_flows__(struct ofputil_flow_mod *fms, size_t
n_fms)
for (i = 0; i < n_fms; i++) {
struct ofputil_flow_mod *fm = &fms[i];
struct ofpbuf *msg;
+ enum ofperr error;
- msg = ofputil_encode_flow_mod(fm, protocol);
+ error = ofputil_encode_flow_mod(fm, protocol, &msg);
+ if (error) {
+ ovs_fatal(0, "ofputil_encode_flow_mod: %s",
+ ofperr_get_name(error));
+ }
ofp_print(stdout, msg->data, msg->size, verbosity);
ofpbuf_delete(msg);
@@ -2301,7 +2323,10 @@ ofctl_parse_ofp10_actions(int argc OVS_UNUSED, char
*argv[] OVS_UNUSED)
/* Convert back to ofp10 actions and print differences from input. */
ofpbuf_init(&of10_out, 0);
- ofpacts_put_openflow10(ofpacts.data, ofpacts.size, &of10_out);
+ error = ofpacts_put_openflow10(ofpacts.data, ofpacts.size, &of10_out);
+ if (error) {
+ ovs_fatal(0, "bad OF1.1 encoding: %s\n\n", ofperr_get_name(error));
+ }
print_differences("", of10_in.data, of10_in.size,
of10_out.data, of10_out.size);
--
1.7.1.1
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
