On 6/27/22 17:22, Terry Wilson wrote: > The only way to configure settings on a remote (e.g. inactivity_probe) > is via --remote=db:DB,table,row. There is no way to do this via the > existing CLI options. > > For a clustered DB with multiple servers listening on unique addresses > there is no way to store these entries in the DB as the DB is shared. > For example, three servers listening on 1.1.1.1, 1.1.1.2, and 1.1.1.3 > respectively would require a Manager/Connection row each, but then > all three servers would try to listen on all three addresses. > > It is possible for ovsdb-server to serve multiple databases. This > means that we can have a local "config" database in addition to > the main database we are servering (Open_vSwitch, OVN_Southbound, etc.) > and this patch adds a Local_Config schema that currently just mirrors > the Connection table and a Config table with a 'connections' row that > stores each Connection. > > Signed-off-by: Terry Wilson <[email protected]> > ---
Hi Terry, Overall the change looks good to me, just a few small comments below. > NEWS | 5 + > debian/openvswitch-common.manpages | 1 + > ovsdb/.gitignore | 2 + > ovsdb/automake.mk | 20 ++ > ovsdb/local_config.ovsschema | 43 +++++ > ovsdb/local_config.xml | 281 +++++++++++++++++++++++++++++ > rhel/openvswitch-fedora.spec.in | 1 + > tests/ovsdb-cluster.at | 42 ++++- > 8 files changed, 388 insertions(+), 7 deletions(-) > create mode 100644 ovsdb/local_config.ovsschema Nit: At least in OVN, we use dashes instead of underscores for all the schema file names. > create mode 100644 ovsdb/local_config.xml > > diff --git a/NEWS b/NEWS > index 9fe3f44f4..b9e6f0216 100644 > --- a/NEWS > +++ b/NEWS > @@ -32,6 +32,11 @@ Post-v2.17.0 > - DPDK: > * OVS validated with DPDK 21.11.1. It is recommended to use this > version > until further releases. > + - Local_Config schema added: > + * Clustered ovsdb-servers listening on unique addresses can pass a > second > + DB created with the new Local_Config schema in order to configure > their > + Connections independent from each other. See the ovsdb.local-config.5 > + manpage for schema details. > > > v2.17.0 - 17 Feb 2022 > diff --git a/debian/openvswitch-common.manpages > b/debian/openvswitch-common.manpages > index 95004122c..7c46a2acf 100644 > --- a/debian/openvswitch-common.manpages > +++ b/debian/openvswitch-common.manpages > @@ -5,3 +5,4 @@ debian/tmp/usr/share/man/man8/ovs-appctl.8 > utilities/ovs-ofctl.8 > debian/tmp/usr/share/man/man8/ovs-parse-backtrace.8 > debian/tmp/usr/share/man/man8/ovs-pki.8 > +ovsdb/ovsdb.local-config.5 > diff --git a/ovsdb/.gitignore b/ovsdb/.gitignore > index fbcefafc6..747f119a9 100644 > --- a/ovsdb/.gitignore > +++ b/ovsdb/.gitignore > @@ -1,5 +1,7 @@ > /_server.ovsschema.inc > /_server.ovsschema.stamp > +/local_config.ovsschema.stamp > +/ovsdb.local-config.5 > /ovsdb-client > /ovsdb-client.1 > /ovsdb-doc > diff --git a/ovsdb/automake.mk b/ovsdb/automake.mk > index 62cc02686..c53675a87 100644 > --- a/ovsdb/automake.mk > +++ b/ovsdb/automake.mk > @@ -148,4 +148,24 @@ ovsdb/ovsdb-server.5: \ > $(srcdir)/ovsdb/_server.xml > [email protected] && \ > mv [email protected] $@ > > +EXTRA_DIST += ovsdb/local_config.ovsschema It's probably useful to install the schema for users to find it in a common place. I think we can add: pkgdata_DATA += ovsdb/local_config.ovsschema Then "make install" will install it along with the other schema (vtep, vswitch). > + > +# Version checking for local_config.ovsschema. > +ALL_LOCAL += ovsdb/local_config.ovsschema.stamp > +ovsdb/local_config.ovsschema.stamp: ovsdb/local_config.ovsschema > + $(srcdir)/build-aux/cksum-schema-check $? $@ > +CLEANFILES += ovsdb/local_config.ovsschema.stamp > + > +# Local_Config schema documentation > +EXTRA_DIST += ovsdb/local_config.xml > +CLEANFILES += ovsdb/ovsdb.local-config.5 > +man_MANS += ovsdb/ovsdb.local-config.5 > +ovsdb/ovsdb.local-config.5: \ > + ovsdb/ovsdb-doc ovsdb/ ovsdb/local_config.xml > ovsdb/local_config.ovsschema > + $(AM_V_GEN)$(OVSDB_DOC) \ > + --version=$(VERSION) \ > + $(srcdir)/ovsdb/local_config.ovsschema \ > + $(srcdir)/ovsdb/local_config.xml > [email protected] && \ > + mv [email protected] $@ > + > EXTRA_DIST += ovsdb/TODO.rst > diff --git a/ovsdb/local_config.ovsschema b/ovsdb/local_config.ovsschema > new file mode 100644 > index 000000000..bd86d0f4f > --- /dev/null > +++ b/ovsdb/local_config.ovsschema > @@ -0,0 +1,43 @@ > +{ > + "name": "Local_Config", > + "version": "1.0.0", > + "cksum": "2048726482 1858", > + "tables": { > + "Config": { > + "columns": { > + "connections": { > + "type": {"key": {"type": "uuid", > + "refTable": "Connection"}, > + "min": 0, > + "max": "unlimited"}}}, > + "maxRows": 1, > + "isRoot": true}, > + "Connection": { > + "columns": { > + "target": {"type": "string"}, > + "max_backoff": {"type": {"key": {"type": "integer", > + "minInteger": 1000}, > + "min": 0, > + "max": 1}}, > + "inactivity_probe": {"type": {"key": "integer", > + "min": 0, > + "max": 1}}, > + "read_only": {"type": "boolean"}, > + "role": {"type": "string"}, > + "other_config": {"type": {"key": "string", > + "value": "string", > + "min": 0, > + "max": "unlimited"}}, > + "external_ids": {"type": {"key": "string", > + "value": "string", > + "min": 0, > + "max": "unlimited"}}, > + "is_connected": {"type": "boolean", "ephemeral": true}, > + "status": {"type": {"key": "string", > + "value": "string", > + "min": 0, > + "max": "unlimited"}, > + "ephemeral": true}}, > + "indexes": [["target"]]} > + } > +} > diff --git a/ovsdb/local_config.xml b/ovsdb/local_config.xml > new file mode 100644 > index 000000000..a21509872 > --- /dev/null > +++ b/ovsdb/local_config.xml > @@ -0,0 +1,281 @@ > +<?xml version="1.0" encoding="utf-8"?> > +<database name="Local_Config" title="OVS Local Configuration Database"> > + <p> > + This database is for local configuration of an ovsdb-server. The > + database is meant to be unique, even among multiple clustered db > + servers, so that configuration that is local to that server can > + be configured separately (e.g. Connection information) Nit: missing '.' at the end of the phrase. > + </p> > + > + <table name="Config" title="ovsdb-server configuration"> > + <p> > + The root local configuration table for an ovsdb-server. This table > + must have exactly one row. > + </p> > + <group title="Connection Options"> > + <column name="connections"> > + Database clients to which the Open vSwitch database server should > + connect or on which it should listen, along with options for how > these > + connections should be configured. See the <ref table="Connection"/> > + table for more information. > + </column> > + </group> > + </table> > + > + <table name="Connection" title="OVSDB client connections."> > + <p> > + Configuration for a database connection to an Open vSwitch database > + (OVSDB) client. > + </p> > + > + <p> > + This table primarily configures the Open vSwitch database server > + (<code>ovsdb-server</code>). > + </p> > + > + <p> > + The Open vSwitch database server can initiate and maintain active > + connections to remote clients. It can also listen for database > + connections. > + </p> > + > + <group title="Core Features"> > + <column name="target"> > + <p>Connection methods for clients.</p> > + <p> > + The following connection methods are currently supported: > + </p> > + <dl> > + <dt> > + <code>ssl:<var>host</var></code>[<code>:<var>port</var></code>] > + </dt> > + <dd> > + <p> > + The specified SSL <var>port</var> on the host at the given > + <var>host</var>, which can either be a DNS name (if built with > + unbound library) or an IP address. A valid SSL configuration > must > + be provided when this form is used, this configuration can be > + specified via command-line options or the <ref table="SSL"/> > + table. > + </p> > + <p> > + If <var>port</var> is not specified, it defaults to 6640. > + </p> > + <p> > + SSL support is an optional feature that is not always > + built as part of Open vSwitch. > + </p> > + </dd> > + > + <dt> > + <code>tcp:<var>host</var></code>[<code>:<var>port</var></code>] > + </dt> > + <dd> > + <p> > + The specified TCP <var>port</var> on the host at the given > + <var>host</var>, which can either be a DNS name (if built with > + unbound library) or an IP address. If <var>host</var> is an > IPv6 > + address, wrap it in square brackets, e.g. > + <code>tcp:[::1]:6640</code>. > + </p> > + <p> > + If <var>port</var> is not specified, it defaults to 6640. > + </p> > + </dd> > + <dt> > + > <code>pssl:</code>[<var>port</var>][<code>:<var>host</var></code>] > + </dt> > + <dd> > + <p> > + Listens for SSL connections on the specified TCP > <var>port</var>. > + Specify 0 for <var>port</var> to have the kernel automatically > + choose an available port. If <var>host</var>, which can either > + be a DNS name (if built with unbound library) or an IP address, > + is specified, then connections are restricted to the resolved > or > + specified local IPaddress (either IPv4 or IPv6 address). If > + <var>host</var> is an IPv6 address, wrap in square brackets, > + e.g. <code>pssl:6640:[::1]</code>. If <var>host</var> is not > + specified then it listens only on IPv4 (but not IPv6) > addresses. > + A valid SSL configuration must be provided when this form is > + used, this can be specified either via command-line options or > + the <ref table="SSL"/> table. > + </p> > + <p> > + If <var>port</var> is not specified, it defaults to 6640. > + </p> > + <p> > + SSL support is an optional feature that is not always built as > + part of Open vSwitch. > + </p> > + </dd> > + <dt> > + > <code>ptcp:</code>[<var>port</var>][<code>:<var>host</var></code>] > + </dt> > + <dd> > + <p> > + Listens for connections on the specified TCP <var>port</var>. > + Specify 0 for <var>port</var> to have the kernel automatically > + choose an available port. If <var>host</var>, which can either > + be a DNS name (if built with unbound library) or an IP address, > + is specified, then connections are restricted to the resolved > or > + specified local IP address (either IPv4 or IPv6 address). If > + <var>host</var> is an IPv6 address, wrap it in square brackets, > + e.g. <code>ptcp:6640:[::1]</code>. If <var>host</var> is not > + specified then it listens only on IPv4 addresses. > + </p> > + <p> > + If <var>port</var> is not specified, it defaults to 6640. > + </p> > + </dd> > + </dl> > + <p>When multiple clients are configured, the <ref column="target"/> > + values must be unique. Duplicate <ref column="target"/> values yield > + unspecified results.</p> > + </column> > + > + <column name="read_only"> > + <code>true</code> to restrict these connections to read-only > + transactions, <code>false</code> to allow them to modify the > database. > + </column> > + > + <column name="role"> > + String containing role name for this connection entry. > + </column> > + </group> > + > + <group title="Client Failure Detection and Handling"> > + <column name="max_backoff"> > + Maximum number of milliseconds to wait between connection attempts. > + Default is implementation-specific. > + </column> > + > + <column name="inactivity_probe"> > + Maximum number of milliseconds of idle time on connection to the > client > + before sending an inactivity probe message. If Open vSwitch does not > + communicate with the client for the specified number of seconds, it > + will send a probe. If a response is not received for the same > + additional amount of time, Open vSwitch assumes the connection has > been > + broken and attempts to reconnect. Default is > implementation-specific. > + A value of 0 disables inactivity probes. > + </column> > + </group> > + > + <group title="Status"> > + <p> > + Key-value pair of <ref column="is_connected"/> is always updated. > + Other key-value pairs in the status columns may be updated depends > + on the <ref column="target"/> type. > + </p> > + > + <p> > + When <ref column="target"/> specifies a connection method that > + listens for inbound connections (e.g. <code>ptcp:</code> or > + <code>punix:</code>), both <ref column="n_connections"/> and > + <ref column="is_connected"/> may also be updated while the > + remaining key-value pairs are omitted. > + </p> > + > + <p> > + On the other hand, when <ref column="target"/> specifies an > + outbound connection, all key-value pairs may be updated, except > + the above-mentioned two key-value pairs associated with inbound > + connection targets. They are omitted. > + </p> > + > + <column name="is_connected"> > + <code>true</code> if currently connected to this client, > + <code>false</code> otherwise. > + </column> > + > + <column name="status" key="last_error"> > + A human-readable description of the last error on the connection > + to the manager; i.e. <code>strerror(errno)</code>. This key > + will exist only if an error has occurred. > + </column> > + > + <column name="status" key="state" > + type='{"type": "string", "enum": ["set", ["VOID", "BACKOFF", > + "CONNECTING", "ACTIVE", "IDLE"]]}'> > + <p> > + The state of the connection to the manager: > + </p> > + <dl> > + <dt><code>VOID</code></dt> > + <dd>Connection is disabled.</dd> > + > + <dt><code>BACKOFF</code></dt> > + <dd>Attempting to reconnect at an increasing period.</dd> > + > + <dt><code>CONNECTING</code></dt> > + <dd>Attempting to connect.</dd> > + > + <dt><code>ACTIVE</code></dt> > + <dd>Connected, remote host responsive.</dd> > + > + <dt><code>IDLE</code></dt> > + <dd>Connection is idle. Waiting for response to keep-alive.</dd> > + </dl> > + <p> > + These values may change in the future. They are provided only for > + human consumption. > + </p> > + </column> > + > + <column name="status" key="sec_since_connect" > + type='{"type": "integer", "minInteger": 0}'> > + The amount of time since this client last successfully connected > + to the database (in seconds). Value is empty if client has never > + successfully been connected. > + </column> > + > + <column name="status" key="sec_since_disconnect" > + type='{"type": "integer", "minInteger": 0}'> > + The amount of time since this client last disconnected from the > + database (in seconds). Value is empty if client has never > + disconnected. > + </column> > + > + <column name="status" key="locks_held"> > + Space-separated list of the names of OVSDB locks that the connection > + holds. Omitted if the connection does not hold any locks. > + </column> > + > + <column name="status" key="locks_waiting"> > + Space-separated list of the names of OVSDB locks that the connection > is > + currently waiting to acquire. Omitted if the connection is not > waiting > + for any locks. > + </column> > + > + <column name="status" key="locks_lost"> > + Space-separated list of the names of OVSDB locks that the connection > + has had stolen by another OVSDB client. Omitted if no locks have > been > + stolen from this connection. > + </column> > + > + <column name="status" key="n_connections" > + type='{"type": "integer", "minInteger": 2}'> > + When <ref column="target"/> specifies a connection method that > + listens for inbound connections (e.g. <code>ptcp:</code> or > + <code>pssl:</code>) and more than one connection is actually active, > + the value is the number of active connections. Otherwise, this > + key-value pair is omitted. > + </column> > + > + <column name="status" key="bound_port" type='{"type": "integer"}'> > + When <ref column="target"/> is <code>ptcp:</code> or > + <code>pssl:</code>, this is the TCP port on which the OVSDB server is > + listening. (This is particularly useful when <ref > + column="target"/> specifies a port of 0, allowing the kernel to > + choose any available port.) > + </column> > + </group> > + > + <group title="Common Columns"> > + The overall purpose of these columns is described under <code>Common > + Columns</code> at the beginning of this document. > + We don't have a "Common Columns" section at the beginning of this document and I don't really think we need one. Let's just add a brief description inline, here, what do you think? > + <column name="external_ids"/> > + <column name="other_config"/> As far as I can tell the only valid key is other_config:dscp, should we document it explicitly? Thanks, Dumitru > + </group> > + </table> > +</database> > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in > index 16ef1ac3a..42c2b0528 100644 > --- a/rhel/openvswitch-fedora.spec.in > +++ b/rhel/openvswitch-fedora.spec.in > @@ -476,6 +476,7 @@ fi > %{_mandir}/man1/ovsdb-server.1* > %{_mandir}/man1/ovsdb-tool.1* > %{_mandir}/man5/ovsdb-server.5* > +%{_mandir}/man5/ovsdb.local-config.5* > %{_mandir}/man5/ovs-vswitchd.conf.db.5* > %{_mandir}/man5/ovsdb.5* > %{_mandir}/man5/vtep.5* > diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at > index 0f7076a05..ea6a4dea2 100644 > --- a/tests/ovsdb-cluster.at > +++ b/tests/ovsdb-cluster.at > @@ -1,12 +1,25 @@ > OVS_START_SHELL_HELPERS > -# ovsdb_check_cluster N_SERVERS SCHEMA_FUNC OUTPUT TRANSACTION... > +# ovsdb_check_cluster N_SERVERS SCHEMA_FUNC OUTPUT USE_LOCAL_CONFIG > TRANSACTION... > ovsdb_check_cluster () { > - local n=$1 schema_func=$2 output=$3 > - shift; shift; shift > + set -x > + local n=$1 schema_func=$2 output=$3 local_config=$4 > + shift; shift; shift; shift > > $schema_func > schema > schema=`ovsdb-tool schema-name schema` > AT_CHECK([ovsdb-tool '-vPATTERN:console:%c|%p|%m' create-cluster s1.db > schema unix:s1.raft], [0], [], [stderr]) > + if test X$local_config == X"yes"; then > + for i in `seq $n`; do > + AT_CHECK([ovsdb-tool create c$i.db > $top_srcdir/ovsdb/local_config.ovsschema], [0], [], [stderr]) > + local ctxn="[[\"Local_Config\", > + {\"op\": \"insert\", \"table\": \"Config\", > + \"row\": {\"connections\": > [\"named-uuid\",\"conn$n\"]}}, > + {\"op\": \"insert\", \"table\": \"Connection\", > \"uuid-name\": \"conn$n\", > + \"row\": {\"target\": \"punix:s$i.ovsdb\"}}]]" > + > + AT_CHECK([ovsdb-tool transact c$i.db "$ctxn"], [0], [ignore], > [stderr]) > + done > + fi > AT_CHECK([grep -v 'from ephemeral to persistent' stderr], [1]) > cid=`ovsdb-tool db-cid s1.db` > for i in `seq 2 $n`; do > @@ -15,7 +28,13 @@ ovsdb_check_cluster () { > > on_exit 'kill `cat *.pid`' > for i in `seq $n`; do > - AT_CHECK([ovsdb-server -vraft -vconsole:off -vsyslog:off --detach > --no-chdir --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i > --remote=punix:s$i.ovsdb s$i.db]) > + local remote=punix:s$i.ovsdb > + local config_db= > + if test X$local_config == X"yes"; then > + remote=db:Local_Config,Config,connections > + config_db=c$i.db > + fi > + AT_CHECK([ovsdb-server -vraft -vconsole:off -vsyslog:off --detach > --no-chdir --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i > --remote=$remote s$i.db $config_db]) > done > for i in `seq $n`; do > AT_CHECK([ovsdb_client_wait unix:s$i.ovsdb $schema connected]) > @@ -40,7 +59,7 @@ AT_BANNER([OVSDB - clustered transactions (1 server)]) > m4_define([OVSDB_CHECK_EXECUTION], > [AT_SETUP([$1 - cluster of 1]) > AT_KEYWORDS([ovsdb server positive unix cluster cluster1 $5]) > - ovsdb_check_cluster 1 "$2" '$4' m4_foreach([txn], [$3], ['txn' ]) > + ovsdb_check_cluster 1 "$2" '$4' no m4_foreach([txn], [$3], ['txn' ]) > AT_CLEANUP]) > EXECUTION_EXAMPLES > > @@ -49,7 +68,7 @@ AT_BANNER([OVSDB - clustered transactions (3 servers)]) > m4_define([OVSDB_CHECK_EXECUTION], > [AT_SETUP([$1 - cluster of 3]) > AT_KEYWORDS([ovsdb server positive unix cluster cluster3 $5]) > - ovsdb_check_cluster 3 "$2" '$4' m4_foreach([txn], [$3], ['txn' ]) > + ovsdb_check_cluster 3 "$2" '$4' no m4_foreach([txn], [$3], ['txn' ]) > AT_CLEANUP]) > EXECUTION_EXAMPLES > > @@ -58,7 +77,16 @@ AT_BANNER([OVSDB - clustered transactions (5 servers)]) > m4_define([OVSDB_CHECK_EXECUTION], > [AT_SETUP([$1 - cluster of 5]) > AT_KEYWORDS([ovsdb server positive unix cluster cluster5 $5]) > - ovsdb_check_cluster 5 "$2" '$4' m4_foreach([txn], [$3], ['txn' ]) > + ovsdb_check_cluster 5 "$2" '$4' no m4_foreach([txn], [$3], ['txn' ]) > + AT_CLEANUP]) > +EXECUTION_EXAMPLES > + > +# Test a 3-server cluster using a Local_Config db. > +AT_BANNER([OVSDB - clustered transactions Local_Config (3 servers)]) > +m4_define([OVSDB_CHECK_EXECUTION], > + [AT_SETUP([$1 - cluster of 3]) > + AT_KEYWORDS([ovsdb server positive unix cluster cluster3 Local_Config $5]) > + ovsdb_check_cluster 3 "$2" '$4' yes m4_foreach([txn], [$3], ['txn' ]) > AT_CLEANUP]) > EXECUTION_EXAMPLES > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
