[ovs-dev] [ovn-controller-vtep V7 1/3] ovn-controller-vtep: Add vtep module.

Alex Wang alexw at nicira.com
Thu Aug 20 16:07:33 UTC 2015


As well as a test to cover that~ ;D

On Thu, Aug 20, 2015 at 9:06 AM, Alex Wang <alexw at nicira.com> wrote:

>
>
> On Thu, Aug 20, 2015 at 8:24 AM, Russell Bryant <rbryant at redhat.com>
> wrote:
>
>> acked this earlier, but just came across something while reviewing the
>> next patch ... let me know if I'm missing something.
>>
>> On 08/18/2015 05:58 PM, Alex Wang wrote:
>> > This commit adds the vtep module to ovn-controller-vtep.  The
>> > module will scan through the Port_Binding table in OVN-SB database,
>> > and update the vtep logcial switches tunnel keys.
>> >
>> > Signed-off-by: Alex Wang <alexw at nicira.com>
>> > ---
>> > V6->V7:
>> > - change the assertion to VLOG_ERR in vtep_lswitch_run().
>> > - refine the vtep_lswitch_run() as suggested by Russell.
>> > - refine vtep_lswitch_cleanup() as suggested by Russell.
>> >
>> > V5->V6:
>> > - rebase.
>> >
>> > V5: new patch.
>> > ---
>> >  ovn/controller-vtep/automake.mk           |    4 +-
>> >  ovn/controller-vtep/binding.c             |    3 +-
>> >  ovn/controller-vtep/gateway.c             |    3 +-
>> >  ovn/controller-vtep/ovn-controller-vtep.c |    3 +
>> >  ovn/controller-vtep/vtep.c                |  149
>> +++++++++++++++++++++++++++++
>> >  ovn/controller-vtep/vtep.h                |   27 ++++++
>> >  tests/ovn-controller-vtep.at              |   32 +++++++
>> >  7 files changed, 218 insertions(+), 3 deletions(-)
>> >  create mode 100644 ovn/controller-vtep/vtep.c
>> >  create mode 100644 ovn/controller-vtep/vtep.h
>> >
>> > diff --git a/ovn/controller-vtep/automake.mk b/ovn/controller-vtep/
>> automake.mk
>> > index 33f063f..cacfae6 100644
>> > --- a/ovn/controller-vtep/automake.mk
>> > +++ b/ovn/controller-vtep/automake.mk
>> > @@ -5,7 +5,9 @@ ovn_controller_vtep_ovn_controller_vtep_SOURCES = \
>> >       ovn/controller-vtep/gateway.c \
>> >       ovn/controller-vtep/gateway.h \
>> >       ovn/controller-vtep/ovn-controller-vtep.c \
>> > -     ovn/controller-vtep/ovn-controller-vtep.h
>> > +     ovn/controller-vtep/ovn-controller-vtep.h \
>> > +     ovn/controller-vtep/vtep.c \
>> > +     ovn/controller-vtep/vtep.h
>> >  ovn_controller_vtep_ovn_controller_vtep_LDADD = ovn/lib/libovn.la lib/
>> libopenvswitch.la vtep/libvtep.la
>> >  man_MANS += ovn/controller-vtep/ovn-controller-vtep.8
>> >  EXTRA_DIST += ovn/controller-vtep/ovn-controller-vtep.8.xml
>> > diff --git a/ovn/controller-vtep/binding.c
>> b/ovn/controller-vtep/binding.c
>> > index 652852d..d65f43c 100644
>> > --- a/ovn/controller-vtep/binding.c
>> > +++ b/ovn/controller-vtep/binding.c
>> > @@ -226,7 +226,8 @@ binding_run(struct controller_vtep_ctx *ctx)
>> >  }
>> >
>> >  /* Removes all port binding association with vtep gateway chassis.
>> > - * Returns true when all done. */
>> > + * Returns true when done (i.e. there is no change made to
>> 'ovnsb_idl'),
>> > + * otherwise returns false. */
>> >  bool
>> >  binding_cleanup(struct controller_vtep_ctx *ctx)
>> >  {
>> > diff --git a/ovn/controller-vtep/gateway.c
>> b/ovn/controller-vtep/gateway.c
>> > index 025aff8..963d419 100644
>> > --- a/ovn/controller-vtep/gateway.c
>> > +++ b/ovn/controller-vtep/gateway.c
>> > @@ -189,7 +189,8 @@ gateway_run(struct controller_vtep_ctx *ctx)
>> >  }
>> >
>> >  /* Destroys the chassis table entries for vtep physical switches.
>> > - * Returns true when all done. */
>> > + * Returns true when done (i.e. there is no change made to
>> 'ovnsb_idl'),
>> > + * otherwise returns false. */
>> >  bool
>> >  gateway_cleanup(struct controller_vtep_ctx *ctx)
>> >  {
>> > diff --git a/ovn/controller-vtep/ovn-controller-vtep.c
>> b/ovn/controller-vtep/ovn-controller-vtep.c
>> > index 7e98f69..429ac23 100644
>> > --- a/ovn/controller-vtep/ovn-controller-vtep.c
>> > +++ b/ovn/controller-vtep/ovn-controller-vtep.c
>> > @@ -39,6 +39,7 @@
>> >
>> >  #include "binding.h"
>> >  #include "gateway.h"
>> > +#include "vtep.h"
>> >  #include "ovn-controller-vtep.h"
>> >
>> >  static unixctl_cb_func ovn_controller_vtep_exit;
>> > @@ -99,6 +100,7 @@ main(int argc, char *argv[])
>> >
>> >          gateway_run(&ctx);
>> >          binding_run(&ctx);
>> > +        vtep_run(&ctx);
>> >          unixctl_server_run(unixctl);
>> >
>> >          unixctl_server_wait(unixctl);
>> > @@ -127,6 +129,7 @@ main(int argc, char *argv[])
>> >           * We're done if all of them return true. */
>> >          done = binding_cleanup(&ctx);
>> >          done = gateway_cleanup(&ctx) && done;
>> > +        done = vtep_cleanup(&ctx) && done;
>> >          if (done) {
>> >              poll_immediate_wake();
>> >          }
>> > diff --git a/ovn/controller-vtep/vtep.c b/ovn/controller-vtep/vtep.c
>> > new file mode 100644
>> > index 0000000..9870296
>> > --- /dev/null
>> > +++ b/ovn/controller-vtep/vtep.c
>> > @@ -0,0 +1,149 @@
>> > +/* Copyright (c) 2015 Nicira, Inc.
>> > + *
>> > + * Licensed under the Apache License, Version 2.0 (the "License");
>> > + * you may not use this file except in compliance with the License.
>> > + * You may obtain a copy of the License at:
>> > + *
>> > + *     http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + */
>> > +
>> > +#include <config.h>
>> > +
>> > +#include "vtep.h"
>> > +
>> > +#include "lib/hash.h"
>> > +#include "lib/hmap.h"
>> > +#include "lib/smap.h"
>> > +#include "lib/util.h"
>> > +#include "ovn-controller-vtep.h"
>> > +#include "openvswitch/vlog.h"
>> > +#include "ovn/lib/ovn-sb-idl.h"
>> > +#include "vtep/vtep-idl.h"
>> > +
>> > +VLOG_DEFINE_THIS_MODULE(vtep);
>> > +
>> > +/*
>> > + * Scans through the Binding table in ovnsb and updates the vtep
>> logical
>> > + * switch tunnel keys.
>> > + *
>> > + */
>> > +
>> > +/* Updates the vtep Logical_Switch table entries' tunnel keys based
>> > + * on the port bindings. */
>> > +static void
>> > +vtep_lswitch_run(struct controller_vtep_ctx *ctx)
>> > +{
>> > +    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);
>> > +    }
>> > +
>> > +    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;
>> > +        }
>> > +        const char *lswitch_name = smap_get(&port_binding_rec->options,
>> > +                                            "vtep-logical-switch");
>> > +
>> > +        /* If 'port_binding_rec->chassis' exists then 'lswitch_name'
>> > +         * must also exist. */
>> > +        if (!lswitch_name) {
>> > +            VLOG_ERR("logical port (%s) with no 'options:vtep-logical-"
>> > +                     "switch' specified is somehow bound to chassis
>> (%s). "
>> > +                     "this could only happen when someone is messing
>> up "
>> > +                     "using ovn-sbctl",
>> > +                     port_binding_rec->logical_port,
>> > +                     port_binding_rec->chassis->name);
>> > +            continue;
>> > +        }
>> > +        vtep_ls = shash_find_and_delete(&vtep_lswitches, lswitch_name);
>> > +        if (!vtep_ls) {
>> > +            VLOG_ERR("logical port (%s) bound to non-exist vtep
>> logical "
>> > +                     "switch (%s), something is seriously wrong",
>> > +                     port_binding_rec->logical_port, lswitch_name);
>>
>> It looks like there's some missing validation to ensure that the port
>> binding we're looking at is actually bound to this chassis and not just
>> any chassis.
>>
>> It seems this error condition could get easily hit if the port is bound
>> to a logical switch of the same on a different vtep gateway.
>>
>>
>
> Thx for pointing this out~!  I'll change the 'shash_find_and_delete' to
> 'shash_find' and use a sset to help with the garbage collection just like
> in the next patch~
>
> Alex Wang,
>
>
>
>> > +        } else {
>> > +            int64_t tnl_key;
>> > +
>> > +            tnl_key = port_binding_rec->datapath->tunnel_key;
>> > +            if (vtep_ls->n_tunnel_key
>> > +                && vtep_ls->tunnel_key[0] != tnl_key) {
>> > +                VLOG_DBG("set vtep logical switch (%s) tunnel key from
>> "
>> > +                         "(%"PRId64") to (%"PRId64")", vtep_ls->name,
>> > +                         vtep_ls->tunnel_key[0], tnl_key);
>> > +            }
>> > +            vteprec_logical_switch_set_tunnel_key(vtep_ls, &tnl_key,
>> 1);
>> > +        }
>> > +    }
>> > +    struct shash_node *node;
>> > +    /* Resets the tunnel keys for the rest of vtep logical switches. */
>> > +    SHASH_FOR_EACH (node, &vtep_lswitches) {
>> > +        int64_t tnl_key = 0;
>> > +
>> > +        vteprec_logical_switch_set_tunnel_key(node->data, &tnl_key, 1);
>> > +    }
>> > +
>> > +    shash_destroy(&vtep_lswitches);
>> > +}
>> > +
>> > +/* Since we do not own any vtep logical switch, just sets their tunnel
>> key
>> > + * to 0. */
>> > +static bool
>> > +vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
>> > +{
>> > +    const struct vteprec_logical_switch *vtep_ls;
>> > +    int64_t tnl_key = 0;
>> > +    bool done = true;
>> > +
>> > +    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, vtep_idl) {
>> > +        if (vtep_ls->n_tunnel_key != 1
>> > +            || vtep_ls->tunnel_key[0] != tnl_key) {
>> > +            vteprec_logical_switch_set_tunnel_key(vtep_ls, &tnl_key,
>> 1);
>> > +            done = false;
>> > +        }
>> > +    }
>> > +
>> > +    return done;
>> > +}
>> > +
>> > +
>> > +/* Updates vtep logical switch tunnel keys. */
>> > +void
>> > +vtep_run(struct controller_vtep_ctx *ctx)
>> > +{
>> > +    if (!ctx->vtep_idl_txn) {
>> > +        return;
>> > +    }
>> > +    vtep_lswitch_run(ctx);
>> > +}
>> > +
>> > +/* Cleans up all related entries in vtep.  Returns true when done (i.e.
>> > + * there is no change made to 'vtep_idl'), otherwise returns false. */
>> > +bool
>> > +vtep_cleanup(struct controller_vtep_ctx *ctx)
>> > +{
>> > +    if (!ctx->vtep_idl_txn) {
>> > +        return false;
>> > +    }
>> > +
>> > +    ovsdb_idl_txn_add_comment(ctx->vtep_idl_txn,
>> > +                              "cleans up vtep configuration");
>> > +    return vtep_lswitch_cleanup(ctx->vtep_idl);
>> > +}
>> > diff --git a/ovn/controller-vtep/vtep.h b/ovn/controller-vtep/vtep.h
>> > new file mode 100644
>> > index 0000000..ae6c79b
>> > --- /dev/null
>> > +++ b/ovn/controller-vtep/vtep.h
>> > @@ -0,0 +1,27 @@
>> > +/* Copyright (c) 2015 Nicira, Inc.
>> > + *
>> > + * Licensed under the Apache License, Version 2.0 (the "License");
>> > + * you may not use this file except in compliance with the License.
>> > + * You may obtain a copy of the License at:
>> > + *
>> > + *     http://www.apache.org/licenses/LICENSE-2.0
>> > + *
>> > + * Unless required by applicable law or agreed to in writing, software
>> > + * distributed under the License is distributed on an "AS IS" BASIS,
>> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
>> implied.
>> > + * See the License for the specific language governing permissions and
>> > + * limitations under the License.
>> > + */
>> > +
>> > +
>> > +#ifndef OVN_VTEP_H
>> > +#define OVN_VTEP_H 1
>> > +
>> > +#include <stdbool.h>
>> > +
>> > +struct controller_vtep_ctx;
>> > +
>> > +void vtep_run(struct controller_vtep_ctx *);
>> > +bool vtep_cleanup(struct controller_vtep_ctx *);
>> > +
>> > +#endif /* ovn/controller-gw/vtep.h */
>> > diff --git a/tests/ovn-controller-vtep.at b/tests/
>> ovn-controller-vtep.at
>> > index d7fe234..1460188 100644
>> > --- a/tests/ovn-controller-vtep.at
>> > +++ b/tests/ovn-controller-vtep.at
>> > @@ -274,3 +274,35 @@ ${chassis_uuid}
>> >
>> >  OVN_CONTROLLER_VTEP_STOP(["/has already been associated with logical
>> datapath/d"])
>> >  AT_CLEANUP
>> > +
>> > +
>> > +# Tests vtep module vtep logical switch tunnel key update.
>> > +AT_SETUP([ovn-controller-vtep - test vtep-lswitch])
>> > +OVN_CONTROLLER_VTEP_START
>> > +
>> > +# creates the logical switch in vtep and adds the corresponding logical
>> > +# port to 'br-test'.
>> > +AT_CHECK([vtep-ctl add-ls lswitch0 -- bind-ls br-vtep p0 100 lswitch0])
>> > +OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep_lswitch0], [br-vtep],
>> [lswitch0])
>> > +OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep
>> br-vtep_lswitch0`"])
>> > +
>> > +# retrieves the expected tunnel key.
>> > +datapath_uuid=$(ovn-sbctl --columns=datapath list Port_Binding
>> br-vtep_lswitch0 | cut -d ':' -f2 | tr -d ' ')
>> > +tunnel_key=$(ovn-sbctl --columns=tunnel_key list Datapath_Binding
>> ${datapath_uuid} | cut -d ':' -f2 | tr -d ' ')
>> > +OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=tunnel_key list
>> Logical_Switch | grep 0`"])
>> > +# checks the vtep logical switch tunnel key configuration.
>> > +AT_CHECK_UNQUOTED([vtep-ctl --columns=tunnel_key list Logical_Switch |
>> cut -d ':' -f2 | tr -d ' '], [0], [dnl
>> > +${tunnel_key}
>> > +])
>> > +
>> > +# changes the ovn-nb logical port type so that it is no longer
>> > +# vtep port.
>> > +AT_CHECK([ovn-nbctl lport-set-type br-vtep_lswitch0 void])
>> > +OVS_WAIT_UNTIL([test -z "`vtep-ctl --columns=tunnel_key list
>> Logical_Switch | grep 1`"])
>> > +# now should see the tunnel key reset.
>> > +AT_CHECK([vtep-ctl --columns=tunnel_key list Logical_Switch | cut -d
>> ':' -f2 | tr -d ' '], [0], [dnl
>> > +0
>> > +])
>> > +
>> > +OVN_CONTROLLER_VTEP_STOP
>> > +AT_CLEANUP
>> >
>>
>>
>



More information about the dev mailing list