On Tue, 2017-04-25 at 17:02 -0700, Andy Zhou wrote:
> vswitchd can gracefully shutdown after receiving the 'appctl exit'
> command.  But the datapath resources vswitchd created can
> linger on. This is not a problem, and in fact desireable when the
> shutdown is transient in nature. Since restarted vswitchd usually
> sees a similar configuration. By keeping the datapath resources
> minimize the perturbation to the running data traffic.
> 
> However, when vswitchd shutdown is permanent, there should be a
> way to release the all datapath resources, so that they won't
> be linger on forever.  Add 'appctl quit' command for such purpose.

Otherwise looks fine to me but s/quiting/quitting/

Thanks,

- Greg

> 
> Signed-off-by: Andy Zhou <[email protected]>
> ---
>  NEWS                       |  1 +
>  ofproto/ofproto-dpif.c     | 11 +++++++----
>  ofproto/ofproto-provider.h |  2 +-
>  ofproto/ofproto.c          |  2 +-
>  vswitchd/bridge.c          |  4 ++--
>  vswitchd/bridge.h          |  4 +++-
>  vswitchd/ovs-vswitchd.8.in |  3 +++
>  vswitchd/ovs-vswitchd.c    |  7 ++++---
>  8 files changed, 22 insertions(+), 12 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index ea97d84a2dea..3f65654f5124 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -26,6 +26,7 @@ Post-v2.7.0
>       * Bundles now support hashing by just nw_src or nw_dst.
>       * The "learn" action now supports a "limit" option (see ovs-ofctl(8)).
>       * The port status bit OFPPS_LIVE now reflects link aliveness.
> +   - Add the command 'ovs-appctl quit' (see ovs-vswitchd(8)).
>  
>  v2.7.0 - 21 Feb 2017
>  ---------------------
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index c73c2738c91c..bd2eaa60d36b 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -645,7 +645,7 @@ dealloc(struct ofproto *ofproto_)
>  }
>  
>  static void
> -close_dpif_backer(struct dpif_backer *backer)
> +close_dpif_backer(struct dpif_backer *backer, bool del)
>  {
>      ovs_assert(backer->refcount > 0);
>  
> @@ -661,6 +661,9 @@ close_dpif_backer(struct dpif_backer *backer)
>      shash_find_and_delete(&all_dpif_backers, backer->type);
>      free(backer->type);
>      free(backer->dp_version_string);
> +    if (del) {
> +        dpif_delete(backer->dpif);
> +    }
>      dpif_close(backer->dpif);
>      free(backer);
>  }
> @@ -772,7 +775,7 @@ open_dpif_backer(const char *type, struct dpif_backer 
> **backerp)
>      if (error) {
>          VLOG_ERR("failed to listen on datapath of type %s: %s",
>                   type, ovs_strerror(error));
> -        close_dpif_backer(backer);
> +        close_dpif_backer(backer, false);
>          return error;
>      }
>  
> @@ -1452,7 +1455,7 @@ add_internal_flows(struct ofproto_dpif *ofproto)
>  }
>  
>  static void
> -destruct(struct ofproto *ofproto_)
> +destruct(struct ofproto *ofproto_, bool del)
>  {
>      struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
>      struct ofproto_async_msg *am;
> @@ -1505,7 +1508,7 @@ destruct(struct ofproto *ofproto_)
>  
>      seq_destroy(ofproto->ams_seq);
>  
> -    close_dpif_backer(ofproto->backer);
> +    close_dpif_backer(ofproto->backer, del);
>  }
>  
>  static int
> diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
> index b7b12cdfd5f4..ef993d0afc4d 100644
> --- a/ofproto/ofproto-provider.h
> +++ b/ofproto/ofproto-provider.h
> @@ -828,7 +828,7 @@ struct ofproto_class {
>       */
>      struct ofproto *(*alloc)(void);
>      int (*construct)(struct ofproto *ofproto);
> -    void (*destruct)(struct ofproto *ofproto);
> +    void (*destruct)(struct ofproto *ofproto, bool del);
>      void (*dealloc)(struct ofproto *ofproto);
>  
>      /* Performs any periodic activity required by 'ofproto'.  It should:
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index ca0f3e49bd67..7bc7b7f99d0d 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -1651,7 +1651,7 @@ ofproto_destroy(struct ofproto *p, bool del)
>          free(usage);
>      }
>  
> -    p->ofproto_class->destruct(p);
> +    p->ofproto_class->destruct(p, del);
>  
>      /* We should not postpone this because it involves deleting a listening
>       * socket which we may want to reopen soon. 'connmgr' may be used by 
> other
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index ebb6249416fa..e741f34f19ec 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -496,14 +496,14 @@ bridge_init(const char *remote)
>  }
>  
>  void
> -bridge_exit(void)
> +bridge_exit(bool delete_datpath)
>  {
>      struct bridge *br, *next_br;
>  
>      if_notifier_destroy(ifnotifier);
>      seq_destroy(ifaces_changed);
>      HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
> -        bridge_destroy(br, false);
> +        bridge_destroy(br, delete_datpath);
>      }
>      ovsdb_idl_destroy(idl);
>  }
> diff --git a/vswitchd/bridge.h b/vswitchd/bridge.h
> index 3783a21e3b4c..7835611568cf 100644
> --- a/vswitchd/bridge.h
> +++ b/vswitchd/bridge.h
> @@ -16,10 +16,12 @@
>  #ifndef VSWITCHD_BRIDGE_H
>  #define VSWITCHD_BRIDGE_H 1
>  
> +#include <stdbool.h>
> +
>  struct simap;
>  
>  void bridge_init(const char *remote);
> -void bridge_exit(void);
> +void bridge_exit(bool delete_datpath);
>  
>  void bridge_run(void);
>  void bridge_wait(void);
> diff --git a/vswitchd/ovs-vswitchd.8.in b/vswitchd/ovs-vswitchd.8.in
> index 8496aa68af97..bb855861992b 100644
> --- a/vswitchd/ovs-vswitchd.8.in
> +++ b/vswitchd/ovs-vswitchd.8.in
> @@ -111,6 +111,9 @@ how to configure Open vSwitch.
>  .SS "GENERAL COMMANDS"
>  .IP "\fBexit\fR"
>  Causes \fBovs\-vswitchd\fR to gracefully terminate.
> +.IP "\fBquit\fR"
> +Similar to "\fBexit\fR", but also release datapath resources.
> +.
>  .IP "\fBqos/show-types\fR \fIinterface\fR"
>  Queries the interface for a list of Quality of Service types that are
>  configurable via Open vSwitch for the given \fIinterface\fR.
> diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c
> index bed3fb5c374d..10953bb1bd47 100644
> --- a/vswitchd/ovs-vswitchd.c
> +++ b/vswitchd/ovs-vswitchd.c
> @@ -66,7 +66,7 @@ main(int argc, char *argv[])
>      char *unixctl_path = NULL;
>      struct unixctl_server *unixctl;
>      char *remote;
> -    bool exiting;
> +    bool exiting, quiting;
>      int retval;
>  
>      set_program_name(argv[0]);
> @@ -93,6 +93,7 @@ main(int argc, char *argv[])
>          exit(EXIT_FAILURE);
>      }
>      unixctl_command_register("exit", "", 0, 0, ovs_vswitchd_exit, &exiting);
> +    unixctl_command_register("quit", "", 0, 0, ovs_vswitchd_exit, &quiting);
>  
>      bridge_init(remote);
>      free(remote);
> @@ -120,11 +121,11 @@ main(int argc, char *argv[])
>              poll_immediate_wake();
>          }
>          poll_block();
> -        if (should_service_stop()) {
> +        if (quiting || should_service_stop()) {
>              exiting = true;
>          }
>      }
> -    bridge_exit();
> +    bridge_exit(quiting);
>      unixctl_server_destroy(unixctl);
>      service_stop();
>  



_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to