Looks Good.
Ethan
On Tue, Apr 26, 2011 at 09:24, Ben Pfaff <b...@nicira.com> wrote:
> These new semantics are less efficient in the case where the flood_vlans
> actually changed, but that should be very rare.
>
> There are no advantages to this change on its own, but upcoming commits
> will add multiple layers between the code supplying the flood_vlans and
> actually calling mac_learning_set_flood_vlans(). Consistency in this
> multilayered interface seems valuable, and the rest of it does not transfer
> ownership from the caller to the callee.
> ---
> lib/bitmap.h | 8 +++++++-
> lib/mac-learning.c | 20 +++++++++++---------
> lib/mac-learning.h | 2 +-
> lib/vlan-bitmap.h | 7 +++++++
> vswitchd/bridge.c | 1 +
> 5 files changed, 27 insertions(+), 11 deletions(-)
>
> diff --git a/lib/bitmap.h b/lib/bitmap.h
> index fd05d3d..f6feff0 100644
> --- a/lib/bitmap.h
> +++ b/lib/bitmap.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> + * Copyright (c) 2008, 2009, 2010, 2011 Nicira Networks.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -53,6 +53,12 @@ bitmap_allocate(size_t n_bits)
> return xzalloc(bitmap_n_bytes(n_bits));
> }
>
> +static inline unsigned long *
> +bitmap_clone(const unsigned long *bitmap, size_t n_bits)
> +{
> + return xmemdup(bitmap, bitmap_n_bytes(n_bits));
> +}
> +
> static inline void
> bitmap_free(unsigned long *bitmap)
> {
> diff --git a/lib/mac-learning.c b/lib/mac-learning.c
> index a57bbc7..981578d 100644
> --- a/lib/mac-learning.c
> +++ b/lib/mac-learning.c
> @@ -139,17 +139,19 @@ mac_learning_destroy(struct mac_learning *ml)
> }
>
> /* Provides a bitmap of VLANs which have learning disabled, that is, VLANs on
> - * which all packets are flooded. It takes ownership of the bitmap. Returns
> - * true if the set has changed from the previous value. */
> + * which all packets are flooded. Returns true if the set has changed from
> the
> + * previous value. */
> bool
> -mac_learning_set_flood_vlans(struct mac_learning *ml, unsigned long *bitmap)
> +mac_learning_set_flood_vlans(struct mac_learning *ml,
> + const unsigned long *bitmap)
> {
> - bool ret = vlan_bitmap_equal(ml->flood_vlans, bitmap);
> -
> - bitmap_free(ml->flood_vlans);
> - ml->flood_vlans = bitmap;
> -
> - return ret;
> + if (vlan_bitmap_equal(ml->flood_vlans, bitmap)) {
> + return false;
> + } else {
> + bitmap_free(ml->flood_vlans);
> + ml->flood_vlans = vlan_bitmap_clone(bitmap);
> + return true;
> + }
> }
>
> static bool
> diff --git a/lib/mac-learning.h b/lib/mac-learning.h
> index 51a7ac7..d9fa433 100644
> --- a/lib/mac-learning.h
> +++ b/lib/mac-learning.h
> @@ -96,7 +96,7 @@ void mac_learning_wait(struct mac_learning *);
>
> /* Configuration. */
> bool mac_learning_set_flood_vlans(struct mac_learning *,
> - unsigned long *bitmap);
> + const unsigned long *bitmap);
>
> /* Learning. */
> bool mac_learning_may_learn(const struct mac_learning *,
> diff --git a/lib/vlan-bitmap.h b/lib/vlan-bitmap.h
> index eca42fe..6d74d40 100644
> --- a/lib/vlan-bitmap.h
> +++ b/lib/vlan-bitmap.h
> @@ -37,4 +37,11 @@ vlan_bitmap_contains(const unsigned long *vlans, uint16_t
> vid)
> return !vlans || bitmap_is_set(vlans, vid);
> }
>
> +/* Returns a new copy of 'vlans'. */
> +static inline unsigned long *
> +vlan_bitmap_clone(const unsigned long *vlans)
> +{
> + return vlans ? bitmap_clone(vlans, 4096) : NULL;
> +}
> +
> #endif /* lib/vlan-bitmap.h */
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index e3e5a4c..9e9566c 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -3621,6 +3621,7 @@ mirror_reconfigure(struct bridge *br)
> bridge_flush(br);
> mac_learning_flush(br->ml);
> }
> + free(rspan_vlans);
> }
>
> 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
