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-boun...@openvswitch.org > > [mailto:ovs-dev-boun...@openvswitch.org] On Behalf Of Ben Pfaff > > Sent: Tuesday, 07 November, 2017 05:56 > > To: d...@openvswitch.org > > Cc: Ben Pfaff <b...@ovn.org>; Periyasamy Palanisamy > > <periyasamy.palanis...@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.palanis...@ericsson.com> > > Signed-off-by: Ben Pfaff <b...@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 pa...@iki.fi > > Patrik Andersson R patrik.r.anders...@ericsson.com > > Paulo Cravero pcrav...@as2594.net > > Pawan Shukla shuk...@vmware.com > > +Periyasamy Palanisamy periyasamy.palanis...@ericsson.com > > Peter Amidon pe...@picnicpark.org > > Peter Balland pe...@nicira.com > > Peter Phaal peter.ph...@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 > > d...@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev