[ovs-dev] [PATCH] Use initializers for struct ofputil_flow_mod instead of assignments.
Ben Pfaff
blp at ovn.org
Mon Jan 4 19:24:01 UTC 2016
A few bugs have been fixed lately that were related to struct
ofputil_flow_mod not being fully initialized in a few places. This commit
changes several pieces of code from using individual assignments to fields
in struct ofputil_flow_mod, to using whole initializers or assignments to
a whole struct. This should help prevent similar problems in the future.
CC: Ilya Maximets <i.maximets at samsung.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
lib/learning-switch.c | 66 +++++++++++++++++++++++++-------------------------
lib/ofp-parse.c | 41 +++++++++++++++----------------
ofproto/ofproto-dpif.c | 58 +++++++++++++++++++++++---------------------
ofproto/ofproto.c | 39 ++++++++++++++---------------
utilities/ovs-ofctl.c | 35 +++++++++++++-------------
5 files changed, 122 insertions(+), 117 deletions(-)
diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index a45cf98..89c1215 100644
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -191,7 +191,6 @@ lswitch_handshake(struct lswitch *sw)
/* OpenFlow 1.3 and later by default drop packets that miss in the flow
* table. Set up a flow to send packets to the controller by
* default. */
- struct ofputil_flow_mod fm;
struct ofpact_output output;
struct ofpbuf *msg;
int error;
@@ -200,24 +199,26 @@ lswitch_handshake(struct lswitch *sw)
output.port = OFPP_CONTROLLER;
output.max_len = OFP_DEFAULT_MISS_SEND_LEN;
- match_init_catchall(&fm.match);
- fm.priority = 0;
- fm.cookie = 0;
- fm.cookie_mask = 0;
- fm.new_cookie = 0;
- fm.modify_cookie = false;
- fm.table_id = 0;
- fm.command = OFPFC_ADD;
- fm.idle_timeout = 0;
- fm.hard_timeout = 0;
- fm.importance = 0;
- fm.buffer_id = UINT32_MAX;
- fm.out_port = OFPP_NONE;
- fm.out_group = OFPG_ANY;
- fm.flags = 0;
- fm.ofpacts = &output.ofpact;
- fm.ofpacts_len = sizeof output;
- fm.delete_reason = 0;
+ struct ofputil_flow_mod fm = {
+ .match = MATCH_CATCHALL_INITIALIZER,
+ .priority = 0,
+ .cookie = 0,
+ .cookie_mask = 0,
+ .new_cookie = 0,
+ .modify_cookie = false,
+ .table_id = 0,
+ .command = OFPFC_ADD,
+ .idle_timeout = 0,
+ .hard_timeout = 0,
+ .importance = 0,
+ .buffer_id = UINT32_MAX,
+ .out_port = OFPP_NONE,
+ .out_group = OFPG_ANY,
+ .flags = 0,
+ .ofpacts = &output.ofpact,
+ .ofpacts_len = sizeof output,
+ .delete_reason = 0,
+ };
msg = ofputil_encode_flow_mod(&fm, protocol);
error = rconn_send(sw->rconn, msg, NULL);
@@ -667,23 +668,22 @@ process_packet_in(struct lswitch *sw, const struct ofp_header *oh)
/* Send the packet, and possibly the whole flow, to the output port. */
if (sw->max_idle >= 0 && (!sw->ml || out_port != OFPP_FLOOD)) {
- struct ofputil_flow_mod fm;
- struct ofpbuf *buffer;
-
/* The output port is known, or we always flood everything, so add a
* new flow. */
- memset(&fm, 0, sizeof fm);
+ struct ofputil_flow_mod fm = {
+ .priority = 1, /* Must be > 0 because of table-miss flow entry. */
+ .table_id = 0xff,
+ .command = OFPFC_ADD,
+ .idle_timeout = sw->max_idle,
+ .buffer_id = pi.buffer_id,
+ .out_port = OFPP_NONE,
+ .ofpacts = ofpacts.data,
+ .ofpacts_len = ofpacts.size,
+ };
match_init(&fm.match, &flow, &sw->wc);
ofputil_normalize_match_quiet(&fm.match);
- fm.priority = 1; /* Must be > 0 because of table-miss flow entry. */
- fm.table_id = 0xff;
- fm.command = OFPFC_ADD;
- fm.idle_timeout = sw->max_idle;
- fm.buffer_id = pi.buffer_id;
- fm.out_port = OFPP_NONE;
- fm.ofpacts = ofpacts.data;
- fm.ofpacts_len = ofpacts.size;
- buffer = ofputil_encode_flow_mod(&fm, sw->protocol);
+
+ struct ofpbuf *buffer = ofputil_encode_flow_mod(&fm, sw->protocol);
queue_tx(sw, buffer);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index fdc30d9..8a96756 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -332,27 +332,26 @@ parse_ofp_str__(struct ofputil_flow_mod *fm, int command, char *string,
OVS_NOT_REACHED();
}
- match_init_catchall(&fm->match);
- fm->priority = OFP_DEFAULT_PRIORITY;
- fm->cookie = htonll(0);
- fm->cookie_mask = htonll(0);
- if (command == OFPFC_MODIFY || command == OFPFC_MODIFY_STRICT) {
+ *fm = (struct ofputil_flow_mod) {
+ .match = MATCH_CATCHALL_INITIALIZER,
+ .priority = OFP_DEFAULT_PRIORITY,
+ .cookie = htonll(0),
+ .cookie_mask = htonll(0),
/* For modify, by default, don't update the cookie. */
- fm->new_cookie = OVS_BE64_MAX;
- } else{
- fm->new_cookie = htonll(0);
- }
- fm->modify_cookie = false;
- fm->table_id = 0xff;
- fm->command = command;
- fm->idle_timeout = OFP_FLOW_PERMANENT;
- fm->hard_timeout = OFP_FLOW_PERMANENT;
- fm->buffer_id = UINT32_MAX;
- fm->out_port = OFPP_ANY;
- fm->flags = 0;
- fm->importance = 0;
- fm->out_group = OFPG_ANY;
- fm->delete_reason = OFPRR_DELETE;
+ .new_cookie = (command == OFPFC_MODIFY
+ || command == OFPFC_MODIFY_STRICT
+ ? OVS_BE64_MAX : 0),
+ .modify_cookie = false,
+ .table_id = 0xff,
+ .command = command,
+ .idle_timeout = OFP_FLOW_PERMANENT,
+ .hard_timeout = OFP_FLOW_PERMANENT,
+ .buffer_id = UINT32_MAX,
+ .out_port = OFPP_ANY,
+ .out_group = OFPG_ANY,
+ .delete_reason = OFPRR_DELETE,
+ };
+
if (fields & F_ACTIONS) {
act_str = extract_actions(string);
if (!act_str) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 2f42886..e2b82b1 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -5660,23 +5660,25 @@ ofproto_dpif_add_internal_flow(struct ofproto_dpif *ofproto,
struct rule_dpif *rule;
int error;
- ofm.fm.match = *match;
- ofm.fm.priority = priority;
- ofm.fm.new_cookie = htonll(0);
- ofm.fm.cookie = htonll(0);
- ofm.fm.cookie_mask = htonll(0);
- ofm.fm.modify_cookie = false;
- ofm.fm.table_id = TBL_INTERNAL;
- ofm.fm.command = OFPFC_ADD;
- ofm.fm.idle_timeout = idle_timeout;
- ofm.fm.hard_timeout = 0;
- ofm.fm.importance = 0;
- ofm.fm.buffer_id = 0;
- ofm.fm.out_port = 0;
- ofm.fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY;
- ofm.fm.ofpacts = ofpacts->data;
- ofm.fm.ofpacts_len = ofpacts->size;
- ofm.fm.delete_reason = OVS_OFPRR_NONE;
+ ofm.fm = (struct ofputil_flow_mod) {
+ .match = *match,
+ .priority = priority,
+ .new_cookie = htonll(0),
+ .cookie = htonll(0),
+ .cookie_mask = htonll(0),
+ .modify_cookie = false,
+ .table_id = TBL_INTERNAL,
+ .command = OFPFC_ADD,
+ .idle_timeout = idle_timeout,
+ .hard_timeout = 0,
+ .importance = 0,
+ .buffer_id = 0,
+ .out_port = 0,
+ .flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY,
+ .ofpacts = ofpacts->data,
+ .ofpacts_len = ofpacts->size,
+ .delete_reason = OVS_OFPRR_NONE,
+ };
error = ofproto_flow_mod(&ofproto->up, &ofm);
if (error) {
@@ -5705,15 +5707,17 @@ ofproto_dpif_delete_internal_flow(struct ofproto_dpif *ofproto,
struct ofproto_flow_mod ofm;
int error;
- ofm.fm.match = *match;
- ofm.fm.priority = priority;
- ofm.fm.new_cookie = htonll(0);
- ofm.fm.cookie = htonll(0);
- ofm.fm.cookie_mask = htonll(0);
- ofm.fm.modify_cookie = false;
- ofm.fm.table_id = TBL_INTERNAL;
- ofm.fm.flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY;
- ofm.fm.command = OFPFC_DELETE_STRICT;
+ ofm.fm = (struct ofputil_flow_mod) {
+ .match = *match,
+ .priority = priority,
+ .new_cookie = htonll(0),
+ .cookie = htonll(0),
+ .cookie_mask = htonll(0),
+ .modify_cookie = false,
+ .table_id = TBL_INTERNAL,
+ .flags = OFPUTIL_FF_HIDDEN_FIELDS | OFPUTIL_FF_NO_READONLY,
+ .command = OFPFC_DELETE_STRICT,
+ };
error = ofproto_flow_mod(&ofproto->up, &ofm);
if (error) {
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index a5ce04e..4a2725e 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009-2015 Nicira, Inc.
+ * Copyright (c) 2009-2016 Nicira, Inc.
* Copyright (c) 2010 Jean Tourrilhes - HP-Labs.
*
* Licensed under the Apache License, Version 2.0 (the "License");
@@ -2033,24 +2033,25 @@ flow_mod_init(struct ofputil_flow_mod *fm,
const struct ofpact *ofpacts, size_t ofpacts_len,
enum ofp_flow_mod_command command)
{
- memset(fm, 0, sizeof *fm);
- fm->match = *match;
- fm->priority = priority;
- fm->cookie = 0;
- fm->new_cookie = 0;
- fm->modify_cookie = false;
- fm->table_id = 0;
- fm->command = command;
- fm->idle_timeout = 0;
- fm->hard_timeout = 0;
- fm->importance = 0;
- fm->buffer_id = UINT32_MAX;
- fm->out_port = OFPP_ANY;
- fm->out_group = OFPG_ANY;
- fm->flags = 0;
- fm->ofpacts = CONST_CAST(struct ofpact *, ofpacts);
- fm->ofpacts_len = ofpacts_len;
- fm->delete_reason = OFPRR_DELETE;
+ *fm = (struct ofputil_flow_mod) {
+ .match = *match,
+ .priority = priority,
+ .cookie = 0,
+ .new_cookie = 0,
+ .modify_cookie = false,
+ .table_id = 0,
+ .command = command,
+ .idle_timeout = 0,
+ .hard_timeout = 0,
+ .importance = 0,
+ .buffer_id = UINT32_MAX,
+ .out_port = OFPP_ANY,
+ .out_group = OFPG_ANY,
+ .flags = 0,
+ .ofpacts = CONST_CAST(struct ofpact *, ofpacts),
+ .ofpacts_len = ofpacts_len,
+ .delete_reason = OFPRR_DELETE,
+ };
}
static int
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 0d57f85..27035ea 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -2899,24 +2899,26 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command,
enum ofputil_protocol protocol, struct ovs_list *packets)
{
const struct fte_version *version = fte->versions[index];
- struct ofputil_flow_mod fm;
struct ofpbuf *ofm;
+ struct ofputil_flow_mod fm = {
+ .priority = fte->rule.priority,
+ .cookie = htonll(0),
+ .cookie_mask = htonll(0),
+ .new_cookie = version->cookie,
+ .modify_cookie = true,
+ .table_id = version->table_id,
+ .command = command,
+ .idle_timeout = version->idle_timeout,
+ .hard_timeout = version->hard_timeout,
+ .importance = version->importance,
+ .buffer_id = UINT32_MAX,
+ .out_port = OFPP_ANY,
+ .out_group = OFPG_ANY,
+ .flags = version->flags,
+ .delete_reason = OFPRR_DELETE,
+ };
minimatch_expand(&fte->rule.match, &fm.match);
- fm.priority = fte->rule.priority;
- fm.cookie = htonll(0);
- fm.cookie_mask = htonll(0);
- fm.new_cookie = version->cookie;
- fm.modify_cookie = true;
- fm.table_id = version->table_id;
- fm.command = command;
- fm.idle_timeout = version->idle_timeout;
- fm.hard_timeout = version->hard_timeout;
- fm.importance = version->importance;
- fm.buffer_id = UINT32_MAX;
- fm.out_port = OFPP_ANY;
- fm.out_group = OFPG_ANY;
- fm.flags = version->flags;
if (command == OFPFC_ADD || command == OFPFC_MODIFY ||
command == OFPFC_MODIFY_STRICT) {
fm.ofpacts = version->ofpacts;
@@ -2925,7 +2927,6 @@ fte_make_flow_mod(const struct fte *fte, int index, uint16_t command,
fm.ofpacts = NULL;
fm.ofpacts_len = 0;
}
- fm.delete_reason = OFPRR_DELETE;
ofm = ofputil_encode_flow_mod(&fm, protocol);
list_push_back(packets, &ofm->list_node);
--
2.1.3
More information about the dev
mailing list