From 477902bd2e8c1e978ad43d22dba1f28525bb797a Mon Sep 17 00:00:00 2001 From: Olivier Houchard Date: Wed, 22 Jan 2020 18:08:48 +0100 Subject: [PATCH] MEDIUM: connections: Get ride of the xprt_done callback. The xprt_done_cb callback was used to defer some connection initialization until we're connected and the handshake are done. As it mostly consists of creating the mux, instead of using the callback, introduce a conn_create_mux() function, that will just call conn_complete_session() for frontend, and create the mux for backend. In h2_wake(), make sure we call the wake method of the stream_interface, as we no longer wakeup the stream task. --- include/proto/connection.h | 16 +++----------- include/proto/session.h | 1 + include/types/connection.h | 6 +----- src/backend.c | 26 ---------------------- src/connection.c | 44 +++++++++++++++++++++++++++++--------- src/mux_h2.c | 1 + src/session.c | 14 +++--------- src/ssl_sock.c | 13 ++++++----- src/xprt_handshake.c | 10 ++++----- 9 files changed, 54 insertions(+), 77 deletions(-) diff --git a/include/proto/connection.h b/include/proto/connection.h index b0771cbf9d..af0eaaeb92 100644 --- a/include/proto/connection.h +++ b/include/proto/connection.h @@ -67,6 +67,9 @@ int conn_sock_drain(struct connection *conn); int conn_send_socks4_proxy_request(struct connection *conn); int conn_recv_socks4_proxy_response(struct connection *conn); +/* If we delayed the mux creation because we were waiting for the handshake, do it now */ +int conn_create_mux(struct connection *conn); + __decl_hathreads(extern HA_SPINLOCK_T toremove_lock[MAX_THREADS]); /* returns true is the transport layer is ready */ @@ -398,7 +401,6 @@ static inline void conn_init(struct connection *conn) conn->handle.fd = DEAD_FD_MAGIC; conn->err_code = CO_ER_NONE; conn->target = NULL; - conn->xprt_done_cb = NULL; conn->destroy_cb = NULL; conn->proxy_netns = NULL; LIST_INIT(&conn->list); @@ -417,18 +419,6 @@ static inline void conn_set_owner(struct connection *conn, void *owner, void (*c conn->destroy_cb = cb; } -/* registers as a callback to notify for transport's readiness or failure */ -static inline void conn_set_xprt_done_cb(struct connection *conn, int (*cb)(struct connection *)) -{ - conn->xprt_done_cb = cb; -} - -/* unregisters the callback to notify for transport's readiness or failure */ -static inline void conn_clear_xprt_done_cb(struct connection *conn) -{ - conn->xprt_done_cb = NULL; -} - /* Allocates a struct sockaddr from the pool if needed, assigns it to *sap and * returns it. If is NULL, the address is always allocated and returned. * if is non-null, an address will only be allocated if it points to a diff --git a/include/proto/session.h b/include/proto/session.h index a80f97dc13..3eaa357530 100644 --- a/include/proto/session.h +++ b/include/proto/session.h @@ -40,6 +40,7 @@ extern struct pool_head *pool_head_sess_srv_list; struct session *session_new(struct proxy *fe, struct listener *li, enum obj_type *origin); void session_free(struct session *sess); int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr); +int conn_complete_session(struct connection *conn); /* Remove the refcount from the session to the tracked counters, and clear the * pointer to ensure this is only performed once. The caller is responsible for diff --git a/include/types/connection.h b/include/types/connection.h index 4524c2e955..9329654e85 100644 --- a/include/types/connection.h +++ b/include/types/connection.h @@ -437,10 +437,7 @@ struct cs_info { * socket, and can also be made to an internal applet. It can support * several transport schemes (raw, ssl, ...). It can support several * connection control schemes, generally a protocol for socket-oriented - * connections, but other methods for applets. The xprt_done_cb() callback - * is called once the transport layer initialization is done (success or - * failure). It may return < 0 to report an error and require an abort of the - * connection being instanciated. It must be removed once done. + * connections, but other methods for applets. */ struct connection { /* first cache line */ @@ -462,7 +459,6 @@ struct connection { struct list session_list; /* List of attached connections to a session */ union conn_handle handle; /* connection handle at the socket layer */ const struct netns_entry *proxy_netns; - int (*xprt_done_cb)(struct connection *conn); /* callback to notify of end of handshake */ /* third cache line and beyond */ void (*destroy_cb)(struct connection *conn); /* callback to notify of imminent death of the connection */ diff --git a/src/backend.c b/src/backend.c index 4bb30adfb9..7a75ed3c65 100644 --- a/src/backend.c +++ b/src/backend.c @@ -1076,27 +1076,6 @@ static void assign_tproxy_address(struct stream *s) #endif } -#if defined(USE_OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation) -/* Wake the stream up to finish the connection (attach the mux etc). We should - * now have an alpn if available, so we are now able to choose. In this specific - * case the connection's context is &si[i].end. - */ -static int conn_complete_server(struct connection *conn) -{ - struct conn_stream *cs = conn->ctx; - struct stream *s = si_strm((struct stream_interface *)cs->data); - - task_wakeup(s->task, TASK_WOKEN_IO); - conn_clear_xprt_done_cb(conn); - /* Verify if the connection just established. */ - if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) - conn->flags |= CO_FL_CONNECTED; - - return 0; -} -#endif - - /* * This function initiates a connection to the server assigned to this stream * (s->target, s->si[1].addr.to). It will assign a server if none @@ -1420,11 +1399,6 @@ int connect_server(struct stream *s) srv->mux_proto || s->be->mode != PR_MODE_HTTP)) #endif init_mux = 1; -#if defined(USE_OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation) - else - conn_set_xprt_done_cb(srv_conn, conn_complete_server); - -#endif /* process the case where the server requires the PROXY protocol to be sent */ srv_conn->send_proxy_ofs = 0; diff --git a/src/connection.c b/src/connection.c index 5c13a39a86..8754d6e000 100644 --- a/src/connection.c +++ b/src/connection.c @@ -41,6 +41,34 @@ struct mux_proto_list mux_proto_list = { .list = LIST_HEAD_INIT(mux_proto_list.list) }; +int conn_create_mux(struct connection *conn) +{ + /* Verify if the connection just established. */ + if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) + conn->flags |= CO_FL_CONNECTED; + + if (conn_is_back(conn)) { + struct server *srv; + struct conn_stream *cs = conn->ctx; + + if (conn->flags & CO_FL_ERROR) + goto fail; + if (conn_install_mux_be(conn, conn->ctx, conn->owner) < 0) + goto fail; + srv = objt_server(conn->target); + if (srv && ((srv->proxy->options & PR_O_REUSE_MASK) != PR_O_REUSE_NEVR) && + conn->mux->avail_streams(conn) > 0) + LIST_ADD(&srv->idle_conns[tid], &conn->list); + return 0; +fail: + /* let the upper layer know the connection failed */ + cs->data_cb->wake(cs); + return -1; + } else + return conn_complete_session(conn); + +} + /* I/O callback for fd-based connections. It calls the read/write handlers * provided by the connection's sock_ops, which must be valid. */ @@ -111,17 +139,13 @@ void conn_fd_handler(int fd) if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) conn->flags |= CO_FL_CONNECTED; - /* The connection owner might want to be notified about failures - * and/or handshake completeion. The callback may fail and cause the - * connection to be destroyed, thus we must not use it anymore and - * should immediately leave instead. The caller must immediately - * unregister itself once called. + /* If we don't yet have a mux, that means we were waiting for + * informations to create one, typically from the ALPN. If we're + * done with the handshake, attempt to create one. */ - if (unlikely(conn->xprt_done_cb) && - (!(conn->flags & CO_FL_HANDSHAKE) || - ((conn->flags ^ flags) & CO_FL_NOTIFY_DONE)) && - conn->xprt_done_cb(conn) < 0) - return; + if (unlikely(!conn->mux) && !(conn->flags & CO_FL_HANDSHAKE)) + if (conn_create_mux(conn) < 0) + return; /* The wake callback is normally used to notify the data layer about * data layer activity (successful send/recv), connection establishment, diff --git a/src/mux_h2.c b/src/mux_h2.c index d1e93161e0..9c1a77e66a 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -3650,6 +3650,7 @@ static int h2_wake(struct connection *conn) TRACE_ENTER(H2_EV_H2C_WAKE, conn); ret = h2_process(h2c); + h2_wake_some_streams(h2c, 0); TRACE_LEAVE(H2_EV_H2C_WAKE); return ret; } diff --git a/src/session.c b/src/session.c index 111fc61e37..d80392d1d2 100644 --- a/src/session.c +++ b/src/session.c @@ -32,7 +32,7 @@ DECLARE_POOL(pool_head_session, "session", sizeof(struct session)); DECLARE_POOL(pool_head_sess_srv_list, "session server list", sizeof(struct sess_srv_list)); -static int conn_complete_session(struct connection *conn); +int conn_complete_session(struct connection *conn); static struct task *session_expire_embryonic(struct task *t, void *context, unsigned short state); /* Create a a new session and assign it to frontend , listener
  • , @@ -274,8 +274,6 @@ int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr if (unlikely((sess->task = task_new(tid_bit)) == NULL)) goto out_free_sess; - conn_set_xprt_done_cb(cli_conn, conn_complete_session); - sess->task->context = sess; sess->task->nice = l->nice; sess->task->process = session_expire_embryonic; @@ -422,22 +420,16 @@ static struct task *session_expire_embryonic(struct task *t, void *context, unsi /* Finish initializing a session from a connection, or kills it if the * connection shows and error. Returns <0 if the connection was killed. It may - * be called either asynchronously as an xprt_done callback with an embryonic + * be called either asynchronously when ssl handshake is done with an embryonic * session, or synchronously to finalize the session. The distinction is made * on sess->task which is only set in the embryonic session case. */ -static int conn_complete_session(struct connection *conn) +int conn_complete_session(struct connection *conn) { struct session *sess = conn->owner; sess->t_handshake = tv_ms_elapsed(&sess->tv_accept, &now); - conn_clear_xprt_done_cb(conn); - - /* Verify if the connection just established. */ - if (unlikely(!(conn->flags & (CO_FL_WAIT_L4_CONN | CO_FL_WAIT_L6_CONN | CO_FL_CONNECTED)))) - conn->flags |= CO_FL_CONNECTED; - if (conn->flags & CO_FL_ERROR) goto fail; diff --git a/src/ssl_sock.c b/src/ssl_sock.c index 1aac1caa44..88611dd665 100644 --- a/src/ssl_sock.c +++ b/src/ssl_sock.c @@ -6423,8 +6423,7 @@ static struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned short ssl_sock_handshake(ctx->conn, CO_FL_SSL_WAIT_HS); /* If we had an error, or the handshake is done and I/O is available, * let the upper layer know. - * If no mux was set up yet, and nobody subscribed, then call - * xprt_done_cb() ourself if it's set, or destroy the connection, + * If no mux was set up yet, then call conn_create_mux() * we can't be sure conn_fd_handler() will be called again. */ if ((ctx->conn->flags & CO_FL_ERROR) || @@ -6441,13 +6440,13 @@ static struct task *ssl_sock_io_cb(struct task *t, void *context, unsigned short } /* If we're the first xprt for the connection, let the - * upper layers know. If xprt_done_cb() is set, call it, - * otherwise, we should have a mux, so call its wake - * method if we didn't woke a tasklet already. + * upper layers know. If we have no mux, create it, + * and once we have a mux, call its wake method if we didn't + * woke a tasklet already. */ if (ctx->conn->xprt_ctx == ctx) { - if (ctx->conn->xprt_done_cb) - ret = ctx->conn->xprt_done_cb(ctx->conn); + if (!ctx->conn->mux) + ret = conn_create_mux(ctx->conn); if (ret >= 0 && !woke && ctx->conn->mux && ctx->conn->mux->wake) ctx->conn->mux->wake(ctx->conn); return NULL; diff --git a/src/xprt_handshake.c b/src/xprt_handshake.c index f48d7e8bd9..aaf5b6eed5 100644 --- a/src/xprt_handshake.c +++ b/src/xprt_handshake.c @@ -112,13 +112,13 @@ static struct task *xprt_handshake_io_cb(struct task *t, void *bctx, unsigned sh conn->xprt->remove_xprt(conn, conn->xprt_ctx, ctx, ctx->xprt, ctx->xprt_ctx); /* If we're the first xprt for the connection, let the - * upper layers know. If xprt_done_cb() is set, call it, - * and if we have a mux, and it has a wake method, call it - * too. + * upper layers know. If no mux was set up yet, then call + * conn_create_mux, and if we have a mux, and it has a wake + * method, call it too. */ if (was_conn_ctx) { - if (ctx->conn->xprt_done_cb) - ret = ctx->conn->xprt_done_cb(ctx->conn); + if (!ctx->conn->mux) + ret = conn_create_mux(ctx->conn); if (ret >= 0 && !woke && ctx->conn->mux && ctx->conn->mux->wake) ret = ctx->conn->mux->wake(ctx->conn); }