From 39cc5c4adea207465e88d309dedf61990201301b Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 4 Jan 2016 11:36:14 -0800 Subject: [PATCH] Use initializers for struct ofputil_flow_mod instead of assignments. 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 Signed-off-by: Ben Pfaff Acked-by: Russell Bryant --- lib/learning-switch.c | 57 ++++++++++++++++++------------------------ lib/ofp-parse.c | 32 ++++++++++-------------- ofproto/ofproto-dpif.c | 46 ++++++++++++++-------------------- ofproto/ofproto.c | 32 ++++++++++-------------- utilities/ovs-ofctl.c | 33 ++++++++++++------------ 5 files changed, 85 insertions(+), 115 deletions(-) diff --git a/lib/learning-switch.c b/lib/learning-switch.c index a45cf98b9..28f381917 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,17 @@ 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, + .table_id = 0, + .command = OFPFC_ADD, + .buffer_id = UINT32_MAX, + .out_port = OFPP_NONE, + .out_group = OFPG_ANY, + .ofpacts = &output.ofpact, + .ofpacts_len = sizeof output, + }; msg = ofputil_encode_flow_mod(&fm, protocol); error = rconn_send(sw->rconn, msg, NULL); @@ -667,23 +659,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 fdc30d9b7..c346f102b 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,21 @@ 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); + *fm = (struct ofputil_flow_mod) { + .match = MATCH_CATCHALL_INITIALIZER, + .priority = OFP_DEFAULT_PRIORITY, + .table_id = 0xff, + .command = command, + .buffer_id = UINT32_MAX, + .out_port = OFPP_ANY, + .out_group = OFPG_ANY, + .delete_reason = OFPRR_DELETE, + }; + /* For modify, by default, don't update the cookie. */ if (command == OFPFC_MODIFY || command == OFPFC_MODIFY_STRICT) { - /* 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; + 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 2f4288649..0a9ba4d0a 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,17 @@ 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, + .table_id = TBL_INTERNAL, + .command = OFPFC_ADD, + .idle_timeout = idle_timeout, + .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 +5699,13 @@ 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, + .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 a5ce04eed..4fa045ff4 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,18 @@ 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, + .table_id = 0, + .command = command, + .buffer_id = UINT32_MAX, + .out_port = OFPP_ANY, + .out_group = OFPG_ANY, + .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 0d57f8548..4ab83b9aa 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,24 @@ 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, + .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 +2925,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.20.1