From d3b8483300532167ece1307252b441de24f07dfa Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 8 Jan 2018 13:13:34 -0800 Subject: [PATCH] ofproto: Delete all groups and meters when (un)configuring a controller. Open vSwitch has always deleted all flows from the flow table whenever a controller is configured or whenever all the controllers are unconfigured. After this commit, OVS additionally deletes all OpenFlow groups and meters. Suggested-by: Periyasamy Palanisamy Suggested-by: Jan Scheurich Signed-off-by: Ben Pfaff Acked-by: Jan Scheurich Tested-by: Jan Scheurich Reviewed-by: Greg Rose Tested-by: Greg Rose --- AUTHORS.rst | 1 + NEWS | 2 ++ ofproto/ofproto.c | 37 +++++++++++++++++++--------- tests/ofproto.at | 58 ++++++++++++++++++++++++++++++++++++++++++++ vswitchd/vswitch.xml | 15 ++++++------ 5 files changed, 94 insertions(+), 19 deletions(-) diff --git a/AUTHORS.rst b/AUTHORS.rst index 695b778a9..0c89fc2e1 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -524,6 +524,7 @@ Pasi Kärkkäinen pasik@iki.fi Patrik Andersson R patrik.r.andersson@ericsson.com Paulo Cravero pcravero@as2594.net Pawan Shukla shuklap@vmware.com +Periyasamy Palanisamy periyasamy.palanisamy@ericsson.com Peter Amidon peter@picnicpark.org Peter Balland peter@nicira.com Peter Phaal peter.phaal@inmon.com diff --git a/NEWS b/NEWS index a7f2defea..7d62d8c84 100644 --- a/NEWS +++ b/NEWS @@ -31,6 +31,8 @@ Post-v2.8.0 * All the netdev-dpdk appctl commands described in ovs-vswitchd man page. - vswitchd: * Datapath IDs may now be specified as 0x1 (etc.) instead of 16 digits. + * Configuring a controller, or unconfiguring all controllers, now deletes + all groups and meters (as well as all flows). v2.8.0 - 31 Aug 2017 -------------------- diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c index 84eb18e90..a09dbb2c4 100644 --- a/ofproto/ofproto.c +++ b/ofproto/ofproto.c @@ -251,6 +251,8 @@ static void delete_flows__(struct rule_collection *, const struct openflow_mod_requester *) OVS_REQUIRES(ofproto_mutex); +static void ofproto_group_delete_all__(struct ofproto *) + OVS_REQUIRES(ofproto_mutex); static bool ofproto_group_exists(const struct ofproto *, uint32_t group_id); static void handle_openflow(struct ofconn *, const struct ofpbuf *); static enum ofperr ofproto_flow_mod_init(struct ofproto *, @@ -1566,6 +1568,8 @@ ofproto_flush__(struct ofproto *ofproto) } delete_flows__(&rules, OFPRR_DELETE, NULL); } + ofproto_group_delete_all__(ofproto); + meter_delete_all(ofproto); /* XXX: Concurrent handler threads may insert new learned flows based on * learn actions of the now deleted flows right after we release * 'ofproto_mutex'. */ @@ -1599,6 +1603,8 @@ ofproto_destroy__(struct ofproto *ofproto) } free(ofproto->tables); + hmap_destroy(&ofproto->meters); + ovs_mutex_lock(&ofproto->vl_mff_map.mutex); mf_vl_mff_map_clear(&ofproto->vl_mff_map, true); ovs_mutex_unlock(&ofproto->vl_mff_map.mutex); @@ -1637,9 +1643,6 @@ ofproto_destroy(struct ofproto *p, bool del) return; } - meter_delete_all(p); - hmap_destroy(&p->meters); - ofproto_flush__(p); HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, &p->ports) { ofport_destroy(ofport, del); @@ -7361,6 +7364,24 @@ ofproto_group_mod_finish(struct ofproto *ofproto, } } +/* Delete all groups from 'ofproto'. + * + * This is intended for use within an ofproto provider's 'destruct' + * function. */ +static void +ofproto_group_delete_all__(struct ofproto *ofproto) + OVS_REQUIRES(ofproto_mutex) +{ + struct ofproto_group_mod ogm; + ogm.gm.command = OFPGC11_DELETE; + ogm.gm.group_id = OFPG_ALL; + ogm.version = ofproto->tables_version + 1; + + ofproto_group_mod_start(ofproto, &ogm); + ofproto_bump_tables_version(ofproto); + ofproto_group_mod_finish(ofproto, &ogm, NULL); +} + /* Delete all groups from 'ofproto'. * * This is intended for use within an ofproto provider's 'destruct' @@ -7369,16 +7390,8 @@ void ofproto_group_delete_all(struct ofproto *ofproto) OVS_EXCLUDED(ofproto_mutex) { - struct ofproto_group_mod ogm; - - ogm.gm.command = OFPGC11_DELETE; - ogm.gm.group_id = OFPG_ALL; - ovs_mutex_lock(&ofproto_mutex); - ogm.version = ofproto->tables_version + 1; - ofproto_group_mod_start(ofproto, &ogm); - ofproto_bump_tables_version(ofproto); - ofproto_group_mod_finish(ofproto, &ogm, NULL); + ofproto_group_delete_all__(ofproto); ovs_mutex_unlock(&ofproto_mutex); } diff --git a/tests/ofproto.at b/tests/ofproto.at index 74a30cbef..5d90a21a0 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -6050,3 +6050,61 @@ OVS_VSWITCHD_STOP(["/NXFMFC_INVALID_TLV_FIELD/d /tun_metadata0/d /OFPBAC_BAD_SET_LEN/d"]) AT_CLEANUP + +AT_SETUP([ofproto - flush flows, groups, and meters for controller change]) +AT_KEYWORDS([flow flows group group meter]) +OVS_VSWITCHD_START + +add_flow_group_and_meter () { + AT_CHECK([ovs-ofctl add-flow br0 in_port=1,actions=2]) + AT_CHECK([ovs-ofctl -O OpenFlow11 add-group br0 group_id=1234,type=all,bucket=output:10 + AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=1 pktps burst stats bands=type=drop rate=1 burst_size=1']) +]) +} + +verify_added () { + AT_CHECK([ovs-ofctl --no-stats dump-flows br0], [0], [dnl + in_port=1 actions=output:2 +]) + AT_CHECK([ovs-ofctl -O OpenFlow11 dump-groups br0], [0], [dnl +OFPST_GROUP_DESC reply (OF1.1) (xid=0x2): + group_id=1234,type=all,bucket=actions=output:10 +]) + AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0], [0], [dnl +OFPST_METER_CONFIG reply (OF1.3) (xid=0x2): +meter=1 pktps burst stats bands= +type=drop rate=1 burst_size=1 +]) +} + +verify_deleted () { + AT_CHECK([ovs-ofctl --no-stats dump-flows br0]) + AT_CHECK([ovs-ofctl -O OpenFlow11 dump-groups br0], [0], [dnl +OFPST_GROUP_DESC reply (OF1.1) (xid=0x2): +]) + AT_CHECK([ovs-ofctl -O OpenFlow13 dump-meters br0], [0], [dnl +OFPST_METER_CONFIG reply (OF1.3) (xid=0x2): +]) +} + +# Add flow, group, meter and check that they're there, without a controller. +add_flow_group_and_meter +verify_added + +# Set up a controller and verify that the flow and group were deleted, +# then add them back. +AT_CHECK([ovs-vsctl set-controller br0 'tcp::6653']) +verify_deleted +add_flow_group_and_meter +verify_added + +# Change the controller and verify that the flow and group are still there. +AT_CHECK([ovs-vsctl set-controller br0 'tcp::6653']) +verify_added + +# Clear the controller and verify that the flow and group were deleted. +AT_CHECK([ovs-vsctl del-controller br0]) +verify_deleted + +OVS_VSWITCHD_STOP(["/ If there are primary controllers, removing all of them clears the - flow table. If there are no primary controllers, adding one also - clears the flow table. Other changes to the set of controllers, such - as adding or removing a service controller, adding another primary - controller to supplement an existing primary controller, or removing - only one of two primary controllers, have no effect on the flow - table. + OpenFlow flow tables, group table, and meter table. If there are no + primary controllers, adding one also clears these tables. Other + changes to the set of controllers, such as adding or removing a + service controller, adding another primary controller to supplement + an existing primary controller, or removing only one of two primary + controllers, have no effect on these tables.

@@ -821,7 +821,8 @@ configured controllers can be contacted.

Changing when no primary controllers are - configured clears the flow table. + configured clears the OpenFlow flow tables, group table, and meter + table.