[ovs-dev] [PATCH 2/2] in-band: Delete remaining rules when disabling in-band control.

Ethan Jackson ethan at nicira.com
Wed Aug 3 23:03:15 UTC 2011


The series looks good to me.

Ethan

On Wed, Aug 3, 2011 at 15:01, Ben Pfaff <blp at nicira.com> wrote:
> in_band_destroy() doesn't remove all of the rules that in-band control
> adds (and it cannot, because that might require waiting for an existing
> asynchronous flow modification or addition to complete), so turning on
> other-config:disable-in-band or deleting all of the OpenFlow controllers
> did not delete all of the in-band rules.
>
> This commit fixes the problem by making the in-band control object hang
> around until all of the flows that it set up have actually been deleted.
>
> This problem was introduced as part of commit 7ee20df "ofproto: Implement
> asynchronous OFPT_FLOW_MOD commands."
>
> Reported-by: Brad Hall <brad at nicira.com>
> ---
>  ofproto/connmgr.c |   15 +++++++++------
>  ofproto/in-band.c |   11 +++++++++--
>  ofproto/in-band.h |    2 +-
>  3 files changed, 19 insertions(+), 9 deletions(-)
>
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index 865fa29..38052ac 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -240,7 +240,10 @@ connmgr_run(struct connmgr *mgr,
>     size_t i;
>
>     if (handle_openflow && mgr->in_band) {
> -        in_band_run(mgr->in_band);
> +        if (!in_band_run(mgr->in_band)) {
> +            in_band_destroy(mgr->in_band);
> +            mgr->in_band = NULL;
> +        }
>     }
>
>     LIST_FOR_EACH_SAFE (ofconn, next_ofconn, node, &mgr->all_conns) {
> @@ -604,13 +607,13 @@ update_in_band_remotes(struct connmgr *mgr)
>         if (!mgr->in_band) {
>             in_band_create(mgr->ofproto, mgr->local_port_name, &mgr->in_band);
>         }
> -        if (mgr->in_band) {
> -            in_band_set_remotes(mgr->in_band, addrs, n_addrs);
> -        }
>         in_band_set_queue(mgr->in_band, mgr->in_band_queue);
>     } else {
> -        in_band_destroy(mgr->in_band);
> -        mgr->in_band = NULL;
> +        /* in_band_run() needs a chance to delete any existing in-band flows.
> +         * We will destroy mgr->in_band after it's done with that. */
> +    }
> +    if (mgr->in_band) {
> +        in_band_set_remotes(mgr->in_band, addrs, n_addrs);
>     }
>
>     /* Clean up. */
> diff --git a/ofproto/in-band.c b/ofproto/in-band.c
> index ae1f1b1..764b252 100644
> --- a/ofproto/in-band.c
> +++ b/ofproto/in-band.c
> @@ -310,7 +310,7 @@ update_rules(struct in_band *ib)
>         ib_rule->op = DELETE;
>     }
>
> -    if (!eth_addr_is_zero(ib->local_mac)) {
> +    if (ib->n_remotes && !eth_addr_is_zero(ib->local_mac)) {
>         /* (a) Allow DHCP requests sent from the local port. */
>         cls_rule_init_catchall(&rule, IBR_FROM_LOCAL_DHCP);
>         cls_rule_set_in_port(&rule, ODPP_LOCAL);
> @@ -395,7 +395,12 @@ update_rules(struct in_band *ib)
>     }
>  }
>
> -void
> +/* Updates the OpenFlow flow table for the current state of in-band control.
> + * Returns true ordinarily.  Returns false if no remotes are configured on 'ib'
> + * and 'ib' doesn't have any rules left to remove from the OpenFlow flow
> + * table.  Thus, a false return value means that the caller can destroy 'ib'
> + * without leaving extra flows hanging around in the flow table. */
> +bool
>  in_band_run(struct in_band *ib)
>  {
>     struct {
> @@ -446,6 +451,8 @@ in_band_run(struct in_band *ib)
>             break;
>         }
>     }
> +
> +    return ib->n_remotes || !hmap_is_empty(&ib->rules);
>  }
>
>  void
> diff --git a/ofproto/in-band.h b/ofproto/in-band.h
> index e2d8e80..f7f2ec6 100644
> --- a/ofproto/in-band.h
> +++ b/ofproto/in-band.h
> @@ -35,7 +35,7 @@ void in_band_set_queue(struct in_band *, int queue_id);
>  void in_band_set_remotes(struct in_band *,
>                          const struct sockaddr_in *, size_t n);
>
> -void in_band_run(struct in_band *);
> +bool in_band_run(struct in_band *);
>  void in_band_wait(struct in_band *);
>
>  bool in_band_msg_in_hook(struct in_band *, const struct flow *,
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>



More information about the dev mailing list