On Thu, Aug 20, 2015 at 9:40 AM, Russell Bryant <rbry...@redhat.com> wrote:
> 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. :) > > Np, btw, it also help me realize that I should not even use 'shash_find_and_delete()' in previous patch,~ > -- > Russell Bryant > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
