Hey Isaku, Thanks for the patch. I have applied it to master.
thanx! mehak On Wed, Sep 26, 2012 at 12:12 AM, Isaku Yamahata <[email protected]>wrote: > Thus ovsdb-client aborts as follows. > > > # ovs-vsctl set-manager ptcp:6634 > > # ovsdb-client get-schema tcp:127.0.0.1:6634 > > 2012-09-14T05:38:26Z|00001|jsonrpc|WARN|tcp:127.0.0.1:6634: receive > error: Connection reset by peer > > 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. 5 > > Signed-off-by: Isaku Yamahata <[email protected]> > --- > Changes v1 -> v2: > - addressed listening socket > --- > lib/jsonrpc.c | 17 ++++++++++++++- > lib/jsonrpc.h | 3 +- > ovsdb/jsonrpc-server.c | 20 ++++++++++++++++++- > tests/ovsdb-server.at | 49 > ++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 85 insertions(+), 4 deletions(-) > > diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c > index 0e1788c..d6e11c3 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,19 @@ 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 44ae06f..b5acf89 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 bc0c89e..6a67f2e 100644 > --- a/ovsdb/jsonrpc-server.c > +++ b/ovsdb/jsonrpc-server.c > @@ -98,6 +98,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( > @@ -193,6 +194,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) { > @@ -269,7 +271,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", > @@ -505,6 +508,21 @@ 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 b0a3377..87046f6 100644 > --- a/tests/ovsdb-server.at > +++ b/tests/ovsdb-server.at > @@ -415,6 +415,55 @@ cat stdout >> output > > 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 <N> 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]) > > # OVSDB_CHECK_EXECUTION(TITLE, SCHEMA, TRANSACTIONS, OUTPUT, [KEYWORDS]) > -- > 1.7.1.1 > >
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
