From 8e3c6ce75a4f4b61e01ddb72d278ad447dbc1ae7 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 28 Aug 2017 15:46:01 +0200 Subject: [PATCH] MEDIUM: connection: get rid of data->init() which was not for data The ->init() callback of the connection's data layer was only used to complete the session's initialisation since sessions and streams were split apart in 1.6. The problem is that it creates a big confusion in the layers' roles as the session has to register a dummy data layer when waiting for a handshake to complete, then hand it off to the stream which will replace it. The real need is to notify that the transport has finished initializing. This should enable a better splitting between these layers. This patch thus introduces a connection-specific callback called xprt_done_cb() which informs about handshake successes or failures. With this, data->init() can disappear, CO_FL_INIT_DATA as well, and we don't need to register a dummy data->wake() callback to be notified of errors. --- contrib/debug/flags.c | 1 - include/proto/connection.h | 19 +++++++++++++++++++ include/types/connection.h | 16 ++++++++-------- src/connection.c | 22 +++++++++++++++++----- src/session.c | 36 +++++++----------------------------- 5 files changed, 51 insertions(+), 43 deletions(-) diff --git a/contrib/debug/flags.c b/contrib/debug/flags.c index 2638dd3430..d45b9684f8 100644 --- a/contrib/debug/flags.c +++ b/contrib/debug/flags.c @@ -118,7 +118,6 @@ void show_conn_flags(unsigned int f) SHOW_FLAG(f, CO_FL_SOCK_RD_SH); SHOW_FLAG(f, CO_FL_DATA_WR_SH); SHOW_FLAG(f, CO_FL_DATA_RD_SH); - SHOW_FLAG(f, CO_FL_INIT_DATA); SHOW_FLAG(f, CO_FL_ADDR_TO_SET); SHOW_FLAG(f, CO_FL_ADDR_FROM_SET); SHOW_FLAG(f, CO_FL_WAIT_ROOM); diff --git a/include/proto/connection.h b/include/proto/connection.h index 66887a5413..db712f58bf 100644 --- a/include/proto/connection.h +++ b/include/proto/connection.h @@ -500,10 +500,29 @@ 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->proxy_netns = NULL; LIST_INIT(&conn->list); } +/* sets as the connection's owner */ +static inline void conn_set_owner(struct connection *conn, void *owner) +{ + conn->owner = owner; +} + +/* 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; +} + /* Tries to allocate a new connection and initialized its main fields. The * connection is returned on success, NULL on failure. The connection must * be released using pool_free2() or conn_free(). diff --git a/include/types/connection.h b/include/types/connection.h index 59b8cff8ec..748af8de61 100644 --- a/include/types/connection.h +++ b/include/types/connection.h @@ -95,8 +95,7 @@ enum { CO_FL_ADDR_FROM_SET = 0x00001000, /* addr.from is set */ CO_FL_ADDR_TO_SET = 0x00002000, /* addr.to is set */ - /* flags indicating what event type the data layer is interested in */ - CO_FL_INIT_DATA = 0x00004000, /* initialize the data layer before using it */ + /* unused : 0x00004000 */ /* unused : 0x00008000 */ /* flags used to remember what shutdown have been performed/reported */ @@ -107,6 +106,7 @@ enum { /* flags used to report connection errors or other closing conditions */ CO_FL_ERROR = 0x00100000, /* a fatal error was reported */ + CO_FL_NOTIFY_DONE = 0x001C0000, /* any xprt shut/error flags above needs to be reported */ CO_FL_NOTIFY_DATA = 0x001F0000, /* any shut/error flags above needs to be reported */ /* flags used to report connection status updates */ @@ -248,16 +248,12 @@ struct xprt_ops { * callbacks are supposed to make use of the xprt_ops above to exchange data * from/to buffers and pipes. The callback is used to report activity * at the transport layer, which can be a connection opening/close, or any - * data movement. The callback may be called by the connection handler - * at the end of a transport handshake, when it is about to transfer data and - * the data layer is not ready yet. Both and may abort a connection - * by returning < 0. + * data movement. It may abort a connection by returning < 0. */ struct data_cb { void (*recv)(struct connection *conn); /* data-layer recv callback */ void (*send)(struct connection *conn); /* data-layer send callback */ int (*wake)(struct connection *conn); /* data-layer callback to report activity */ - int (*init)(struct connection *conn); /* data-layer initialization */ char name[8]; /* data layer name, zero-terminated */ }; @@ -289,7 +285,10 @@ struct conn_src { * 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. + * 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. */ struct connection { enum obj_type obj_type; /* differentiates connection from applet context */ @@ -305,6 +304,7 @@ struct connection { union conn_handle handle; /* connection handle at the socket layer */ enum obj_type *target; /* the target to connect to (server, proxy, applet, ...) */ struct list list; /* attach point to various connection lists (idle, ...) */ + int (*xprt_done_cb)(struct connection *conn); /* callback to notify of end of handshake */ const struct netns_entry *proxy_netns; struct { struct sockaddr_storage from; /* client address, or address to spoof when connecting to the server */ diff --git a/src/connection.c b/src/connection.c index 564c9761b2..54bbc0737b 100644 --- a/src/connection.c +++ b/src/connection.c @@ -86,12 +86,14 @@ void conn_fd_handler(int fd) if (!(conn->flags & CO_FL_POLL_SOCK)) __conn_sock_stop_both(conn); - /* The data layer might not be ready yet (eg: when using embryonic - * sessions). If we're about to move data, we must initialize it first. - * The function may fail and cause the connection to be destroyed, thus - * we must not use it anymore and should immediately leave instead. + /* The connection owner might want to be notified about an end of + * handshake indicating the connection is ready, before we proceed with + * any data exchange. 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 ((conn->flags & CO_FL_INIT_DATA) && conn->data->init(conn) < 0) + if (conn->xprt_done_cb && conn->xprt_done_cb(conn) < 0) return; if (conn->xprt && fd_send_ready(fd) && @@ -137,6 +139,16 @@ 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 to + * complete the handshake. 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 (((conn->flags ^ flags) & CO_FL_NOTIFY_DONE) && + conn->xprt_done_cb && conn->xprt_done_cb(conn) < 0) + return; + /* The wake callback is normally used to notify the data layer about * data layer activity (successful send/recv), connection establishment, * shutdown and fatal errors. We need to consider the following diff --git a/src/session.c b/src/session.c index 62713abce5..cc2a0b87a7 100644 --- a/src/session.c +++ b/src/session.c @@ -31,18 +31,8 @@ struct pool_head *pool2_session; static int conn_complete_session(struct connection *conn); -static int conn_update_session(struct connection *conn); static struct task *session_expire_embryonic(struct task *t); -/* data layer callbacks for an embryonic stream */ -struct data_cb sess_conn_cb = { - .recv = NULL, - .send = NULL, - .wake = conn_update_session, - .init = conn_complete_session, - .name = "SESS", -}; - /* Create a a new session and assign it to frontend , listener
  • , * origin , set the current date and clear the stick counters pointers. * Returns the session upon success or NULL. The session may be released using @@ -251,11 +241,11 @@ int session_accept_fd(struct listener *l, int cfd, struct sockaddr_storage *addr * conn -- owner ---> task */ if (cli_conn->flags & CO_FL_HANDSHAKE) { - conn_attach(cli_conn, t, &sess_conn_cb); + conn_set_owner(cli_conn, t); + conn_set_xprt_done_cb(cli_conn, conn_complete_session); t->process = session_expire_embryonic; t->expire = tick_add_ifset(now_ms, p->timeout.client); task_queue(t); - cli_conn->flags |= CO_FL_INIT_DATA; return 1; } @@ -423,9 +413,14 @@ static int conn_complete_session(struct connection *conn) struct task *task = conn->owner; struct session *sess = task->context; + conn_clear_xprt_done_cb(conn); + if (conn->flags & CO_FL_ERROR) goto fail; + if (conn->flags & CO_FL_HANDSHAKE) + return 0; /* wait more */ + /* if logs require transport layer information, note it on the connection */ if (sess->fe->to_log & LW_XPRT) conn->flags |= CO_FL_XPRT_TRACKED; @@ -439,8 +434,6 @@ static int conn_complete_session(struct connection *conn) if (!stream_new(sess, task, &conn->obj_type)) goto fail; - conn->flags &= ~CO_FL_INIT_DATA; - task_wakeup(task, TASK_WOKEN_INIT); return 0; @@ -449,21 +442,6 @@ static int conn_complete_session(struct connection *conn) return -1; } -/* Update a session status. The connection is killed in case of - * error, and <0 will be returned. Otherwise it does nothing. - */ -static int conn_update_session(struct connection *conn) -{ - struct task *task = conn->owner; - struct session *sess = task->context; - - if (conn->flags & CO_FL_ERROR) { - session_kill_embryonic(sess, task); - return -1; - } - return 0; -} - /* * Local variables: * c-indent-level: 8 -- 2.39.5