[ovs-dev,v2] ofproto: Delete all groups and meters when (un)configuring a controller.

Message ID 20171108030402.26837-1-blp@ovn.org
State New
Headers show
Series
  • [ovs-dev,v2] ofproto: Delete all groups and meters when (un)configuring a controller.
Related show

Commit Message

Ben Pfaff Nov. 8, 2017, 3:04 a.m.
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>
---
v1->v2: Clear the meter table too (thanks Jan).

 AUTHORS.rst          |  1 +
 NEWS                 |  3 +++
 ofproto/ofproto.c    | 26 +++++++++++++++++------
 tests/ofproto.at     | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 vswitchd/vswitch.xml | 15 +++++++-------
 5 files changed, 90 insertions(+), 13 deletions(-)

Patch

diff --git a/AUTHORS.rst b/AUTHORS.rst
index 139e99b330d8..26f81508d3d5 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -519,6 +519,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 047f34b9f402..7ef4d6728d28 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,8 @@ 
 Post-v2.8.0
 --------------------
+   - ovs-vswitchd:
+     * Configuring a controller, or unconfiguring all controllers, now deletes
+       all groups and meters (as well as all flows).
    - OVN:
      * The "requested-chassis" option for a logical switch port now accepts a
        chassis "hostname" in addition to a chassis "name".
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 82c2bb27d348..e762888b746e 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'. */
@@ -7348,20 +7352,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 31cb5208fc1c..f51fd7e97859 100644
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -6023,3 +6023,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 conroller 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 d7f68393b25e..a12ec0f81545 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -751,12 +751,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>
 
@@ -806,7 +806,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>