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>

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

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
 --------------------

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_delete_all__(ofproto);
-    ofproto_group_mod_start(ofproto, &ogm);
-    ofproto_bump_tables_version(ofproto);
-    ofproto_group_mod_finish(ofproto, &ogm, NULL);
     ovs_mutex_unlock(&ofproto_mutex);
 }
 

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

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
+          OpenFlow flow tables, group table, and meter table.  If there are no
-          clears the flow table.  Other changes to the set of controllers, such
+          primary controllers, adding one also clears these tables.  Other
-          as adding or removing a service controller, adding another primary
+          changes to the set of controllers, such as adding or removing a
-          controller to supplement an existing primary controller, or removing
+          service controller, adding another primary controller to supplement
-          only one of two primary controllers, have no effect on the flow
+          an existing primary controller, or removing only one of two primary
-          table.
+          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>