[ovs-dev] [next 02/35] bridge: Move logic for flushing flows and standalone mode into connmgr.

Ethan Jackson ethan at nicira.com
Mon May 2 22:11:50 UTC 2011


I'm noticing now. this patch does not make check.  It looks like the
testsuite wasn't updated in a recent rebase.  The HEAD of the branch
does appear to make check though so I'm not sure.

Ethan

On Tue, Apr 26, 2011 at 09:24, Ben Pfaff <blp at nicira.com> wrote:
> This improves the abstraction behind ofproto and connmgr.
>
> Some of this could even go into fail_open, but I'm not sure that it would
> make anything easier to understand.
> ---
>  ofproto/connmgr.c |   30 ++++++++++++++++++++++++++++--
>  ofproto/ofproto.c |    6 ------
>  ofproto/ofproto.h |    1 -
>  vswitchd/bridge.c |   32 --------------------------------
>  4 files changed, 28 insertions(+), 41 deletions(-)
>
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index 83b3159..b27fe94 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -390,6 +390,7 @@ connmgr_set_controllers(struct connmgr *mgr,
>                         const struct ofproto_controller *controllers,
>                         size_t n_controllers)
>  {
> +    bool had_controllers = connmgr_has_controllers(mgr);
>     struct shash new_controllers;
>     struct ofconn *ofconn, *next_ofconn;
>     struct ofservice *ofservice, *next_ofservice;
> @@ -451,6 +452,9 @@ connmgr_set_controllers(struct connmgr *mgr,
>
>     update_in_band_remotes(mgr);
>     update_fail_open(mgr);
> +    if (had_controllers != connmgr_has_controllers(mgr)) {
> +        ofproto_flush_flows(mgr->ofproto);
> +    }
>  }
>
>  /* Drops the connections between 'mgr' and all of its primary and secondary
> @@ -1081,8 +1085,13 @@ connmgr_get_fail_mode(const struct connmgr *mgr)
>  void
>  connmgr_set_fail_mode(struct connmgr *mgr, enum ofproto_fail_mode fail_mode)
>  {
> -    mgr->fail_mode = fail_mode;
> -    update_fail_open(mgr);
> +    if (mgr->fail_mode != fail_mode) {
> +        mgr->fail_mode = fail_mode;
> +        update_fail_open(mgr);
> +        if (!connmgr_has_controllers(mgr)) {
> +            ofproto_flush_flows(mgr->ofproto);
> +        }
> +    }
>  }
>
>  /* Fail-open implementation. */
> @@ -1265,6 +1274,23 @@ connmgr_flushed(struct connmgr *mgr)
>     if (mgr->fail_open) {
>         fail_open_flushed(mgr->fail_open);
>     }
> +
> +    /* If there are no controllers and we're in standalone mode, set up a flow
> +     * that matches every packet and directs them to OFPP_NORMAL (which goes to
> +     * us).  Otherwise, the switch is in secure mode and we won't pass any
> +     * traffic until a controller has been defined and it tells us to do so. */
> +    if (!connmgr_has_controllers(mgr)
> +        && mgr->fail_mode == OFPROTO_FAIL_STANDALONE) {
> +        union ofp_action action;
> +        struct cls_rule rule;
> +
> +        memset(&action, 0, sizeof action);
> +        action.type = htons(OFPAT_OUTPUT);
> +        action.output.len = htons(sizeof action);
> +        action.output.port = htons(OFPP_NORMAL);
> +        cls_rule_init_catchall(&rule, 0);
> +        ofproto_add_flow(mgr->ofproto, &rule, &action, 1);
> +    }
>  }
>
>  /* Creates a new ofservice for 'target' in 'mgr'.  Returns 0 if successful,
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index fb63606..6745a49 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -657,12 +657,6 @@ ofproto_get_datapath_id(const struct ofproto *ofproto)
>     return ofproto->datapath_id;
>  }
>
> -bool
> -ofproto_has_primary_controller(const struct ofproto *ofproto)
> -{
> -    return connmgr_has_controllers(ofproto->connmgr);
> -}
> -
>  enum ofproto_fail_mode
>  ofproto_get_fail_mode(const struct ofproto *p)
>  {
> diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
> index 9a56bee..d999b8d 100644
> --- a/ofproto/ofproto.h
> +++ b/ofproto/ofproto.h
> @@ -134,7 +134,6 @@ const struct cfm *ofproto_iface_get_cfm(struct ofproto *, uint32_t port_no);
>
>  /* Configuration querying. */
>  uint64_t ofproto_get_datapath_id(const struct ofproto *);
> -bool ofproto_has_primary_controller(const struct ofproto *);
>  enum ofproto_fail_mode ofproto_get_fail_mode(const struct ofproto *);
>  void ofproto_get_listeners(const struct ofproto *, struct sset *);
>  bool ofproto_has_snoops(const struct ofproto *);
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 20ecca3..eb10cf0 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1923,16 +1923,8 @@ bridge_reconfigure_one(struct bridge *br)
>                 || !strcmp(br->cfg->fail_mode, "standalone")
>                     ? OFPROTO_FAIL_STANDALONE
>                     : OFPROTO_FAIL_SECURE;
> -    if (ofproto_get_fail_mode(br->ofproto) != fail_mode
> -        && !ofproto_has_primary_controller(br->ofproto)) {
> -        ofproto_flush_flows(br->ofproto);
> -    }
>     ofproto_set_fail_mode(br->ofproto, fail_mode);
>
> -    /* Delete all flows if we're switching from connected to standalone or vice
> -     * versa.  (XXX Should we delete all flows if we are switching from one
> -     * controller to another?) */
> -
>     /* Configure OpenFlow controller connection snooping. */
>     if (!ofproto_has_snoops(br->ofproto)) {
>         struct sset snoops;
> @@ -2033,7 +2025,6 @@ bridge_reconfigure_remotes(struct bridge *br,
>
>     struct ovsrec_controller **controllers;
>     size_t n_controllers;
> -    bool had_primary;
>
>     struct ofproto_controller *ocs;
>     size_t n_ocs;
> @@ -2055,7 +2046,6 @@ bridge_reconfigure_remotes(struct bridge *br,
>     } else {
>         ofproto_set_extra_in_band_remotes(br->ofproto, managers, n_managers);
>     }
> -    had_primary = ofproto_has_primary_controller(br->ofproto);
>
>     n_controllers = bridge_get_controllers(br, &controllers);
>
> @@ -2089,28 +2079,6 @@ bridge_reconfigure_remotes(struct bridge *br,
>     ofproto_set_controllers(br->ofproto, ocs, n_ocs);
>     free(ocs[0].target); /* From bridge_ofproto_controller_for_mgmt(). */
>     free(ocs);
> -
> -    if (had_primary != ofproto_has_primary_controller(br->ofproto)) {
> -        ofproto_flush_flows(br->ofproto);
> -    }
> -
> -    /* If there are no controllers and the bridge is in standalone
> -     * mode, set up a flow that matches every packet and directs
> -     * them to OFPP_NORMAL (which goes to us).  Otherwise, the
> -     * switch is in secure mode and we won't pass any traffic until
> -     * a controller has been defined and it tells us to do so. */
> -    if (!n_controllers
> -        && ofproto_get_fail_mode(br->ofproto) == OFPROTO_FAIL_STANDALONE) {
> -        union ofp_action action;
> -        struct cls_rule rule;
> -
> -        memset(&action, 0, sizeof action);
> -        action.type = htons(OFPAT_OUTPUT);
> -        action.output.len = htons(sizeof action);
> -        action.output.port = htons(OFPP_NORMAL);
> -        cls_rule_init_catchall(&rule, 0);
> -        ofproto_add_flow(br->ofproto, &rule, &action, 1);
> -    }
>  }
>
>  static void
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list