On 08/20/2015 09:34 AM, Alex Wang wrote: > > > On Thu, Aug 20, 2015 at 9:09 AM, Russell Bryant <rbry...@redhat.com > <mailto:rbry...@redhat.com>> wrote: > > Looks good to me except for the one thing I noticed that was introduced > in the last patch. > > On 08/18/2015 05:58 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 <mailto:al...@nicira.com>> > > --- > > V6->V7: > > - rebase. > > - adopt suggestions from Russell. > > > > V5->V6: > > - rebase. > > > > V4->V5: > > - rebase on top of master. > > - rewrite the feature since a lot have changed. > > > > V3->V4: > > - add logic to remove Ucast_Macs_Remote for non-existent MACs. > > > > V2->V3: > > - rebase to master. > > > > PATCH->V2: > > - split into separate commit. > > - few optimizations. > > --- > > ovn/controller-vtep/vtep.c | 303 > ++++++++++++++++++++++++++++++++++++++---- > > tests/ovn-controller-vtep.at <http://ovn-controller-vtep.at> | > 136 +++++++++++++++++++ > > 2 files changed, 411 insertions(+), 28 deletions(-) > > > > diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c > > index 9870296..8f9572c 100644 > > --- a/ovn/controller-vtep/vtep.c > > +++ b/ovn/controller-vtep/vtep.c > > @@ -19,7 +19,8 @@ > > > > #include "lib/hash.h" > > #include "lib/hmap.h" > > -#include "lib/smap.h" > > +#include "lib/shash.h" > > +#include "lib/sset.h" > > #include "lib/util.h" > > #include "ovn-controller-vtep.h" > > #include "openvswitch/vlog.h" > > @@ -29,39 +30,75 @@ > > VLOG_DEFINE_THIS_MODULE(vtep); > > > > /* > > - * Scans through the Binding table in ovnsb and updates the vtep > logical > > - * switch tunnel keys. > > + * Scans through the Binding table in ovnsb, and updates the vtep > logical > > + * switch tunnel keys and the 'Ucast_Macs_Remote' table in the VTEP > > + * database. > > * > > */ > > > > +/* Searches the 'chassis_rec->encaps' for the first vtep tunnel > > + * configuration, returns the 'ip'. */ > > +static const char * > > +get_chassis_vtep_ip(const struct sbrec_chassis *chassis_rec) > > +{ > > + if (chassis_rec) { > > + size_t i; > > + > > + for (i = 0; i < chassis_rec->n_encaps; i++) { > > + if (!strcmp(chassis_rec->encaps[i]->type, "vxlan")) { > > + return chassis_rec->encaps[i]->ip; > > + } > > + } > > + } > > + > > + return NULL; > > +} > > + > > +/* Creates a new 'Ucast_Macs_Remote'. */ > > +static struct vteprec_ucast_macs_remote * > > +create_umr(struct ovsdb_idl_txn *vtep_idl_txn, const char *mac, > > + const struct vteprec_logical_switch *vtep_ls) > > +{ > > + struct vteprec_ucast_macs_remote *new_umr; > > + > > + new_umr = vteprec_ucast_macs_remote_insert(vtep_idl_txn); > > + vteprec_ucast_macs_remote_set_MAC(new_umr, mac); > > + vteprec_ucast_macs_remote_set_logical_switch(new_umr, vtep_ls); > > + > > + return new_umr; > > +} > > + > > +/* Creates a new 'Physical_Locator'. */ > > +static struct vteprec_physical_locator * > > +create_pl(struct ovsdb_idl_txn *vtep_idl_txn, const char *chassis_ip) > > +{ > > + struct vteprec_physical_locator *new_pl; > > + > > + new_pl = vteprec_physical_locator_insert(vtep_idl_txn); > > + vteprec_physical_locator_set_dst_ip(new_pl, chassis_ip); > > + vteprec_physical_locator_set_encapsulation_type(new_pl, > VTEP_ENCAP_TYPE); > > + > > + return new_pl; > > +} > > + > > + > > /* Updates the vtep Logical_Switch table entries' tunnel keys based > > * on the port bindings. */ > > static void > > -vtep_lswitch_run(struct controller_vtep_ctx *ctx) > > +vtep_lswitch_run(struct shash *vtep_pbs, struct shash > *vtep_lswitches) > > { > > - struct shash vtep_lswitches = SHASH_INITIALIZER(&vtep_lswitches); > > - const struct sbrec_port_binding *port_binding_rec; > > - const struct vteprec_logical_switch *vtep_ls; > > - > > - /* Stores all logical switches to 'vtep_lswitches' with name > as key. */ > > - VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, ctx->vtep_idl) { > > - shash_add(&vtep_lswitches, vtep_ls->name, vtep_ls); > > - } > > + struct sset used_ls = SSET_INITIALIZER(&used_ls); > > + struct shash_node *node; > > > > - ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn, > > - "ovn-controller-vtep: update > logical switch " > > - "tunnel keys"); > > /* Collects the logical switch bindings from port binding > entries. > > * Since the binding module has already guaranteed that each vtep > > * logical switch is bound only to one ovn-sb logical datapath, > > * we can just iterate and assign tunnel key to vtep logical > switch. */ > > - SBREC_PORT_BINDING_FOR_EACH(port_binding_rec, ctx->ovnsb_idl) { > > - if (strcmp(port_binding_rec->type, "vtep") > > - || !port_binding_rec->chassis) { > > - continue; > > - } > > + SHASH_FOR_EACH (node, vtep_pbs) { > > + const struct sbrec_port_binding *port_binding_rec = > node->data; > > const char *lswitch_name = > smap_get(&port_binding_rec->options, > > "vtep-logical-switch"); > > + const struct vteprec_logical_switch *vtep_ls; > > > > I went back and mentioned this on the last patch, but I think we're > missing some validation here to ensure that the vtep port binding we're > looking at is bound to this chassis and not another one that happens to > have a logical switch of the same name. > > > > By 'this chassis', I think you mean all 'chassis' registered in one VTEP > database. In other words, if we have another VTEP database with same- > named logical switches, the logic here will mistakenly report warning for > Port_Binding entries for other VTEP database, right? I think I'll bring the > 'options:vtep_physical_switch' in for help,
Yes, exactly. Sorry for not being clear. :) -- Russell Bryant _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
