On Dec 10, 2013, at 11:20 PM, Ben Pfaff <[email protected]> wrote: > This greatly simplifies the reconfiguration code, making it much easier > to understand and modify. The old multi-pass configuration had the > property that it didn't delay block packet processing as much, but that's > not much of a worry anymore now that latency critical activities have > been moved outside the main thread. >
You’ll find two minor comments below, but otherwise seems good to me. I have no history with this code, though, so with that caveat: Acked-by: Jarno Rajahalme <[email protected]> … > +static void > +bridge_delete_ports(struct bridge *br) IMO this function should be renamed, as it only deletes ports that should not exists, are of wrong type, or cannot be successfully configured. How about “bridge_conf_or_del_ports” etc.? > +{ > + struct ofproto_port ofproto_port; > + struct ofproto_port_dump dump; > > - /* Remove ports from any datapath with the same name as 'br'. If we > - * don't do this, creating 'br''s ofproto will fail because a port > with > - * the same name as its local port already exists. */ > - HMAP_FOR_EACH (br2, node, &all_bridges) { > - struct ofproto_port ofproto_port; > + ofp_port_t *del; > + size_t n, allocated; > + size_t i; > + … > @@ -2824,44 +2625,23 @@ bridge_get_controllers(const struct bridge *br, > } > > static void > -bridge_queue_if_cfg(struct bridge *br, > - const struct ovsrec_interface *cfg, > - const struct ovsrec_port *parent) > +bridge_collect_wanted_ports(struct bridge *br, > + const unsigned long int *splinter_vlans, > + struct shash *wanted_ports) > { > - struct if_cfg *if_cfg = xmalloc(sizeof *if_cfg); > - > - if_cfg->cfg = cfg; > - if_cfg->parent = parent; > - if_cfg->ofport = iface_pick_ofport(cfg); > - hmap_insert(&br->if_cfg_todo, &if_cfg->hmap_node, > - hash_string(if_cfg->cfg->name, 0)); > -} > - > -/* Deletes "struct port"s and "struct iface"s under 'br' which aren't > - * consistent with 'br->cfg'. Updates 'br->if_cfg_queue' with interfaces > which > - * 'br' needs to complete its configuration. */ > -static void > -bridge_add_del_ports(struct bridge *br, > - const unsigned long int *splinter_vlans) > -{ > - struct shash_node *port_node; > - struct port *port, *next; > - struct shash new_ports; > size_t i; > + Extra trailing whitespace on this line. > + shash_init(wanted_ports); > > - ovs_assert(hmap_is_empty(&br->if_cfg_todo)); > - ... _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
