[ovs-dev] [bug 14265 1/3] vconn: Fix parameters for vconn_open(), vconn_open_block(), pvconn_open().

Ethan Jackson ethan at nicira.com
Fri Jan 4 00:41:33 UTC 2013


This has annoyed me for a while too.  Thanks.

Acked-by: Ethan Jackson <ethan at nicira.com>


On Wed, Jan 2, 2013 at 5:10 PM, Ben Pfaff <blp at nicira.com> wrote:

> The customary parameter order in Open vSwitch is to put input parameters
> before output parameters, but vconn_open() and pvconn_open() had the 'dscp'
> input parameter at the end, which bugged me a bit.  Also,
> vconn_open_block() didn't take a 'dscp' parameter at all even though it's
> otherwise a wrapper around vconn_open().  This commit fixes all that up.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  lib/rconn.c                |    6 +++---
>  lib/vconn.c                |   14 +++++++-------
>  lib/vconn.h                |   10 +++++-----
>  ofproto/connmgr.c          |    6 +++---
>  tests/test-vconn.c         |   10 +++++-----
>  utilities/ovs-controller.c |    8 ++++----
>  utilities/ovs-ofctl.c      |    9 +++++----
>  7 files changed, 32 insertions(+), 31 deletions(-)
>
> diff --git a/lib/rconn.c b/lib/rconn.c
> index c9163c5..7a2bcf5 100644
> --- a/lib/rconn.c
> +++ b/lib/rconn.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -370,8 +370,8 @@ reconnect(struct rconn *rc)
>          VLOG_INFO("%s: connecting...", rc->name);
>      }
>      rc->n_attempted_connections++;
> -    retval = vconn_open(rc->target, rc->allowed_versions,
> -                        &rc->vconn, rc->dscp);
> +    retval = vconn_open(rc->target, rc->allowed_versions, rc->dscp,
> +                        &rc->vconn);
>      if (!retval) {
>          rc->remote_ip = vconn_get_remote_ip(rc->vconn);
>          rc->local_ip = vconn_get_local_ip(rc->vconn);
> diff --git a/lib/vconn.c b/lib/vconn.c
> index 4e92834..a3792ec 100644
> --- a/lib/vconn.c
> +++ b/lib/vconn.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -223,8 +223,8 @@ vconn_verify_name(const char *name)
>   * stores a pointer to the new connection in '*vconnp', otherwise a null
>   * pointer.  */
>  int
> -vconn_open(const char *name, uint32_t allowed_versions,
> -           struct vconn **vconnp, uint8_t dscp)
> +vconn_open(const char *name, uint32_t allowed_versions, uint8_t dscp,
> +           struct vconn **vconnp)
>  {
>      struct vconn_class *class;
>      struct vconn *vconn;
> @@ -295,7 +295,7 @@ vconn_run_wait(struct vconn *vconn)
>  }
>
>  int
> -vconn_open_block(const char *name, uint32_t allowed_versions,
> +vconn_open_block(const char *name, uint32_t allowed_versions, uint8_t
> dscp,
>                   struct vconn **vconnp)
>  {
>      struct vconn *vconn;
> @@ -303,7 +303,7 @@ vconn_open_block(const char *name, uint32_t
> allowed_versions,
>
>      fatal_signal_run();
>
> -    error = vconn_open(name, allowed_versions, &vconn, DSCP_DEFAULT);
> +    error = vconn_open(name, allowed_versions, dscp, &vconn);
>      if (!error) {
>          error = vconn_connect_block(vconn);
>      }
> @@ -986,8 +986,8 @@ pvconn_verify_name(const char *name)
>   * stores a pointer to the new connection in '*pvconnp', otherwise a null
>   * pointer.  */
>  int
> -pvconn_open(const char *name, uint32_t allowed_versions,
> -            struct pvconn **pvconnp, uint8_t dscp)
> +pvconn_open(const char *name, uint32_t allowed_versions, uint8_t dscp,
> +            struct pvconn **pvconnp)
>  {
>      struct pvconn_class *class;
>      struct pvconn *pvconn;
> diff --git a/lib/vconn.h b/lib/vconn.h
> index 3f69a51..81bdcc9 100644
> --- a/lib/vconn.h
> +++ b/lib/vconn.h
> @@ -34,8 +34,8 @@ void vconn_usage(bool active, bool passive, bool
> bootstrap);
>
>  /* Active vconns: virtual connections to OpenFlow devices. */
>  int vconn_verify_name(const char *name);
> -int vconn_open(const char *name, uint32_t allowed_versions,
> -               struct vconn **vconnp, uint8_t dscp);
> +int vconn_open(const char *name, uint32_t allowed_versions, uint8_t dscp,
> +               struct vconn **vconnp);
>  void vconn_close(struct vconn *);
>  const char *vconn_get_name(const struct vconn *);
>  uint32_t vconn_get_allowed_versions(const struct vconn *vconn);
> @@ -58,7 +58,7 @@ int vconn_transact_multiple_noreply(struct vconn *,
> struct list *requests,
>  void vconn_run(struct vconn *);
>  void vconn_run_wait(struct vconn *);
>
> -int vconn_open_block(const char *name, uint32_t allowed_versions,
> +int vconn_open_block(const char *name, uint32_t allowed_versions, uint8_t
> dscp,
>                       struct vconn **);
>  int vconn_connect_block(struct vconn *);
>  int vconn_send_block(struct vconn *, struct ofpbuf *);
> @@ -76,8 +76,8 @@ void vconn_send_wait(struct vconn *);
>
>  /* Passive vconns: virtual listeners for incoming OpenFlow connections. */
>  int pvconn_verify_name(const char *name);
> -int pvconn_open(const char *name, uint32_t allowed_versions,
> -                struct pvconn **pvconnp, uint8_t dscp);
> +int pvconn_open(const char *name, uint32_t allowed_versions, uint8_t dscp,
> +                struct pvconn **pvconnp);
>  const char *pvconn_get_name(const struct pvconn *);
>  void pvconn_close(struct pvconn *);
>  int pvconn_accept(struct pvconn *, struct vconn **);
> diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
> index 2dc5249..1dc9082 100644
> --- a/ofproto/connmgr.c
> +++ b/ofproto/connmgr.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -753,7 +753,7 @@ set_pvconns(struct pvconn ***pvconnsp, size_t
> *n_pvconnsp,
>      SSET_FOR_EACH (name, sset) {
>          struct pvconn *pvconn;
>          int error;
> -        error = pvconn_open(name, 0, &pvconn, 0);
> +        error = pvconn_open(name, 0, 0, &pvconn);
>          if (!error) {
>              pvconns[n_pvconns++] = pvconn;
>          } else {
> @@ -1690,7 +1690,7 @@ ofservice_create(struct connmgr *mgr, const char
> *target,
>      struct pvconn *pvconn;
>      int error;
>
> -    error = pvconn_open(target, allowed_versions, &pvconn, dscp);
> +    error = pvconn_open(target, allowed_versions, dscp, &pvconn);
>      if (error) {
>          return error;
>      }
> diff --git a/tests/test-vconn.c b/tests/test-vconn.c
> index 11ff492..1e4e787 100644
> --- a/tests/test-vconn.c
> +++ b/tests/test-vconn.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -148,7 +148,7 @@ test_refuse_connection(int argc OVS_UNUSED, char
> *argv[])
>      int error;
>
>      fpv_create(type, &fpv);
> -    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, &vconn, DSCP_DEFAULT), 0);
> +    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, DSCP_DEFAULT, &vconn), 0);
>      fpv_close(&fpv);
>      vconn_run(vconn);
>
> @@ -185,7 +185,7 @@ test_accept_then_close(int argc OVS_UNUSED, char
> *argv[])
>      int error;
>
>      fpv_create(type, &fpv);
> -    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, &vconn, DSCP_DEFAULT), 0);
> +    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, DSCP_DEFAULT, &vconn), 0);
>      vconn_run(vconn);
>      stream_close(fpv_accept(&fpv));
>      fpv_close(&fpv);
> @@ -217,7 +217,7 @@ test_read_hello(int argc OVS_UNUSED, char *argv[])
>      int error;
>
>      fpv_create(type, &fpv);
> -    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, &vconn, DSCP_DEFAULT), 0);
> +    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, DSCP_DEFAULT, &vconn), 0);
>      vconn_run(vconn);
>      stream = fpv_accept(&fpv);
>      fpv_destroy(&fpv);
> @@ -270,7 +270,7 @@ test_send_hello(const char *type, const void *out,
> size_t out_size,
>      size_t n_sent;
>
>      fpv_create(type, &fpv);
> -    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, &vconn, DSCP_DEFAULT), 0);
> +    CHECK_ERRNO(vconn_open(fpv.vconn_name, 0, DSCP_DEFAULT, &vconn), 0);
>      vconn_run(vconn);
>      stream = fpv_accept(&fpv);
>      fpv_destroy(&fpv);
> diff --git a/utilities/ovs-controller.c b/utilities/ovs-controller.c
> index bf0ae92..51765bd 100644
> --- a/utilities/ovs-controller.c
> +++ b/utilities/ovs-controller.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -116,8 +116,8 @@ main(int argc, char *argv[])
>          const char *name = argv[i];
>          struct vconn *vconn;
>
> -        retval = vconn_open(name, get_allowed_ofp_versions(),
> -                            &vconn, DSCP_DEFAULT);
> +        retval = vconn_open(name, get_allowed_ofp_versions(),
> DSCP_DEFAULT,
> +                            &vconn);
>          if (!retval) {
>              if (n_switches >= MAX_SWITCHES) {
>                  ovs_fatal(0, "max %d switch connections", n_switches);
> @@ -127,7 +127,7 @@ main(int argc, char *argv[])
>          } else if (retval == EAFNOSUPPORT) {
>              struct pvconn *pvconn;
>              retval = pvconn_open(name, get_allowed_ofp_versions(),
> -                                 &pvconn, DSCP_DEFAULT);
> +                                 DSCP_DEFAULT, &pvconn);
>              if (!retval) {
>                  if (n_listeners >= MAX_LISTENERS) {
>                      ovs_fatal(0, "max %d passive connections",
> n_listeners);
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 26736bb..239f317 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -357,8 +357,8 @@ open_vconn_socket(const char *name, struct vconn
> **vconnp)
>      char *vconn_name = xasprintf("unix:%s", name);
>      int error;
>
> -    error = vconn_open(vconn_name, get_allowed_ofp_versions(), vconnp,
> -                       DSCP_DEFAULT);
> +    error = vconn_open(vconn_name, get_allowed_ofp_versions(),
> DSCP_DEFAULT,
> +                       vconnp);
>      if (error && error != ENOENT) {
>          ovs_fatal(0, "%s: failed to open socket (%s)", name,
>                    strerror(error));
> @@ -387,7 +387,8 @@ open_vconn__(const char *name, const char
> *default_suffix,
>      free(datapath_type);
>
>      if (strchr(name, ':')) {
> -        run(vconn_open_block(name, get_allowed_ofp_versions(), vconnp),
> +        run(vconn_open_block(name, get_allowed_ofp_versions(),
> DSCP_DEFAULT,
> +                             vconnp),
>              "connecting to %s", name);
>      } else if (!open_vconn_socket(name, vconnp)) {
>          /* Fall Through. */
> --
> 1.7.2.5
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-dev/attachments/20130103/50908c45/attachment-0003.html>


More information about the dev mailing list