Looks good to me,
Thanks for writing this up. I'll drop the equivalent patch from my series.
Ethan
On Wed, Nov 30, 2011 at 12:09, Ben Pfaff <b...@nicira.com> wrote:
> The design intent is for LACP ports to use the datapath ID as the default
> system ID when none is specifically configured. However, the datapath ID
> is not available that early. This commit makes it available earlier.
>
> This commit does not fix another bug that prevents the LACP system ID from
> being set properly (nothing sets it at all, in fact, so it always uses 0).
>
> Build and unit tested only.
> ---
> vswitchd/bridge.c | 24 ++++++++++++++++++++++--
> 1 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index d2dcd02..ec40927 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -28,6 +28,7 @@
> #include "dynamic-string.h"
> #include "hash.h"
> #include "hmap.h"
> +#include "hmapx.h"
> #include "jsonrpc.h"
> #include "lacp.h"
> #include "list.h"
> @@ -444,6 +445,10 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> *ovs_cfg)
> HMAP_FOR_EACH (br, node, &all_bridges) {
> struct port *port;
>
> + /* We need the datapath ID early to allow LACP ports to use it as the
> + * default system ID. */
> + bridge_configure_datapath_id(br);
> +
> HMAP_FOR_EACH (port, hmap_node, &br->ports) {
> struct iface *iface;
>
> @@ -456,7 +461,6 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
> *ovs_cfg)
> }
> }
> bridge_configure_mirrors(br);
> - bridge_configure_datapath_id(br);
> bridge_configure_flow_eviction_threshold(br);
> bridge_configure_forward_bpdu(br);
> bridge_configure_remotes(br, managers, n_managers);
> @@ -1278,10 +1282,12 @@ static void
> bridge_pick_local_hw_addr(struct bridge *br, uint8_t ea[ETH_ADDR_LEN],
> struct iface **hw_addr_iface)
> {
> + struct hmapx mirror_output_ports;
> const char *hwaddr;
> struct port *port;
> bool found_addr = false;
> int error;
> + int i;
>
> *hw_addr_iface = NULL;
>
> @@ -1298,6 +1304,18 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t
> ea[ETH_ADDR_LEN],
> }
> }
>
> + /* Mirror output ports don't participate in picking the local hardware
> + * address. ofproto can't help us find out whether a given port is a
> + * mirror output because we haven't configured mirrors yet, so we need to
> + * accumulate them ourselves. */
> + hmapx_init(&mirror_output_ports);
> + for (i = 0; i < br->cfg->n_mirrors; i++) {
> + struct ovsrec_mirror *m = br->cfg->mirrors[i];
> + if (m->output_port) {
> + hmapx_add(&mirror_output_ports, m->output_port);
> + }
> + }
> +
> /* Otherwise choose the minimum non-local MAC address among all of the
> * interfaces. */
> HMAP_FOR_EACH (port, hmap_node, &br->ports) {
> @@ -1306,7 +1324,7 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t
> ea[ETH_ADDR_LEN],
> struct iface *iface;
>
> /* Mirror output ports don't participate. */
> - if (ofproto_is_mirror_output_bundle(br->ofproto, port)) {
> + if (hmapx_contains(&mirror_output_ports, port->cfg)) {
> continue;
> }
>
> @@ -1369,6 +1387,8 @@ bridge_pick_local_hw_addr(struct bridge *br, uint8_t
> ea[ETH_ADDR_LEN],
> VLOG_WARN("bridge %s: using default bridge Ethernet "
> "address "ETH_ADDR_FMT, br->name, ETH_ADDR_ARGS(ea));
> }
> +
> + hmapx_destroy(&mirror_output_ports);
> }
>
> /* Choose and returns the datapath ID for bridge 'br' given that the bridge
> --
> 1.7.2.5
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
