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

@@ -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);

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 <owner> as the connection's owner */
+static inline void conn_set_owner(struct connection *conn, void *owner)
+{
+	conn->owner = owner;
+}
+
+/* registers <cb> 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().

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 <wake> callback is used to report activity
  * at the transport layer, which can be a connection opening/close, or any
- * data movement. The <init> 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 <wake> and <init> 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 */

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

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 <fe>, listener <li>,
  * origin <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