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 <periyasamy.palanisamy@ericsson.com>
Suggested-by: Jan Scheurich <jan.scheurich@ericsson.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Jan Scheurich <jan.scheurich@ericsson.com>
Tested-by: Jan Scheurich <jan.scheurich@ericsson.com>
Reviewed-by: Greg Rose <gvrose8192@gmail.com>
Tested-by: Greg Rose <gvrose8192@gmail.com>

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 695b778a96bd7c2aba76e5c6432a59dc97a4e825..0c89fc2e117193232f28d219b5bebe2fcda1d5d1 100644 (file)
--- 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 a7f2defea25dd7f69c90ae6c20e249b52261e120..7d62d8c84e450b95c7fe4f274c602320040e0874 100644 (file)
--- 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 84eb18e901edd6a519c4c506cddcbfd0859b2248..a09dbb2c413f8835f42e0bdc5c6e10debafb7ed6 100644 (file)
--- 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);
@@ -7365,20 +7368,30 @@ ofproto_group_mod_finish(struct ofproto *ofproto,
  *
  * This is intended for use within an ofproto provider's 'destruct'
  * function. */
-void
-ofproto_group_delete_all(struct ofproto *ofproto)
-    OVS_EXCLUDED(ofproto_mutex)
+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;
-
-    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);
+}
+
+/* Delete all groups from 'ofproto'.
+ *
+ * This is intended for use within an ofproto provider's 'destruct'
+ * function. */
+void
+ofproto_group_delete_all(struct ofproto *ofproto)
+    OVS_EXCLUDED(ofproto_mutex)
+{
+    ovs_mutex_lock(&ofproto_mutex);
+    ofproto_group_delete_all__(ofproto);
     ovs_mutex_unlock(&ofproto_mutex);
 }
 

diff --git a/tests/ofproto.at b/tests/ofproto.at
index 74a30cbefe3bf063e727527b6847f6d836e99dbc..5d90a21a0e66a646e2555bbcc41c781050f7a049 100644 (file)
--- 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:<invalid>: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:<invalid2>: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(["/<invalid/d"])
+AT_CLEANUP

diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 37d04b7cf9a4fe4dca4d723c2acd5c5728d014ce..58c0ebd4b206aebf985f039a95b26e7e35190ee7 100644 (file)
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -766,12 +766,12 @@
 
         <p>
           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.
         </p>
       </column>
 
@@ -821,7 +821,8 @@
         configured controllers can be contacted.</p>
         <p>
           Changing <ref column="fail_mode"/> when no primary controllers are
-          configured clears the flow table.
+          configured clears the OpenFlow flow tables, group table, and meter
+          table.
         </p>
       </column>