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 <b...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
