[ovs-dev] [ovn-controller-vtep V7 2/3] ovn-controller-vtep: Extend vtep module to install Ucast_Macs_Remote.

Alex Wang alexw at nicira.com
Thu Aug 20 16:42:56 UTC 2015


On Thu, Aug 20, 2015 at 9:40 AM, Russell Bryant <rbryant at redhat.com> wrote:

> On 08/20/2015 09:34 AM, Alex Wang wrote:
> >
> >
> > On Thu, Aug 20, 2015 at 9:09 AM, Russell Bryant <rbryant at redhat.com
> > <mailto:rbryant at 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 <alexw at nicira.com <mailto:
> alexw at 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
>



More information about the dev mailing list