[ovs-dev] [PATCH] ofproto: Delete all groups when (un)configuring a controller.

Ben Pfaff blp at ovn.org
Wed Nov 8 03:04:30 UTC 2017


That's a good idea.  I sent v2:
        https://patchwork.ozlabs.org/patch/835563/

On Tue, Nov 07, 2017 at 02:37:01PM +0000, Jan Scheurich wrote:
> Hi Ben,
> 
> Do you think we should also delete any OpenFlow meters in this case?
> 
> Regards, Jan
> 
> > -----Original Message-----
> > From: ovs-dev-bounces at openvswitch.org [mailto:ovs-dev-bounces at openvswitch.org] On Behalf Of Ben Pfaff
> > Sent: Tuesday, 07 November, 2017 05:56
> > To: dev at openvswitch.org
> > Cc: Ben Pfaff <blp at ovn.org>; Periyasamy Palanisamy <periyasamy.palanisamy at ericsson.com>
> > Subject: [ovs-dev] [PATCH] ofproto: Delete all groups 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.
> > 
> > Suggested-by: Periyasamy Palanisamy <periyasamy.palanisamy at ericsson.com>
> > Signed-off-by: Ben Pfaff <blp at ovn.org>
> > ---
> >  AUTHORS.rst          |  1 +
> >  NEWS                 |  2 ++
> >  ofproto/ofproto.c    | 25 +++++++++++++++++++------
> >  tests/ofproto.at     | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  vswitchd/vswitch.xml | 14 +++++++-------
> >  5 files changed, 77 insertions(+), 13 deletions(-)
> > 
> > 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 at iki.fi
> >  Patrik Andersson R              patrik.r.andersson at ericsson.com
> >  Paulo Cravero                   pcravero at as2594.net
> >  Pawan Shukla                    shuklap at vmware.com
> > +Periyasamy Palanisamy           periyasamy.palanisamy at ericsson.com
> >  Peter Amidon                    peter at picnicpark.org
> >  Peter Balland                   peter at nicira.com
> >  Peter Phaal                     peter.phaal at inmon.com
> > diff --git a/NEWS b/NEWS
> > index 047f34b9f402..e6bd71c31fdb 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,5 +1,7 @@
> >  Post-v2.8.0
> >  --------------------
> > +   - ovs-vswitchd:
> > +     * Configuring a controller now deletes all groups (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..3de1f6e7d20f 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,7 @@ ofproto_flush__(struct ofproto *ofproto)
> >          }
> >          delete_flows__(&rules, OFPRR_DELETE, NULL);
> >      }
> > +    ofproto_group_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 +7351,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..cb9471494916 100644
> > --- a/tests/ofproto.at
> > +++ b/tests/ofproto.at
> > @@ -6023,3 +6023,51 @@ OVS_VSWITCHD_STOP(["/NXFMFC_INVALID_TLV_FIELD/d
> >  /tun_metadata0/d
> >  /OFPBAC_BAD_SET_LEN/d"])
> >  AT_CLEANUP
> > +
> > +AT_SETUP([ofproto - flush flows and groups for controller change])
> > +OVS_VSWITCHD_START
> > +
> > +add_flow_and_group () {
> > +    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
> > +])
> > +}
> > +
> > +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
> > +])
> > +}
> > +
> > +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):
> > +])
> > +}
> > +
> > +# Add flow and group and check that they're there, without a controller.
> > +add_flow_and_group
> > +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_and_group
> > +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..e35b8a0c47f3 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 and group table.  If there are no primary
> > +          controllers, adding one also clears the flow tables and group 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 tables or group table.
> >          </p>
> >        </column>
> > 
> > @@ -806,7 +806,7 @@
> >          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 and group table.
> >          </p>
> >        </column>
> > 
> > --
> > 2.10.2
> > 
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list