Hey Russell, Sorry for this very delayed reply,~
Please see my reply inline, Thanks, Alex Wang, On Mon, Aug 24, 2015 at 10:48 AM, Russell Bryant <rbry...@redhat.com> wrote: > On 08/23/2015 02:06 PM, Alex Wang wrote: > > This commit extends the vtep module to support creating the > > 'Ucast_Macs_Remote' table entries in the vtep database for > > MAC addresses on the ovn logical ports. > > > > Signed-off-by: Alex Wang <al...@nicira.com> > > Acked-by: Russell Bryant <rbry...@redhat.com> > > > +/* Updates the vtep 'Ucast_Macs_Remote' table based on non-vtep port > > + * bindings. */ > > +static void > > +vtep_macs_run(struct ovsdb_idl_txn *vtep_idl_txn, struct shash > *ucast_macs_rmts, > > + struct shash *physical_locators, struct shash > *vtep_lswitches, > > + struct shash *other_pbs) > > +{ > > + struct shash_node *node; > > + struct hmap ls_map; > > + > > + /* Maps from ovn logical datapath tunnel key (which is also the vtep > > + * logical switch tunnel key) to the corresponding vtep logical > switch > > + * instance. Also, the shash map 'added_macs' is used for checking > > + * duplicated MAC addresses in the same ovn logical datapath. */ > > + struct ls_hash_node { > > + struct hmap_node hmap_node; > > + > > + const struct vteprec_logical_switch *vtep_ls; > > + struct shash added_macs; > > + }; > > + > > + hmap_init(&ls_map); > > + SHASH_FOR_EACH (node, vtep_lswitches) { > > + struct ls_hash_node *ls_node = xmalloc(sizeof *ls_node); > > + > > + ls_node->vtep_ls = node->data; > > + shash_init(&ls_node->added_macs); > > + hmap_insert(&ls_map, &ls_node->hmap_node, > > + hash_uint64((uint64_t) > ls_node->vtep_ls->tunnel_key[0])); > > + } > > + > > + SHASH_FOR_EACH (node, other_pbs) { > > + const struct sbrec_port_binding *port_binding_rec = node->data; > > + const struct sbrec_chassis *chassis_rec; > > + struct ls_hash_node *ls_node; > > + const char *chassis_ip; > > + int64_t tnl_key; > > + size_t i; > > + > > + chassis_rec = port_binding_rec->chassis; > > + chassis_ip = get_chassis_vtep_ip(chassis_rec); > > + > > + /* Unreachable chassis, continue. */ > > + if (!chassis_ip) { > > It may be worth logging a rate-limited warning here if chassis_rec is > non-NULL. It seems like this could be pretty easy to hit with a config > mistake (forgetting to configure the vxlan encap on a regular chassis). > > At this stage, we do not know if this port_binding entry is in a logical network extended by vtep switch or not... so this could be a false alarm, So, I move it a bit later and add the VLOG_INFO_RL(). > > + continue; > > + } > > + > > + tnl_key = port_binding_rec->datapath->tunnel_key; > > + HMAP_FOR_EACH_WITH_HASH (ls_node, hmap_node, > > + hash_uint64((uint64_t) tnl_key), > > + &ls_map) { > > + if (ls_node->vtep_ls->tunnel_key[0] == tnl_key) { > > + break; > > + } > > + } > > + /* If 'ls_node' is NULL, that means no vtep logical switch is > > + * attached to the corresponding ovn logical datapath, so pass. > > + * 'ls_node' will be NULL, if the previous loop does not break, > > + * since the 'hmap_node' is the first member of the struct. */ > > It's not a big deal, but Ben merged my patch that ensures ls_node will > always be NULL, even if it isn't the first struct member, so the comment > isn't needed anymore. > > Yeah, I removed the part explaining 'hmap_node' is the first member of struct. > > > -- > Russell Bryant > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev