From: Mehak Mahajan Date: Thu, 4 Oct 2012 19:33:05 +0000 (-0700) Subject: ovsdb/jsonrpc-server: ovsdb-server closes accepted connections immediately. X-Git-Url: https://pintos-os.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e879d33e8398219d5c9af8fd565c97303f126809;p=openvswitch ovsdb/jsonrpc-server: ovsdb-server closes accepted connections immediately. 2012-09-14T05:38:26Z|00001|jsonrpc|WARN|tcp:127.0.0.1:6634: receive error: Con ovsdb-client: transaction failed (Connection reset by peer) NOTE: This occurs intermittently depending on how ovsdb-server runs. Running ovsdb-client on a remote machine increases the possibility. This is because ovsdb-server closes newly accepted tcp connection. The following changesets caused it. struct jsonrpc_session::dscp isn't set based on listening socket's dscp value. - ovsdb-server creates passive connection and listens on it. - ovsdb-server accepts connection by ovsdb_jsonrpc_server_run(). The accepted socket inherits from the listening sockets. ovsdb_jsonrpc_server_run() creates json session, but leaves dscp of struct jsonrpc_session zero. - On calling reconfigure_from_db(), it resets dscp value to all jsonrpc sessions. Eventually jsonrpc_session_set_dscp() is called. Then jsonrpc_session_force_reconnect() closes the connection. With this patch, - struct jsonrpc_session::dscp is correctly set based on listening sockets dscp value. - dscp of listening socket is changed dynamically by setsockopt. This leaves a window where accepted socket may have old dscp. But it is ignored for now because it would complicates codes too much. The related change sets: - 0442efd9b1a88d923b56eab6b72b6be8231a49f7 Reapplying the dscp changes: No need to restart DB/OVS on changing dscp value. - 59efa47adf3234ec51541405726d033173851285 Revert DSCP update changes. - b2e18db292cd4962af3248f11e9f17e6eaf9c033 No need to restart DB / OVS on changing dscp value. - f125905cdd3dc0339ad968c0a70128807884b400 Allow configuring DSCP on controller and manager connections. Signed-off-by: Isaku Yamahata Signed-off-by: Mehak Mahajan --- diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c index 0e1788ca..552de04c 100644 --- a/lib/jsonrpc.c +++ b/lib/jsonrpc.c @@ -791,7 +791,7 @@ jsonrpc_session_open(const char *name) * On the assumption that such connections are likely to be short-lived * (e.g. from ovs-vsctl), informational logging for them is suppressed. */ struct jsonrpc_session * -jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc) +jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc, uint8_t dscp) { struct jsonrpc_session *s; @@ -801,7 +801,7 @@ jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc) reconnect_set_name(s->reconnect, jsonrpc_get_name(jsonrpc)); reconnect_set_max_tries(s->reconnect, 0); reconnect_connected(s->reconnect, time_msec()); - s->dscp = 0; + s->dscp = dscp; s->rpc = jsonrpc; s->stream = NULL; s->pstream = NULL; @@ -1093,6 +1093,20 @@ jsonrpc_session_set_dscp(struct jsonrpc_session *s, uint8_t dscp) { if (s->dscp != dscp) { + if (s->pstream) { + int error; + + error = pstream_set_dscp(s->pstream, dscp); + if (error) { + VLOG_ERR("%s: failed set_dscp %s", + reconnect_get_name(s->reconnect), strerror(error)); + } + /* + * TODO:XXX race window between setting dscp to listening socket + * and accepting socket. accepted socket may have old dscp value. + * Ignore this race window for now. + */ + } s->dscp = dscp; jsonrpc_session_force_reconnect(s); } diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h index 44ae06f2..b5acf893 100644 --- a/lib/jsonrpc.h +++ b/lib/jsonrpc.h @@ -99,7 +99,8 @@ struct json *jsonrpc_msg_to_json(struct jsonrpc_msg *); /* A JSON-RPC session with reconnection. */ struct jsonrpc_session *jsonrpc_session_open(const char *name); -struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *); +struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *, + uint8_t); void jsonrpc_session_close(struct jsonrpc_session *); void jsonrpc_session_run(struct jsonrpc_session *); diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c index 279ea97e..1149efd6 100644 --- a/ovsdb/jsonrpc-server.c +++ b/ovsdb/jsonrpc-server.c @@ -99,6 +99,7 @@ struct ovsdb_jsonrpc_remote { struct ovsdb_jsonrpc_server *server; struct pstream *listener; /* Listener, if passive. */ struct list sessions; /* List of "struct ovsdb_jsonrpc_session"s. */ + uint8_t dscp; }; static struct ovsdb_jsonrpc_remote *ovsdb_jsonrpc_server_add_remote( @@ -206,6 +207,7 @@ ovsdb_jsonrpc_server_add_remote(struct ovsdb_jsonrpc_server *svr, remote->server = svr; remote->listener = listener; list_init(&remote->sessions); + remote->dscp = options->dscp; shash_add(&svr->remotes, name, remote); if (!listener) { @@ -282,7 +284,8 @@ ovsdb_jsonrpc_server_run(struct ovsdb_jsonrpc_server *svr) error = pstream_accept(remote->listener, &stream); if (!error) { struct jsonrpc_session *js; - js = jsonrpc_session_open_unreliably(jsonrpc_open(stream)); + js = jsonrpc_session_open_unreliably(jsonrpc_open(stream), + remote->dscp); ovsdb_jsonrpc_session_create(remote, js); } else if (error != EAGAIN) { VLOG_WARN_RL(&rl, "%s: accept failed: %s", @@ -518,6 +521,22 @@ ovsdb_jsonrpc_session_set_all_options( { struct ovsdb_jsonrpc_session *s; + if (remote->listener) { + int error; + + error = pstream_set_dscp(remote->listener, options->dscp); + if (error) { + VLOG_ERR("%s: set_dscp failed %s", + pstream_get_name(remote->listener), strerror(error)); + } else { + remote->dscp = options->dscp; + } + /* + * TODO:XXX race window between setting dscp to listening socket + * and accepting socket. Accepted socket may have old dscp value. + * Ignore this race window for now. + */ + } LIST_FOR_EACH (s, node, &remote->sessions) { ovsdb_jsonrpc_session_set_options(s, options); } diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at index f787f5ac..6dcf2f5e 100644 --- a/tests/ovsdb-server.at +++ b/tests/ovsdb-server.at @@ -446,6 +446,55 @@ cat stdout >> output OVSDB_SERVER_SHUTDOWN AT_CLEANUP]) +EXECUTION_EXAMPLES + +AT_BANNER([OVSDB -- ovsdb-server transactions (TCP sockets)]) + +AT_SETUP([ovsdb-client get-schema-version - tcp socket]) +AT_KEYWORDS([ovsdb server positive tcp]) +ordinal_schema > schema +AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout]) +TCP_PORT=`cat stdout` +AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore]) +AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --unixctl="`pwd`"/unixctl --remote=ptcp:$TCP_PORT:127.0.0.1 db], [0], [ignore], [ignore]) +AT_CHECK([ovsdb-client get-schema-version tcp:127.0.0.1:$TCP_PORT ordinals], [0], [5.1.3 +]) +OVSDB_SERVER_SHUTDOWN +AT_CLEANUP]) + +# OVSDB_CHECK_EXECUTION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS]) +# +# Creates a database with the given SCHEMA, starts an ovsdb-server on +# that database, and runs each of the TRANSACTIONS (which should be a +# quoted list of quoted strings) against it with ovsdb-client one at a +# time. +# +# Checks that the overall output is OUTPUT, but UUIDs in the output +# are replaced by markers of the form where N is a number. The +# first unique UUID is replaced by <0>, the next by <1>, and so on. +# If a given UUID appears more than once it is always replaced by the +# same marker. +# +# TITLE is provided to AT_SETUP and KEYWORDS to AT_KEYWORDS. +m4_define([OVSDB_CHECK_EXECUTION], + [AT_SETUP([$1]) + AT_KEYWORDS([ovsdb server positive tcp $5]) + $2 > schema + AT_CHECK([perl $srcdir/choose-port.pl], [0], [stdout]) + TCP_PORT=`cat stdout` + PKIDIR=$abs_top_builddir/tests + AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore]) + AT_CHECK([ovsdb-server --detach --no-chdir --pidfile="`pwd`"/pid --remote=ptcp:$TCP_PORT:127.0.0.1 --unixctl="`pwd`"/unixctl db], [0], [ignore], [ignore]) + m4_foreach([txn], [$3], + [AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT 'txn'], [0], [stdout], [ignore], + [test ! -e pid || kill `cat pid`]) +cat stdout >> output +]) + AT_CHECK([perl $srcdir/uuidfilt.pl output], [0], [$4], [ignore], + [test ! -e pid || kill `cat pid`]) + OVSDB_SERVER_SHUTDOWN + AT_CLEANUP]) + EXECUTION_EXAMPLES AT_BANNER([OVSDB -- transactions on transient ovsdb-server])