[ovs-dev] [PATCH 1/2] stream: Eliminate pstream_set_dscp().
Alex Wang
alexw at nicira.com
Fri Feb 20 19:24:03 UTC 2015
Acked-by: Alex Wang <alexw at nicira.com>
On Fri, Feb 20, 2015 at 8:44 AM, Ben Pfaff <blp at nicira.com> wrote:
> This function is really of marginal utility. This commit drops it and
> makes the existing callers instead open a new pstream with the desired
> dscp.
>
> The ulterior motive here is that the set_dscp() function that actually sets
> the DSCP on a socket really wants to know the address family (AF_INET vs.
> AF_INET6). We could plumb that down through the stream code, and that's
> one reasonable option, but I thought that simply eliminating some calls
> to set_dscp() where we don't already have the address family handy was
> another reasonable way to go.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
> lib/jsonrpc.c | 18 +++---------------
> lib/stream-fd.c | 14 --------------
> lib/stream-fd.h | 3 +--
> lib/stream-provider.h | 5 +----
> lib/stream-ssl.c | 10 +---------
> lib/stream-tcp.c | 6 ++----
> lib/stream-unix.c | 5 ++---
> lib/stream.c | 11 +----------
> lib/stream.h | 3 +--
> ovsdb/jsonrpc-server.c | 35 +++++++++++++----------------------
> 10 files changed, 25 insertions(+), 85 deletions(-)
>
> diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
> index f15adca..5323e07 100644
> --- a/lib/jsonrpc.c
> +++ b/lib/jsonrpc.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -1134,21 +1134,9 @@ jsonrpc_session_set_dscp(struct jsonrpc_session *s,
> uint8_t dscp)
> {
> if (s->dscp != dscp) {
> - if (s->pstream) {
> - int error;
> + pstream_close(s->pstream);
> + s->pstream = NULL;
>
> - error = pstream_set_dscp(s->pstream, dscp);
> - if (error) {
> - VLOG_ERR("%s: failed set_dscp %s",
> - reconnect_get_name(s->reconnect),
> - ovs_strerror(error));
> - }
> - /*
> - * 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/stream-fd.c b/lib/stream-fd.c
> index 2cb4b4c..31bfc6e 100644
> --- a/lib/stream-fd.c
> +++ b/lib/stream-fd.c
> @@ -181,7 +181,6 @@ struct fd_pstream
> int fd;
> int (*accept_cb)(int fd, const struct sockaddr_storage *, size_t
> ss_len,
> struct stream **);
> - int (*set_dscp_cb)(int fd, uint8_t dscp);
> char *unlink_path;
> };
>
> @@ -212,14 +211,12 @@ int
> new_fd_pstream(const char *name, int fd,
> int (*accept_cb)(int fd, const struct sockaddr_storage *ss,
> size_t ss_len, struct stream **streamp),
> - int (*set_dscp_cb)(int fd, uint8_t dscp),
> char *unlink_path, struct pstream **pstreamp)
> {
> struct fd_pstream *ps = xmalloc(sizeof *ps);
> pstream_init(&ps->pstream, &fd_pstream_class, name);
> ps->fd = fd;
> ps->accept_cb = accept_cb;
> - ps->set_dscp_cb = set_dscp_cb;
> ps->unlink_path = unlink_path;
> *pstreamp = &ps->pstream;
> return 0;
> @@ -273,16 +270,6 @@ pfd_wait(struct pstream *pstream)
> poll_fd_wait(ps->fd, POLLIN);
> }
>
> -static int
> -pfd_set_dscp(struct pstream *pstream, uint8_t dscp)
> -{
> - struct fd_pstream *ps = fd_pstream_cast(pstream);
> - if (ps->set_dscp_cb) {
> - return ps->set_dscp_cb(ps->fd, dscp);
> - }
> - return 0;
> -}
> -
> static const struct pstream_class fd_pstream_class = {
> "pstream",
> false,
> @@ -290,7 +277,6 @@ static const struct pstream_class fd_pstream_class = {
> pfd_close,
> pfd_accept,
> pfd_wait,
> - pfd_set_dscp,
> };
>
> /* Helper functions. */
> diff --git a/lib/stream-fd.h b/lib/stream-fd.h
> index d2937c1..12452dd 100644
> --- a/lib/stream-fd.h
> +++ b/lib/stream-fd.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2012, 2014 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2012, 2014, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -33,7 +33,6 @@ int new_fd_stream(const char *name, int fd, int
> connect_status,
> int new_fd_pstream(const char *name, int fd,
> int (*accept_cb)(int fd, const struct sockaddr_storage
> *ss,
> size_t ss_len, struct stream **),
> - int (*set_dscp_cb)(int fd, uint8_t dscp),
> char *unlink_path,
> struct pstream **pstreamp);
>
> diff --git a/lib/stream-provider.h b/lib/stream-provider.h
> index 8347ac6..2226a80 100644
> --- a/lib/stream-provider.h
> +++ b/lib/stream-provider.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2012, 2013, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -183,9 +183,6 @@ struct pstream_class {
> /* Arranges for the poll loop to wake up when a connection is ready
> to be
> * accepted on 'pstream'. */
> void (*wait)(struct pstream *pstream);
> -
> - /* Set DSCP value of the listening socket. */
> - int (*set_dscp)(struct pstream *pstream, uint8_t dscp);
> };
>
> /* Active and passive stream classes. */
> diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
> index 47c5312..c2ace71 100644
> --- a/lib/stream-ssl.c
> +++ b/lib/stream-ssl.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira,
> Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -871,13 +871,6 @@ pssl_wait(struct pstream *pstream)
> poll_fd_wait(pssl->fd, POLLIN);
> }
>
> -static int
> -pssl_set_dscp(struct pstream *pstream, uint8_t dscp)
> -{
> - struct pssl_pstream *pssl = pssl_pstream_cast(pstream);
> - return set_dscp(pssl->fd, dscp);
> -}
> -
> const struct pstream_class pssl_pstream_class = {
> "pssl",
> true,
> @@ -885,7 +878,6 @@ const struct pstream_class pssl_pstream_class = {
> pssl_close,
> pssl_accept,
> pssl_wait,
> - pssl_set_dscp,
> };
>
> /*
> diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
> index 3e19dbbb..1f53e86 100644
> --- a/lib/stream-tcp.c
> +++ b/lib/stream-tcp.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2012, 2013, 2014, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -161,8 +161,7 @@ new_pstream(char *suffix, const char *name, struct
> pstream **pstreamp,
> conn_name = bound_name;
> }
>
> - error = new_fd_pstream(conn_name, fd, ptcp_accept, set_dscp,
> unlink_path,
> - pstreamp);
> + error = new_fd_pstream(conn_name, fd, ptcp_accept, unlink_path,
> pstreamp);
> if (!error) {
> pstream_set_bound_port(*pstreamp, htons(port));
> }
> @@ -196,7 +195,6 @@ const struct pstream_class ptcp_pstream_class = {
> NULL,
> NULL,
> NULL,
> - NULL,
> };
>
> #ifdef _WIN32
> diff --git a/lib/stream-unix.c b/lib/stream-unix.c
> index daade3f..cadd180 100644
> --- a/lib/stream-unix.c
> +++ b/lib/stream-unix.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira,
> Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -102,7 +102,7 @@ punix_open(const char *name OVS_UNUSED, char *suffix,
> return error;
> }
>
> - return new_fd_pstream(name, fd, punix_accept, NULL, bind_path,
> pstreamp);
> + return new_fd_pstream(name, fd, punix_accept, bind_path, pstreamp);
> }
>
> static int
> @@ -128,6 +128,5 @@ const struct pstream_class punix_pstream_class = {
> NULL,
> NULL,
> NULL,
> - NULL,
> };
>
> diff --git a/lib/stream.c b/lib/stream.c
> index ca469fb..a7e07d0 100644
> --- a/lib/stream.c
> +++ b/lib/stream.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
> + * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -592,15 +592,6 @@ pstream_wait(struct pstream *pstream)
> (pstream->class->wait)(pstream);
> }
>
> -int
> -pstream_set_dscp(struct pstream *pstream, uint8_t dscp)
> -{
> - if (pstream->class->set_dscp) {
> - return pstream->class->set_dscp(pstream, dscp);
> - }
> - return 0;
> -}
> -
> /* Returns the transport port on which 'pstream' is listening, or 0 if the
> * concept doesn't apply. */
> ovs_be16
> diff --git a/lib/stream.h b/lib/stream.h
> index 3087f45..0db50da 100644
> --- a/lib/stream.h
> +++ b/lib/stream.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2009, 2010, 2011, 2013 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2013, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -66,7 +66,6 @@ void pstream_close(struct pstream *);
> int pstream_accept(struct pstream *, struct stream **);
> int pstream_accept_block(struct pstream *, struct stream **);
> void pstream_wait(struct pstream *);
> -int pstream_set_dscp(struct pstream *, uint8_t dscp);
>
> ovs_be16 pstream_get_bound_port(const struct pstream *);
>
> diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
> index e406758..caef515 100644
> --- a/ovsdb/jsonrpc-server.c
> +++ b/ovsdb/jsonrpc-server.c
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> +/* Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
> *
> * Licensed under the Apache License, Version 2.0 (the "License");
> * you may not use this file except in compliance with the License.
> @@ -198,10 +198,16 @@ ovsdb_jsonrpc_server_set_remotes(struct
> ovsdb_jsonrpc_server *svr,
> struct shash_node *node, *next;
>
> SHASH_FOR_EACH_SAFE (node, next, &svr->remotes) {
> - if (!shash_find(new_remotes, node->name)) {
> + struct ovsdb_jsonrpc_remote *remote = node->data;
> + struct ovsdb_jsonrpc_options *options
> + = shash_find_data(new_remotes, node->name);
> +
> + if (!options) {
> VLOG_INFO("%s: remote deconfigured", node->name);
> ovsdb_jsonrpc_server_del_remote(node);
> - }
> + } else if (options->dscp != remote->dscp) {
> + ovsdb_jsonrpc_server_del_remote(node);
> + }
> }
> SHASH_FOR_EACH (node, new_remotes) {
> const struct ovsdb_jsonrpc_options *options = node->data;
> @@ -384,8 +390,6 @@ static int ovsdb_jsonrpc_session_run(struct
> ovsdb_jsonrpc_session *);
> static void ovsdb_jsonrpc_session_wait(struct ovsdb_jsonrpc_session *);
> static void ovsdb_jsonrpc_session_get_memory_usage(
> const struct ovsdb_jsonrpc_session *, struct simap *usage);
> -static void ovsdb_jsonrpc_session_set_options(
> - struct ovsdb_jsonrpc_session *, const struct ovsdb_jsonrpc_options *);
> static void ovsdb_jsonrpc_session_got_request(struct
> ovsdb_jsonrpc_session *,
> struct jsonrpc_msg *);
> static void ovsdb_jsonrpc_session_got_notify(struct ovsdb_jsonrpc_session
> *,
> @@ -556,7 +560,10 @@ ovsdb_jsonrpc_session_reconnect_all(struct
> ovsdb_jsonrpc_remote *remote)
> }
>
> /* Sets the options for all of the JSON-RPC sessions managed by 'remote'
> to
> - * 'options'. */
> + * 'options'.
> + *
> + * (The dscp value can't be changed directly; the caller must instead
> close and
> + * re-open the session.) */
> static void
> ovsdb_jsonrpc_session_set_all_options(
> struct ovsdb_jsonrpc_remote *remote,
> @@ -564,22 +571,6 @@ 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),
> ovs_strerror(error));
> - } else {
> - remote->dscp = options->dscp;
> - }
> - /*
> - * 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);
> }
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list