[ovs-dev] [ovn-controller-vtep V5 06/12] ovn: Add controller for VTEP gateway.

[Russell Bryant]

On 08/07/2015 03:46 AM, Alex Wang wrote:
> This commit lays down the foundation for a new controller in OVN, the
> ovn-controller-vtep, for controlling the vtep enabled gateways.
> 
> Signed-off-by: Alex Wang <alexw at nicira.com>

> diff --git a/ovn/controller-vtep/ovn-controller-vtep.c b/ovn/controller-vtep/ovn-controller-vtep.c
> new file mode 100644
> index 0000000..e8bfcbd
> --- /dev/null
> +++ b/ovn/controller-vtep/ovn-controller-vtep.c
> @@ -0,0 +1,245 @@
> +/* 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 <errno.h>
> +#include <getopt.h>
> +#include <signal.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "ovn-controller-vtep.h"
> +
> +#include "command-line.h"
> +#include "compiler.h"
> +#include "daemon.h"
> +#include "dirs.h"
> +#include "dynamic-string.h"
> +#include "fatal-signal.h"
> +#include "poll-loop.h"
> +#include "stream.h"
> +#include "stream-ssl.h"
> +#include "unixctl.h"
> +#include "util.h"
> +#include "openvswitch/vconn.h"
> +#include "openvswitch/vlog.h"
> +#include "ovn/lib/ovn-sb-idl.h"
> +#include "vtep/vtep-idl.h"
> +
> +VLOG_DEFINE_THIS_MODULE(ovn_vtep);

The patch doesn't build because of this line:

ovn/controller-vtep/ovn-controller-vtep.c:42:1: error: unused variable
'THIS_MODULE' [-Werror,-Wunused-const-variable]
VLOG_DEFINE_THIS_MODULE(ovn_vtep);
^
./include/openvswitch/vlog.h:185:42: note: expanded from macro
'VLOG_DEFINE_THIS_MODULE'
        static struct vlog_module *const THIS_MODULE = &VLM_##MODULE

So, I guess it should be added later when you introduce some logging?

> +
> +static unixctl_cb_func ovn_controller_vtep_exit;
> +
> +static void parse_options(int argc, char *argv[]);
> +OVS_NO_RETURN static void usage(void);
> +
> +static char *vtep_remote;
> +static char *ovnsb_remote;
> +
> +static void
> +get_initial_snapshot(struct ovsdb_idl *idl)
> +{
> +    while (1) {
> +        ovsdb_idl_run(idl);
> +        if (ovsdb_idl_has_ever_connected(idl)) {
> +            return;
> +        }
> +        ovsdb_idl_wait(idl);
> +        poll_block();
> +    }
> +}
> +
> +int
> +main(int argc, char *argv[])
> +{
> +    struct unixctl_server *unixctl;
> +    bool exiting;
> +    int retval;
> +
> +    ovs_cmdl_proctitle_init(argc, argv);
> +    set_program_name(argv[0]);
> +    parse_options(argc, argv);
> +    fatal_ignore_sigpipe();
> +
> +    daemonize_start();
> +
> +    retval = unixctl_server_create(NULL, &unixctl);
> +    if (retval) {
> +        exit(EXIT_FAILURE);
> +    }
> +    unixctl_command_register("exit", "", 0, 0, ovn_controller_vtep_exit,
> +                             &exiting);
> +
> +    daemonize_complete();
> +
> +    vteprec_init();
> +    sbrec_init();
> +
> +    /* Connect to VTEP database. */
> +    struct ovsdb_idl_loop vtep_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> +        ovsdb_idl_create(vtep_remote, &vteprec_idl_class, false, true));
> +    ovsdb_idl_add_table(vtep_idl_loop.idl, &vteprec_table_global);
> +    ovsdb_idl_add_column(vtep_idl_loop.idl,
> +                         &vteprec_global_col_switches);
> +    get_initial_snapshot(vtep_idl_loop.idl);
> +
> +    /* Connect to OVN SB database. */
> +    struct ovsdb_idl_loop ovnsb_idl_loop = OVSDB_IDL_LOOP_INITIALIZER(
> +        ovsdb_idl_create(ovnsb_remote, &sbrec_idl_class, false, true));
> +    get_initial_snapshot(ovnsb_idl_loop.idl);
> +
> +    /* Main loop. */
> +    exiting = false;
> +    while (!exiting) {

I expected ovsdb_idl_loop_run() in here.  I see you add it a couple of
patches later.

You could add it here and mark it as temporarily unused, though:

        struct controller_vtep_ctx OVS_UNUSED ctx = {
            .vtep_idl = vtep_idl_loop.idl,
            .vtep_idl_txn = ovsdb_idl_loop_run(&vtep_idl_loop),
            .ovnsb_idl = ovnsb_idl_loop.idl,
            .ovnsb_idl_txn = ovsdb_idl_loop_run(&ovnsb_idl_loop),
        };



> +        unixctl_server_run(unixctl);
> +
> +        unixctl_server_wait(unixctl);
> +        if (exiting) {
> +            poll_immediate_wake();
> +        }
> +        ovsdb_idl_loop_commit_and_wait(&vtep_idl_loop);
> +        ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
> +        poll_block();
> +    }
> +
> +    unixctl_server_destroy(unixctl);
> +
> +    ovsdb_idl_loop_destroy(&vtep_idl_loop);
> +    ovsdb_idl_loop_destroy(&ovnsb_idl_loop);
> +
> +    free(ovnsb_remote);
> +    free(vtep_remote);
> +
> +    exit(retval);
> +}

-- 
Russell Bryant