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.
diff --git a/contrib/debug/flags.c b/contrib/debug/flags.c
index 2638dd3..d45b968 100644
--- a/contrib/debug/flags.c
+++ b/contrib/debug/flags.c
@@ -118,7 +118,6 @@
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 66887a5..db712f5 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -500,10 +500,29 @@
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().
diff --git a/include/types/connection.h b/include/types/connection.h
index 59b8cff..748af8d 100644
--- a/include/types/connection.h
+++ b/include/types/connection.h
@@ -95,8 +95,7 @@
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 @@
/* 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 @@
* 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 @@
* 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 @@
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 564c976..54bbc07 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -86,12 +86,14 @@
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 @@
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 62713ab..cc2a0b8 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 <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 @@
* 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 @@
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 @@
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 @@
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