> From: "JaiSingh Rana" <jaisingh.r...@cavium.com>
> To: "Jai Singh Rana" <jai.r...@gmail.com>, d...@openvswitch.org
> Sent: Thursday, August 24, 2017 9:50:51 AM
> Subject: Re: [ovs-dev] [PATCH] controller: Remote connection option to        
> OpenFlow switch.
> 
> Hi,
> 
> 
> Any feedback on this patch. Appreciate any suggestions for moving this
> forward.
> 

It has already been pointed out, but this patch makes a fundamental
assumption that ovn-controller and ovs-vswitchd can run on different hosts.
This isn't a good assumption, for example, ovn-controller implements QoS
features by configuring qdiscs on kernel netdevices corresponding to
interfaces on the ovs integration bridge.

Regards,

   Lance Richardson

> 
> Thanks,
> 
> Jai
> 
> 
> ________________________________
> From: Jai Singh Rana <jai.r...@gmail.com>
> Sent: 07 August 2017 20:48
> To: d...@openvswitch.org
> Cc: jai.r...@gmail.com; Rana, JaiSingh
> Subject: [PATCH] controller: Remote connection option to OpenFlow switch.
> 
> Currently ovn-controller uses default unix domain socket in Open
> vSwitch's "run" directory to connect to OpenFlow switch. If
> ovn-controller/ovsdb-server and vswitchd are running on different
> systems, remote connection method needs to be provided.
> 
> Added configuration option to override default connection by using OVSDB
> external-ids "ovn-ofswitch-remote". Using this external-id, desired tcp
> or unix connection to OpenFlow switch can be specified.
> 
> Tested this by using tcp/unix method configured through external-id
> "ovn-ofswitch-remote" and confirmed connection as flows getting updated
> in Open vSwitch.
> 
> Signed-off-by: Jai Singh Rana <jaisingh.r...@caviumnetworks.com>
> ---
>  ovn/controller/ofctrl.c             | 13 +++++++------
>  ovn/controller/ofctrl.h             |  4 ++--
>  ovn/controller/ovn-controller.8.xml |  9 +++++++++
>  ovn/controller/ovn-controller.c     | 37
>  ++++++++++++++++++++++++++++++++++---
>  ovn/controller/pinctrl.c            |  5 +++--
>  ovn/controller/pinctrl.h            |  3 ++-
>  6 files changed, 57 insertions(+), 14 deletions(-)
> 
> diff --git a/ovn/controller/ofctrl.c b/ovn/controller/ofctrl.c
> index fc88a41..c2ea1a5 100644
> --- a/ovn/controller/ofctrl.c
> +++ b/ovn/controller/ofctrl.c
> @@ -451,14 +451,15 @@ recv_S_UPDATE_FLOWS(const struct ofp_header *oh, enum
> ofptype type,
>      }
>  }
> 
> -/* Runs the OpenFlow state machine against 'br_int', which is local to the
> - * hypervisor on which we are running.  Attempts to negotiate a Geneve
> option
> - * field for class OVN_GENEVE_CLASS, type OVN_GENEVE_TYPE.  If successful,
> - * returns the MFF_* field ID for the option, otherwise returns 0. */
> +/* Runs the OpenFlow state machine against 'ovn_ofswitch_remote', which can
> + * be local or remote to hypervisor on which we are running. Attempts to
> + * negotiate a Geneve option field for class OVN_GENEVE_CLASS,
> + * type OVN_GENEVE_TYPE. If successful, returns the MFF_* field ID for the
> + * option, otherwise returns 0. */
>  enum mf_field_id
> -ofctrl_run(const struct ovsrec_bridge *br_int, struct shash
> *pending_ct_zones)
> +ofctrl_run(struct shash *pending_ct_zones, char *ovn_ofswitch_remote)
>  {
> -    char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
> +    char *target = xstrdup(ovn_ofswitch_remote);
>      if (strcmp(target, rconn_get_target(swconn))) {
>          VLOG_INFO("%s: connecting to switch", target);
>          rconn_connect(swconn, target, target);
> diff --git a/ovn/controller/ofctrl.h b/ovn/controller/ofctrl.h
> index d83f6ae..a23ed4c 100644
> --- a/ovn/controller/ofctrl.h
> +++ b/ovn/controller/ofctrl.h
> @@ -32,8 +32,8 @@ struct shash;
> 
>  /* Interface for OVN main loop. */
>  void ofctrl_init(struct group_table *group_table);
> -enum mf_field_id ofctrl_run(const struct ovsrec_bridge *br_int,
> -                            struct shash *pending_ct_zones);
> +enum mf_field_id ofctrl_run(struct shash *pending_ct_zones,
> +                            char *ovn_ofswitch_remote);
>  bool ofctrl_can_put(void);
>  void ofctrl_put(struct hmap *flow_table, struct shash *pending_ct_zones,
>                  int64_t nb_cfg);
> diff --git a/ovn/controller/ovn-controller.8.xml
> b/ovn/controller/ovn-controller.8.xml
> index 5641abc..4cd68c4 100644
> --- a/ovn/controller/ovn-controller.8.xml
> +++ b/ovn/controller/ovn-controller.8.xml
> @@ -151,6 +151,15 @@
>          network interface card, enabling encapsulation checksum may incur
>          performance loss. In such cases, encapsulation checksums can be
>          disabled.
>        </dd>
> +
> +      <dt><code>external_ids:ovn-ofswitch-remote</code></dt>
> +      <dd>
> +        The OpenFlow connection method that this system can connect to reach
> +        Open vSwitch using the <code>unix</code> or <code>tcp</code> forms
> +        documented above for the <var>ovs-database</var>. If not configured,
> +        default local unix domain socket file in the local Open vSwitch's
> +        "run" directory is used.
> +      </dd>
>      </dl>
> 
>      <p>
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index e2c9652..358b98b 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -328,6 +328,30 @@ get_ovnsb_remote(struct ovsdb_idl *ovs_idl)
>      }
>  }
> 
> +/* Retrieves the OVN OpenFlow switch remote location from the
> + * "external-ids:ovn-ofswitch-remote" key in 'ovs_idl' and returns
> + *  a copy of it.
> + */
> +static char *
> +get_ovn_ofswitch_remote(struct ovsdb_idl *ovs_idl,
> +                        const struct ovsrec_bridge *br_int)
> +{
> +    const struct ovsrec_open_vswitch *cfg
> +        = ovsrec_open_vswitch_first(ovs_idl);
> +    if (cfg) {
> +        const char *remote =
> +            smap_get(&cfg->external_ids, "ovn-ofswitch-remote");
> +        if (remote) {
> +            return xstrdup(remote);
> +        }
> +    }
> +
> +    char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
> +    VLOG_INFO("OVN OVSDB OpenFlow switch remote not specified."
> +              "Using default %s", target);
> +    return target;
> +}
> +
>  static void
>  update_ct_zones(struct sset *lports, const struct hmap *local_datapaths,
>                  struct simap *ct_zones, unsigned long *ct_zone_bitmap,
> @@ -690,15 +714,21 @@ main(int argc, char *argv[])
>          }
>          if (br_int && chassis) {
>              struct shash addr_sets = SHASH_INITIALIZER(&addr_sets);
> +
> +            /* Get ovn connection method to OpenFlow switch. */
> +            char *ovn_ofswitch_remote = get_ovn_ofswitch_remote(ctx.ovs_idl,
> +                                                                br_int);
> +
>              addr_sets_init(&ctx, &addr_sets);
> 
>              patch_run(&ctx, br_int, chassis);
> 
> -            enum mf_field_id mff_ovn_geneve = ofctrl_run(br_int,
> -                                                         &pending_ct_zones);
> +            enum mf_field_id mff_ovn_geneve = ofctrl_run(&pending_ct_zones,
> +
> ovn_ofswitch_remote);
> 
>              pinctrl_run(&ctx, br_int, chassis, &chassis_index,
> -                        &local_datapaths, &active_tunnels);
> +                        &local_datapaths, &active_tunnels,
> +                        ovn_ofswitch_remote);
>              update_ct_zones(&local_lports, &local_datapaths, &ct_zones,
>                              ct_zone_bitmap, &pending_ct_zones);
>              if (ctx.ovs_idl_txn) {
> @@ -750,6 +780,7 @@ main(int argc, char *argv[])
> 
>              expr_addr_sets_destroy(&addr_sets);
>              shash_destroy(&addr_sets);
> +            free(ovn_ofswitch_remote);
>          }
> 
>          /* If we haven't handled the pending packet insertion
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 469a355..eda8d76 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -1025,9 +1025,10 @@ pinctrl_run(struct controller_ctx *ctx,
>              const struct sbrec_chassis *chassis,
>              const struct chassis_index *chassis_index,
>              struct hmap *local_datapaths,
> -            struct sset *active_tunnels)
> +            struct sset *active_tunnels,
> +            char *ovn_ofswitch_remote)
>  {
> -    char *target = xasprintf("unix:%s/%s.mgmt", ovs_rundir(), br_int->name);
> +    char *target = xstrdup(ovn_ofswitch_remote);
>      if (strcmp(target, rconn_get_target(swconn))) {
>          VLOG_INFO("%s: connecting to switch", target);
>          rconn_connect(swconn, target, target);
> diff --git a/ovn/controller/pinctrl.h b/ovn/controller/pinctrl.h
> index fc9cca8..6abf0f8 100644
> --- a/ovn/controller/pinctrl.h
> +++ b/ovn/controller/pinctrl.h
> @@ -33,7 +33,8 @@ void pinctrl_init(void);
>  void pinctrl_run(struct controller_ctx *,
>                   const struct ovsrec_bridge *, const struct sbrec_chassis *,
>                   const struct chassis_index *, struct hmap *local_datapaths,
> -                 struct sset *active_tunnels);
> +                 struct sset *active_tunnels,
> +                 char *ovn_ofswitch_remote);
>  void pinctrl_wait(struct controller_ctx *);
>  void pinctrl_destroy(void);
> 
> --
> 1.8.3.1
> 
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to