Resubmitted rebased and with all comments (hopefully) addressed. See v5. Thanks, Ihar
On Tue, Sep 8, 2020 at 7:52 AM Numan Siddique <[email protected]> wrote: > > On Wed, Sep 2, 2020 at 6:54 AM Ihar Hrachyshka <[email protected]> wrote: > > > > User stories: > > 1) NFV: an admin wants to run two separate instances of OVN controller > > using the same database but configuring ports on different bridges. > > Some of these bridges may use DPDK while others may not. > > > > 2) Parallel OVN instances: an admin wants to run two separate > > instances of OVN controller using different databases. The > > instances are completely independent and serve different consumers. > > For example, the same machine runs both OpenStack and OpenShift > > stacks, each running its own separate OVN stack. > > > > To serve these use cases, several features should be added to > > ovn-controller: > > > > - use different database configuration for multiple controllers; > > - customize chassis name used by controller. > > > > ===== > > > > For each of the following database configuration options, their > > extended chassis specific counterparts are introduced: > > > > external_ids:ovn-remote -> external_ids:ovn-remote-<chassis-name>= > > external_ids:ovn-encap-type -> external_ids:ovn-encap-type-<chassis-name>= > > external_ids:ovn-encap-ip -> external_ids:ovn-encap-ip-<chassis-name>= > > external_ids:ovn-bridge -> external_ids:ovn-bridge-<chassis-name>= > > > > Priority wise, <chassis-name> specific options take precedence. > > > > ===== > > > > For system-id, > > > > You can now pass intended chassis name via CLI argument: > > > > $ ovn-controller ... -n <chassis_name> > > > > Alternatively, you can configure a chassis name by putting it into the > > ${ovs_sysconfdir}/system-id file before running the controller. > > > > The latter option may be more useful in container environment where > > the same image may be reused for multiple controller instances, where > > ovs_sysconfigdir/system-id is a volume mounted into this generic > > image. > > > > Priority wise, this is the order in which different means to configure > > the chassis name are used: > > > > - ovn-controller ... -n <chassis_name> CLI argument. > > - ${ovs_sysconfdir}/system-id file; > > - external_ids:system-id= ovsdb option; > > > > -- > > > > Hi Ihar, > > Can you please resubmit v4 after rebase. This patch doesn't apply > cleanly with the present master. > You could also specify the commit id on top of which your patch applies > cleanly. > > Also please move the change log after "---", before NEWS.Otherwise it > will be part of the commit message. > > Thanks > Numan > > > > v1: initial implementation. > > v2: fixed test case to check ports are claimed by proper chassis. > > v2: added NEWS entry. > > v2: fixed some compiler warnings. > > v2: moved file_system_id declaration inside a function that uses it. > > v2: removed unneeded binding.h #include. > > v2: docs: better explanation of alternatives to select chassis name. > > v3: reverted priority order for chassis configuration: first CLI, then > > system-id file, then ovsdb. > > > > Signed-off-by: Ihar Hrachyshka <[email protected]> > > --- > > NEWS | 4 ++ > > controller/binding.h | 1 + > > controller/chassis.c | 23 +++++++++- > > controller/ovn-controller.c | 85 ++++++++++++++++++++++++++++++++----- > > controller/ovn-controller.h | 3 ++ > > ovn-sb.xml | 10 +++-- > > tests/ovn-controller.at | 9 ++-- > > tests/ovn-macros.at | 49 +++++++++++++++++---- > > tests/ovn.at | 55 +++++++++++++++++++++++- > > 9 files changed, 209 insertions(+), 30 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index a1ce4e8ec..d1819a541 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -11,6 +11,10 @@ Post-v20.06.0 > > called Chassis_Private now contains the nb_cfg column which is updated > > by incrementing the value in the NB_Global table, CMSes relying on > > this mechanism should update their code to use this new table. > > + - Added support for multiple ovn-controller instances on the same host > > + (virtual chassis). Now bridge, tunnel type, IP, and chassis name can > > be > > + customized for each controller instance running on the same host. > > Chassis > > + name can be passed via config file or CLI. > > > > OVN v20.06.0 > > -------------------------- > > diff --git a/controller/binding.h b/controller/binding.h > > index c9740560f..12557b3b9 100644 > > --- a/controller/binding.h > > +++ b/controller/binding.h > > @@ -32,6 +32,7 @@ struct ovsrec_port_table; > > struct ovsrec_qos_table; > > struct ovsrec_bridge_table; > > struct ovsrec_open_vswitch_table; > > +struct ovsrec_open_vswitch; > > struct sbrec_chassis; > > struct sbrec_port_binding_table; > > struct sset; > > diff --git a/controller/chassis.c b/controller/chassis.c > > index d392d4f20..d417fbe36 100644 > > --- a/controller/chassis.c > > +++ b/controller/chassis.c > > @@ -271,8 +271,27 @@ chassis_parse_ovs_config(const struct > > ovsrec_open_vswitch_table *ovs_table, > > return false; > > } > > > > - const char *encap_type = smap_get(&cfg->external_ids, > > "ovn-encap-type"); > > - const char *encap_ips = smap_get(&cfg->external_ids, "ovn-encap-ip"); > > + const char *encap_type = NULL; > > + const char *chassis_id = get_ovs_chassis_id(cfg); > > + if (chassis_id != NULL) { > > + char *type_key = xasprintf("ovn-encap-type-%s", chassis_id); > > + encap_type = smap_get(&cfg->external_ids, type_key); > > + free(type_key); > > + } > > + if (!encap_type) { > > + encap_type = smap_get(&cfg->external_ids, "ovn-encap-type"); > > + } > > + > > + const char *encap_ips = NULL; > > + if (chassis_id != NULL) { > > + char *ip_key = xasprintf("ovn-encap-ip-%s", chassis_id); > > + encap_ips = smap_get(&cfg->external_ids, ip_key); > > + free(ip_key); > > + } > > + if (!encap_ips) { > > + encap_ips = smap_get(&cfg->external_ids, "ovn-encap-ip"); > > + } > > + > > if (!encap_type || !encap_ips) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); > > VLOG_INFO_RL(&rl, "Need to specify an encap type and ip"); > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > > index d299c6153..e1da2ef7b 100644 > > --- a/controller/ovn-controller.c > > +++ b/controller/ovn-controller.c > > @@ -18,10 +18,14 @@ > > #include "ovn-controller.h" > > > > #include <errno.h> > > +#include <fcntl.h> > > #include <getopt.h> > > #include <signal.h> > > #include <stdlib.h> > > #include <string.h> > > +#include <sys/types.h> > > +#include <sys/stat.h> > > +#include <unistd.h> > > > > #include "bfd.h" > > #include "binding.h" > > @@ -80,6 +84,8 @@ static unixctl_cb_func cluster_state_reset_cmd; > > > > #define CONTROLLER_LOOP_STOPWATCH_NAME "ovn-controller-flow-generation" > > > > +static const char *controller_chassis = NULL; > > + > > static char *parse_options(int argc, char *argv[]); > > OVS_NO_RETURN static void usage(void); > > > > @@ -255,7 +261,18 @@ out: > > static const char * > > br_int_name(const struct ovsrec_open_vswitch *cfg) > > { > > - return smap_get_def(&cfg->external_ids, "ovn-bridge", > > DEFAULT_BRIDGE_NAME); > > + const char *bridge_name = NULL; > > + const char *chassis_id = get_ovs_chassis_id(cfg); > > + if (chassis_id != NULL) { > > + char *key = xasprintf("ovn-bridge-%s", chassis_id); > > + bridge_name = smap_get(&cfg->external_ids, key); > > + free(key); > > + } > > + if (!bridge_name) { > > + bridge_name = smap_get_def( > > + &cfg->external_ids, "ovn-bridge", DEFAULT_BRIDGE_NAME); > > + } > > + return bridge_name; > > } > > > > static const struct ovsrec_bridge * > > @@ -367,17 +384,47 @@ process_br_int(struct ovsdb_idl_txn *ovs_idl_txn, > > return br_int; > > } > > > > -static const char * > > -get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table) > > +static const char *get_file_system_id(void) > > { > > - const struct ovsrec_open_vswitch *cfg > > - = ovsrec_open_vswitch_table_first(ovs_table); > > + const char *ret = NULL; > > + char *filename = xasprintf("%s/system-id", ovs_sysconfdir()); > > + errno = 0; > > + int fd = open(filename, O_RDONLY); > > + if (fd != -1) { > > + static char file_system_id[64]; > > + int nread = read(fd, file_system_id, sizeof file_system_id); > > + if (nread) { > > + file_system_id[nread] = '\0'; > > + if (file_system_id[nread - 1] == '\n') { > > + file_system_id[nread - 1] = '\0'; > > + } > > + ret = file_system_id; > > + } > > + close(fd); > > + } > > + > > + free(filename); > > + return ret; > > +} > > + > > +const char * > > +get_ovs_chassis_id(const struct ovsrec_open_vswitch *cfg) > > +{ > > + if (controller_chassis) { > > + return controller_chassis; > > + } > > + > > + const char *id_from_file = get_file_system_id(); > > + if (id_from_file) { > > + return id_from_file; > > + } > > + > > const char *chassis_id = cfg ? smap_get(&cfg->external_ids, > > "system-id") > > : NULL; > > - > > if (!chassis_id) { > > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > - VLOG_WARN_RL(&rl, "'system-id' in Open_vSwitch database is > > missing."); > > + VLOG_WARN_RL(&rl, "Failed to detect system-id, " > > + "configuration not found."); > > } > > > > return chassis_id; > > @@ -490,7 +537,16 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct > > ovsdb_idl *ovnsb_idl, > > } > > > > /* Set remote based on user configuration. */ > > - const char *remote = smap_get(&cfg->external_ids, "ovn-remote"); > > + const char *remote = NULL; > > + const char *chassis_id = get_ovs_chassis_id(cfg); > > + if (chassis_id != NULL) { > > + char *key = xasprintf("ovn-remote-%s", chassis_id); > > + remote = smap_get(&cfg->external_ids, key); > > + free(key); > > + } > > + if (!remote) { > > + remote = smap_get(&cfg->external_ids, "ovn-remote"); > > + } > > ovsdb_idl_set_remote(ovnsb_idl, remote, true); > > > > /* Set probe interval, based on user configuration and the remote. */ > > @@ -1151,7 +1207,9 @@ init_binding_ctx(struct engine_node *node, > > struct ovsrec_bridge_table *bridge_table = > > (struct ovsrec_bridge_table *)EN_OVSDB_GET( > > engine_get_input("OVS_bridge", node)); > > - const char *chassis_id = get_ovs_chassis_id(ovs_table); > > + const struct ovsrec_open_vswitch *cfg = > > + ovsrec_open_vswitch_table_first(ovs_table); > > + const char *chassis_id = get_ovs_chassis_id(cfg); > > const struct ovsrec_bridge *br_int = get_br_int(bridge_table, > > ovs_table); > > > > ovs_assert(br_int && chassis_id); > > @@ -2402,7 +2460,9 @@ main(int argc, char *argv[]) > > sbrec_chassis_table_get(ovnsb_idl_loop.idl); > > const struct ovsrec_bridge *br_int = > > process_br_int(ovs_idl_txn, bridge_table, ovs_table); > > - const char *chassis_id = get_ovs_chassis_id(ovs_table); > > + const struct ovsrec_open_vswitch *cfg = > > + ovsrec_open_vswitch_table_first(ovs_table); > > + const char *chassis_id = get_ovs_chassis_id(cfg); > > const struct sbrec_chassis *chassis = NULL; > > const struct sbrec_chassis_private *chassis_private = NULL; > > if (chassis_id) { > > @@ -2698,6 +2758,7 @@ parse_options(int argc, char *argv[]) > > STREAM_SSL_LONG_OPTIONS, > > {"peer-ca-cert", required_argument, NULL, OPT_PEER_CA_CERT}, > > {"bootstrap-ca-cert", required_argument, NULL, > > OPT_BOOTSTRAP_CA_CERT}, > > + {"chassis", required_argument, NULL, 'n'}, > > {NULL, 0, NULL, 0} > > }; > > char *short_options = > > ovs_cmdl_long_options_to_short_options(long_options); > > @@ -2730,6 +2791,10 @@ parse_options(int argc, char *argv[]) > > stream_ssl_set_ca_cert_file(optarg, true); > > break; > > > > + case 'n': > > + controller_chassis = xstrdup(optarg); > > + break; > > + > > case '?': > > exit(EXIT_FAILURE); > > > > diff --git a/controller/ovn-controller.h b/controller/ovn-controller.h > > index 5d9466880..df962b8e9 100644 > > --- a/controller/ovn-controller.h > > +++ b/controller/ovn-controller.h > > @@ -21,6 +21,7 @@ > > #include "lib/ovn-sb-idl.h" > > > > struct ovsrec_bridge_table; > > +struct ovsrec_open_vswitch; > > > > /* Linux supports a maximum of 64K zones, which seems like a fine default. > > */ > > #define MAX_CT_ZONES 65535 > > @@ -87,4 +88,6 @@ enum chassis_tunnel_type { > > > > uint32_t get_tunnel_type(const char *name); > > > > +const char *get_ovs_chassis_id(const struct ovsrec_open_vswitch *cfg); > > + > > #endif /* controller/ovn-controller.h */ > > diff --git a/ovn-sb.xml b/ovn-sb.xml > > index 59b21711b..f84bd5302 100644 > > --- a/ovn-sb.xml > > +++ b/ovn-sb.xml > > @@ -240,10 +240,12 @@ > > > > <column name="name"> > > OVN does not prescribe a particular format for chassis names. > > - ovn-controller populates this column using <ref key="system-id" > > - table="Open_vSwitch" column="external_ids" db="Open_vSwitch"/> > > - in the Open_vSwitch database's <ref table="Open_vSwitch" > > - db="Open_vSwitch"/> table. ovn-controller-vtep populates this > > + ovn-controller populates this column using the <code>system-id</code> > > + configuration file, or <code>-n</code> CLI argument, or > > + <ref key="system-id" table="Open_vSwitch" column="external_ids" > > + db="Open_vSwitch"/> in the Open_vSwitch database's > > + <ref table="Open_vSwitch" db="Open_vSwitch"/> table. > > + ovn-controller-vtep populates this > > column with <ref table="Physical_Switch" column="name" > > db="hardware_vtep"/> in the hardware_vtep database's > > <ref table="Physical_Switch" db="hardware_vtep"/> table. > > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > > index 1b96934b1..2f1fe7387 100644 > > --- a/tests/ovn-controller.at > > +++ b/tests/ovn-controller.at > > @@ -50,8 +50,7 @@ patch > > # is mirrored into the Chassis record in the OVN_Southbound db. > > check_bridge_mappings () { > > local_mappings=$1 > > - sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id) > > - OVS_WAIT_UNTIL([test x"${local_mappings}" = x$(ovn-sbctl get Chassis > > ${sysid} other_config:ovn-bridge-mappings | sed -e 's/\"//g')]) > > + OVS_WAIT_UNTIL([test x"${local_mappings}" = x$(ovn-sbctl get Chassis > > ${sandbox} other_config:ovn-bridge-mappings | sed -e 's/\"//g')]) > > } > > > > # Initially there should be no patch ports. > > @@ -133,13 +132,13 @@ ovs-vsctl \ > > -- add-br br-eth2 > > ovn_attach n1 br-phys 192.168.0.1 > > > > -sysid=$(ovs-vsctl get Open_vSwitch . external_ids:system-id) > > +sysid=${sandbox} > > > > # Make sure that the datapath_type set in the Bridge table > > # is mirrored into the Chassis record in the OVN_Southbound db. > > check_datapath_type () { > > datapath_type=$1 > > - chassis_datapath_type=$(ovn-sbctl get Chassis ${sysid} > > other_config:datapath-type | sed -e 's/"//g') #" > > + chassis_datapath_type=$(ovn-sbctl get Chassis ${sandbox} > > other_config:datapath-type | sed -e 's/"//g') #" > > test "${datapath_type}" = "${chassis_datapath_type}" > > } > > > > @@ -187,7 +186,7 @@ OVS_WAIT_UNTIL([ > > test "${expected_iface_types}" = "${chassis_iface_types}" > > ]) > > > > -# Change the value of external_ids:system-id and make sure it's mirrored > > +# Set the value of external_ids:system-id and make sure it's mirrored > > # in the Chassis record in the OVN_Southbound database. > > sysid=${sysid}-foo > > ovs-vsctl set Open_vSwitch . external-ids:system-id="${sysid}" > > diff --git a/tests/ovn-macros.at b/tests/ovn-macros.at > > index c639c0ceb..1005350ab 100644 > > --- a/tests/ovn-macros.at > > +++ b/tests/ovn-macros.at > > @@ -215,7 +215,7 @@ net_attach () { > > > > # ovn_az_attach AZ NETWORK BRIDGE IP [MASKLEN] > > ovn_az_attach() { > > - local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24} > > + local az=$1 net=$2 bridge=$3 ip=$4 masklen=${5-24} intbr=${6-br-int} > > chassis=$7 > > net_attach $net $bridge || return 1 > > > > mac=`ovs-vsctl get Interface $bridge mac_in_use | sed s/\"//g` > > @@ -229,15 +229,48 @@ ovn_az_attach() { > > else > > ovn_remote=unix:$ovs_base/$az/ovn-sb/ovn-sb.sock > > fi > > + > > + if [[ -n "${chassis}" ]]; then > > + bridge_key=ovn-bridge-${chassis} > > + remote_key=ovn-remote-${chassis} > > + encap_type_key=ovn-encap-type-${chassis} > > + encap_ip_key=ovn-encap-ip-${chassis} > > + chassis_args="-n $chassis" > > + chassis_vsctl_args= > > + else > > + bridge_key=ovn-bridge > > + remote_key=ovn-remote > > + encap_type_key=ovn-encap-type > > + encap_ip_key=ovn-encap-ip > > + chassis=$sandbox > > + chassis_args= > > + chassis_vsctl_args="-- set Open_vSwitch . > > external-ids:system-id=$chassis" > > + fi > > + > > ovs-vsctl \ > > - -- set Open_vSwitch . external-ids:system-id=$sandbox \ > > - -- set Open_vSwitch . external-ids:ovn-remote=$ovn_remote \ > > - -- set Open_vSwitch . external-ids:ovn-encap-type=geneve,vxlan \ > > - -- set Open_vSwitch . external-ids:ovn-encap-ip=$ip \ > > - -- --may-exist add-br br-int \ > > - -- set bridge br-int fail-mode=secure > > other-config:disable-in-band=true \ > > + $chassis_vsctl_args \ > > + -- set Open_vSwitch . external-ids:$bridge_key=$intbr \ > > + -- set Open_vSwitch . external-ids:$remote_key=$ovn_remote \ > > + -- set Open_vSwitch . external-ids:$encap_type_key=geneve,vxlan \ > > + -- set Open_vSwitch . external-ids:$encap_ip_key=$ip \ > > + -- --may-exist add-br ${intbr} \ > > + -- set bridge ${intbr} fail-mode=secure > > other-config:disable-in-band=true \ > > || return 1 > > - start_daemon ovn-controller || return 1 > > + > > + if [[ "${intbr}" = br-int ]]; then > > + pidfile="${OVS_RUNDIR}/ovn-controller.pid" > > + logfile="${OVS_LOGDIR}/ovn-controller.log" > > + else > > + pidfile="${OVS_RUNDIR}/ovn-controller-${intbr}.pid" > > + logfile="${OVS_LOGDIR}/ovn-controller-${chassis}.log" > > + fi > > + > > + ovn-controller \ > > + ${chassis_args} \ > > + -vconsole:off --detach --no-chdir \ > > + --pidfile=${pidfile} \ > > + --log-file=${logfile} || return 1 > > + on_exit "test -e \"$pidfile\" && kill \`cat \"$pidfile\"\`" > > } > > > > # ovn_attach NETWORK BRIDGE IP [MASKLEN] > > diff --git a/tests/ovn.at b/tests/ovn.at > > index c4edbd9e1..0d6f43e4b 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -1727,7 +1727,60 @@ AT_CLEANUP > > > > AT_BANNER([OVN end-to-end tests]) > > > > -# 3 hypervisors, one logical switch, 3 logical ports per hypervisor > > +AT_SETUP([ovn -- 3 virtual hosts, same node]) > > +AT_KEYWORDS([ovn]) > > +ovn_start > > +ovn-nbctl ls-add lsw0 > > +net_add n1 > > +sim_add hv > > + > > +as hv > > +for i in 1 2 3; do > > + chassis=host-$i > > + ovs-vsctl add-br br-phys-$i > > + ovn_attach n1 br-phys-$i 192.168.0.$i 24 br-int-$i $chassis > > + > > + for j in 1 2 3; do > > + lpname=lp$i$j > > + ovs-vsctl add-port br-int-$i vif$i$j -- set Interface vif$i$j > > external-ids:iface-id=$lpname options:tx_pcap=hv$i/vif$i$j-tx.pcap > > options:rxq_pcap=hv$i/vif$i$j-rx.pcap ofport-request=$i$j > > + ovn-nbctl lsp-add lsw0 $lpname > > + OVS_WAIT_UNTIL([test x`ovn-nbctl lsp-get-up $lpname` = xup]) > > + > > + pb_chassis_id=$(ovn-sbctl --bare --columns chassis list > > port_binding $lpname) > > + pb_chassis_name=$(ovn-sbctl get chassis $pb_chassis_id name) > > + AT_FAIL_IF([test x$pb_chassis_name != x$chassis]) > > + done > > +done > > + > > +for i in 1 2 3; do > > + > expout > > + for vif in 1 2 3; do > > + echo vif$i$vif >> expout > > + done > > + AT_CHECK([ovs-vsctl list-ports br-int-$i | grep vif], [0], [expout]) > > +done > > + > > +AT_CLEANUP > > + > > +AT_SETUP([ovn -- system-id in file]) > > +AT_KEYWORDS([ovn]) > > + > > +ovn_start > > +net_add n1 > > +sim_add hv > > + > > +as hv > > + > > +echo otherid > ${OVS_SYSCONFDIR}/system-id > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.1 > > + > > +# system-id file overrides chassis name selected via cli > > +echo otherid > expout > > +AT_CHECK([ovn-sbctl --bare --columns name list chassis], [0], [expout]) > > + > > +AT_CLEANUP > > + > > AT_SETUP([ovn -- 3 HVs, 1 LS, 3 lports/HV]) > > AT_KEYWORDS([ovnarp]) > > ovn_start > > -- > > 2.26.2 > > > > _______________________________________________ > > dev mailing list > > [email protected] > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
