diff mbox

[ovs-dev,V10] ovn-controller-vtep: Add vtep module.

Message ID 1442177926-21373-1-git-send-email-ee07b291@gmail.com
State Accepted
Headers show

Commit Message

alex wang Sept. 13, 2015, 8:58 p.m. UTC
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 <ee07b291@gmail.com>
Acked-by: Russell Bryant <rbryant@redhat.com>
Acked-by: Justin Pettit <jpettit@nicira.com>
---
V9->V10:
- Refine comments with suggestions from Justin.
- Make vtep_lswitch_cleanup() reset all logical switch tunnel key to NULL.

V8->V9:
- Add Ack from Russell.
- Fix minor comments.

V7->V8:
- Fix bug pointed out by Russell.

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                | 164 ++++++++++++++++++++++++++++++
 ovn/controller-vtep/vtep.h                |  27 +++++
 tests/ovn-controller-vtep.at              |  52 ++++++++++
 7 files changed, 253 insertions(+), 3 deletions(-)
 create mode 100644 ovn/controller-vtep/vtep.c
 create mode 100644 ovn/controller-vtep/vtep.h

Comments

Justin Pettit Sept. 14, 2015, 11:12 p.m. UTC | #1
> On Sep 13, 2015, at 1:58 PM, Alex Wang <ee07b291@gmail.com> wrote:
> 
> --- 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. */

My only comment is that I'd use "ctx->ovnsb_idl".  I think it's ready to merge.

Acked-by: Justin Pettit <jpettit@nicira.com>

--Justin
alex wang Sept. 15, 2015, 6:40 a.m. UTC | #2
Thx for the review,

Applied to master~,

On 14 September 2015 at 16:12, Justin Pettit <jpettit@nicira.com> wrote:

>
> > On Sep 13, 2015, at 1:58 PM, Alex Wang <ee07b291@gmail.com> wrote:
> >
> > --- 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. */
>
> My only comment is that I'd use "ctx->ovnsb_idl".  I think it's ready to
> merge.
>
> Acked-by: Justin Pettit <jpettit@nicira.com>
>
> --Justin
>
>
>
diff mbox

Patch

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..65070d6 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 'ctx->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 b54b29d..9908f8d 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..bae8bd5
--- /dev/null
+++ b/ovn/controller-vtep/vtep.c
@@ -0,0 +1,164 @@ 
+/* 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/sset.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);
+    struct sset vtep_pswitches = SSET_INITIALIZER(&vtep_pswitches);
+    struct sset used_ls = SSET_INITIALIZER(&used_ls);
+    const struct vteprec_physical_switch *pswitch;
+    const struct sbrec_port_binding *port_binding_rec;
+    const struct vteprec_logical_switch *vtep_ls;
+
+    VTEPREC_PHYSICAL_SWITCH_FOR_EACH (pswitch, ctx->vtep_idl) {
+        sset_add(&vtep_pswitches, pswitch->name);
+    }
+    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 *pswitch_name = smap_get(&port_binding_rec->options,
+                                            "vtep-physical-switch");
+        const char *lswitch_name = smap_get(&port_binding_rec->options,
+                                            "vtep-logical-switch");
+
+        /* If 'port_binding_rec->chassis' exists then 'pswitch_name'
+         * and 'lswitch_name' must also exist. */
+        if (!pswitch_name || !lswitch_name) {
+            /* This could only happen when someone directly modifies the
+             * database.  (e.g. using ovn-sbctl) */
+            VLOG_ERR("logical port (%s) with no 'options:vtep-physical-"
+                     "switch' or 'options:vtep-logical-switch' specified "
+                     "is bound to chassis (%s).",
+                     port_binding_rec->logical_port,
+                     port_binding_rec->chassis->name);
+            continue;
+        }
+        vtep_ls = shash_find_data(&vtep_lswitches, lswitch_name);
+        /* Also checks 'pswitch_name' since the same 'lswitch_name' could
+         * exist in multiple vtep database instances and be bound to different
+         * ovn logical networks. */
+        if (vtep_ls && sset_find(&vtep_pswitches, pswitch_name)) {
+            int64_t tnl_key;
+
+            if (sset_find(&used_ls, lswitch_name)) {
+                continue;
+            }
+
+            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);
+            sset_add(&used_ls, lswitch_name);
+        }
+    }
+    struct shash_node *node;
+    /* Resets the tunnel keys for the rest of vtep logical switches. */
+    SHASH_FOR_EACH (node, &vtep_lswitches) {
+        if (!sset_find(&used_ls, node->name)) {
+            int64_t tnl_key = 0;
+
+            vteprec_logical_switch_set_tunnel_key(node->data, &tnl_key, 1);
+        }
+    }
+
+    shash_destroy(&vtep_lswitches);
+    sset_destroy(&vtep_pswitches);
+    sset_destroy(&used_ls);
+}
+
+/* Resets all logical switches' 'tunnel_key' to NULL */
+static bool
+vtep_lswitch_cleanup(struct ovsdb_idl *vtep_idl)
+{
+   const struct vteprec_logical_switch *vtep_ls;
+    bool done = true;
+
+    VTEPREC_LOGICAL_SWITCH_FOR_EACH (vtep_ls, vtep_idl) {
+        if (vtep_ls->n_tunnel_key) {
+            vteprec_logical_switch_set_tunnel_key(vtep_ls, NULL, 0);
+            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 'ctx->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,
+                              "ovn-controller-vtep: cleaning 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..97c87b7
--- /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-vtep/vtep.h */
diff --git a/tests/ovn-controller-vtep.at b/tests/ovn-controller-vtep.at
index 5db9a35..0aea936 100644
--- a/tests/ovn-controller-vtep.at
+++ b/tests/ovn-controller-vtep.at
@@ -275,3 +275,55 @@  ${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}
+])
+
+# creates a second physical switch in vtep database, and binds its p0 vlan-100
+# to the same logical switch 'lswitch0'.
+AT_CHECK([vtep-ctl add-ps br-vtep-void -- add-port br-vtep-void p0 -- bind-ls br-vtep-void p0 100 lswitch0])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl --columns=name list Chassis  | grep -- br-vtep-void`"])
+OVN_NB_ADD_VTEP_PORT([br-test], [br-vtep-void_lswitch0], [br-vtep-void], [lswitch0])
+OVS_WAIT_UNTIL([test -n "`ovn-sbctl list Port_Binding  | grep -- br-vtep-void_lswitch0`"])
+
+# 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}
+])
+
+# now, deletes br-vtep-void.
+AT_CHECK([vtep-ctl del-ps br-vtep-void])
+OVS_WAIT_UNTIL([test -z "`ovn-sbctl --columns=name list Chassis  | grep -- br-vtep-void`"])
+# 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