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

Russell Bryant rbryant at redhat.com
Thu Aug 20 16:40:20 UTC 2015


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.  :)

-- 
Russell Bryant



More information about the dev mailing list