diff mbox series

[ovs-dev,v2,11/17] ovsdb: Embed jsonrpc session options into ovsdb jsonrpc options.

Message ID 20240109225142.1987981-12-i.maximets@ovn.org
State Accepted
Commit 9a1b79c154310f28aec6e39d63331f74b08385c5
Delegated to: Ilya Maximets
Headers show
Series ovsdb-server: Configuration via config-file. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test fail github build: failed
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Ilya Maximets Jan. 9, 2024, 10:49 p.m. UTC 
Just introduced structure 'jsonrpc_session_options' is the same
as part of the 'ovsdb_jsonrpc_options'.  In fact, these options
do really belong to a lower layer.  So, replace a copy of these
fields with a structure, so it can be easily passed to jsonrpc's
'jsonrpc_session_set_options()'.

Not creating separate JSON parsing/formatting functions to avoid
creating an extra nesting level for the users who will write
the JSON definition in a configuration file.  I.e. keeping the
JSON object flat.  Also, not changing the 'db_config->options'
to be 'jsonrpc_session_options', even though we don't need the
'role' or 'read-only' fields.  This allows us to use the same
JSON parsing function for both the remotes ans database sources.
Can be changed in the future, but for now keeping as is to avoid
extra code complication.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 ovsdb/jsonrpc-server.c | 40 ++++++++++++++++------------------------
 ovsdb/jsonrpc-server.h |  5 ++---
 ovsdb/ovsdb-server.c   | 31 +++++++++++++++++--------------
 3 files changed, 35 insertions(+), 41 deletions(-)

Comments

Ilya Maximets Jan. 10, 2024, 11:28 a.m. UTC | #1 
On 1/9/24 23:49, Ilya Maximets wrote:
> Just introduced structure 'jsonrpc_session_options' is the same
> as part of the 'ovsdb_jsonrpc_options'.  In fact, these options
> do really belong to a lower layer.  So, replace a copy of these
> fields with a structure, so it can be easily passed to jsonrpc's
> 'jsonrpc_session_set_options()'.
> 
> Not creating separate JSON parsing/formatting functions to avoid
> creating an extra nesting level for the users who will write
> the JSON definition in a configuration file.  I.e. keeping the
> JSON object flat.  Also, not changing the 'db_config->options'
> to be 'jsonrpc_session_options', even though we don't need the
> 'role' or 'read-only' fields.  This allows us to use the same
> JSON parsing function for both the remotes ans database sources.
> Can be changed in the future, but for now keeping as is to avoid
> extra code complication.
> 
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  ovsdb/jsonrpc-server.c | 40 ++++++++++++++++------------------------
>  ovsdb/jsonrpc-server.h |  5 ++---
>  ovsdb/ovsdb-server.c   | 31 +++++++++++++++++--------------
>  3 files changed, 35 insertions(+), 41 deletions(-)

Recheck-request: github-robot
Dumitru Ceara Jan. 12, 2024, 8:46 p.m. UTC | #2 
On 1/9/24 23:49, Ilya Maximets wrote:
> Just introduced structure 'jsonrpc_session_options' is the same
> as part of the 'ovsdb_jsonrpc_options'.  In fact, these options
> do really belong to a lower layer.  So, replace a copy of these
> fields with a structure, so it can be easily passed to jsonrpc's
> 'jsonrpc_session_set_options()'.
> 
> Not creating separate JSON parsing/formatting functions to avoid
> creating an extra nesting level for the users who will write
> the JSON definition in a configuration file.  I.e. keeping the
> JSON object flat.  Also, not changing the 'db_config->options'
> to be 'jsonrpc_session_options', even though we don't need the
> 'role' or 'read-only' fields.  This allows us to use the same
> JSON parsing function for both the remotes ans database sources.
> Can be changed in the future, but for now keeping as is to avoid
> extra code complication.
> 
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

Looks good to me, thanks!

Acked-by: Dumitru Ceara <dceara@redhat.com>
diff mbox series

Patch

diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index f3b4961f3..5b3b5f451 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -211,11 +211,12 @@  struct ovsdb_jsonrpc_options *
 ovsdb_jsonrpc_default_options(const char *target)
 {
     struct ovsdb_jsonrpc_options *options = xzalloc(sizeof *options);
-    options->max_backoff = RECONNECT_DEFAULT_MAX_BACKOFF;
-    options->probe_interval = (stream_or_pstream_needs_probes(target)
-                               ? RECONNECT_DEFAULT_PROBE_INTERVAL
-                               : 0);
-    options->dscp = DSCP_DEFAULT;
+    struct jsonrpc_session_options *rpc_opt = &options->rpc;
+
+    rpc_opt->max_backoff = RECONNECT_DEFAULT_MAX_BACKOFF;
+    rpc_opt->probe_interval = (stream_or_pstream_needs_probes(target)
+                               ? RECONNECT_DEFAULT_PROBE_INTERVAL : 0);
+    rpc_opt->dscp = DSCP_DEFAULT;
     return options;
 }
 
@@ -237,10 +238,10 @@  ovsdb_jsonrpc_options_to_json(const struct ovsdb_jsonrpc_options *options,
     struct json *json = json_object_create();
 
     json_object_put(json, "max-backoff",
-                    json_integer_create(options->max_backoff));
+                    json_integer_create(options->rpc.max_backoff));
     json_object_put(json, "inactivity-probe",
-                    json_integer_create(options->probe_interval));
-    json_object_put(json, "dscp", json_integer_create(options->dscp));
+                    json_integer_create(options->rpc.probe_interval));
+    json_object_put(json, "dscp", json_integer_create(options->rpc.dscp));
 
     if (jsonrpc_session_only) {
         /* Caller is not interested in OVSDB-specific options. */
@@ -270,18 +271,18 @@  ovsdb_jsonrpc_options_update_from_json(struct ovsdb_jsonrpc_options *options,
     max_backoff = ovsdb_parser_member(&parser, "max-backoff",
                                       OP_INTEGER | OP_OPTIONAL);
     if (max_backoff) {
-        options->max_backoff = json_integer(max_backoff);
+        options->rpc.max_backoff = json_integer(max_backoff);
     }
 
     probe_interval = ovsdb_parser_member(&parser, "inactivity-probe",
                                          OP_INTEGER | OP_OPTIONAL);
     if (probe_interval) {
-        options->probe_interval = json_integer(probe_interval);
+        options->rpc.probe_interval = json_integer(probe_interval);
     }
 
     dscp = ovsdb_parser_member(&parser, "dscp", OP_INTEGER | OP_OPTIONAL);
     if (dscp) {
-        options->dscp = json_integer(dscp);
+        options->rpc.dscp = json_integer(dscp);
     }
 
     if (jsonrpc_session_only) {
@@ -330,7 +331,7 @@  ovsdb_jsonrpc_server_set_remotes(struct ovsdb_jsonrpc_server *svr,
         if (!options) {
             VLOG_INFO("%s: remote deconfigured", node->name);
             ovsdb_jsonrpc_server_del_remote(node);
-        } else if (options->dscp != remote->dscp
+        } else if (options->rpc.dscp != remote->dscp
                    || !nullable_string_is_equal(options->role, remote->role)) {
             ovsdb_jsonrpc_server_del_remote(node);
          }
@@ -360,7 +361,7 @@  ovsdb_jsonrpc_server_add_remote(struct ovsdb_jsonrpc_server *svr,
     struct pstream *listener;
     int error;
 
-    error = jsonrpc_pstream_open(name, &listener, options->dscp);
+    error = jsonrpc_pstream_open(name, &listener, options->rpc.dscp);
     switch (error) {
     case 0:
     case EAFNOSUPPORT:
@@ -368,7 +369,7 @@  ovsdb_jsonrpc_server_add_remote(struct ovsdb_jsonrpc_server *svr,
         remote->server = svr;
         remote->listener = listener;
         ovs_list_init(&remote->sessions);
-        remote->dscp = options->dscp;
+        remote->dscp = options->rpc.dscp;
         remote->read_only = options->read_only;
         remote->role = nullable_xstrdup(options->role);
         shash_add(&svr->remotes, name, remote);
@@ -678,15 +679,6 @@  ovsdb_jsonrpc_session_run(struct ovsdb_jsonrpc_session *s)
     return jsonrpc_session_is_alive(s->js) ? 0 : ETIMEDOUT;
 }
 
-static void
-ovsdb_jsonrpc_session_set_options(struct ovsdb_jsonrpc_session *session,
-                                  const struct ovsdb_jsonrpc_options *options)
-{
-    jsonrpc_session_set_max_backoff(session->js, options->max_backoff);
-    jsonrpc_session_set_probe_interval(session->js, options->probe_interval);
-    jsonrpc_session_set_dscp(session->js, options->dscp);
-}
-
 static void
 ovsdb_jsonrpc_session_run_all(struct ovsdb_jsonrpc_remote *remote)
 {
@@ -805,7 +797,7 @@  ovsdb_jsonrpc_session_set_all_options(
     struct ovsdb_jsonrpc_session *s;
 
     LIST_FOR_EACH (s, node, &remote->sessions) {
-        ovsdb_jsonrpc_session_set_options(s, options);
+        jsonrpc_session_set_options(s->js, &options->rpc);
     }
 }
 
diff --git a/ovsdb/jsonrpc-server.h b/ovsdb/jsonrpc-server.h
index 0d8f87da7..2fe381010 100644
--- a/ovsdb/jsonrpc-server.h
+++ b/ovsdb/jsonrpc-server.h
@@ -18,6 +18,7 @@ 
 
 #include <stdbool.h>
 #include "openvswitch/types.h"
+#include "jsonrpc.h"
 
 struct ovsdb;
 struct shash;
@@ -33,10 +34,8 @@  void ovsdb_jsonrpc_server_destroy(struct ovsdb_jsonrpc_server *);
 
 /* Options for a remote. */
 struct ovsdb_jsonrpc_options {
-    int max_backoff;            /* Maximum reconnection backoff, in msec. */
-    int probe_interval;         /* Max idle time before probing, in msec. */
+    struct jsonrpc_session_options rpc; /* JSON-RPC options. */
     bool read_only;             /* Only read-only transactions are allowed. */
-    int dscp;                   /* Dscp value for manager connections */
     char *role;                 /* Role, for role-based access controls */
 };
 struct ovsdb_jsonrpc_options *ovsdb_jsonrpc_default_options(
diff --git a/ovsdb/ovsdb-server.c b/ovsdb/ovsdb-server.c
index 3765cf066..eceed7b54 100644
--- a/ovsdb/ovsdb-server.c
+++ b/ovsdb/ovsdb-server.c
@@ -464,9 +464,9 @@  get_jsonrpc_options(const char *target, enum service_model model)
 
     options = ovsdb_jsonrpc_default_options(target);
     if (model == SM_ACTIVE_BACKUP) {
-        options->probe_interval = REPLICATION_DEFAULT_PROBE_INTERVAL;
+        options->rpc.probe_interval = REPLICATION_DEFAULT_PROBE_INTERVAL;
     } else if (model == SM_RELAY) {
-        options->probe_interval = RELAY_SOURCE_DEFAULT_PROBE_INTERVAL;
+        options->rpc.probe_interval = RELAY_SOURCE_DEFAULT_PROBE_INTERVAL;
     }
 
     return options;
@@ -1017,14 +1017,14 @@  open_db(struct server_config *server_config,
 
     if (model == SM_RELAY) {
         ovsdb_relay_add_db(db->db, conf->source, update_schema, server_config,
-                           conf->options->probe_interval);
+                           conf->options->rpc.probe_interval);
     }
     if (model == SM_ACTIVE_BACKUP && conf->ab.backup) {
         const struct uuid *server_uuid;
 
         server_uuid = ovsdb_jsonrpc_server_get_uuid(server_config->jsonrpc);
         replication_set_db(db->db, conf->source, conf->ab.sync_exclude,
-                           server_uuid, conf->options->probe_interval);
+                           server_uuid, conf->options->rpc.probe_interval);
     }
     return NULL;
 }
@@ -1241,11 +1241,11 @@  add_manager_options(struct shash *remotes, const struct ovsdb_row *row)
 
     options = add_remote(remotes, target, NULL);
     if (ovsdb_util_read_integer_column(row, "max_backoff", &max_backoff)) {
-        options->max_backoff = max_backoff;
+        options->rpc.max_backoff = max_backoff;
     }
     if (ovsdb_util_read_integer_column(row, "inactivity_probe",
                                        &probe_interval)) {
-        options->probe_interval = probe_interval;
+        options->rpc.probe_interval = probe_interval;
     }
     if (ovsdb_util_read_bool_column(row, "read_only", &read_only)) {
         options->read_only = read_only;
@@ -1257,13 +1257,13 @@  add_manager_options(struct shash *remotes, const struct ovsdb_row *row)
         options->role = xstrdup(role);
     }
 
-    options->dscp = DSCP_DEFAULT;
+    options->rpc.dscp = DSCP_DEFAULT;
     dscp_string = ovsdb_util_read_map_string_column(row, "other_config",
                                                     "dscp");
     if (dscp_string) {
         int dscp = atoi(dscp_string);
         if (dscp >= 0 && dscp <= 63) {
-            options->dscp = dscp;
+            options->rpc.dscp = dscp;
         }
     }
 }
@@ -1722,7 +1722,7 @@  ovsdb_server_connect_active_ovsdb_server(struct unixctl_conn *conn,
                 conf->model = SM_ACTIVE_BACKUP;
                 conf->source = xstrdup(*config->sync_from);
                 conf->options = ovsdb_jsonrpc_default_options(conf->source);
-                conf->options->probe_interval =
+                conf->options->rpc.probe_interval =
                     *config->replication_probe_interval;
                 conf->ab.sync_exclude =
                     nullable_xstrdup(*config->sync_exclude);
@@ -1731,7 +1731,8 @@  ovsdb_server_connect_active_ovsdb_server(struct unixctl_conn *conn,
 
             if (conf->model == SM_ACTIVE_BACKUP && !conf->ab.backup) {
                 replication_set_db(db->db, conf->source, conf->ab.sync_exclude,
-                                   server_uuid, conf->options->probe_interval);
+                                   server_uuid,
+                                   conf->options->rpc.probe_interval);
                 conf->ab.backup = true;
             }
         }
@@ -1797,10 +1798,11 @@  ovsdb_server_set_active_ovsdb_server_probe_interval(struct unixctl_conn *conn,
         struct db_config *conf = db->config;
 
         if (conf->model == SM_ACTIVE_BACKUP) {
-            conf->options->probe_interval = probe_interval;
+            conf->options->rpc.probe_interval = probe_interval;
             if (conf->ab.backup) {
                 replication_set_db(db->db, conf->source, conf->ab.sync_exclude,
-                                   server_uuid, conf->options->probe_interval);
+                                   server_uuid,
+                                   conf->options->rpc.probe_interval);
             }
         }
     }
@@ -1836,7 +1838,7 @@  ovsdb_server_set_relay_source_interval(struct unixctl_conn *conn,
         struct db_config *conf = db->config;
 
         if (conf->model == SM_RELAY) {
-            conf->options->probe_interval = probe_interval;
+            conf->options->rpc.probe_interval = probe_interval;
         }
     }
 
@@ -1879,7 +1881,8 @@  ovsdb_server_set_sync_exclude_tables(struct unixctl_conn *conn,
             conf->ab.sync_exclude = xstrdup(argv[1]);
             if (conf->ab.backup) {
                 replication_set_db(db->db, conf->source, conf->ab.sync_exclude,
-                                   server_uuid, conf->options->probe_interval);
+                                   server_uuid,
+                                   conf->options->rpc.probe_interval);
             }
         }
     }