Hey Isaku,<div><br></div><div>Thanks for the patch.</div><div>I have applied it to master.</div><div><br></div><div>thanx!</div><div>mehak<br><br><div class="gmail_quote">On Wed, Sep 26, 2012 at 12:12 AM, Isaku Yamahata <span dir="ltr"><<a href="mailto:yamahata@valinux.co.jp" target="_blank">yamahata@valinux.co.jp</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Thus ovsdb-client aborts as follows.<br>
<br>
> # ovs-vsctl set-manager ptcp:6634<br>
> # ovsdb-client get-schema tcp:<a href="http://127.0.0.1:6634" target="_blank">127.0.0.1:6634</a><br>
> 2012-09-14T05:38:26Z|00001|jsonrpc|WARN|tcp:<a href="http://127.0.0.1:6634" target="_blank">127.0.0.1:6634</a>: receive error: Connection reset by peer<br>
> ovsdb-client: transaction failed (Connection reset by peer)<br>
NOTE: This occurs intermittently depending on how ovsdb-server runs.<br>
Running ovsdb-client on a remote machine increases the possibility.<br>
<br>
This is because ovsdb-server closes newly accepted tcp connection.<br>
The following changesets caused it. struct jsonrpc_session::dscp isn't set<br>
based on listening socket's dscp value.<br>
<br>
- ovsdb-server creates passive connection and listens on it.<br>
- ovsdb-server accepts connection by ovsdb_jsonrpc_server_run().<br>
The accepted socket inherits from the listening sockets.<br>
ovsdb_jsonrpc_server_run() creates json session, but leaves dscp of<br>
struct jsonrpc_session zero.<br>
- On calling reconfigure_from_db(), it resets dscp value to<br>
all jsonrpc sessions. Eventually jsonrpc_session_set_dscp() is called.<br>
Then jsonrpc_session_force_reconnect() closes the connection.<br>
<br>
With this patch,<br>
- struct jsonrpc_session::dscp is correctly set based on<br>
listening sockets dscp value.<br>
- dscp of listening socket is changed dynamically by setsockopt.<br>
This leaves a window where accepted socket may have old dscp.<br>
But it is ignored for now because it would complicates codes<br>
too much.<br>
<br>
The related change sets:<br>
- 0442efd9b1a88d923b56eab6b72b6be8231a49f7<br>
Reapplying the dscp changes: No need to restart DB/OVS on changing<br>
dscp value.<br>
- 59efa47adf3234ec51541405726d033173851285<br>
Revert DSCP update changes.<br>
- b2e18db292cd4962af3248f11e9f17e6eaf9c033<br>
No need to restart DB / OVS on changing dscp value.<br>
- f125905cdd3dc0339ad968c0a70128807884b400<br>
Allow configuring DSCP on controller and manager connections. 5<br>
<br>
Signed-off-by: Isaku Yamahata <<a href="mailto:yamahata@valinux.co.jp" target="_blank">yamahata@valinux.co.jp</a>><br>
---<br>
Changes v1 -> v2:<br>
- addressed listening socket<br>
---<br>
lib/jsonrpc.c | 17 ++++++++++++++-<br>
lib/jsonrpc.h | 3 +-<br>
ovsdb/jsonrpc-server.c | 20 ++++++++++++++++++-<br>
tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a> | 49 ++++++++++++++++++++++++++++++++++++++++++++++++<br>
4 files changed, 85 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c<br>
index 0e1788c..d6e11c3 100644<br>
--- a/lib/jsonrpc.c<br>
+++ b/lib/jsonrpc.c<br>
@@ -791,7 +791,7 @@ jsonrpc_session_open(const char *name)<br>
* On the assumption that such connections are likely to be short-lived<br>
* (e.g. from ovs-vsctl), informational logging for them is suppressed. */<br>
struct jsonrpc_session *<br>
-jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc)<br>
+jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc, uint8_t dscp)<br>
{<br>
struct jsonrpc_session *s;<br>
<br>
@@ -801,7 +801,7 @@ jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc)<br>
reconnect_set_name(s->reconnect, jsonrpc_get_name(jsonrpc));<br>
reconnect_set_max_tries(s->reconnect, 0);<br>
reconnect_connected(s->reconnect, time_msec());<br>
- s->dscp = 0;<br>
+ s->dscp = dscp;<br>
s->rpc = jsonrpc;<br>
s->stream = NULL;<br>
s->pstream = NULL;<br>
@@ -1093,6 +1093,19 @@ jsonrpc_session_set_dscp(struct jsonrpc_session *s,<br>
uint8_t dscp)<br>
{<br>
if (s->dscp != dscp) {<br>
+ if (s->pstream) {<br>
+ int error;<br>
+ error = pstream_set_dscp(s->pstream, dscp);<br>
+ if (error) {<br>
+ VLOG_ERR("%s: failed set_dscp %s",<br>
+ reconnect_get_name(s->reconnect), strerror(error));<br>
+ }<br>
+ /*<br>
+ * TODO:XXX race window between setting dscp to listening socket<br>
+ * and accepting socket. accepted socket may have old dscp value.<br>
+ * Ignore this race window for now.<br>
+ */<br>
+ }<br>
s->dscp = dscp;<br>
jsonrpc_session_force_reconnect(s);<br>
}<br>
diff --git a/lib/jsonrpc.h b/lib/jsonrpc.h<br>
index 44ae06f..b5acf89 100644<br>
--- a/lib/jsonrpc.h<br>
+++ b/lib/jsonrpc.h<br>
@@ -99,7 +99,8 @@ struct json *jsonrpc_msg_to_json(struct jsonrpc_msg *);<br>
/* A JSON-RPC session with reconnection. */<br>
<br>
struct jsonrpc_session *jsonrpc_session_open(const char *name);<br>
-struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *);<br>
+struct jsonrpc_session *jsonrpc_session_open_unreliably(struct jsonrpc *,<br>
+ uint8_t);<br>
void jsonrpc_session_close(struct jsonrpc_session *);<br>
<br>
void jsonrpc_session_run(struct jsonrpc_session *);<br>
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c<br>
index bc0c89e..6a67f2e 100644<br>
--- a/ovsdb/jsonrpc-server.c<br>
+++ b/ovsdb/jsonrpc-server.c<br>
@@ -98,6 +98,7 @@ struct ovsdb_jsonrpc_remote {<br>
struct ovsdb_jsonrpc_server *server;<br>
struct pstream *listener; /* Listener, if passive. */<br>
struct list sessions; /* List of "struct ovsdb_jsonrpc_session"s. */<br>
+ uint8_t dscp;<br>
};<br>
<br>
static struct ovsdb_jsonrpc_remote *ovsdb_jsonrpc_server_add_remote(<br>
@@ -193,6 +194,7 @@ ovsdb_jsonrpc_server_add_remote(struct ovsdb_jsonrpc_server *svr,<br>
remote->server = svr;<br>
remote->listener = listener;<br>
list_init(&remote->sessions);<br>
+ remote->dscp = options->dscp;<br>
shash_add(&svr->remotes, name, remote);<br>
<br>
if (!listener) {<br>
@@ -269,7 +271,8 @@ ovsdb_jsonrpc_server_run(struct ovsdb_jsonrpc_server *svr)<br>
error = pstream_accept(remote->listener, &stream);<br>
if (!error) {<br>
struct jsonrpc_session *js;<br>
- js = jsonrpc_session_open_unreliably(jsonrpc_open(stream));<br>
+ js = jsonrpc_session_open_unreliably(jsonrpc_open(stream),<br>
+ remote->dscp);<br>
ovsdb_jsonrpc_session_create(remote, js);<br>
} else if (error != EAGAIN) {<br>
VLOG_WARN_RL(&rl, "%s: accept failed: %s",<br>
@@ -505,6 +508,21 @@ ovsdb_jsonrpc_session_set_all_options(<br>
{<br>
struct ovsdb_jsonrpc_session *s;<br>
<br>
+ if (remote->listener) {<br>
+ int error;<br>
+ error = pstream_set_dscp(remote->listener, options->dscp);<br>
+ if (error) {<br>
+ VLOG_ERR("%s: set_dscp failed %s",<br>
+ pstream_get_name(remote->listener), strerror(error));<br>
+ } else {<br>
+ remote->dscp = options->dscp;<br>
+ }<br>
+ /*<br>
+ * TODO:XXX race window between setting dscp to listening socket<br>
+ * and accepting socket. Accepted socket may have old dscp value.<br>
+ * Ignore this race window for now.<br>
+ */<br>
+ }<br>
LIST_FOR_EACH (s, node, &remote->sessions) {<br>
ovsdb_jsonrpc_session_set_options(s, options);<br>
}<br>
diff --git a/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a> b/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a><br>
index b0a3377..87046f6 100644<br>
--- a/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a><br>
+++ b/tests/<a href="http://ovsdb-server.at" target="_blank">ovsdb-server.at</a><br>
@@ -415,6 +415,55 @@ cat stdout >> output<br>
<br>
EXECUTION_EXAMPLES<br>
<br>
+AT_BANNER([OVSDB -- ovsdb-server transactions (TCP sockets)])<br>
+<br>
+AT_SETUP([ovsdb-client get-schema-version - tcp socket])<br>
+AT_KEYWORDS([ovsdb server positive tcp])<br>
+ordinal_schema > schema<br>
+AT_CHECK([perl $srcdir/<a href="http://choose-port.pl" target="_blank">choose-port.pl</a>], [0], [stdout])<br>
+TCP_PORT=`cat stdout`<br>
+AT_CHECK([ovsdb-tool create db schema], [0], [ignore], [ignore])<br>
+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])<br>
+AT_CHECK([ovsdb-client get-schema-version tcp:127.0.0.1:$TCP_PORT ordinals], [0], [5.1.3<br>
+])<br>
+OVSDB_SERVER_SHUTDOWN<br>
+AT_CLEANUP])<br>
+<br>
+# OVSDB_CHECK_EXECUTION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS])<br>
+#<br>
+# Creates a database with the given SCHEMA, starts an ovsdb-server on<br>
+# that database, and runs each of the TRANSACTIONS (which should be a<br>
+# quoted list of quoted strings) against it with ovsdb-client one at a<br>
+# time.<br>
+#<br>
+# Checks that the overall output is OUTPUT, but UUIDs in the output<br>
+# are replaced by markers of the form <N> where N is a number. The<br>
+# first unique UUID is replaced by <0>, the next by <1>, and so on.<br>
+# If a given UUID appears more than once it is always replaced by the<br>
+# same marker.<br>
+#<br>
+# TITLE is provided to AT_SETUP and KEYWORDS to AT_KEYWORDS.<br>
+m4_define([OVSDB_CHECK_EXECUTION],<br>
+ [AT_SETUP([$1])<br>
+ AT_KEYWORDS([ovsdb server positive tcp $5])<br>
+ $2 > schema<br>
+ AT_CHECK([perl $srcdir/<a href="http://choose-port.pl" target="_blank">choose-port.pl</a>], [0], [stdout])<br>
+ TCP_PORT=`cat stdout`<br>
+ PKIDIR=$abs_top_builddir/tests<br>
+ AT_CHECK([ovsdb-tool create db schema], [0], [stdout], [ignore])<br>
+ 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])<br>
+ m4_foreach([txn], [$3],<br>
+ [AT_CHECK([ovsdb-client transact tcp:127.0.0.1:$TCP_PORT 'txn'], [0], [stdout], [ignore],<br>
+ [test ! -e pid || kill `cat pid`])<br>
+cat stdout >> output<br>
+])<br>
+ AT_CHECK([perl $srcdir/<a href="http://uuidfilt.pl" target="_blank">uuidfilt.pl</a> output], [0], [$4], [ignore],<br>
+ [test ! -e pid || kill `cat pid`])<br>
+ OVSDB_SERVER_SHUTDOWN<br>
+ AT_CLEANUP])<br>
+<br>
+EXECUTION_EXAMPLES<br>
+ <br>
AT_BANNER([OVSDB -- transactions on transient ovsdb-server])<br>
<br>
# OVSDB_CHECK_EXECUTION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS])<br>
<span><font color="#888888">--<br>
1.7.1.1<br>
<br>
</font></span></blockquote></div><br></div>